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

From: Péter Ujfalusi
Date: Thu Jun 06 2024 - 13:24:35 EST




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