[PATCH 0/3] ALSA: pcm: More mmap coverages
Hi,
the recent regression fix enlightened the potentially missing pieces in a couple of drivers for mmap on non-x86 architectures. This patch series tries to cover those.
Takashi
===
Takashi Iwai (3): ALSA: pcm: Check mmap capability of runtime dma buffer at first ALSA: pci: rme: Set up buffer type properly ALSA: pci: cs46xx: Fix set up buffer type properly
sound/core/pcm_native.c | 9 +++++++-- sound/pci/cs46xx/cs46xx_lib.c | 30 ++++++++---------------------- sound/pci/rme9652/hdsp.c | 6 ++---- sound/pci/rme9652/rme9652.c | 6 ++---- 4 files changed, 19 insertions(+), 32 deletions(-)
Currently we check only the substream->dma_buffer as the preset of the buffer configuration for verifying the availability of mmap. But a few drivers rather set up the buffer in the own way without the standard buffer preallocation using substream->dma_buffer, and they miss the proper checks. (Now it's working more or less fine as most of them are running only on x86).
Actually, they may set up the runtime dma_buffer (referred via snd_pcm_get_dma_buf()) at the open callback, though. That is, this could have been used as the primary source.
This patch changes the hw_support_mmap() function to check the runtime dma buffer at first. It's usually NULL with the standard buffer preallocation, and in that case, we continue checking substream->dma_buffer as fallback.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_native.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 71323d807dbf..dc9fa312fadd 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -243,13 +243,18 @@ int snd_pcm_info_user(struct snd_pcm_substream *substream,
static bool hw_support_mmap(struct snd_pcm_substream *substream) { + struct snd_dma_buffer *dmabuf; + if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP)) return false;
if (substream->ops->mmap || substream->ops->page) return true;
- switch (substream->dma_buffer.dev.type) { + dmabuf = snd_pcm_get_dma_buf(substream); + if (!dmabuf) + dmabuf = &substream->dma_buffer; + switch (dmabuf->dev.type) { case SNDRV_DMA_TYPE_UNKNOWN: /* we can't know the device, so just assume that the driver does * everything right @@ -259,7 +264,7 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) case SNDRV_DMA_TYPE_VMALLOC: return true; default: - return dma_can_mmap(substream->dma_buffer.dev.dev); + return dma_can_mmap(dmabuf->dev.dev); } }
Although the regression of the mmap was fixed in the recent commit dc0dc8a73e8e ("ALSA: pcm: Fix mmap breakage without explicit buffer setup"), RME9652 and HDSP drivers have still potential issues with their mmap handling. Namely, they use the default mmap handler without the standard buffer preallocation, and PCM core wouldn't use the coherent DMA mapping. It's practically OK on x86, but on some exotic architectures, it wouldn't work.
This patch addresses the potential breakage by replacing the buffer setup with the proper macro. It also simplifies the source code, too.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme9652/hdsp.c | 6 ++---- sound/pci/rme9652/rme9652.c | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index 982278f8724d..75aa2ea733a5 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -4507,8 +4507,7 @@ static int snd_hdsp_playback_open(struct snd_pcm_substream *substream) snd_pcm_set_sync(substream);
runtime->hw = snd_hdsp_playback_subinfo; - runtime->dma_area = hdsp->playback_buffer; - runtime->dma_bytes = HDSP_DMA_AREA_BYTES; + snd_pcm_set_runtime_buffer(substream, hdsp->playback_dma_buf);
hdsp->playback_pid = current->pid; hdsp->playback_substream = substream; @@ -4584,8 +4583,7 @@ static int snd_hdsp_capture_open(struct snd_pcm_substream *substream) snd_pcm_set_sync(substream);
runtime->hw = snd_hdsp_capture_subinfo; - runtime->dma_area = hdsp->capture_buffer; - runtime->dma_bytes = HDSP_DMA_AREA_BYTES; + snd_pcm_set_runtime_buffer(substream, hdsp->capture_dma_buf);
hdsp->capture_pid = current->pid; hdsp->capture_substream = substream; diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 45448a97070c..e76f737ac9e8 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -2259,8 +2259,7 @@ static int snd_rme9652_playback_open(struct snd_pcm_substream *substream) snd_pcm_set_sync(substream);
runtime->hw = snd_rme9652_playback_subinfo; - runtime->dma_area = rme9652->playback_buffer; - runtime->dma_bytes = RME9652_DMA_AREA_BYTES; + snd_pcm_set_runtime_buffer(substream, rme9652->playback_dma_buf);
if (rme9652->capture_substream == NULL) { rme9652_stop(rme9652); @@ -2319,8 +2318,7 @@ static int snd_rme9652_capture_open(struct snd_pcm_substream *substream) snd_pcm_set_sync(substream);
runtime->hw = snd_rme9652_capture_subinfo; - runtime->dma_area = rme9652->capture_buffer; - runtime->dma_bytes = RME9652_DMA_AREA_BYTES; + snd_pcm_set_runtime_buffer(substream, rme9652->capture_dma_buf);
if (rme9652->playback_substream == NULL) { rme9652_stop(rme9652);
CS46xx driver switches the buffer depending on the number of periods, and in some cases it switches to the own buffer without updating the buffer type properly. This may cause a problem with the mmap on exotic architectures that require the own mmap call for the coherent DMA buffer.
This patch addresses the potential breakage by replacing the buffer setup with the proper macro. It also simplifies the source code, too.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/cs46xx/cs46xx_lib.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c index 8877afb66704..62f45847b351 100644 --- a/sound/pci/cs46xx/cs46xx_lib.c +++ b/sound/pci/cs46xx/cs46xx_lib.c @@ -1121,9 +1121,7 @@ static int snd_cs46xx_playback_hw_params(struct snd_pcm_substream *substream, if (params_periods(hw_params) == CS46XX_FRAGS) { if (runtime->dma_area != cpcm->hw_buf.area) snd_pcm_lib_free_pages(substream); - runtime->dma_area = cpcm->hw_buf.area; - runtime->dma_addr = cpcm->hw_buf.addr; - runtime->dma_bytes = cpcm->hw_buf.bytes; + snd_pcm_set_runtime_buffer(substream, &cpcm->hw_buf);
#ifdef CONFIG_SND_CS46XX_NEW_DSP @@ -1143,11 +1141,8 @@ static int snd_cs46xx_playback_hw_params(struct snd_pcm_substream *substream, #endif
} else { - if (runtime->dma_area == cpcm->hw_buf.area) { - runtime->dma_area = NULL; - runtime->dma_addr = 0; - runtime->dma_bytes = 0; - } + if (runtime->dma_area == cpcm->hw_buf.area) + snd_pcm_set_runtime_buffer(substream, NULL); err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (err < 0) { #ifdef CONFIG_SND_CS46XX_NEW_DSP @@ -1196,9 +1191,7 @@ static int snd_cs46xx_playback_hw_free(struct snd_pcm_substream *substream) if (runtime->dma_area != cpcm->hw_buf.area) snd_pcm_lib_free_pages(substream);
- runtime->dma_area = NULL; - runtime->dma_addr = 0; - runtime->dma_bytes = 0; + snd_pcm_set_runtime_buffer(substream, NULL);
return 0; } @@ -1287,16 +1280,11 @@ static int snd_cs46xx_capture_hw_params(struct snd_pcm_substream *substream, if (runtime->periods == CS46XX_FRAGS) { if (runtime->dma_area != chip->capt.hw_buf.area) snd_pcm_lib_free_pages(substream); - runtime->dma_area = chip->capt.hw_buf.area; - runtime->dma_addr = chip->capt.hw_buf.addr; - runtime->dma_bytes = chip->capt.hw_buf.bytes; + snd_pcm_set_runtime_buffer(substream, &chip->capt.hw_buf); substream->ops = &snd_cs46xx_capture_ops; } else { - if (runtime->dma_area == chip->capt.hw_buf.area) { - runtime->dma_area = NULL; - runtime->dma_addr = 0; - runtime->dma_bytes = 0; - } + if (runtime->dma_area == chip->capt.hw_buf.area) + snd_pcm_set_runtime_buffer(substream, NULL); err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (err < 0) return err; @@ -1313,9 +1301,7 @@ static int snd_cs46xx_capture_hw_free(struct snd_pcm_substream *substream)
if (runtime->dma_area != chip->capt.hw_buf.area) snd_pcm_lib_free_pages(substream); - runtime->dma_area = NULL; - runtime->dma_addr = 0; - runtime->dma_bytes = 0; + snd_pcm_set_runtime_buffer(substream, NULL);
return 0; }
participants (1)
-
Takashi Iwai