copier: fix allocation and configuration size#6930
Conversation
|
@lyakh Now copier doesn't save gateway config (blob data) in variable array since it is a little huge like DMIC blob. Why do we need to save it ? These blob data is sent to dai directly from memory windows |
@RanderWang ah, so it is on purpose? Ok, then we need an explanation there, I'll add a comment. And we don't need that cache-invalidation, I'll remove it. And in principle this PR isn't wrong, right? So it is strange why it's causing PR failures. I'll investigate that. |
There was a problem hiding this comment.
I wonder how did this work until now?
There was a problem hiding this comment.
Wonder why this passed build ? @lyakh have you been able to build this with GCC ?
There was a problem hiding this comment.
@dbaluta as @RanderWang explained, it wasn't used. And indeed in some cases that array is large, and if we don't need to keep that configuration, it's ok to not do so, but then we need to make it explicit in the code.
@lgirdwood sure, it builds both ways, because that array isn't declared as variable size, it's declared as size 1 because of problems with some compilers I guess
There was a problem hiding this comment.
Wonder why this passed build ? @lyakh have you been able to build this with GCC ?
I think MTL-003 has already been announced but I will let know the team to take a look at this. |
|
Now this PR is causing CI failures like https://sof-ci.01.org/sofpr/PR6930/build3364/devicetest/index.html?model=TGLU_RVP_NOCODEC_IPC4ZPH&testcase=check-capture-3times but I think it isn't the fault of this PR that those tests are now failing, it is because it's fixing something. E.g. in the SSP case I narrowed down the failure to calling |
kv2019i
left a comment
There was a problem hiding this comment.
Code looks good. Not approving until the issue found by CI is sorted out.
f27ab70 to
9870206
Compare
|
Only CI failures now are the known SDW failure https://sof-ci.01.org/sofpr/PR6930/build3399/devicetest/index.html?model=ADLP_RVP_SDW_IPC4ZPH&testcase=check-capture-10sec like in https://sof-ci.01.org/sofpr/PR6785/build3402/devicetest/index.html?model=ADLP_RVP_SDW_IPC4ZPH&testcase=check-capture-10sec and an alsabat failure. There's also a typo in a commit message, let me fix that. |
Some DAI types share properties objects between playback and capture directions and write to them and read from them with no locking. Add a DAI spinlock to protect them. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
rtos/interrupt.h is included twice in dai-zephyr.c, remove one copy. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
With IPC4 the top level component that receives trigger events is a copier and not the associated with it DAI. We need to retrieve the DAI device pointer for its timing parameters. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
No need to invalidate cache again - comp_new() already has done it. Also document why we don't need to copy config_data[] and use memcpy() directly instead of mailbox_hostbox_read() - we already have a pointer and cached have been invalidated already. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Copier configuration size must include the variable size array in struct ipc4_copier_gateway_cfg.