| Message ID | 20201218154544.25513-1-lars@metafoo.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: meson: Use managed DMA buffer allocation | expand |
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 ? > > return 0; > }
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. 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.
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.
On Fri, 18 Dec 2020 16:45:44 +0100, Lars-Peter Clausen 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. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: meson: Use managed DMA buffer allocation commit: e05cde84eabccc0441f837f0661cd4c6f4820513 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
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); return 0; }
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(-)