Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

west.yml: update Zephyr (subset of PR8804 , no SMP changes) #8805

Closed

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Jan 26, 2024

No description provided.

Starting with Zephyr commit e021ccfc745221c6 ("drivers: dma:
intel-adsp-hda: add delay to stop host dma"), the pause-resume
sof-test cases started failing on Intel cAVS2.5 platforms.

Add a delay loop around DMA stop code in chain DMA to workaround
the issue while a proper fix is under investigation. This allows
to resume integration of newer Zephyr versions to SOF and ensure
we detect any new regressions in time.

Link: thesofproject#8792
Signed-off-by: Kai Vehmanen <[email protected]>
Update Zephyr to the most recent commit before the SMP interface
change (which requires SOF side code changes).

Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i kv2019i marked this pull request as ready for review January 26, 2024 22:43
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 26, 2024

One configuration was missed due to missing, but results are good:
https://sof-ci.01.org/sofpr/PR8805/build2381/devicetest/index.html
https://sof-ci.01.org/sofpr/PR8805/build2380/devicetest/index.html

The pause-resume fail seen with newer Zephyr revisions in #8804 and #8764 is not seen in results for this PR, so the the SMP interface change is causing the new failure on pause-resume.

if (err < 0)
if (err < 0) {
if (err == -EBUSY) {
int retries = 100;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you probably want to fix the misplaced brace at line 143, and while at it (optionally) you could also consider doing

for (int i = 0; i < 100 && err == -EBUSY; i++) {
    if (!i)
        comp_warn(...);
    k_busy_wait(10);
    err = dma_stop(...);
}
if (err < 0)
    return err;

*/
while (err == -EBUSY && --retries) {
k_busy_wait(10);
err = dma_stop(cd->chan_link->dma->z_dev, cd->chan_link->index);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, @ujfalusi noted this is likely not doing actual stop from second iterations onwards, so this is a no-op operation (with the HD-DMA driver). Let me revise the workaround to be more explicit...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this would be identical and more explicit:

diff --git a/src/audio/chain_dma.c b/src/audio/chain_dma.c
index 696973146404..cea2a2e5c8f3 100644
--- a/src/audio/chain_dma.c
+++ b/src/audio/chain_dma.c
@@ -121,7 +121,7 @@ static int chain_host_stop(struct comp_dev *dev)
 	int err;
 
 	err = dma_stop(cd->chan_host->dma->z_dev, cd->chan_host->index);
-	if (err < 0)
+	if (err < 0 && err != -EBUSY)
 		return err;
 
 	comp_info(dev, "chain_host_stop(): dma_stop() host chan_index = %u",

With added comment

@kv2019i kv2019i marked this pull request as draft January 29, 2024 16:20
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 29, 2024

Workaround requires work, converting to a draft.

@kv2019i kv2019i closed this Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants