Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     "Mukunda,Vijendar" <vijendar.mukunda@amd.com>,
        Mark Brown <broonie@kernel.org>
Cc:     alsa-devel@alsa-project.org, amistry@google.com,
        nartemiev@google.com, Alexander.Deucher@amd.com,
        Basavaraj.Hiregoudar@amd.com, Sunil-kumar.Dommati@amd.com,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
        Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
        open list <linux-kernel@vger.kernel.org>
References: <1619195089-29710-1-git-send-email-Vijendar.Mukunda@amd.com>
 <20210423164617.GG5507@sirena.org.uk>
 <e1268120-7a91-da49-0bb6-89d5cb4e2cce@gmail.com>
 <b32fcc42-d67e-bfbd-ed83-7f7274fb2f79@amd.com>
From:   =?UTF-8?Q?P=c3=a9ter_Ujfalusi?= <peter.ujfalusi@gmail.com>
Subject: Re: [PATCH 1/2] ASoC: dwc: add a quirk DW_I2S_QUIRK_STOP_ON_SHUTDOWN
 to dwc driver
Message-ID: <ac5244d1-643d-6577-80cd-bf6867e75ca2@gmail.com>
Date:   Tue, 27 Apr 2021 09:40:17 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.0
MIME-Version: 1.0
In-Reply-To: <b32fcc42-d67e-bfbd-ed83-7f7274fb2f79@amd.com>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 4/26/21 3:21 PM, Mukunda,Vijendar wrote:

>>> What is the design constraint here - can't we fix the design?? Or is it
>>> a hardware design constraint (presumably broken signalling between the
>>> I2S and DMA blocks)?
> 
> Its a hardware design constraint.
> 
> 
> I2S driver is not directly exposing DMA interface to host.
> ACP 2.x has unique design where ACP driver controls data flow between
> host and I2S as mentioned above.
> ACP IP has different IP blocks within it which includes I2S controller
> and DMA controller.
> 
> 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?

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

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.

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.

Still the issue sounds like exactly what I have described. One of the
DMA is failing to drain because the IP is stopped?

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

-- 
P?ter