Re: [PATCH] ASoC: meson: Use managed DMA buffer allocation

On Fri 18 Dec 2020 at 18:41, Lars-Peter Clausen lars@metafoo.de wrote:

On 12/18/20 5:28 PM, Jerome Brunet wrote:

...

On Fri 18 Dec 2020 at 16:45, Lars-Peter Clausen lars@metafoo.de wrote:

...

Using a managed buffer will pre-allocate the buffer using snd_pcm_lib_preallocate_pages() and automatically free it when the PCM is destroyed.

In addition it will call snd_pcm_lib_malloc_pages() before the driver's hw_params() callback and snd_pcm_lib_free_pages() after the driver's hw_free() callback.

This slightly reduces the boilerplate code of the driver.

Signed-off-by: Lars-Peter Clausen lars@metafoo.de

sound/soc/meson/aiu-fifo-i2s.c | 1 - sound/soc/meson/aiu-fifo-spdif.c | 1 - sound/soc/meson/aiu-fifo.c | 18 ++---------------- 3 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/sound/soc/meson/aiu-fifo-i2s.c b/sound/soc/meson/aiu-fifo-i2s.c index d91b0d874342..2388a2d0b3a6 100644 --- a/sound/soc/meson/aiu-fifo-i2s.c +++ b/sound/soc/meson/aiu-fifo-i2s.c @@ -124,7 +124,6 @@ const struct snd_soc_dai_ops aiu_fifo_i2s_dai_ops = { .trigger = aiu_fifo_i2s_trigger, .prepare = aiu_fifo_i2s_prepare, .hw_params = aiu_fifo_i2s_hw_params,

• .hw_free = aiu_fifo_hw_free, .startup = aiu_fifo_startup, .shutdown = aiu_fifo_shutdown, };

diff --git a/sound/soc/meson/aiu-fifo-spdif.c b/sound/soc/meson/aiu-fifo-spdif.c index 44eb6faacf44..2fb30f89bf7a 100644 --- a/sound/soc/meson/aiu-fifo-spdif.c +++ b/sound/soc/meson/aiu-fifo-spdif.c @@ -158,7 +158,6 @@ const struct snd_soc_dai_ops aiu_fifo_spdif_dai_ops = { .trigger = fifo_spdif_trigger, .prepare = fifo_spdif_prepare, .hw_params = fifo_spdif_hw_params,

• .hw_free = aiu_fifo_hw_free, .startup = aiu_fifo_startup, .shutdown = aiu_fifo_shutdown, };

diff --git a/sound/soc/meson/aiu-fifo.c b/sound/soc/meson/aiu-fifo.c index aa88aae8e517..4ad23267cace 100644 --- a/sound/soc/meson/aiu-fifo.c +++ b/sound/soc/meson/aiu-fifo.c @@ -99,11 +99,6 @@ int aiu_fifo_hw_params(struct snd_pcm_substream *substream, struct snd_soc_component *component = dai->component; struct aiu_fifo *fifo = dai->playback_dma_data; dma_addr_t end;

• int ret;
• ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
• if (ret < 0)

• return ret;
  /* Setup the fifo boundaries */
  

  end = runtime->dma_addr + runtime->dma_bytes - fifo->fifo_block;

@@ -124,12 +119,6 @@ int aiu_fifo_hw_params(struct snd_pcm_substream *substream, return 0; } -int aiu_fifo_hw_free(struct snd_pcm_substream *substream,

•      struct snd_soc_dai *dai)
  

-{

• return snd_pcm_lib_free_pages(substream);

-}

• static irqreturn_t aiu_fifo_isr(int irq, void *dev_id) { struct snd_pcm_substream *playback = dev_id;

@@ -187,15 +176,12 @@ void aiu_fifo_shutdown(struct snd_pcm_substream *substream, int aiu_fifo_pcm_new(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *dai) {

• struct snd_pcm_substream *substream =

• rtd->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
  

  struct snd_card *card = rtd->card->snd_card; struct aiu_fifo *fifo = dai->playback_dma_data; size_t size = fifo->pcm->buffer_bytes_max;
  • snd_pcm_lib_preallocate_pages(substream,

• 		      SNDRV_DMA_TYPE_DEV,
  

• 		      card->dev, size, size);
  

• snd_pcm_set_managed_buffer_all(rtd->pcm, SNDRV_DMA_TYPE_DEV,

• 		       card->dev, size, size);
  

Hi Lars-Peter,

These FIFOs only do playback so to avoid wasting memory s/snd_pcm_set_managed_buffer_all/snd_pcm_set_managed_buffer ?

snd_pcm_set_managed_buffer_all() will skip substreams that do not exist. E.g. if the there is not capture support it wont allocate memory for it.

Indeed, Thanks !

Reviewed-by: Jerome Brunet jbrunet@baylibre.com Tested-by: Jerome Brunet jbrunet@baylibre.com

To keep things simple I prefer snd_pcm_set_managed_buffer_all(). snd_pcm_set_managed_buffer() only makes sense if you have a different DMA device for capture and playback or you want different buffer sizes.