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: <9b689495-e956-6242-0784-af3ccf7c3238@amd.com>
Date:   Mon, 10 May 2021 22:57:25 +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



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