dai-zephyr: fixes to dma_stop() usage #7110
Conversation
kv2019i
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
kv2019i
requested review from
softwarecki,
abonislawski,
aborisovich and
juimonen
kv2019i
changed the title
|
Something wrong with the changes and/or these cannot be merged until we update Zephyr. Marking as DNM until I have rootcaused the problem. |
The component state dependent actions in dai_comp_trigger_internal() are not working as intended as dev->state is already updated to new target state at the start of the function. Update the logic to correctly evaluate previous state in trigger actions. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
To ensure DMA is stopped in all cases, we need to call dma_stop()
also when the previous state is PAUSED.
With Zephyr commit ec9d94e06d4a ("dma: Document expected behavior of
start/stop") the dma interface semantics were clarified. Build on
this and call dma_stop() in all cases and make the code simpler to
follow.
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
force-pushed
the
202302-dma-trigger-fixes
branch
from
b6ede01
to
f104114
Compare
|
V2:
|
kv2019i
changed the title
arrrgh, looks like we cannot see failures in #7102 any more :-( It would've been good to have a direct link to those failures here... Do we have those failures anywhere else? |
|
@lyakh wrote:
Sorry for non-Intel folks, the log is there in public but I know of no easy way to find the link given the PR is now closed and I did not add the link manually to the bug comments. I do nowadays almost always add a link (just for this reason), but we are human and make mistakes. @lyakh you can see the log in Intel internal database for test plan 21013 |
| @@ -1111,7 +1112,7 @@ static int dai_comp_trigger_internal(struct comp_dev *dev, int cmd) | |||
| break; | |||
| case COMP_TRIGGER_PAUSE: | |||
| comp_dbg(dev, "dai_comp_trigger_internal(), PAUSE"); | |||
| if (dev->state == COMP_STATE_ACTIVE) { | |||
| if (prev_state == COMP_STATE_ACTIVE) { | |||
There was a problem hiding this comment.
@juimonen Yup, I originally had three places to replace with "prev_state", but one change was not correct. I removed the first check in this PR already., so that only leaves this one check before dma_suspend(). Reading the Zephyr dma.h docs again, it seems also dma_suspend() is safe to call (even if channel is not active. So probably we can make the change and simplify this series.
|
I think that now when it's safe from power management perspective to call multiple times DMA start and stop, we can just revert this commit fa0456e |
|
@tmleman wrote:
Problem is that the check on "COMP_TRIGGER_RELEASE" in that patch actually seems to be still needed. I tried to revert all, and I got a bunch of failures still with Zephyr main. UPDATE: @tmleman I actually didn't revert the whole change. Hmm, you are probably right, full revert should actually do the trick. Let's try this in the baseline update. |
|
This needs to be merged together with Zephyr baseline update. Replaced with #7125 |
To fix SOF SSP tests with latest Zephyr main, see failures without this fix in #7102
For all tests to pass, zephyrproject-rtos/zephyr#54900 is also needed.