Re: [PATCH 1/2] ASoC: dwc: add a quirk DW_I2S_QUIRK_STOP_ON_SHUTDOWN to dwc driver

[Mukunda,Vijendar]

On 5/1/21 12:05 AM, Mukunda,Vijendar wrote:
> On 4/30/21 11:12 AM, Péter Ujfalusi wrote:
> > On 28.4.2021 18.35, Mukunda,Vijendar wrote:
> > > > Thanks for the explanation.
> > > > This is not upstream, right?
> > >
> > > Driver is already upstreamed.
> > > Stoneyridge platform based products already into market and working fine
> > > with 4.14 kernel version.
> > > Currently Kernel migration from v4.14 to v5.10 is in progress for
> > > Stoneyridge platform and release got blocked due to Audio use cases
> > > failures.
> > > In v5.10 kernel base, re-ordering of stop trigger sequence is causing
> > > DMA channel stop failure for both playback & capture use cases.
> >
> > The dai - pcm start/stop ordering has changed in v5.5, more than a year
> > ago. If the support is upstream it should have been noticed by users.
>
> AMD partner using their own repositories where kernel is not migrated to 
> 5.10. Still products are running with v4.14 kernel only.
> That's why users hasn't faced this issue.
> > > > What is still not clear to me is which channel fails?
> > > > A) the DMA between ACP FIFO and the I2S
> > > > B) the DMA between ACP FIFO and system memory
> > >
> > > There is difference for playback and Capture use cases.
> > >
> > > Playback:
> > >
> > > channel 1 : DMA transfer from System memory -> ACP memory
> > > channel 2 : DMA transfer from ACP memory -> I2S memory
> > >
> > > Capture:
> > >
> > > channel 1: DMA transfer from I2S memory to ACP memory
> > > channel 2: DMA transfer from ACP memory to System memory
> >
> > Yes, this is why I used A and B to refer to the DMA channels.
> >
> > > In case of playback, Channel 2 is failing where as in case of
> > > capture channel 1 is failing.
> >
> > So channel A is failing.
> >
> > > > in acp-pcm-dma.c on stop you have a busy loop (10000 iterations) to
> > > > check if the channel is in fact stopped in response to the cleared run,
> > > > IOCEn bits and the set Rst bit.
> > >
> > > DMA channel run bit is cleared and Ioc bit also cleared for channel 2 in
> > > case of playback .
> > > After that as part of DMA stop sequence, DMA channel reset is applied.
> > > When DMA channel status is polled for stop, its failed to stop.
> > > > Channel closer to the destination is stopped first which sounds
> > > > reasonable, but on playback you ignore timeout from A, on capture you
> > > > ignore the timeout from B.
> > >
> > > Please refer above explanation.
> > >
> > > > Still the issue sounds like exactly what I have described. One of the
> > > > DMA is failing to drain because the IP is stopped?
> > >
> > > As per our understanding, failing to stop the DMA by hardware is causing
> > > the issue.
> > >
> > > > > Need find a right place to implement a work around only for AMD
> > > > > stoneyridge platform.
> > > >
> > > > Is this really only affecting stoneyridge platform? Are there other
> > > > platforms using drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c ?
> > > This design is being used only in stoneyridge and Carrizo platforms.
> > > But only stoneyridge platform is productized.
> > > New design is implemented for later generations of APU series.
> >
> > Right.
> >
> > This version of ACP is only used with the designware I2S IP?
> Yes this version of ACP only uses designware I2S controller.
> > I would try to find a way to force stop the DMA in case the DAI has been
> > already stopped.
> >
> > If this is not possible than the only solution is to do this in core, 
> > imho.
> >
> > For that you would need a flag to say that the platform (DMA) needs the
> > DAI to be active when stopping it.
> >
> > If the same ACP have problems with different DAIs, then it is a property
> > of the platform driver.
> > If the ACP only have problem against the designware I2S then it is a
> > link property.
> > If this ACP only used with the designware I2S then it is again, most
> > likely the property of the platform driver.
> Hardware signal broken between ACP and Designware I2S controller with 
> re-ordering the sequence.
>   > It is surely not a designware IP issue, trying to solve it there is 
> wrong.
> >
> As it's not a designware IP issue, initially we started idea with 
> introducing quirk that applies for this ACP version based AMD platforms.

Hi Peter,

Any suggestion on the work around for this issue?
How about declaring a flag in sound card structure and this flag will be 
set in stoneyridge machine driver.

Based on flag check trigger stop sequence will be re-ordered.

Thanks,
Vijendar