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]

Message-ID: <b86ad586-9513-8fa9-54e3-a0b4a3a7fd92@amd.com>
Date:   Wed, 28 Apr 2021 21:05:45 +0530
From:   "Mukunda,Vijendar" <vijendar.mukunda@....com>
To:     Péter Ujfalusi <peter.ujfalusi@...il.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


>> ACP DMA Driver is responsible for DMA transactions between system memory
>> and I2S controller.It uses two step DMA mechanism to copy data between
>> system memory <-> ACP SRAM and ACP SRAM <-> I2S FIFO for
>> playback/capture use cases.
>> ACP driver program two DMA channels for DMA transfers between System
>> memory & I2S FIFO.
>>
>> ACP DMA driver isn't general purpose DMA controller driver where we can
>> implement terminate_all() API.
>>
>> I2S controller DMA transactions are tightly coupled with ACP DMA
>> controller.
>> while DMA transfer ongoing between ACP SRAM and I2S FIFO, Stopping I2S
>> DMA prior to ACP DMA stop resulting DMA Channel stop failure.
>> Its not related to I2S FIFO flushing related handling.
>> Once the DMA channel failure observed during the closure of the stream,
>> when again new stream opened, DMA won't progress at all.
> 
> 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.

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

In case of playback, Channel 2 is failing where as in case of
capture channel 1 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.