[alsa-devel] Incorrect buffer handling in dw-hdmi bridge audio

[Neil Armstrong]

Hi,

On 05/11/2019 08:55, Takashi Iwai wrote:
> Hi,
> 
> while recently working on the ALSA memory allocator API cleanup, I
> noticed that dw-hdmi bridge driver seems doing weird about the buffer
> management.  It pre-allocates the usual device buffers fully at the
> probe time, while each stream allocates the buffer via the vmalloc
> helpers and replaces with it at each open.
> 
> I guess it's no expected behavior?  It's basically a full waste of
> resources, and the vmalloc buffer isn't guaranteed to work well for
> mmap on every architecture.
> 
> Below is the patch to address it.  Can anyone check whether this still
> works?

I don't have the setup to check, but this has been pushed by Russell I Added in CC.

I also added the imx maintainer since it's (AFAIK) only used on iMX SoCs.

Neil

> 
> Since I have a cleanup series and this is involved, I'd like to take
> the fix through my tree once after it's confirmed (and get ACK if
> possible).
> 
> 
> Thanks!
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai at suse.de>
> Subject: [PATCH] drm/bridge: dw-hdmi: Fix the incorrect buffer allocations
> 
> The driver sets up the buffer preallocation with SNDRV_DMA_TYPE_DEV,
> while it re-allocates and releases vmalloc pages.  This is not only a
> lot of waste resources but also causes the mmap malfunction.
> 
> Change / drop the vmalloc-related code and use the standard buffer
> allocation / release code instead.
> 
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 6 ++----
>  1 file changed, 2 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 2b7539701b42..8fe7a6e8ff94 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -384,15 +384,14 @@ 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);
> +	return snd_pcm_lib_free_pages(substream);
>  }
>  
>  static int dw_hdmi_hw_params(struct snd_pcm_substream *substream,
>  	struct snd_pcm_hw_params *params)
>  {
>  	/* Allocate the PCM runtime buffer, which is exposed to userspace. */
> -	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
> -						params_buffer_bytes(params));
> +	return snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
>  }
>  
>  static int dw_hdmi_prepare(struct snd_pcm_substream *substream)
> @@ -511,7 +510,6 @@ 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,
>  };
>  
>  static int snd_dw_hdmi_probe(struct platform_device *pdev)
>