Re: [PATCH 1/2] ALSA: dmaengine: Synchronize dma channel in prepare()

[Péter Ujfalusi]

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

> Signed-off-by: Jai Luthra <j-luthra@xxxxxx>
> ---
>  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