Skip to content

dai: remove wait on stop - #284

Merged
lgirdwood merged 1 commit into
thesofproject:masterfrom
tlauda:topic/dai_remove_wait
Sep 6, 2018
Merged

dai: remove wait on stop#284
lgirdwood merged 1 commit into
thesofproject:masterfrom
tlauda:topic/dai_remove_wait

Conversation

@tlauda

@tlaudatlauda commented Aug 30, 2018

Copy link
Copy Markdown
Contributor

Removes wait on stop, as it's not needed.
Also this change fixes problems with sending IPC as
wait function lowers interrupt level, while IPCs are
now executed on irq task level.

Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com

@tlauda
tlaudaforce-pushed the topic/dai_remove_wait branch from 76c867c to 6373c93CompareAugust 30, 2018 16:39
lgirdwood
lgirdwood reviewed Aug 30, 2018
View reviewed changes

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The wait was intended for DMA FIFO draining, but if its no longer needed it can be dropped. Btw, have you tested pause/release or XRUNs with this update ?

@lgirdwood

lgirdwood commented Aug 30, 2018

Copy link
Copy Markdown
Member

@ZhendanYang can you test this PR for XRUNs and pause/release.

@keqiaozhang

keqiaozhang commented Aug 31, 2018

Copy link
Copy Markdown
Collaborator

@tlauda@lgirdwood
I tested this patch on UP^2+ hifiberry codec
Pause/release:

  1. Pause/release will introduce noise and XRUN issue, audio cannot recover by doing pause/release.
  2. No XRUN issues without pause/release.
  3. After several pause/release(2~6 times), playback will be stopped with I/O error.
  4. playback can re-run.
    pause-release-dmesg.log
    pause-release-rmbox-t.log

XRUN injection:

  1. XRUN injection will introduce noise and can recover after several XRUN. if keep inject xrun, playback will be stopped with I/Oerror.
  2. Playback can re-run after I/O error.
    xrun-dmesg.log
    xrun-rmbox-t.log

@tlauda

tlauda commented Aug 31, 2018

Copy link
Copy Markdown
ContributorAuthor

@keqiaozhang@lgirdwood
This patch doesn't touch XRUN injection scenario.
Can you confirm, that pause/release is working without my patch?

@keqiaozhang

keqiaozhang commented Aug 31, 2018

Copy link
Copy Markdown
Collaborator

@tlauda
There's no noise or XRUN error without your patch, but in an extreme way, like keep pressing the space key will cause DSP panic #272.

@tlauda
tlaudaforce-pushed the topic/dai_remove_wait branch from 6373c93 to bf68b0bCompareSeptember 3, 2018 11:32
@tlauda
dai: remove wait on stop
344085d
Removes wait on stop, as it's not needed.
Also this change fixes problems with sending IPC as
wait function lowers interrupt level, while IPCs are
now executed on irq task level.
Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
@tlauda
tlaudaforce-pushed the topic/dai_remove_wait branch from bf68b0b to 344085dCompareSeptember 3, 2018 11:58
tlauda
tlauda commented Sep 3, 2018
View reviewed changes
Comment threadsrc/audio/dai.c
ret = dma_stop(dd->dma, dd->chan);
}
ret = dma_stop(dd->dma, dd->chan);
dai_trigger(dd->dai, COMP_TRIGGER_STOP, dev->params.direction);

@tlaudatlaudaSep 3, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Intentionally changed in order to properly clear SSP fifo (PR #302).

@tlauda

tlauda commented Sep 3, 2018

Copy link
Copy Markdown
ContributorAuthor

@keqiaozhang Could you check with the current version of this PR? I've tested the pause/release scenario and it shouldn't generate glitches anymore.

@tlauda

tlauda commented Sep 5, 2018

Copy link
Copy Markdown
ContributorAuthor

@keqiaozhang Have you had a chance to check it out?

@tlaudatlauda mentioned this pull request Sep 5, 2018
Closed
@ZhendanYang

ZhendanYang commented Sep 6, 2018

Copy link
Copy Markdown

@tlauda@lgirdwood
I tested this patch on UP^2+ hifiberry codec. I applied this patch to master branch, and there were some conflicts with stable-1.2 branch.

When playback, sometimes XRUN will occur, followed by noise issue.

Pause/Release:

  1. Pause/release will introduce XRUN issue and noise, audio cannot recover by doing pause/release.
  2. Pause/release will not cause I/O error any more.
  3. In an extreme way, like keep pressing the space key will cause ipc timeout.
    ([ 80.679313] sof-audio sof-audio: ipc rx: 0x90020000: GLB_TRACE_MSG
    [ 80.679331] sof-audio sof-audio: ipc rx done: 0x90020000: GLB_TRACE_MSG
    [ 81.152198] sof-audio sof-audio: error: ipc timed out for 0x40010000 size 0x20
    [ 81.152240] sof-audio sof-audio: status: fw entered - code 00000005
    [ 81.152262] sof-audio sof-audio: error: unexpected fault 0x00000000 trace 0x00004000
    [ 81.152266] sof-audio sof-audio: error: waking up any trace sleepers
    [ 81.152278] sof-audio sof-audio: error: ctx_save ipc error during suspend -110
    [ 81.152295] sof-audio-pci 0000:00:0e.0: can't suspend (snd_sof_runtime_suspend [snd_sof] returned -110)

dmesg-PR284.log
rmbox-PR284.log
rmbox-t-PR284.log

XRUN injection:

  1. XRUN injection will introduce noise and can not recover.
  2. Playback will be stopped with I/O error sometimes. And also ipc time out in dmesg.
    ([ 297.900713] sof-audio sof-audio: ipc rx: 0x90020000: GLB_TRACE_MSG
    [ 297.900735] sof-audio sof-audio: ipc rx done: 0x90020000: GLB_TRACE_MSG
    [ 298.272298] sof-audio sof-audio: error: ipc timed out for 0x40010000 size 0x20
    [ 298.272318] sof-audio sof-audio: status: fw entered - code 00000005
    [ 298.272345] sof-audio sof-audio: error: unexpected fault 0x00000000 trace 0x00004000
    [ 298.272352] sof-audio sof-audio: error: waking up any trace sleepers
    [ 298.272369] sof-audio sof-audio: error: ctx_save ipc error during suspend -110
    [ 298.272396] sof-audio-pci 0000:00:0e.0: can't suspend (snd_sof_runtime_suspend [snd_sof] returned -110)

dmesg-XRUN.log
rmbox-t-XRUN.log
rmbox-XRUN.log

@lgirdwood
lgirdwood merged commit d27ec1d into thesofproject:masterSep 6, 2018
@xiulipan

xiulipan commented Sep 6, 2018

Copy link
Copy Markdown
Contributor

@lgirdwood
Could this PR be merged to stable-1.2 too?

@lgirdwood

lgirdwood commented Sep 6, 2018

Copy link
Copy Markdown
Member

@xiulipan yes pls do and send PR.

@xiulipan

xiulipan commented Sep 6, 2018

Copy link
Copy Markdown
Contributor

@lgirdwood@tlauda
PR for stable-1.2 send #320

This was referenced Sep 7, 2018
Merged
Closed
@xiulipanxiulipan mentioned this pull request Dec 19, 2018
Merged
@tlauda
tlauda deleted the topic/dai_remove_wait branch November 22, 2019 11:26
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgirdwoodlgirdwoodlgirdwood left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tlauda@lgirdwood@keqiaozhang@ZhendanYang@xiulipan