[alsa-devel] [PATCH 0/4] ALSA: enhancement / cleanup on memalloc stuff
Hi,
this is a small series of patches to enhance / clean up the core memory allocation stuff. The basic changes are:
- The memalloc code accepts NULL device pointer to treat as the default mode for the continuous pages - The new SNDRV_DMA_TYPE_VMALLOC type in the core allocator, so that we can drop the PCM-specific helpers - The PCM mmap default handler checks the buffer type, and the PCM page ops can be dropped in almost all cases.
These whole core changes are still compatible with the old code.
The actual cleanup patch for each driver will be posted later once when this core change set is accepted.
thanks,
Takashi
===
Takashi Iwai (4): ALSA: memalloc: Allow NULL device for SNDRV_DMA_TYPE_CONTINOUS type ALSA: memalloc: Add vmalloc buffer allocation support ALSA: pcm: Handle special page mapping in the default mmap handler ALSA: docs: Update documentation about SG- and vmalloc-buffers
.../sound/kernel-api/writing-an-alsa-driver.rst | 80 ++++++++++++---------- include/sound/memalloc.h | 1 + sound/core/memalloc.c | 23 ++++++- sound/core/pcm_native.c | 14 +++- 4 files changed, 80 insertions(+), 38 deletions(-)
Currently we pass the artificial device pointer to the allocation helper in the case of SNDRV_DMA_TYPE_CONTINUOUS for passing the GFP flags. But all common cases are the allocations with GFP_KERNEL, and it's messy to put this in each place.
In this patch, the memalloc core helper is changed to accept the NULL device pointer and it treats as the default mode, GFP_KERNEL, so that all callers can omit the complex argument but just leave NULL.
Signed-off-by: Takashi Iwai tiwai@suse.de --- Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 14 ++++++++------ sound/core/memalloc.c | 9 ++++++++- 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 132f5eb9b530..5385618fd881 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -3523,12 +3523,14 @@ The second argument (type) and the third argument (device pointer) are dependent on the bus. For normal devices, pass the device pointer (typically identical as ``card->dev``) to the third argument with ``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to the -bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type and the -``snd_dma_continuous_data(GFP_KERNEL)`` device pointer, where -``GFP_KERNEL`` is the kernel allocation flag to use. For the -scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the device -pointer (see the `Non-Contiguous Buffers`_ -section). +bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type. +You can pass NULL to the device pointer in that case, which is the +default mode implying to allocate with ``GFP_KRENEL`` flag. +If you need a different GFP flag, you can pass it by encoding the flag +into the device pointer via a special macro +:c:func:`snd_dma_continuous_data()`. +For the scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the +device pointer (see the `Non-Contiguous Buffers`_ section).
Once the buffer is pre-allocated, you can use the allocator in the ``hw_params`` callback: diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 6850d13aa98c..e56f84fbd659 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -99,6 +99,13 @@ static void snd_free_dev_iram(struct snd_dma_buffer *dmab) * */
+static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev) +{ + if (!dev) + return GFP_KRENEL; + else + return (__force gfp_t)(unsigned long)dev; +}
/** * snd_dma_alloc_pages - allocate the buffer area according to the given type @@ -129,7 +136,7 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, switch (type) { case SNDRV_DMA_TYPE_CONTINUOUS: dmab->area = alloc_pages_exact(size, - (__force gfp_t)(unsigned long)device); + snd_mem_get_gfp_flags(device)); dmab->addr = 0; break; #ifdef CONFIG_HAS_DMA
On Tue, 2019-11-05 at 09:01 +0100, Takashi Iwai wrote:
Currently we pass the artificial device pointer to the allocation helper in the case of SNDRV_DMA_TYPE_CONTINUOUS for passing the GFP flags. But all common cases are the allocations with GFP_KERNEL, and it's messy to put this in each place.
In this patch, the memalloc core helper is changed to accept the NULL device pointer and it treats as the default mode, GFP_KERNEL, so that all callers can omit the complex argument but just leave NULL.
Signed-off-by: Takashi Iwai tiwai@suse.de
Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 14 ++++++++------ sound/core/memalloc.c | 9 ++++++++- 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa- driver.rst b/Documentation/sound/kernel-api/writing-an-alsa- driver.rst index 132f5eb9b530..5385618fd881 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -3523,12 +3523,14 @@ The second argument (type) and the third argument (device pointer) are dependent on the bus. For normal devices, pass the device pointer (typically identical as ``card->dev``) to the third argument with ``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to
Hi Takashi,
I have a question about the usage of snd_dma_alloc_pages() with the SNDRV_DMA_TYPE_DEV type. I am working on adding a platform-device for audio which is a child device of the top-level PCI device in SOF. When I use the handle for the platform-device as the "dev" argument for snd_dma_alloc_pages(), the dma alloc fails on some platforms (ex: Ice Lake). But it works fine if I use the top-level PCI device instead. Why would that be? Are there any restrictions to what devices can be used for dma alloc?
Thanks, Ranjani
On Tue, 05 Nov 2019 18:58:53 +0100, Ranjani Sridharan wrote:
On Tue, 2019-11-05 at 09:01 +0100, Takashi Iwai wrote:
Currently we pass the artificial device pointer to the allocation helper in the case of SNDRV_DMA_TYPE_CONTINUOUS for passing the GFP flags. But all common cases are the allocations with GFP_KERNEL, and it's messy to put this in each place.
In this patch, the memalloc core helper is changed to accept the NULL device pointer and it treats as the default mode, GFP_KERNEL, so that all callers can omit the complex argument but just leave NULL.
Signed-off-by: Takashi Iwai tiwai@suse.de
Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 14 ++++++++------ sound/core/memalloc.c | 9 ++++++++- 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa- driver.rst b/Documentation/sound/kernel-api/writing-an-alsa- driver.rst index 132f5eb9b530..5385618fd881 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -3523,12 +3523,14 @@ The second argument (type) and the third argument (device pointer) are dependent on the bus. For normal devices, pass the device pointer (typically identical as ``card->dev``) to the third argument with ``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to
Hi Takashi,
I have a question about the usage of snd_dma_alloc_pages() with the SNDRV_DMA_TYPE_DEV type. I am working on adding a platform-device for audio which is a child device of the top-level PCI device in SOF. When I use the handle for the platform-device as the "dev" argument for snd_dma_alloc_pages(), the dma alloc fails on some platforms (ex: Ice Lake). But it works fine if I use the top-level PCI device instead. Why would that be? Are there any restrictions to what devices can be used for dma alloc?
This pretty much depends on the device. Basically the ALSA memalloc stuff simply calls dma_alloc_coherent() if the buffer type is SNDRV_DMA_TYPE_DEV, and the rest goes deeply into the code in kernel/dma/*.
My wild guess is that the significant difference in your case is about the DMA coherence mask set on the device. As default the platform device keeps 32bit DMA while the modern PCI drivers often sets up 64bit DMA mask.
Takashi
Hi Takashi,
I have a question about the usage of snd_dma_alloc_pages() with the SNDRV_DMA_TYPE_DEV type. I am working on adding a platform-device for audio which is a child device of the top-level PCI device in SOF. When I use the handle for the platform-device as the "dev" argument for snd_dma_alloc_pages(), the dma alloc fails on some platforms (ex: Ice Lake). But it works fine if I use the top-level PCI device instead. Why would that be? Are there any restrictions to what devices can be used for dma alloc?
This pretty much depends on the device. Basically the ALSA memalloc stuff simply calls dma_alloc_coherent() if the buffer type is SNDRV_DMA_TYPE_DEV, and the rest goes deeply into the code in kernel/dma/*.
My wild guess is that the significant difference in your case is about the DMA coherence mask set on the device. As default the platform device keeps 32bit DMA while the modern PCI drivers often sets up 64bit DMA mask.
Thanks, Takashi. So, in this case, would you recommend to always use the PCI device for dma alloc?
Thanks, Ranjani
On Tue, 05 Nov 2019 19:17:40 +0100, Sridharan, Ranjani wrote:
> > > Hi Takashi, > > I have a question about the usage of snd_dma_alloc_pages() with the > SNDRV_DMA_TYPE_DEV type. I am working on adding a platform-device for > audio which is a child device of the top-level PCI device in SOF. > When I use the handle for the platform-device as the "dev" argument for > snd_dma_alloc_pages(), the dma alloc fails on some platforms (ex: Ice > Lake). But it works fine if I use the top-level PCI device instead. > Why would that be? Are there any restrictions to what devices can be > used for dma alloc? This pretty much depends on the device. Basically the ALSA memalloc stuff simply calls dma_alloc_coherent() if the buffer type is SNDRV_DMA_TYPE_DEV, and the rest goes deeply into the code in kernel/dma/*. My wild guess is that the significant difference in your case is about the DMA coherence mask set on the device. As default the platform device keeps 32bit DMA while the modern PCI drivers often sets up 64bit DMA mask.Thanks, Takashi. So, in this case, would you recommend to always use the PCI device for dma alloc?
Yes, if the PCI bus is used in the backend, using the PCI device is a better choice.
Takashi
On Tue, 5 Nov 2019 09:01:35 +0100 Takashi Iwai tiwai@suse.de wrote:
Currently we pass the artificial device pointer to the allocation helper in the case of SNDRV_DMA_TYPE_CONTINUOUS for passing the GFP flags. But all common cases are the allocations with GFP_KERNEL, and it's messy to put this in each place.
In this patch, the memalloc core helper is changed to accept the NULL device pointer and it treats as the default mode, GFP_KERNEL, so that all callers can omit the complex argument but just leave NULL.
Signed-off-by: Takashi Iwai tiwai@suse.de
Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 14 ++++++++------ sound/core/memalloc.c | 9 ++++++++- 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 132f5eb9b530..5385618fd881 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -3523,12 +3523,14 @@ The second argument (type) and the third argument (device pointer) are dependent on the bus. For normal devices, pass the device pointer (typically identical as ``card->dev``) to the third argument with ``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to the -bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type and the -``snd_dma_continuous_data(GFP_KERNEL)`` device pointer, where -``GFP_KERNEL`` is the kernel allocation flag to use. For the -scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the device -pointer (see the `Non-Contiguous Buffers`_ -section). +bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type. +You can pass NULL to the device pointer in that case, which is the +default mode implying to allocate with ``GFP_KRENEL`` flag. +If you need a different GFP flag, you can pass it by encoding the flag +into the device pointer via a special macro +:c:func:`snd_dma_continuous_data()`. +For the scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the +device pointer (see the `Non-Contiguous Buffers`_ section).
Once the buffer is pre-allocated, you can use the allocator in the ``hw_params`` callback: diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 6850d13aa98c..e56f84fbd659 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -99,6 +99,13 @@ static void snd_free_dev_iram(struct snd_dma_buffer *dmab)
*/
+static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev) +{
- if (!dev)
return GFP_KRENEL;
There is a typo, you remove it in next patch, but it may cause problem with bisects.
- else
return (__force gfp_t)(unsigned long)dev;+}
/**
- snd_dma_alloc_pages - allocate the buffer area according to the given type
@@ -129,7 +136,7 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, switch (type) { case SNDRV_DMA_TYPE_CONTINUOUS: dmab->area = alloc_pages_exact(size,
(__force gfp_t)(unsigned long)device);
snd_mem_get_gfp_flags(device));#ifdef CONFIG_HAS_DMA
On Tue, 05 Nov 2019 19:28:44 +0100, Amadeusz Sławiński wrote:
On Tue, 5 Nov 2019 09:01:35 +0100 Takashi Iwai tiwai@suse.de wrote:
Currently we pass the artificial device pointer to the allocation helper in the case of SNDRV_DMA_TYPE_CONTINUOUS for passing the GFP flags. But all common cases are the allocations with GFP_KERNEL, and it's messy to put this in each place.
In this patch, the memalloc core helper is changed to accept the NULL device pointer and it treats as the default mode, GFP_KERNEL, so that all callers can omit the complex argument but just leave NULL.
Signed-off-by: Takashi Iwai tiwai@suse.de
Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 14 ++++++++------ sound/core/memalloc.c | 9 ++++++++- 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 132f5eb9b530..5385618fd881 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -3523,12 +3523,14 @@ The second argument (type) and the third argument (device pointer) are dependent on the bus. For normal devices, pass the device pointer (typically identical as ``card->dev``) to the third argument with ``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to the -bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type and the -``snd_dma_continuous_data(GFP_KERNEL)`` device pointer, where -``GFP_KERNEL`` is the kernel allocation flag to use. For the -scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the device -pointer (see the `Non-Contiguous Buffers`_ -section). +bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type. +You can pass NULL to the device pointer in that case, which is the +default mode implying to allocate with ``GFP_KRENEL`` flag. +If you need a different GFP flag, you can pass it by encoding the flag +into the device pointer via a special macro +:c:func:`snd_dma_continuous_data()`. +For the scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the +device pointer (see the `Non-Contiguous Buffers`_ section).
Once the buffer is pre-allocated, you can use the allocator in the ``hw_params`` callback: diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 6850d13aa98c..e56f84fbd659 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -99,6 +99,13 @@ static void snd_free_dev_iram(struct snd_dma_buffer *dmab)
*/
+static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev) +{
- if (!dev)
return GFP_KRENEL;There is a typo, you remove it in next patch, but it may cause problem with bisects.
Indeed, will fix up.
thanks,
Takashi
Currently we pass the artificial device pointer to the allocation helper in the case of SNDRV_DMA_TYPE_CONTINUOUS for passing the GFP flags. But all common cases are the allocations with GFP_KERNEL, and it's messy to put this in each place.
In this patch, the memalloc core helper is changed to accept the NULL device pointer and it treats as the default mode, GFP_KERNEL, so that all callers can omit the complex argument but just leave NULL.
Link: https://lore.kernel.org/r/20191105080138.1260-2-tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de --- v1->v2: typo fixes, fix a remaining WARN_ON() for NULL device
Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 14 ++++++++------ sound/core/memalloc.c | 11 ++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 132f5eb9b530..5385618fd881 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -3523,12 +3523,14 @@ The second argument (type) and the third argument (device pointer) are dependent on the bus. For normal devices, pass the device pointer (typically identical as ``card->dev``) to the third argument with ``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to the -bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type and the -``snd_dma_continuous_data(GFP_KERNEL)`` device pointer, where -``GFP_KERNEL`` is the kernel allocation flag to use. For the -scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the device -pointer (see the `Non-Contiguous Buffers`_ -section). +bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type. +You can pass NULL to the device pointer in that case, which is the +default mode implying to allocate with ``GFP_KRENEL`` flag. +If you need a different GFP flag, you can pass it by encoding the flag +into the device pointer via a special macro +:c:func:`snd_dma_continuous_data()`. +For the scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the +device pointer (see the `Non-Contiguous Buffers`_ section).
Once the buffer is pre-allocated, you can use the allocator in the ``hw_params`` callback: diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 6850d13aa98c..1b1c7620cbda 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -99,6 +99,13 @@ static void snd_free_dev_iram(struct snd_dma_buffer *dmab) * */
+static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev) +{ + if (!dev) + return GFP_KERNEL; + else + return (__force gfp_t)(unsigned long)dev; +}
/** * snd_dma_alloc_pages - allocate the buffer area according to the given type @@ -120,8 +127,6 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, return -ENXIO; if (WARN_ON(!dmab)) return -ENXIO; - if (WARN_ON(!device)) - return -EINVAL;
dmab->dev.type = type; dmab->dev.dev = device; @@ -129,7 +134,7 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, switch (type) { case SNDRV_DMA_TYPE_CONTINUOUS: dmab->area = alloc_pages_exact(size, - (__force gfp_t)(unsigned long)device); + snd_mem_get_gfp_flags(device)); dmab->addr = 0; break; #ifdef CONFIG_HAS_DMA
This patch adds the vmalloc buffer support to ALSA memalloc core. A new type, SNDRV_DMA_TYPE_VMALLOC was added.
The vmalloc buffer has been already supported in the PCM via a few own helper functions, but the user sometimes get confused and misuse them. With this patch, the whole buffer management is integrated into the memalloc core, so they can be used in a sole common way.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/memalloc.h | 1 + sound/core/memalloc.c | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h index 240622d89c0b..6ada3b8ede4e 100644 --- a/include/sound/memalloc.h +++ b/include/sound/memalloc.h @@ -44,6 +44,7 @@ struct snd_dma_device { #else #define SNDRV_DMA_TYPE_DEV_IRAM SNDRV_DMA_TYPE_DEV #endif +#define SNDRV_DMA_TYPE_VMALLOC 7 /* vmalloc'ed buffer */
/* * info for buffer allocation diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index e56f84fbd659..ac4cd94dbd66 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -10,6 +10,7 @@ #include <linux/mm.h> #include <linux/dma-mapping.h> #include <linux/genalloc.h> +#include <linux/vmalloc.h> #ifdef CONFIG_X86 #include <asm/set_memory.h> #endif @@ -99,10 +100,11 @@ static void snd_free_dev_iram(struct snd_dma_buffer *dmab) * */
-static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev) +static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev, + gfp_t default_gfp) { if (!dev) - return GFP_KRENEL; + return default_gfp; else return (__force gfp_t)(unsigned long)dev; } @@ -123,6 +125,8 @@ static inline gfp_t snd_mem_get_gfp_flags(const struct device *dev) int snd_dma_alloc_pages(int type, struct device *device, size_t size, struct snd_dma_buffer *dmab) { + gfp_t gfp; + if (WARN_ON(!size)) return -ENXIO; if (WARN_ON(!dmab)) @@ -135,8 +139,13 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, dmab->bytes = 0; switch (type) { case SNDRV_DMA_TYPE_CONTINUOUS: - dmab->area = alloc_pages_exact(size, - snd_mem_get_gfp_flags(device)); + gfp = snd_mem_get_gfp_flags(device, GFP_KERNEL); + dmab->area = alloc_pages_exact(size, gfp); + dmab->addr = 0; + break; + case SNDRV_DMA_TYPE_VMALLOC: + gfp = snd_mem_get_gfp_flags(device, GFP_KERNEL | __GFP_HIGHMEM); + dmab->area = __vmalloc(size, gfp, PAGE_KERNEL); dmab->addr = 0; break; #ifdef CONFIG_HAS_DMA @@ -222,6 +231,9 @@ void snd_dma_free_pages(struct snd_dma_buffer *dmab) case SNDRV_DMA_TYPE_CONTINUOUS: free_pages_exact(dmab->area, dmab->bytes); break; + case SNDRV_DMA_TYPE_VMALLOC: + vfree(dmab->area); + break; #ifdef CONFIG_HAS_DMA #ifdef CONFIG_GENERIC_ALLOCATOR case SNDRV_DMA_TYPE_DEV_IRAM:
When a driver needs to deal with a special buffer like a SG or a vmalloc buffer, it has to set up the PCM page ops explicitly for the corresponding helper function. This is rather error-prone and many people forgot or incorrectly used it.
For simplifying the call patterns and avoiding such a potential bug, this patch enhances the PCM default mmap handler to check the (pre-)allocated buffer type and handles the page gracefully depending on the buffer type. If the PCM page ops is given, the ops is still used in a higher priority. The new code path is only for the default (NULL page ops) case.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_native.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c3a139436ac2..998c63192ae4 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -13,6 +13,7 @@ #include <linux/pm_qos.h> #include <linux/io.h> #include <linux/dma-mapping.h> +#include <linux/vmalloc.h> #include <sound/core.h> #include <sound/control.h> #include <sound/info.h> @@ -3335,7 +3336,18 @@ static inline struct page * snd_pcm_default_page_ops(struct snd_pcm_substream *substream, unsigned long ofs) { void *vaddr = substream->runtime->dma_area + ofs; - return virt_to_page(vaddr); + + switch (substream->dma_buffer.dev.type) { +#ifdef CONFIG_SND_DMA_SGBUF + case SNDRV_DMA_TYPE_DEV_SG: + case SNDRV_DMA_TYPE_DEV_UC_SG: + return snd_pcm_sgbuf_ops_page(substream, ofs); +#endif /* CONFIG_SND_DMA_SGBUF */ + case SNDRV_DMA_TYPE_VMALLOC: + return vmalloc_to_page(vaddr); + default: + return virt_to_page(vaddr); + } }
/*
The recent changes simplified the required setup for SG- and vmalloc- buffers. Update the documentation accordingly.
Signed-off-by: Takashi Iwai tiwai@suse.de --- .../sound/kernel-api/writing-an-alsa-driver.rst | 66 ++++++++++++---------- 1 file changed, 37 insertions(+), 29 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 5385618fd881..ba008ce28029 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -2095,10 +2095,12 @@ This callback is atomic as default. page callback ~~~~~~~~~~~~~
-This callback is optional too. This callback is used mainly for -non-contiguous buffers. The mmap calls this callback to get the page -address. Some examples will be explained in the later section `Buffer -and Memory Management`_, too. +This callback is optional too. The mmap calls this callback to get the +page fault address. + +Since the recent changes, you need no special callback any longer for +the standard SG-buffer or vmalloc-buffer. Hence this callback should +be rarely used.
mmap calllback ~~~~~~~~~~~~~~ @@ -3700,8 +3702,15 @@ For creating the SG-buffer handler, call ``SNDRV_DMA_TYPE_DEV_SG`` in the PCM constructor like other PCI pre-allocator. You need to pass ``snd_dma_pci_data(pci)``, where pci is the :c:type:`struct pci_dev <pci_dev>` pointer of the chip as -well. The ``struct snd_sg_buf`` instance is created as -``substream->dma_private``. You can cast the pointer like: +well. + +:: + + snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV_SG, + snd_dma_pci_data(pci), size, max); + +The ``struct snd_sg_buf`` instance is created as +``substream->dma_private`` in turn. You can cast the pointer like:
::
@@ -3717,10 +3726,6 @@ physically non-contiguous. The physical address table is set up in ``sgbuf->table``. You can get the physical address at a certain offset via :c:func:`snd_pcm_sgbuf_get_addr()`.
-When a SG-handler is used, you need to set -:c:func:`snd_pcm_sgbuf_ops_page()` as the ``page`` callback. (See -`page callback`_ section.) - To release the data, call :c:func:`snd_pcm_lib_free_pages()` in the ``hw_free`` callback as usual.
@@ -3728,30 +3733,33 @@ Vmalloc'ed Buffers ------------------
It's possible to use a buffer allocated via :c:func:`vmalloc()`, for -example, for an intermediate buffer. Since the allocated pages are not -contiguous, you need to set the ``page`` callback to obtain the physical -address at every offset. +example, for an intermediate buffer. In the recent version of kernel, +you can simply allocate it via standard +:c:func:`snd_pcm_lib_malloc_pages()` and co after setting up the +buffer preallocation with ``SNDRV_DMA_TYPE_VMALLOC`` type.
-The easiest way to achieve it would be to use -:c:func:`snd_pcm_lib_alloc_vmalloc_buffer()` for allocating the buffer -via :c:func:`vmalloc()`, and set :c:func:`snd_pcm_sgbuf_ops_page()` to -the ``page`` callback. At release, you need to call -:c:func:`snd_pcm_lib_free_vmalloc_buffer()`. +::
-If you want to implementation the ``page`` manually, it would be like -this: + snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_VMALLOC, + NULL, 0, 0);
-:: +The NULL is passed to the device pointer argument, which indicates +that the default pages (GFP_KERNEL and GFP_HIGHMEM) will be +allocated.
- #include <linux/vmalloc.h> +Also, note that zero is passed to both the size and the max size +arguments here. Since each vmalloc call should succeed at any time, +we don't need to pre-allocate the buffers like other continuous +pages.
- /* get the physical page pointer on the given offset */ - static struct page *mychip_page(struct snd_pcm_substream *substream, - unsigned long offset) - { - void *pageptr = substream->runtime->dma_area + offset; - return vmalloc_to_page(pageptr); - } +If you need the 32bit DMA allocation, pass the device pointer encoded +by :c:func:`snd_dma_continuous_data()` with ``GFP_KERNEL|__GFP_DMA32`` +argument. + +:: + + snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_VMALLOC, + snd_dma_continuous_data(GFP_KERNEL | __GFP_DMA32), 0, 0);
Proc Interface ==============
participants (4)
-
Amadeusz Sławiński -
Ranjani Sridharan -
Sridharan, Ranjani -
Takashi Iwai