[alsa-devel] [PATCH] ALSA: usb-audio: Allow non-vmalloc buffer for PCM buffers
Currently, USB-audio driver allocates the PCM buffer via vmalloc(), as this serves merely as an intermediate buffer that is copied to each URB transfer buffer. This works well in general on x86, but on some archs this may result in cache coherency issues when mmap is used. OTOH, it works also on such arch unless mmap is used.
This patch is a step for mitigating the inconvenience; a new module option "use_vmalloc" is provided so that user can choose to allocate the DMA coherent buffer instead of the existing vmalloc buffer. The drawback is that it'd be the standard dma_alloc_coherent() calls and the system would require contiguous pages on non-x86 archs.
Note that it's a global option and not dynamically switchable since the buffer is pre-allocated at the probe time. In theory, it's possible to be switchable, but it'd be trickier and racier.
As default use_vmalloc option is set to true, so that the old behavior is kept. For allowing the coherent mmap on ARM or MIPS, pass use_vmalloc=0 option explicitly.
Reported-by: Daniel Danzberger daniel@dd-wrt.com Signed-off-by: Takashi Iwai tiwai@suse.de --- Documentation/sound/alsa-configuration.rst | 7 ++++ sound/usb/card.c | 4 ++ sound/usb/pcm.c | 59 +++++++++++++++++++++++++++--- sound/usb/pcm.h | 1 + sound/usb/stream.c | 2 + sound/usb/usbaudio.h | 2 + 6 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst index aed6b4fb8e46..b1052e18292d 100644 --- a/Documentation/sound/alsa-configuration.rst +++ b/Documentation/sound/alsa-configuration.rst @@ -2224,6 +2224,13 @@ quirk_alias Quirk alias list, pass strings like ``0123abcd:5678beef``, which applies the existing quirk for the device 5678:beef to a new device 0123:abcd. +use_vmalloc + Use vmalloc() for allocations of the PCM buffers (default: yes). + For architectures with non-coherent memory like ARM or MIPS, the + mmap access may give inconsistent results with vmalloc'ed + buffers. If mmap is used on such architectures, turn off this + option, so that the DMA-coherent buffers are allocated and used + instead.
This module supports multiple devices, autoprobe and hotplugging.
diff --git a/sound/usb/card.c b/sound/usb/card.c index c80224807e8f..a1ed798a1c6b 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -86,6 +86,8 @@ static bool ignore_ctl_error; static bool autoclock = true; static char *quirk_alias[SNDRV_CARDS];
+bool snd_usb_use_vmalloc = true; + module_param_array(index, int, NULL, 0444); MODULE_PARM_DESC(index, "Index value for the USB audio adapter."); module_param_array(id, charp, NULL, 0444); @@ -105,6 +107,8 @@ module_param(autoclock, bool, 0444); MODULE_PARM_DESC(autoclock, "Enable auto-clock selection for UAC2 devices (default: yes)."); module_param_array(quirk_alias, charp, NULL, 0444); MODULE_PARM_DESC(quirk_alias, "Quirk aliases, e.g. 0123abcd:5678beef."); +module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444); +MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
/* * we keep the snd_usb_audio_t instances by ourselves for merging diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 897a2cbef6de..78d1cad08a0a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -728,7 +728,11 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, struct audioformat *fmt; int ret;
- ret = snd_pcm_lib_alloc_vmalloc_buffer(substream, + if (snd_usb_use_vmalloc) + ret = snd_pcm_lib_alloc_vmalloc_buffer(substream, + params_buffer_bytes(hw_params)); + else + ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (ret < 0) return ret; @@ -781,7 +785,11 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) snd_usb_endpoint_deactivate(subs->data_endpoint); snd_usb_unlock_shutdown(subs->stream->chip); } - return snd_pcm_lib_free_vmalloc_buffer(substream); + + if (snd_usb_use_vmalloc) + return snd_pcm_lib_free_vmalloc_buffer(substream); + else + return snd_pcm_lib_free_pages(substream); }
/* @@ -1702,9 +1710,50 @@ static const struct snd_pcm_ops snd_usb_capture_ops = { .mmap = snd_pcm_lib_mmap_vmalloc, };
+static const struct snd_pcm_ops snd_usb_playback_dev_ops = { + .open = snd_usb_pcm_open, + .close = snd_usb_pcm_close, + .ioctl = snd_pcm_lib_ioctl, + .hw_params = snd_usb_hw_params, + .hw_free = snd_usb_hw_free, + .prepare = snd_usb_pcm_prepare, + .trigger = snd_usb_substream_playback_trigger, + .pointer = snd_usb_pcm_pointer, + .page = snd_pcm_sgbuf_ops_page, +}; + +static const struct snd_pcm_ops snd_usb_capture_dev_ops = { + .open = snd_usb_pcm_open, + .close = snd_usb_pcm_close, + .ioctl = snd_pcm_lib_ioctl, + .hw_params = snd_usb_hw_params, + .hw_free = snd_usb_hw_free, + .prepare = snd_usb_pcm_prepare, + .trigger = snd_usb_substream_capture_trigger, + .pointer = snd_usb_pcm_pointer, + .page = snd_pcm_sgbuf_ops_page, +}; + void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream) { - snd_pcm_set_ops(pcm, stream, - stream == SNDRV_PCM_STREAM_PLAYBACK ? - &snd_usb_playback_ops : &snd_usb_capture_ops); + const struct snd_pcm_ops *ops; + + if (snd_usb_use_vmalloc) + ops = stream == SNDRV_PCM_STREAM_PLAYBACK ? + &snd_usb_playback_ops : &snd_usb_capture_ops; + else + ops = stream == SNDRV_PCM_STREAM_PLAYBACK ? + &snd_usb_playback_dev_ops : &snd_usb_capture_dev_ops; + snd_pcm_set_ops(pcm, stream, ops); +} + +void snd_usb_preallocate_buffer(struct snd_usb_substream *subs) +{ + struct snd_pcm *pcm = subs->stream->pcm; + struct snd_pcm_substream *s = pcm->streams[subs->direction].substream; + struct device *dev = subs->dev->bus->controller; + + if (!snd_usb_use_vmalloc) + snd_pcm_lib_preallocate_pages(s, SNDRV_DMA_TYPE_DEV_SG, + dev, 64*1024, 512*1024); } diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h index 35740d5ef268..f77ec58bf1a1 100644 --- a/sound/usb/pcm.h +++ b/sound/usb/pcm.h @@ -10,6 +10,7 @@ void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream); int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface, struct usb_host_interface *alts, struct audioformat *fmt); +void snd_usb_preallocate_buffer(struct snd_usb_substream *subs);
#endif /* __USBAUDIO_PCM_H */ diff --git a/sound/usb/stream.c b/sound/usb/stream.c index d16e1c23f4e9..729afd808cc4 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -106,6 +106,8 @@ static void snd_usb_init_substream(struct snd_usb_stream *as, subs->ep_num = fp->endpoint; if (fp->channels > subs->channels_max) subs->channels_max = fp->channels; + + snd_usb_preallocate_buffer(subs); }
/* kctl callbacks for usb-audio channel maps */ diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 7b28cbde22c0..b9faeca645fd 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -127,4 +127,6 @@ struct snd_usb_audio_quirk { int snd_usb_lock_shutdown(struct snd_usb_audio *chip); void snd_usb_unlock_shutdown(struct snd_usb_audio *chip);
+extern bool snd_usb_use_vmalloc; + #endif /* __USBAUDIO_H */
Hi Takashi,
I love your patch! Yet something to improve:
[auto build test ERROR on sound/for-next] [also build test ERROR on v4.17-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/ALSA-usb-audio-Allow-n... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: i386-randconfig-x079-201821 (attached as .config) compiler: gcc-8 (Debian 8.1.0-3) 8.1.0 reproduce: # save the attached .config to linux build tree make ARCH=i386
All errors (new ones prefixed by >>):
sound/usb/pcm.c:1729:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types]
.open = snd_usb_pcm_open, ^~~~~~~~~~~~~~~~ sound/usb/pcm.c:1729:11: note: (near initialization for 'snd_usb_playback_dev_ops.open') sound/usb/pcm.c:1730:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .close = snd_usb_pcm_close, ^~~~~~~~~~~~~~~~~ sound/usb/pcm.c:1730:11: note: (near initialization for 'snd_usb_playback_dev_ops.close') sound/usb/pcm.c:1741:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .open = snd_usb_pcm_open, ^~~~~~~~~~~~~~~~ sound/usb/pcm.c:1741:11: note: (near initialization for 'snd_usb_capture_dev_ops.open') sound/usb/pcm.c:1742:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .close = snd_usb_pcm_close, ^~~~~~~~~~~~~~~~~ sound/usb/pcm.c:1742:11: note: (near initialization for 'snd_usb_capture_dev_ops.close') cc1: some warnings being treated as errors
vim +1729 sound/usb/pcm.c
1727 1728 static const struct snd_pcm_ops snd_usb_playback_dev_ops = {
1729 .open = snd_usb_pcm_open,
1730 .close = snd_usb_pcm_close, 1731 .ioctl = snd_pcm_lib_ioctl, 1732 .hw_params = snd_usb_hw_params, 1733 .hw_free = snd_usb_hw_free, 1734 .prepare = snd_usb_pcm_prepare, 1735 .trigger = snd_usb_substream_playback_trigger, 1736 .pointer = snd_usb_pcm_pointer, 1737 .page = snd_pcm_sgbuf_ops_page, 1738 }; 1739
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on sound/for-next] [also build test WARNING on v4.17-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/ALSA-usb-audio-Allow-n... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
sound/usb/pcm.c:1078:36: sparse: expression using sizeof(void) sound/usb/pcm.c:1078:36: sparse: expression using sizeof(void) sound/usb/pcm.c:1181:25: sparse: expression using sizeof(void) sound/usb/pcm.c:1181:25: sparse: expression using sizeof(void)
sound/usb/pcm.c:1729:25: sparse: incorrect type in initializer (different argument counts) @@ expected int ( *open )( ... ) @@ got int ( *open )( ... ) @@
sound/usb/pcm.c:1729:25: expected int ( *open )( ... ) sound/usb/pcm.c:1729:25: got int ( *<noident> )( ... )
sound/usb/pcm.c:1730:25: sparse: incorrect type in initializer (different argument counts) @@ expected int ( *close )( ... ) @@ got int ( *close )( ... ) @@
sound/usb/pcm.c:1730:25: expected int ( *close )( ... ) sound/usb/pcm.c:1730:25: got int ( *<noident> )( ... ) sound/usb/pcm.c:1741:25: sparse: incorrect type in initializer (different argument counts) @@ expected int ( *open )( ... ) @@ got int ( *open )( ... ) @@ sound/usb/pcm.c:1741:25: expected int ( *open )( ... ) sound/usb/pcm.c:1741:25: got int ( *<noident> )( ... ) sound/usb/pcm.c:1742:25: sparse: incorrect type in initializer (different argument counts) @@ expected int ( *close )( ... ) @@ got int ( *close )( ... ) @@ sound/usb/pcm.c:1742:25: expected int ( *close )( ... ) sound/usb/pcm.c:1742:25: got int ( *<noident> )( ... ) sound/usb/pcm.c:1729:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .open = snd_usb_pcm_open, ^~~~~~~~~~~~~~~~ sound/usb/pcm.c:1729:11: note: (near initialization for 'snd_usb_playback_dev_ops.open') sound/usb/pcm.c:1730:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .close = snd_usb_pcm_close, ^~~~~~~~~~~~~~~~~ sound/usb/pcm.c:1730:11: note: (near initialization for 'snd_usb_playback_dev_ops.close') sound/usb/pcm.c:1741:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .open = snd_usb_pcm_open, ^~~~~~~~~~~~~~~~ sound/usb/pcm.c:1741:11: note: (near initialization for 'snd_usb_capture_dev_ops.open') sound/usb/pcm.c:1742:11: error: initialization of 'int (*)(struct snd_pcm_substream *)' from incompatible pointer type 'int (*)(struct snd_pcm_substream *, int)' [-Werror=incompatible-pointer-types] .close = snd_usb_pcm_close, ^~~~~~~~~~~~~~~~~ sound/usb/pcm.c:1742:11: note: (near initialization for 'snd_usb_capture_dev_ops.close') cc1: some warnings being treated as errors
vim +1729 sound/usb/pcm.c
1727 1728 static const struct snd_pcm_ops snd_usb_playback_dev_ops = {
1729 .open = snd_usb_pcm_open, 1730 .close = snd_usb_pcm_close,
1731 .ioctl = snd_pcm_lib_ioctl, 1732 .hw_params = snd_usb_hw_params, 1733 .hw_free = snd_usb_hw_free, 1734 .prepare = snd_usb_pcm_prepare, 1735 .trigger = snd_usb_substream_playback_trigger, 1736 .pointer = snd_usb_pcm_pointer, 1737 .page = snd_pcm_sgbuf_ops_page, 1738 }; 1739
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Takashi,
I love your patch! Yet something to improve:
[auto build test ERROR on sound/for-next] [also build test ERROR on v4.17-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/ALSA-usb-audio-Allow-n... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
sound/usb/pcm.c:1729:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.open = snd_usb_pcm_open, ^~~~~~~~~~~~~~~~ sound/usb/pcm.c:1729:11: note: (near initialization for 'snd_usb_playback_dev_ops.open') sound/usb/pcm.c:1730:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .close = snd_usb_pcm_close, ^~~~~~~~~~~~~~~~~ sound/usb/pcm.c:1730:11: note: (near initialization for 'snd_usb_playback_dev_ops.close') sound/usb/pcm.c:1741:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .open = snd_usb_pcm_open, ^~~~~~~~~~~~~~~~ sound/usb/pcm.c:1741:11: note: (near initialization for 'snd_usb_capture_dev_ops.open') sound/usb/pcm.c:1742:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .close = snd_usb_pcm_close, ^~~~~~~~~~~~~~~~~ sound/usb/pcm.c:1742:11: note: (near initialization for 'snd_usb_capture_dev_ops.close') cc1: some warnings being treated as errors
vim +1729 sound/usb/pcm.c
1727 1728 static const struct snd_pcm_ops snd_usb_playback_dev_ops = {
1729 .open = snd_usb_pcm_open,
1730 .close = snd_usb_pcm_close, 1731 .ioctl = snd_pcm_lib_ioctl, 1732 .hw_params = snd_usb_hw_params, 1733 .hw_free = snd_usb_hw_free, 1734 .prepare = snd_usb_pcm_prepare, 1735 .trigger = snd_usb_substream_playback_trigger, 1736 .pointer = snd_usb_pcm_pointer, 1737 .page = snd_pcm_sgbuf_ops_page, 1738 }; 1739
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
participants (2)
-
kbuild test robot
-
Takashi Iwai