Received-SPF: pass (google.com: domain of linux-kernel+bounces-204793-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <43418c73-b240-461f-af4e-49735304eaa5@gmail.com>
Date: Thu, 6 Jun 2024 20:28:00 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] ALSA: dmaengine: Synchronize dma channel in prepare()
To: Jai Luthra <j-luthra@ti.com>, Lars-Peter Clausen <lars@metafoo.de>,
 Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
 Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>
Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
 alsa-devel@alsa-project.org
References: <20240604-asoc_next-v1-0-e895c88e744d@ti.com>
 <20240604-asoc_next-v1-1-e895c88e744d@ti.com>
Content-Language: en-US
From: =?UTF-8?Q?P=C3=A9ter_Ujfalusi?= <peter.ujfalusi@gmail.com>
In-Reply-To: <20240604-asoc_next-v1-1-e895c88e744d@ti.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 6/4/24 1:01 PM, Jai Luthra wrote:
> Sometimes the stream may be stopped due to XRUN events, in which case
> the userspace can call snd_pcm_drop() and snd_pcm_prepare() to stop and
> start the stream again.
> 
> In these cases, we must wait for the DMA channel to synchronize before
> marking the stream as prepared for playback, as the DMA channel gets
> stopped by snd_pcm_drop() without any synchronization.

Right, this is a plausible scenario, the xrun is pointing to unhealthy
system, but before re-using the channel we indeed need to make sure that
it is finished with the stop.

Reviewed-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> ---
>  include/sound/dmaengine_pcm.h         |  1 +
>  sound/core/pcm_dmaengine.c            | 10 ++++++++++
>  sound/soc/soc-generic-dmaengine-pcm.c |  8 ++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index c11aaf8079fb..9c5800e5659f 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -36,6 +36,7 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>  int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>  	struct dma_chan *chan);
>  int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
> +int snd_dmaengine_pcm_prepare(struct snd_pcm_substream *substream);
>  
>  int snd_dmaengine_pcm_open_request_chan(struct snd_pcm_substream *substream,
>  	dma_filter_fn filter_fn, void *filter_data);
> diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c
> index 12aa1cef11a1..dbf5c6136d68 100644
> --- a/sound/core/pcm_dmaengine.c
> +++ b/sound/core/pcm_dmaengine.c
> @@ -349,6 +349,16 @@ int snd_dmaengine_pcm_open_request_chan(struct snd_pcm_substream *substream,
>  }
>  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open_request_chan);
>  
> +int snd_dmaengine_pcm_prepare(struct snd_pcm_substream *substream)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +
> +	dmaengine_synchronize(prtd->dma_chan);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_prepare);
> +
>  /**
>   * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
>   * @substream: PCM substream
> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> index ea3bc9318412..af439486d33a 100644
> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> @@ -318,6 +318,12 @@ static int dmaengine_copy(struct snd_soc_component *component,
>  	return 0;
>  }
>  
> +int dmaengine_pcm_prepare(struct snd_soc_component *component,
> +			  struct snd_pcm_substream *substream)
> +{
> +	return snd_dmaengine_pcm_prepare(substream);
> +}
> +
>  static const struct snd_soc_component_driver dmaengine_pcm_component = {
>  	.name		= SND_DMAENGINE_PCM_DRV_NAME,
>  	.probe_order	= SND_SOC_COMP_ORDER_LATE,
> @@ -327,6 +333,7 @@ static const struct snd_soc_component_driver dmaengine_pcm_component = {
>  	.trigger	= dmaengine_pcm_trigger,
>  	.pointer	= dmaengine_pcm_pointer,
>  	.pcm_construct	= dmaengine_pcm_new,
> +	.prepare	= dmaengine_pcm_prepare,
>  };
>  
>  static const struct snd_soc_component_driver dmaengine_pcm_component_process = {
> @@ -339,6 +346,7 @@ static const struct snd_soc_component_driver dmaengine_pcm_component_process = {
>  	.pointer	= dmaengine_pcm_pointer,
>  	.copy		= dmaengine_copy,
>  	.pcm_construct	= dmaengine_pcm_new,
> +	.prepare	= dmaengine_pcm_prepare,
>  };
>  
>  static const char * const dmaengine_pcm_dma_channel_names[] = {
> 

-- 
Péter