src: dai-zephyr: copy data to all available sinks #9200
Conversation
dnikodem
force-pushed
the
copy_audio_to_all_available_sinks
branch
3 times, most recently
from
2649bd8 to
3c13bc4
Compare
dnikodem
force-pushed
the
copy_audio_to_all_available_sinks
branch
from
3c13bc4 to
9f5f8ce
Compare
dnikodem
requested review from
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
There was a problem hiding this comment.
dai_common_copy() calculates the amount of bytes that should be copied. For playback case that is line 1525. Seems that code should be modified to also check if there is free space in all sink buffers, current version only checks for space available in source and free in dma_buffer.
|
|
||
| sink = container_of(sink_list, struct comp_buffer, source_list); | ||
|
|
||
| if (sink == dd->dma_buffer) |
There was a problem hiding this comment.
dma_buffer is not on the list of sinks, so this condition is always false. However, maybe it is OK to leave it as is or replace with assert() "just in case" someone add dma_buffer to sink list in some future refactoring.
src/audio/dai-zephyr.c
Outdated
| continue; | ||
| } | ||
|
|
||
| if (sink_dev && sink_dev->state == COMP_STATE_ACTIVE) { |
There was a problem hiding this comment.
Those additional sinks are (always?) connected to other pipelines. And so some other pipelines might be responsible to set buffer stream params in .params() handler of components connected to those sinks. For unknown reason checking sink_dev->state == COMP_STATE_ACTIVE is not enough, I recently observed problems with multiple-pause-resume test on my draft PR and have to fix it like this: 4ebd807.
Frame format of uninitialized buffer is 0 which is treated as SOF_IPC_FRAME_S16_LE. If test uses 24 or 32 bit formats that could lead to wrong consume/produce and invalidate/writeback values.
The problem could be fixed in a separate PR, I just comment it here as your PR adds new place there the fix should be applied.
There was a problem hiding this comment.
Sure, thanks for review. I'll take a look at your comments and get back with answers
dnikodem
force-pushed
the
copy_audio_to_all_available_sinks
branch
2 times, most recently
from
71956c1 to
eb726a3
Compare
|
Additionally, I am expecting test results from validation - should be ready tomorrow. |
|
After checking on our dut's with more tests, we don't see any regression on MTL and LNL as for PTL we have to wait until Monday |
Any update @dnikodem ? Will need a rebase before merge. Thanks ! |
|
@lgirdwood No regression found on PTL iteration. |
I will try to rebase it today. Thanks |
dnikodem
force-pushed
the
copy_audio_to_all_available_sinks
branch
from
eb726a3 to
3b214eb
Compare
In the case where the copier has more than one output and, additionally, we will be using a pin other than pin 0 (for example, when we want to copy data from pin 1 of a gateway-type copier to the next pipeline), it is necessary to copy data in playback path from the source to the output for all possible active sinks. Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
dnikodem
force-pushed
the
copy_audio_to_all_available_sinks
branch
from
3b214eb to
b5d522e
Compare
In the case where the copier has more than one output and, additionally, we will be using a pin other than pin 0 (for example, when we want to copy data from pin 1 of a gateway-type copier to the next pipeline), it is necessary to copy data in playback path from the source to the output for all possible sinks.