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

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]

Date: Mon, 10 Jun 2024 17:45:52 -0700
From: Lars-Peter Clausen <lars@...afoo.de>
To: Jai Luthra <j-luthra@...com>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, Peter Ujfalusi <peter.ujfalusi@...il.com>
Cc: linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
 alsa-devel@...a-project.org, Devarsh Thakkar <devarsht@...com>,
 Vignesh Raghavendra <vigneshr@...com>, Jayesh Choudhary <j-choudhary@...com>
Subject: Re: [PATCH v2 1/2] ALSA: dmaengine: Synchronize dma channel in
 prepare()

On 6/10/24 03:56, 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.


We should really implement the sync_stop() PCM callback and let the ALSA 
core let care of the sync.


> Reviewed-by: Peter Ujfalusi <peter.ujfalusi@...il.com>
> Signed-off-by: Jai Luthra <j-luthra@...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..078fcb0ba8a2 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;
>   }
>   
> +static 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[] = {
>