DMA: Fix fw panic after release on SdW platforms by ktrzcinx · Pull Request #2673 · thesofproject/sof

ktrzcinx · 2020-03-31T10:45:37Z

Bug was caused because sdw controller is stopped first,
then DSP is stopped, so DW FIFO will never be consumed,
so timeout occurs, and watch dog will reset hardware.
Moreover polling for FIFO emty in duch a place should have positive
result only when pause take shorten than 1ms what is not
reasonable value.

Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com

Bug was caused because sdw controller is stopped first, then DSP is stopped, so DW FIFO will never be consumed, so timeout occurs, and watch dog will reset hardware. Moreover polling for FIFO empty in duch a place should have positive result only when pause take shorten than 1ms what is not reasonable value. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

lgirdwood

Shouldn't the fix be stopping DMA then SDW ? What do we do with DMIC and SSP ?

slawblauciak · 2020-04-03T08:20:00Z

Shouldn't the fix be stopping DMA then SDW ? What do we do with DMIC and SSP ?

@plbossart would that be a good solution to you in your eyes? I'm not entirely sure if we could guarantee correct flow if we were to do that.

lgirdwood · 2020-04-03T09:23:43Z

@slawblauciak @ktrzcinx we need to have something consistent and not reorder the ops depending on DAI. e.g.

Receive IPC STOP from host.
Set DMA to STOP on next DMA period consumed IRQ
Stop the DAI

Can you confirm the stop flow today.

slawblauciak · 2020-04-03T09:32:12Z

Thing is, the SDW controller is managed on the host side. So, this really has to be synched with kernel.

lgirdwood · 2020-04-03T09:51:43Z

@slawblauciak which parts of SDW are controlled by host and which are FW ?

slawblauciak · 2020-04-03T09:52:34Z

The entirety of SDW is controlled by host. The FW only handles the DMA.

lgirdwood · 2020-04-03T10:17:04Z

@slawblauciak I'm assuming DW DMA is used here ?

The programming partitioning flow with host would be

User stops stream.
Driver sends stop IPC to FW
FW Receive IPC STOP from host.
FW Sets DMA to STOP on next DMA period consumed IRQ
FW IPC replies STOP complete.
Driver stops the DAI.

slawblauciak · 2020-04-03T11:18:22Z

That's right, we use DW DMA for SDW/ALH.

lgirdwood · 2020-04-03T12:43:20Z

@slawblauciak can you confirm the current flow like 1 - 6 above, it may mean we need to make driver and FW changes here.

slawblauciak · 2020-04-03T12:44:31Z

Yeah, I think that's the kind of flow we'd want here.

plbossart · 2020-04-03T14:00:03Z

Sorry I don't understand this entire thread. We ALREADY stop the DMA before stopping the SoundWire transfers, and that creates a pop noise. thesofproject/linux#1897

lgirdwood · 2020-04-03T15:23:12Z

@plbossart so does thesofproject/linux#1897 fix the pop issue for you ?
This will still pop if absolute value of the last sample is large.
The problem here for FW is that DMA FIFO cant be drained if DAI IO is disabled but I've no idea why @slawblauciak is seeing a panic. I would expect to just the the error messages.

plbossart · 2020-04-03T15:34:12Z

@plbossart so does thesofproject/linux#1897 fix the pop issue for you ?
This will still pop if absolute value of the last sample is large.

No, the RT1308 will deal with this. What it can't deal with is a sustained output to -1 as is currently the case.

The problem here for FW is that DMA FIFO cant be drained if DAI IO is disabled but I've no idea why @slawblauciak is seeing a panic. I would expect to just the the error messages.

I have no idea how this error happens since it's different from what is being used today. Unless this is with the python scripts?

At any rate I started an Intel internal thread on the recommended programming sequence. let's not continue here until we know what direction to take.

slawblauciak · 2020-04-06T07:16:35Z

I haven't seen any DSP panics actually. Haven't tried to reproduce the problem so far.

lgirdwood · 2020-04-06T08:56:02Z

I haven't seen any DSP panics actually. Haven't tried to reproduce the problem so far.

ok, then the PR title is misleading. Please align internally with @plbossart, I will close this for the moment, we can re-open once solution is agreed.

slawblauciak · 2020-04-06T08:57:31Z

Uhm, I believe there's a misunderstanding, this is not my PR :)

paulstelian97 · 2020-04-06T10:56:45Z

@ktrzcinx You do still get the FW panics right? If so please reopen if this is the fix you want to go forward with.

RanderWang · 2020-04-09T02:54:25Z

the DSP panic is caused by FW for thesofproject/linux#1897

[563056340.885417] (563056320.000000) c0 WAIT src/lib/wait.c:39 ERROR ewt
[563056354.583333] ( 13.697917) c0 DMA src/drivers/dw/dma.c:362 ERROR dw_dma_release() error: dma 0 channel 0 timeout
[563492634.635417] ( 436280.062500) c0 WAIT src/lib/wait.c:39 ERROR ewt
[563492648.229167] ( 13.593750) c0 DMA src/drivers/dw/dma.c:362 ERROR dw_dma_release() error: dma 0 channel 0 timeout
[563951513.281250] ( 458865.062500) c0 WAIT src/lib/wait.c:39 ERROR ewt
[563951526.822917] ( 13.541667) c0 DMA src/drivers/dw/dma.c:362 ERROR dw_dma_release() error: dma 0 channel 0 timeout
[563490940.260417] ( nan) c0 IPC src/ipc/handler.c:441 ipc: comp 0 -> trigger cmd 0x70000
[563490945.416667] ( 5.156250) c0 PIPE 1.5 src/audio/pipeline.c:725 pipe trigger cmd 3
[563490953.072917] ( 7.656250) c0 DMA src/drivers/dw/dma.c:343 dw_dma_release(): dma 0 channel 0 release
[563492634.635417] ( 1681.562500) c0 WAIT src/lib/wait.c:39 ERROR ewt
[563492641.979167] ( 7.343750) c0 DMA src/drivers/dw/dma.c:413 dw_dma_stop(): dma 0 channel 0 stop
[563492648.229167] ( 6.250000) c0 DMA src/drivers/dw/dma.c:362 ERROR dw_dma_release() error: dma 0 channel 0 timeout
```

lgirdwood · 2020-04-09T07:54:49Z

@RanderWang please provide more logs, I dont see a panic above. I only see that we cannot release a DMA channel. Please discuss internally with @plbossart

tlauda · 2020-04-09T08:15:34Z

This code for sure causes agent panic. Such long wait shouldn't be done with interrupts disabled. Every timeout will kill DSP immediately. I don't understand why we're trying to close this.
Please also note that this problem is reported only with thesofproject/linux#1897, which changes start/stop sequence for ALH and not on master.

lgirdwood · 2020-04-09T08:27:20Z

@tlauda it was closed until internal alignment.

RanderWang · 2020-04-09T09:58:03Z

@tlauda it was closed until internal alignment.

@lgirdwood Pierre, @slawblauciak and me got a conclusion that we need to change the start/stop sequence for sdw. With the sequence of HDA was changed (just tested HDA, not changed. But HDA doesn't use GP-DMA), QA also reported DSP panic. All the kernel or FW logs are at https://sof-ci.01.org/linuxpr/PR1897/build3520/devicetest/CML_RVP_SDW/check-pause-resume-playback-10/

lgirdwood · 2020-04-09T10:07:13Z

@RanderWang I've not seen any internal alignment. Please make sure I am on any email alongside @plbossart and @lbetlej .

RanderWang · 2020-04-14T06:22:56Z

@RanderWang I've not seen any internal alignment. Please make sure I am on any email alongside @plbossart and @lbetlej .

@lgirdwood we discussed int microsoft team. please check the picture (edited)

lgirdwood · 2020-04-15T21:17:52Z

@RanderWang this make no sense, it's unreadable.

plbossart · 2020-04-15T22:25:18Z

@RanderWang this make no sense, it's unreadable.

@lgirdwood the sequence used so far for SoundWire is broken and there's consensus to change it. thesofproject/linux#1897 was updated to use the same sequence for all DAIs, and keep the weird sequence for HDAudio - we still don't have a clue about the firmware underflows and panic issues for the HDaudio link DMA.

lgirdwood

@ktrzcinx what happens here when the DMA channel FIFO has data when the stream is stopped ? How can the data be cleared and channel recovered ?

ktrzcinx · 2020-04-17T06:36:59Z

@lgirdwood As I know FIFO should be cleared after disabling DMA enable bit, what is going to be done in dw_dma_stop() called from dw_dma_release()

lgirdwood · 2020-04-17T09:02:00Z

SOFCI TEST

lgirdwood · 2020-04-20T08:50:57Z

@xiulipan I guess Jenkins is back now ? Do we need to restart CI ?

xiulipan · 2020-04-20T09:52:35Z

@lgirdwood I think for this PR, we may need to restart the test.

xiulipan · 2020-04-20T09:52:50Z

SOFCI TEST

lgirdwood · 2020-04-21T08:42:11Z

CI known issues.

ktrzcinx requested a review from lyakh as a code owner March 31, 2020 10:45

ktrzcinx mentioned this pull request Mar 31, 2020
[BUG][CML]DSP Panic occurred when pause/resume on aplay0,2(RT1308) #2658
Closed

ktrzcinx force-pushed the sdw-fix-release branch from d954a81 to 8f6ff11Compare March 31, 2020 13:15

ktrzcinx force-pushed the sdw-fix-release branch from 8f6ff11 to f01e607Compare March 31, 2020 13:58

lgirdwood reviewed Apr 1, 2020
View reviewed changes

lgirdwood closed this Apr 6, 2020

RanderWang mentioned this pull request Apr 9, 2020
ASoC: SOF: change trigger sequence to fix pop noise when stopping playback on sdw platforms thesofproject/linux#1897
Merged

RanderWang reopened this Apr 9, 2020

mengdonglin added the P1 Blocker bugs or important features label Apr 10, 2020

lgirdwood reviewed Apr 16, 2020
View reviewed changes

lgirdwood merged commit ab0c9e0 into thesofproject:masterApr 21, 2020

Uh oh!

Conversation

ktrzcinx commented Mar 31, 2020

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

slawblauciak commented Apr 3, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

slawblauciak commented Apr 3, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

slawblauciak commented Apr 3, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

slawblauciak commented Apr 3, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

slawblauciak commented Apr 3, 2020

Uh oh!

plbossart commented Apr 3, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

plbossart commented Apr 3, 2020

Uh oh!

slawblauciak commented Apr 6, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Apr 6, 2020

Uh oh!

slawblauciak commented Apr 6, 2020

Uh oh!

paulstelian97 commented Apr 6, 2020

Uh oh!

RanderWang commented Apr 9, 2020

Uh oh!

lgirdwood commented Apr 9, 2020

Uh oh!

tlauda commented Apr 9, 2020

Uh oh!

lgirdwood commented Apr 9, 2020

Uh oh!

RanderWang commented Apr 9, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Apr 9, 2020

Uh oh!

RanderWang commented Apr 14, 2020

Uh oh!

lgirdwood commented Apr 15, 2020

Uh oh!

plbossart commented Apr 15, 2020

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

ktrzcinx commented Apr 17, 2020

Uh oh!

lgirdwood commented Apr 17, 2020

Uh oh!

lgirdwood commented Apr 20, 2020

Uh oh!

xiulipan commented Apr 20, 2020

Uh oh!

xiulipan commented Apr 20, 2020

Uh oh!

lgirdwood commented Apr 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

slawblauciak commented Apr 6, 2020•
edited
Loading

RanderWang commented Apr 9, 2020•
edited
Loading