Repository navigation
dai-zephyr: Fix the ordering of DAI and DMA triggers during - #7295
Conversation
|
SOFCI TEST |
1 similar comment
|
SOFCI TEST |
|
@abonislawski Can you check? The order was changed (compared to legacy) in the original "dma: add Zephyr native DMA support to host & dai" commit. |
abonislawski
left a comment
abonislawski
left a comment
There was a problem hiding this comment.
@kv2019i this one is correct
There was a problem hiding this comment.
Should not be handled like the STOP? there is this comment in that section:
/*
* Some platforms cannot just simple disable
* DMA channel during the transfer,
* because it will hang the whole DMA controller.
* Therefore, stop the DMA first and let the DAI
* drain the FIFO in order to stop the channel
* as soon as possible.
*/
There was a problem hiding this comment.
@dbaluta did NXP have this issue with ordering or am I thinking about another platform. If so, may need a Kconfig.
There was a problem hiding this comment.
I remember having some issues on the NXP side with the ordering indeed. Kconfig sounds good.
There was a problem hiding this comment.
I added the above comment (with this PR https://github.com/thesofproject/sof/pull/3867/files).
And I used a config (CONFIG_DMA_SUSPEND_DRAIN) to have it differently for i.MX, but it was done only on start/stop.
I believe we can do the same for pause/release.
There was a problem hiding this comment.
@lgirdwood we did have this problem. See @iuliana-prodan comment. I think we should create a special CONFIG_ENABLE_DMA_FIRST and put everything under this.
There was a problem hiding this comment.
@iuliana-prodan @dbaluta using CONFIG_DMA_SUSPEND_DRAIN is a bit misleading I think. What I can do is make the default as dma_start()->dai_start for START/RELEASE and dai_stop()->dma_stop() for STOP/PAUSE.
And add a new CONFIG called CONFIG_DAI_TRIGGER_ORDER_REVERSE to reverse the order. Would that be OK for you?
There was a problem hiding this comment.
@iuliana-prodan I think i was wrong. I should only reverse the order for stop/pause right? It doesnt make sense to do dai_start before dma_start() even for IMX. I've fixed it now. have a look please
lyakh
left a comment
lyakh
left a comment
There was a problem hiding this comment.
seems reasonable: you first enable the DMA, so that later when the DAI srarts, it triggers the DMA - either to get data from it or to send data to it, so DMA should already be ready. Why did we have it in reverse odrer before? Has it always been like that or is this a recent change?
729f22c to
85a5b8d
Compare
During start trigger, the DMA is be started first followed by the DAI. The same order should also apply for the release triggers. Conversely, during stop/pause the DAI must be stopped first before the DMA. But some platforms cannot handle the conventional ordering during the stop/pause triggers. Add a new config, COMP_DAI_STOP_TRIGGER_ORDER_REVERSE, that will be used to reverse the DMA and DAI trigger order during stop/pause. Also, fix the stop case to use the new config to reverse the ordering instead of COMP_DMA_SUSPEND_DRAIN. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
85a5b8d to
e731fad
Compare
|
Good to go @iuliana-prodan @dbaluta ? |
| Select if the ordering of DMA and DAI triggers during stop/pause should be reversed. | ||
| The normal order during stop/pause is to stop DAI before stopping DMA. This option will | ||
| allow reversing the order to do DMA stop before stopping DAI. | ||
|
|
There was a problem hiding this comment.
Would be good to align use of the Kconfigs in dai-legacy.c as well.
yes, it's ok. Thanks! |
|
@lyakh I think is fine. We need to update src/audio/dai-legacy.c also. Rename CONFIG_DMA_SUSPEND_DRAIN to COMP_DAI_STOP_TRIGGER_ORDER_REVERSE |
|
One fail in https://sof-ci.01.org/sofpr/PR7295/build4799/devicetest/index.html , but that seems unrelated to this PR, so proceeding with merge. |
pause/release
Pause the DAI before the DMA and start the DMA before starting the DAI during release.