[PATCH 1/3] ASoC: atmel-pdc: Use managed DMA buffer allocation
Instead of manually managing its DMA buffers using dma_{alloc,free}_coherent() lets the sound core take care of this using managed buffers.
On one hand this reduces the amount of boiler plate code, but the main motivation for the change is to use the shared code where possible. This makes it easier to argue about correctness and that the code does not contain subtle bugs like data leakage or similar.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/atmel/atmel-pcm-pdc.c | 78 ++------------------------------- 1 file changed, 4 insertions(+), 74 deletions(-)
diff --git a/sound/soc/atmel/atmel-pcm-pdc.c b/sound/soc/atmel/atmel-pcm-pdc.c index 704f700013d3..3e7ea2021b46 100644 --- a/sound/soc/atmel/atmel-pcm-pdc.c +++ b/sound/soc/atmel/atmel-pcm-pdc.c @@ -34,86 +34,21 @@ #include "atmel-pcm.h"
-static int atmel_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, - int stream) -{ - struct snd_pcm_substream *substream = pcm->streams[stream].substream; - struct snd_dma_buffer *buf = &substream->dma_buffer; - size_t size = ATMEL_SSC_DMABUF_SIZE; - - buf->dev.type = SNDRV_DMA_TYPE_DEV; - buf->dev.dev = pcm->card->dev; - buf->private_data = NULL; - buf->area = dma_alloc_coherent(pcm->card->dev, size, - &buf->addr, GFP_KERNEL); - pr_debug("atmel-pcm: alloc dma buffer: area=%p, addr=%p, size=%zu\n", - (void *)buf->area, (void *)(long)buf->addr, size); - - if (!buf->area) - return -ENOMEM; - - buf->bytes = size; - return 0; -} - -static int atmel_pcm_mmap(struct snd_soc_component *component, - struct snd_pcm_substream *substream, - struct vm_area_struct *vma) -{ - return remap_pfn_range(vma, vma->vm_start, - substream->dma_buffer.addr >> PAGE_SHIFT, - vma->vm_end - vma->vm_start, vma->vm_page_prot); -} - static int atmel_pcm_new(struct snd_soc_component *component, struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_pcm *pcm = rtd->pcm; int ret;
ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32)); if (ret) return ret;
- if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) { - pr_debug("atmel-pcm: allocating PCM playback DMA buffer\n"); - ret = atmel_pcm_preallocate_dma_buffer(pcm, - SNDRV_PCM_STREAM_PLAYBACK); - if (ret) - goto out; - } + snd_pcm_set_managed_buffer_all(rtd->pcm, SNDRV_DMA_TYPE_DEV, + card->dev, ATMEL_SSC_DMABUF_SIZE, + ATMEL_SSC_DMABUF_SIZE);
- if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) { - pr_debug("atmel-pcm: allocating PCM capture DMA buffer\n"); - ret = atmel_pcm_preallocate_dma_buffer(pcm, - SNDRV_PCM_STREAM_CAPTURE); - if (ret) - goto out; - } - out: - return ret; -} - -static void atmel_pcm_free(struct snd_soc_component *component, - struct snd_pcm *pcm) -{ - struct snd_pcm_substream *substream; - struct snd_dma_buffer *buf; - int stream; - - for (stream = 0; stream < 2; stream++) { - substream = pcm->streams[stream].substream; - if (!substream) - continue; - - buf = &substream->dma_buffer; - if (!buf->area) - continue; - dma_free_coherent(pcm->card->dev, buf->bytes, - buf->area, buf->addr); - buf->area = NULL; - } + return 0; }
/*--------------------------------------------------------------------------*\ @@ -210,9 +145,6 @@ static int atmel_pcm_hw_params(struct snd_soc_component *component, /* this may get called several times by oss emulation * with different params */
- snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer); - runtime->dma_bytes = params_buffer_bytes(params); - prtd->params = snd_soc_dai_get_dma_data(asoc_rtd_to_cpu(rtd, 0), substream); prtd->params->dma_intr_handler = atmel_pcm_dma_irq;
@@ -384,9 +316,7 @@ static const struct snd_soc_component_driver atmel_soc_platform = { .prepare = atmel_pcm_prepare, .trigger = atmel_pcm_trigger, .pointer = atmel_pcm_pointer, - .mmap = atmel_pcm_mmap, .pcm_construct = atmel_pcm_new, - .pcm_destruct = atmel_pcm_free, };
int atmel_pcm_pdc_platform_register(struct device *dev)
Instead of manually managing its DMA buffers using dma_{alloc,free}_coherent() lets the sound core take care of this using managed buffers.
On one hand this reduces the amount of boiler plate code, but the main motivation for the change is to use the shared code where possible. This makes it easier to argue about correctness and that the code does not contain subtle bugs like data leakage or similar.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/bcm/cygnus-pcm.c | 107 ++----------------------------------- 1 file changed, 3 insertions(+), 104 deletions(-)
diff --git a/sound/soc/bcm/cygnus-pcm.c b/sound/soc/bcm/cygnus-pcm.c index 7ad07239f99c..56b71b965624 100644 --- a/sound/soc/bcm/cygnus-pcm.c +++ b/sound/soc/bcm/cygnus-pcm.c @@ -636,36 +636,6 @@ static int cygnus_pcm_close(struct snd_soc_component *component, return 0; }
-static int cygnus_pcm_hw_params(struct snd_soc_component *component, - struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params) -{ - struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_pcm_runtime *runtime = substream->runtime; - struct cygnus_aio_port *aio; - - aio = cygnus_dai_get_dma_data(substream); - dev_dbg(asoc_rtd_to_cpu(rtd, 0)->dev, "%s port %d\n", __func__, aio->portnum); - - snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer); - runtime->dma_bytes = params_buffer_bytes(params); - - return 0; -} - -static int cygnus_pcm_hw_free(struct snd_soc_component *component, - struct snd_pcm_substream *substream) -{ - struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct cygnus_aio_port *aio; - - aio = cygnus_dai_get_dma_data(substream); - dev_dbg(asoc_rtd_to_cpu(rtd, 0)->dev, "%s port %d\n", __func__, aio->portnum); - - snd_pcm_set_runtime_buffer(substream, NULL); - return 0; -} - static int cygnus_pcm_prepare(struct snd_soc_component *component, struct snd_pcm_substream *substream) { @@ -730,87 +700,19 @@ static snd_pcm_uframes_t cygnus_pcm_pointer(struct snd_soc_component *component, return bytes_to_frames(substream->runtime, res); }
-static int cygnus_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream) -{ - struct snd_pcm_substream *substream = pcm->streams[stream].substream; - struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_dma_buffer *buf = &substream->dma_buffer; - size_t size; - - size = cygnus_pcm_hw.buffer_bytes_max; - - buf->dev.type = SNDRV_DMA_TYPE_DEV; - buf->dev.dev = pcm->card->dev; - buf->private_data = NULL; - buf->area = dma_alloc_coherent(pcm->card->dev, size, - &buf->addr, GFP_KERNEL); - - dev_dbg(asoc_rtd_to_cpu(rtd, 0)->dev, "%s: size 0x%zx @ %pK\n", - __func__, size, buf->area); - - if (!buf->area) { - dev_err(asoc_rtd_to_cpu(rtd, 0)->dev, "%s: dma_alloc failed\n", __func__); - return -ENOMEM; - } - buf->bytes = size; - - return 0; -} - -static void cygnus_dma_free_dma_buffers(struct snd_soc_component *component, - struct snd_pcm *pcm) -{ - struct snd_pcm_substream *substream; - struct snd_dma_buffer *buf; - - substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream; - if (substream) { - buf = &substream->dma_buffer; - if (buf->area) { - dma_free_coherent(pcm->card->dev, buf->bytes, - buf->area, buf->addr); - buf->area = NULL; - } - } - - substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream; - if (substream) { - buf = &substream->dma_buffer; - if (buf->area) { - dma_free_coherent(pcm->card->dev, buf->bytes, - buf->area, buf->addr); - buf->area = NULL; - } - } -} - static int cygnus_dma_new(struct snd_soc_component *component, struct snd_soc_pcm_runtime *rtd) { + size_t size = cygnus_pcm_hw.buffer_bytes_max; struct snd_card *card = rtd->card->snd_card; - struct snd_pcm *pcm = rtd->pcm; - int ret;
if (!card->dev->dma_mask) card->dev->dma_mask = &cygnus_dma_dmamask; if (!card->dev->coherent_dma_mask) card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
- if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) { - ret = cygnus_pcm_preallocate_dma_buffer(pcm, - SNDRV_PCM_STREAM_PLAYBACK); - if (ret) - return ret; - } - - if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) { - ret = cygnus_pcm_preallocate_dma_buffer(pcm, - SNDRV_PCM_STREAM_CAPTURE); - if (ret) { - cygnus_dma_free_dma_buffers(component, pcm); - return ret; - } - } + snd_pcm_set_managed_buffer_all(rtd->pcm, SNDRV_DMA_TYPE_DEV, + card->dev, size, size);
return 0; } @@ -818,13 +720,10 @@ static int cygnus_dma_new(struct snd_soc_component *component, static struct snd_soc_component_driver cygnus_soc_platform = { .open = cygnus_pcm_open, .close = cygnus_pcm_close, - .hw_params = cygnus_pcm_hw_params, - .hw_free = cygnus_pcm_hw_free, .prepare = cygnus_pcm_prepare, .trigger = cygnus_pcm_trigger, .pointer = cygnus_pcm_pointer, .pcm_construct = cygnus_dma_new, - .pcm_destruct = cygnus_dma_free_dma_buffers, };
int cygnus_soc_platform_register(struct device *dev,
Instead of manually managing its DMA buffers using dma_{alloc,free}_coherent() lets the sound core take care of this using managed buffers.
On one hand this reduces the amount of boiler plate code, but the main motivation for the change is to use the shared code where possible. This makes it easier to argue about correctness and that the code does not contain subtle bugs like data leakage or similar.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/kirkwood/kirkwood-dma.c | 79 ++----------------------------- 1 file changed, 3 insertions(+), 76 deletions(-)
diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c index e037826b2451..c2a5933bfcfc 100644 --- a/sound/soc/kirkwood/kirkwood-dma.c +++ b/sound/soc/kirkwood/kirkwood-dma.c @@ -182,25 +182,6 @@ static int kirkwood_dma_close(struct snd_soc_component *component, return 0; }
-static int kirkwood_dma_hw_params(struct snd_soc_component *component, - struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - - snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer); - runtime->dma_bytes = params_buffer_bytes(params); - - return 0; -} - -static int kirkwood_dma_hw_free(struct snd_soc_component *component, - struct snd_pcm_substream *substream) -{ - snd_pcm_set_runtime_buffer(substream, NULL); - return 0; -} - static int kirkwood_dma_prepare(struct snd_soc_component *component, struct snd_pcm_substream *substream) { @@ -244,82 +225,28 @@ static snd_pcm_uframes_t kirkwood_dma_pointer( return count; }
-static int kirkwood_dma_preallocate_dma_buffer(struct snd_pcm *pcm, - int stream) -{ - struct snd_pcm_substream *substream = pcm->streams[stream].substream; - struct snd_dma_buffer *buf = &substream->dma_buffer; - size_t size = kirkwood_dma_snd_hw.buffer_bytes_max; - - buf->dev.type = SNDRV_DMA_TYPE_DEV; - buf->dev.dev = pcm->card->dev; - buf->area = dma_alloc_coherent(pcm->card->dev, size, - &buf->addr, GFP_KERNEL); - if (!buf->area) - return -ENOMEM; - buf->bytes = size; - buf->private_data = NULL; - - return 0; -} - static int kirkwood_dma_new(struct snd_soc_component *component, struct snd_soc_pcm_runtime *rtd) { + size_t size = kirkwood_dma_snd_hw.buffer_bytes_max; struct snd_card *card = rtd->card->snd_card; - struct snd_pcm *pcm = rtd->pcm; int ret;
ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32)); if (ret) return ret;
- if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) { - ret = kirkwood_dma_preallocate_dma_buffer(pcm, - SNDRV_PCM_STREAM_PLAYBACK); - if (ret) - return ret; - } - - if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) { - ret = kirkwood_dma_preallocate_dma_buffer(pcm, - SNDRV_PCM_STREAM_CAPTURE); - if (ret) - return ret; - } + snd_pcm_set_managed_buffer_all(rtd->pcm, SNDRV_DMA_TYPE_DEV, + card->dev, size, size);
return 0; }
-static void kirkwood_dma_free_dma_buffers(struct snd_soc_component *component, - struct snd_pcm *pcm) -{ - struct snd_pcm_substream *substream; - struct snd_dma_buffer *buf; - int stream; - - for (stream = 0; stream < 2; stream++) { - substream = pcm->streams[stream].substream; - if (!substream) - continue; - buf = &substream->dma_buffer; - if (!buf->area) - continue; - - dma_free_coherent(pcm->card->dev, buf->bytes, - buf->area, buf->addr); - buf->area = NULL; - } -} - const struct snd_soc_component_driver kirkwood_soc_component = { .name = DRV_NAME, .open = kirkwood_dma_open, .close = kirkwood_dma_close, - .hw_params = kirkwood_dma_hw_params, - .hw_free = kirkwood_dma_hw_free, .prepare = kirkwood_dma_prepare, .pointer = kirkwood_dma_pointer, .pcm_construct = kirkwood_dma_new, - .pcm_destruct = kirkwood_dma_free_dma_buffers, };
On 06.01.2021 15:36, Lars-Peter Clausen wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Instead of manually managing its DMA buffers using dma_{alloc,free}_coherent() lets the sound core take care of this using managed buffers.
On one hand this reduces the amount of boiler plate code, but the main motivation for the change is to use the shared code where possible. This makes it easier to argue about correctness and that the code does not contain subtle bugs like data leakage or similar.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Reviewed-by: Codrin Ciubotariu codrin.ciubotariu@microchip.com
Thanks!
On Wed, 6 Jan 2021 14:36:48 +0100, Lars-Peter Clausen wrote:
Instead of manually managing its DMA buffers using dma_{alloc,free}_coherent() lets the sound core take care of this using managed buffers.
On one hand this reduces the amount of boiler plate code, but the main motivation for the change is to use the shared code where possible. This makes it easier to argue about correctness and that the code does not contain subtle bugs like data leakage or similar.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ASoC: atmel-pdc: Use managed DMA buffer allocation commit: 22eee4d3efe370fedf71ee6a9e4dead3f32ad461 [2/3] ASoC: bcm: cygnus: Use managed DMA buffer allocation commit: 5ac813c83483e97a13b59aab34b89cebf9d5dcb8 [3/3] ASoC: kirkwood: Use managed DMA buffer allocation commit: b3c0ae75f5d3efa40174230b8c9c01848e03d4d0
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
participants (3)
-
Codrin.Ciubotariu＠microchip.com -
Lars-Peter Clausen -
Mark Brown