linux-kernel - Re: [PATCH 1/2] ASoC: dwc: add a quirk DW_I2S_QUIRK_STOP_ON

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 30 Apr 2021 08:42:04 +0300
From:   Péter Ujfalusi <peter.ujfalusi@...il.com>
To:     "Mukunda,Vijendar" <vijendar.mukunda@....com>,
        Mark Brown <broonie@...nel.org>
Cc:     alsa-devel@...a-project.org, amistry@...gle.com,
        nartemiev@...gle.com, Alexander.Deucher@....com,
        Basavaraj.Hiregoudar@....com, Sunil-kumar.Dommati@....com,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] ASoC: dwc: add a quirk DW_I2S_QUIRK_STOP_ON_SHUTDOWN
 to dwc driver



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.

>> 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?

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.

It is surely not a designware IP issue, trying to solve it there is wrong.

-- 
Péter