[PATCH 0/2] ALSA/DRM: vmalloc PCM buffer helper cleanup

[PATCH 0/2] ALSA/DRM: vmalloc PCM buffer helper cleanup

[Takashi Iwai]

Hi,

we still keep the legacy vmalloc ALSA PCM helper API since there is
only a single user of it, namely, the DRM dw-hdmi bridge driver.
In the past, I attempted to change the buffer management of the
driver, but due to the lack of testing, it failed to get merged:
  https://lore.kernel.org/20191210154536.29819-1-tiwai@suse.de

So, this is a second try after some years of gap: now the relevant
code snippet is moved from ALSA core to dw-hdmi driver itself, and the
API functions are dropped.  There should be no functional changes in
the driver.

If DRM people don't mind and give an ack, I can take the change for
dw-hdmi code together with ALSA core code drop via sound tree for
6.12.


thanks,

Takashi

===

Takashi Iwai (2):
  drm/bridge: dw-hdmi: Move vmalloc PCM buffer management into the
    driver
  ALSA: pcm: Drop PCM vmalloc buffer helpers

 .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   | 30 ++++++++--
 include/sound/pcm.h                           | 42 -------------
 sound/core/pcm_memory.c                       | 59 -------------------
 3 files changed, 26 insertions(+), 105 deletions(-)

-- 
2.43.0

[PATCH 1/2] drm/bridge: dw-hdmi: Move vmalloc PCM buffer management into the driver

[Takashi Iwai]

The dw-hdmi drm bridge driver is the only one who still uses the ALSA
vmalloc helper API functions.  A previous attempt to change the way of
buffer management wasn't taken for this legacy stuff, as we had little
chance for test and some risk of major breaking.
Instead, this patch moves the vmalloc buffer stuff into the dw-hdmi
driver code itself, so that we can drop them from ALSA core code
afterwards.

There should be no functional changes.

Link: https://lore.kernel.org/20191210154536.29819-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   | 30 ++++++++++++++++---
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
index 67b8d17a722a..221e9a4edb40 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
@@ -8,6 +8,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/vmalloc.h>
 #include <drm/bridge/dw_hdmi.h>
 #include <drm/drm_edid.h>
 
@@ -388,15 +389,36 @@ static int dw_hdmi_close(struct snd_pcm_substream *substream)
 
 static int dw_hdmi_hw_free(struct snd_pcm_substream *substream)
 {
-	return snd_pcm_lib_free_vmalloc_buffer(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	vfree(runtime->dma_area);
+	runtime->dma_area = NULL;
+	return 0;
 }
 
 static int dw_hdmi_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params)
 {
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	size_t size = params_buffer_bytes(params);
+
 	/* Allocate the PCM runtime buffer, which is exposed to userspace. */
-	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
-						params_buffer_bytes(params));
+	if (runtime->dma_area) {
+		if (runtime->dma_bytes >= size)
+			return 0; /* already large enough */
+		vfree(runtime->dma_area);
+	}
+	runtime->dma_area = vzalloc(size);
+	if (!runtime->dma_area)
+		return -ENOMEM;
+	runtime->dma_bytes = size;
+	return 1;
+}
+
+static struct page *dw_hdmi_get_page(struct snd_pcm_substream *substream,
+				     unsigned long offset)
+{
+	return vmalloc_to_page(substream->runtime->dma_area + offset);
 }
 
 static int dw_hdmi_prepare(struct snd_pcm_substream *substream)
@@ -515,7 +537,7 @@ static const struct snd_pcm_ops snd_dw_hdmi_ops = {
 	.prepare = dw_hdmi_prepare,
 	.trigger = dw_hdmi_trigger,
 	.pointer = dw_hdmi_pointer,
-	.page = snd_pcm_lib_get_vmalloc_page,
+	.page = dw_hdmi_get_page,
 };
 
 static int snd_dw_hdmi_probe(struct platform_device *pdev)
-- 
2.43.0

Re: [PATCH 1/2] drm/bridge: dw-hdmi: Move vmalloc PCM buffer management into the driver

[Neil Armstrong]

On 07/08/2024 17:27, Takashi Iwai wrote:
> The dw-hdmi drm bridge driver is the only one who still uses the ALSA
> vmalloc helper API functions.  A previous attempt to change the way of
> buffer management wasn't taken for this legacy stuff, as we had little
> chance for test and some risk of major breaking.
> Instead, this patch moves the vmalloc buffer stuff into the dw-hdmi
> driver code itself, so that we can drop them from ALSA core code
> afterwards.
> 
> There should be no functional changes.
> 
> Link: https://lore.kernel.org/20191210154536.29819-1-tiwai@suse.de
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   | 30 ++++++++++++++++---
>   1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index 67b8d17a722a..221e9a4edb40 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -8,6 +8,7 @@
>   #include <linux/interrupt.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> +#include <linux/vmalloc.h>
>   #include <drm/bridge/dw_hdmi.h>
>   #include <drm/drm_edid.h>
>   
> @@ -388,15 +389,36 @@ static int dw_hdmi_close(struct snd_pcm_substream *substream)
>   
>   static int dw_hdmi_hw_free(struct snd_pcm_substream *substream)
>   {
> -	return snd_pcm_lib_free_vmalloc_buffer(substream);
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +	vfree(runtime->dma_area);
> +	runtime->dma_area = NULL;
> +	return 0;
>   }
>   
>   static int dw_hdmi_hw_params(struct snd_pcm_substream *substream,
>   	struct snd_pcm_hw_params *params)
>   {
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	size_t size = params_buffer_bytes(params);
> +
>   	/* Allocate the PCM runtime buffer, which is exposed to userspace. */
> -	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
> -						params_buffer_bytes(params));
> +	if (runtime->dma_area) {
> +		if (runtime->dma_bytes >= size)
> +			return 0; /* already large enough */
> +		vfree(runtime->dma_area);
> +	}
> +	runtime->dma_area = vzalloc(size);
> +	if (!runtime->dma_area)
> +		return -ENOMEM;
> +	runtime->dma_bytes = size;
> +	return 1;
> +}
> +
> +static struct page *dw_hdmi_get_page(struct snd_pcm_substream *substream,
> +				     unsigned long offset)
> +{
> +	return vmalloc_to_page(substream->runtime->dma_area + offset);
>   }
>   
>   static int dw_hdmi_prepare(struct snd_pcm_substream *substream)
> @@ -515,7 +537,7 @@ static const struct snd_pcm_ops snd_dw_hdmi_ops = {
>   	.prepare = dw_hdmi_prepare,
>   	.trigger = dw_hdmi_trigger,
>   	.pointer = dw_hdmi_pointer,
> -	.page = snd_pcm_lib_get_vmalloc_page,
> +	.page = dw_hdmi_get_page,
>   };
>   
>   static int snd_dw_hdmi_probe(struct platform_device *pdev)

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

[PATCH 2/2] ALSA: pcm: Drop PCM vmalloc buffer helpers

[Takashi Iwai]

As the last-standing user of PCM vmalloc buffer helper API took its
own buffer management, we can finally drop those API functions, which
were leftover after reorganization of ALSA memalloc code.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h     | 42 -----------------------------
 sound/core/pcm_memory.c | 59 -----------------------------------------
 2 files changed, 101 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index ac8f3aef9205..3c56a648bdcd 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1355,48 +1355,6 @@ snd_pcm_set_fixed_buffer_all(struct snd_pcm *pcm, int type,
 	return snd_pcm_set_managed_buffer_all(pcm, type, data, size, 0);
 }
 
-int _snd_pcm_lib_alloc_vmalloc_buffer(struct snd_pcm_substream *substream,
-				      size_t size, gfp_t gfp_flags);
-int snd_pcm_lib_free_vmalloc_buffer(struct snd_pcm_substream *substream);
-struct page *snd_pcm_lib_get_vmalloc_page(struct snd_pcm_substream *substream,
-					  unsigned long offset);
-/**
- * snd_pcm_lib_alloc_vmalloc_buffer - allocate virtual DMA buffer
- * @substream: the substream to allocate the buffer to
- * @size: the requested buffer size, in bytes
- *
- * Allocates the PCM substream buffer using vmalloc(), i.e., the memory is
- * contiguous in kernel virtual space, but not in physical memory.  Use this
- * if the buffer is accessed by kernel code but not by device DMA.
- *
- * Return: 1 if the buffer was changed, 0 if not changed, or a negative error
- * code.
- */
-static inline int snd_pcm_lib_alloc_vmalloc_buffer
-			(struct snd_pcm_substream *substream, size_t size)
-{
-	return _snd_pcm_lib_alloc_vmalloc_buffer(substream, size,
-						 GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
-}
-
-/**
- * snd_pcm_lib_alloc_vmalloc_32_buffer - allocate 32-bit-addressable buffer
- * @substream: the substream to allocate the buffer to
- * @size: the requested buffer size, in bytes
- *
- * This function works like snd_pcm_lib_alloc_vmalloc_buffer(), but uses
- * vmalloc_32(), i.e., the pages are allocated from 32-bit-addressable memory.
- *
- * Return: 1 if the buffer was changed, 0 if not changed, or a negative error
- * code.
- */
-static inline int snd_pcm_lib_alloc_vmalloc_32_buffer
-			(struct snd_pcm_substream *substream, size_t size)
-{
-	return _snd_pcm_lib_alloc_vmalloc_buffer(substream, size,
-						 GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
-}
-
 #define snd_pcm_get_dma_buf(substream) ((substream)->runtime->dma_buffer_p)
 
 /**
diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index 506386959f08..8e4c68e3bbd0 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -9,7 +9,6 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/moduleparam.h>
-#include <linux/vmalloc.h>
 #include <linux/export.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -497,61 +496,3 @@ int snd_pcm_lib_free_pages(struct snd_pcm_substream *substream)
 	return 0;
 }
 EXPORT_SYMBOL(snd_pcm_lib_free_pages);
-
-int _snd_pcm_lib_alloc_vmalloc_buffer(struct snd_pcm_substream *substream,
-				      size_t size, gfp_t gfp_flags)
-{
-	struct snd_pcm_runtime *runtime;
-
-	if (PCM_RUNTIME_CHECK(substream))
-		return -EINVAL;
-	runtime = substream->runtime;
-	if (runtime->dma_area) {
-		if (runtime->dma_bytes >= size)
-			return 0; /* already large enough */
-		vfree(runtime->dma_area);
-	}
-	runtime->dma_area = __vmalloc(size, gfp_flags);
-	if (!runtime->dma_area)
-		return -ENOMEM;
-	runtime->dma_bytes = size;
-	return 1;
-}
-EXPORT_SYMBOL(_snd_pcm_lib_alloc_vmalloc_buffer);
-
-/**
- * snd_pcm_lib_free_vmalloc_buffer - free vmalloc buffer
- * @substream: the substream with a buffer allocated by
- *	snd_pcm_lib_alloc_vmalloc_buffer()
- *
- * Return: Zero if successful, or a negative error code on failure.
- */
-int snd_pcm_lib_free_vmalloc_buffer(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime;
-
-	if (PCM_RUNTIME_CHECK(substream))
-		return -EINVAL;
-	runtime = substream->runtime;
-	vfree(runtime->dma_area);
-	runtime->dma_area = NULL;
-	return 0;
-}
-EXPORT_SYMBOL(snd_pcm_lib_free_vmalloc_buffer);
-
-/**
- * snd_pcm_lib_get_vmalloc_page - map vmalloc buffer offset to page struct
- * @substream: the substream with a buffer allocated by
- *	snd_pcm_lib_alloc_vmalloc_buffer()
- * @offset: offset in the buffer
- *
- * This function is to be used as the page callback in the PCM ops.
- *
- * Return: The page struct, or %NULL on failure.
- */
-struct page *snd_pcm_lib_get_vmalloc_page(struct snd_pcm_substream *substream,
-					  unsigned long offset)
-{
-	return vmalloc_to_page(substream->runtime->dma_area + offset);
-}
-EXPORT_SYMBOL(snd_pcm_lib_get_vmalloc_page);
-- 
2.43.0

Re: [PATCH 0/2] ALSA/DRM: vmalloc PCM buffer helper cleanup

[Takashi Iwai]

On Wed, 07 Aug 2024 17:27:21 +0200,
Takashi Iwai wrote:
> 
> Hi,
> 
> we still keep the legacy vmalloc ALSA PCM helper API since there is
> only a single user of it, namely, the DRM dw-hdmi bridge driver.
> In the past, I attempted to change the buffer management of the
> driver, but due to the lack of testing, it failed to get merged:
>   https://lore.kernel.org/20191210154536.29819-1-tiwai@suse.de
> 
> So, this is a second try after some years of gap: now the relevant
> code snippet is moved from ALSA core to dw-hdmi driver itself, and the
> API functions are dropped.  There should be no functional changes in
> the driver.
> 
> If DRM people don't mind and give an ack, I can take the change for
> dw-hdmi code together with ALSA core code drop via sound tree for
> 6.12.

Gentle ping to any DRM reviewers.  It's merely a safe clean up, so if
you don't mind, I'd like to take for 6.12.


thanks,

Takashi

> 
> 
> thanks,
> 
> Takashi
> 
> ===
> 
> Takashi Iwai (2):
>   drm/bridge: dw-hdmi: Move vmalloc PCM buffer management into the
>     driver
>   ALSA: pcm: Drop PCM vmalloc buffer helpers
> 
>  .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   | 30 ++++++++--
>  include/sound/pcm.h                           | 42 -------------
>  sound/core/pcm_memory.c                       | 59 -------------------
>  3 files changed, 26 insertions(+), 105 deletions(-)
> 
> -- 
> 2.43.0
> 

Re: [PATCH 0/2] ALSA/DRM: vmalloc PCM buffer helper cleanup

[Takashi Iwai]

On Thu, 15 Aug 2024 10:03:40 +0200,
Takashi Iwai wrote:
> 
> On Wed, 07 Aug 2024 17:27:21 +0200,
> Takashi Iwai wrote:
> > 
> > Hi,
> > 
> > we still keep the legacy vmalloc ALSA PCM helper API since there is
> > only a single user of it, namely, the DRM dw-hdmi bridge driver.
> > In the past, I attempted to change the buffer management of the
> > driver, but due to the lack of testing, it failed to get merged:
> >   https://lore.kernel.org/20191210154536.29819-1-tiwai@suse.de
> > 
> > So, this is a second try after some years of gap: now the relevant
> > code snippet is moved from ALSA core to dw-hdmi driver itself, and the
> > API functions are dropped.  There should be no functional changes in
> > the driver.
> > 
> > If DRM people don't mind and give an ack, I can take the change for
> > dw-hdmi code together with ALSA core code drop via sound tree for
> > 6.12.
> 
> Gentle ping to any DRM reviewers.  It's merely a safe clean up, so if
> you don't mind, I'd like to take for 6.12.

Now merged to for-next branch of sound.git tree.


Takashi