[PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Laxman Dewangan]

Some of the ARM based soc allocate the writecombine dma buffer for
pcm substreams. They have the same codes for managing this buffer.
Moving this to the core/pcm files so that they can use that directly.

Remove the code from Tegra PCM and use these new library function.

This is enabled only for ARM specific and can be extended to other
architecture if they support the writecombine dma buffer.

This patch is based on detail discussion on patch:
[PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap
And suggestion from Lars and Takashi.


Laxman Dewangan (3):
  ALSA: pcm: add apis for writecombine dma buffer allocation
  ASoC: add apis for creating/free pcm dma buffer
  ASoC: tegra: use core/pcm library for pcm buffer allocation

 include/sound/pcm.h         |   17 ++++++++
 include/sound/soc.h         |   13 ++++++
 sound/core/pcm_native.c     |   53 +++++++++++++++++++++++++
 sound/soc/soc-core.c        |   50 +++++++++++++++++++++++
 sound/soc/tegra/tegra_pcm.c |   92 ++-----------------------------------------
 5 files changed, 137 insertions(+), 88 deletions(-)

[PATCH 1/3] ALSA: pcm: add apis for writecombine dma buffer allocation

[Laxman Dewangan]

Add APIs for allocation/deallocation of writecombine dma buffer
for pcm substreams.
The write combine dma buffer is not supported on few
architecture like ARM and restricting these APIs support
for ARM specific.

This can be make available to other architecture if they
start supporting this.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 include/sound/pcm.h     |   17 +++++++++++++++
 sound/core/pcm_native.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index e7afcc9..e1c8ac4 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1054,6 +1054,23 @@ int snd_pcm_lib_mmap_iomem(struct snd_pcm_substream *substream, struct vm_area_s
 #define snd_pcm_lib_mmap_iomem	NULL
 #endif
 
+/**
+ * Allocate the write combine DMA buffer for the PCM streams.
+ * Not all architecture support the writecombine dma allocation and
+ * hence restricting this to ARM specific. Once all architecture
+ * support the writecombine dma buffer then these APIs will be available
+ * for all architecture.
+ */
+#ifdef CONFIG_ARM
+int snd_pcm_lib_alloc_writecombine_dma_buffer(struct snd_pcm *pcm,
+		size_t size, int stream);
+void snd_pcm_lib_dealloc_writecombine_dma_buffer(struct snd_pcm *pcm,
+		int stream);
+int snd_pcm_lib_mmap_writecombine_dma_buffer(
+		struct snd_pcm_substream *substream,
+		struct vm_area_struct *vma);
+#endif
+
 #define snd_pcm_lib_mmap_vmalloc NULL
 
 static inline void snd_pcm_limit_isa_dma_size(int dma, size_t *max)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 53b5ada..fa56ad9 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3213,6 +3213,59 @@ int snd_pcm_lib_mmap_iomem(struct snd_pcm_substream *substream,
 EXPORT_SYMBOL(snd_pcm_lib_mmap_iomem);
 #endif /* SNDRV_PCM_INFO_MMAP */
 
+#ifdef CONFIG_ARM
+int snd_pcm_lib_alloc_writecombine_dma_buffer(struct snd_pcm *pcm,
+		size_t size, int stream)
+{
+	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
+	struct snd_dma_buffer *buf = &substream->dma_buffer;
+
+	buf->area = dma_alloc_writecombine(pcm->card->dev, size,
+				&buf->addr, GFP_KERNEL);
+	if (!buf->area)
+		return -ENOMEM;
+
+	buf->dev.type = SNDRV_DMA_TYPE_DEV;
+	buf->dev.dev = pcm->card->dev;
+	buf->private_data = NULL;
+	buf->bytes = size;
+
+	return 0;
+}
+EXPORT_SYMBOL(snd_pcm_lib_alloc_writecombine_dma_buffer);
+
+void snd_pcm_lib_dealloc_writecombine_dma_buffer(struct snd_pcm *pcm,
+		int stream)
+{
+	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
+	struct snd_dma_buffer *buf;
+
+	if (!substream)
+		return;
+
+	buf = &substream->dma_buffer;
+	if (!buf->area)
+		return;
+
+	dma_free_writecombine(pcm->card->dev, buf->bytes,
+			buf->area, buf->addr);
+	buf->area = NULL;
+}
+EXPORT_SYMBOL(snd_pcm_lib_dealloc_writecombine_dma_buffer);
+
+int snd_pcm_lib_mmap_writecombine_dma_buffer(
+		struct snd_pcm_substream *substream,
+		struct vm_area_struct *vma)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
+			runtime->dma_area, runtime->dma_addr,
+			runtime->dma_bytes);
+}
+EXPORT_SYMBOL(snd_pcm_lib_mmap_writecombine_dma_buffer);
+#endif
+
 /*
  * mmap DMA buffer
  */
-- 
1.7.1.1

[PATCH 2/3] ASoC: add apis for creating/free pcm dma buffer

[Laxman Dewangan]

Add APIs for creating and freeing the new pcm writecombine
dma buffer.
Adding these APIs for ARM only and if other architecture
supports the write combine dma buffer then these APIs can
be extended to that architecture.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 include/sound/soc.h  |   13 +++++++++++++
 sound/soc/soc-core.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index e063380..6050ac7 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1153,6 +1153,19 @@ int snd_soc_of_parse_card_name(struct snd_soc_card *card,
 int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 				   const char *propname);
 
+/**
+ * Create the snd_soc_pcm using the writecombine dma buffer.
+ * The writecombine dma buffer is supported on some architecture
+ * like ARM and so restricting this for ARM only.
+ * The more architecture can be added here or make it available for
+ * all architecture if writecombine dma buffer is supported.
+ */
+#ifdef CONFIG_ARM
+int snd_soc_pcm_new_writecombine_dma_buffer(struct snd_soc_pcm_runtime *rtd,
+		size_t max_bytes);
+void snd_soc_pcm_free_writecombine_dma_buffer(struct snd_pcm *pcm);
+#endif
+
 #include <sound/soc-dai.h>
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fe16135..d2c0dc4 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -29,6 +29,7 @@
 #include <linux/pm.h>
 #include <linux/bitops.h>
 #include <linux/debugfs.h>
+#include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
 #include <linux/ctype.h>
 #include <linux/slab.h>
@@ -4094,6 +4095,55 @@ int snd_soc_of_parse_card_name(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_card_name);
 
+#ifdef CONFIG_ARM
+static u64 snd_soc_pcm_wc_dma_mask = DMA_BIT_MASK(32);
+int snd_soc_pcm_new_writecombine_dma_buffer(struct snd_soc_pcm_runtime *rtd,
+		size_t max_bytes)
+{
+	struct snd_card *card = rtd->card->snd_card;
+	struct snd_pcm *pcm = rtd->pcm;
+	int ret = 0;
+
+	if (!card->dev->dma_mask)
+		card->dev->dma_mask = &snd_soc_pcm_wc_dma_mask;
+	if (!card->dev->coherent_dma_mask)
+		card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
+		ret = snd_pcm_lib_alloc_writecombine_dma_buffer(pcm,
+				max_bytes, SNDRV_PCM_STREAM_PLAYBACK);
+		if (ret)
+			goto err;
+	}
+	if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {
+		ret = snd_pcm_lib_alloc_writecombine_dma_buffer(pcm,
+				max_bytes, SNDRV_PCM_STREAM_CAPTURE);
+		if (ret)
+			goto err_free_play;
+	}
+
+	return 0;
+
+err_free_play:
+	snd_pcm_lib_dealloc_writecombine_dma_buffer(pcm,
+				SNDRV_PCM_STREAM_PLAYBACK);
+err:
+	return ret;
+
+}
+EXPORT_SYMBOL_GPL(snd_soc_pcm_new_writecombine_dma_buffer);
+
+void snd_soc_pcm_free_writecombine_dma_buffer(struct snd_pcm *pcm)
+{
+	snd_pcm_lib_dealloc_writecombine_dma_buffer(pcm,
+					SNDRV_PCM_STREAM_CAPTURE);
+	snd_pcm_lib_dealloc_writecombine_dma_buffer(pcm,
+					SNDRV_PCM_STREAM_PLAYBACK);
+}
+EXPORT_SYMBOL_GPL(snd_soc_pcm_free_writecombine_dma_buffer);
+#endif
+
+
 int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 				   const char *propname)
 {
-- 
1.7.1.1

[PATCH 3/3] ASoC: tegra: use core/pcm library for pcm buffer allocation

[Laxman Dewangan]

The soc/core support the APIs for allocate and deallocate the
writecombine dma buffer for pcm substream.
Use these APIs and remove the local implementation for achieving
these functionalities.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 sound/soc/tegra/tegra_pcm.c |   92 ++-----------------------------------------
 1 files changed, 4 insertions(+), 88 deletions(-)

diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c
index 127348d..6754697 100644
--- a/sound/soc/tegra/tegra_pcm.c
+++ b/sound/soc/tegra/tegra_pcm.c
@@ -263,18 +263,6 @@ static snd_pcm_uframes_t tegra_pcm_pointer(struct snd_pcm_substream *substream)
 	return prtd->period_index * runtime->period_size;
 }
 
-
-static int tegra_pcm_mmap(struct snd_pcm_substream *substream,
-				struct vm_area_struct *vma)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-
-	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
-					runtime->dma_area,
-					runtime->dma_addr,
-					runtime->dma_bytes);
-}
-
 static struct snd_pcm_ops tegra_pcm_ops = {
 	.open		= tegra_pcm_open,
 	.close		= tegra_pcm_close,
@@ -283,91 +271,19 @@ static struct snd_pcm_ops tegra_pcm_ops = {
 	.hw_free	= tegra_pcm_hw_free,
 	.trigger	= tegra_pcm_trigger,
 	.pointer	= tegra_pcm_pointer,
-	.mmap		= tegra_pcm_mmap,
+	.mmap		= snd_pcm_lib_mmap_writecombine_dma_buffer,
 };
 
-static int tegra_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 = tegra_pcm_hardware.buffer_bytes_max;
-
-	buf->area = dma_alloc_writecombine(pcm->card->dev, size,
-						&buf->addr, GFP_KERNEL);
-	if (!buf->area)
-		return -ENOMEM;
-
-	buf->dev.type = SNDRV_DMA_TYPE_DEV;
-	buf->dev.dev = pcm->card->dev;
-	buf->private_data = NULL;
-	buf->bytes = size;
-
-	return 0;
-}
-
-static void tegra_pcm_deallocate_dma_buffer(struct snd_pcm *pcm, int stream)
-{
-	struct snd_pcm_substream *substream;
-	struct snd_dma_buffer *buf;
-
-	substream = pcm->streams[stream].substream;
-	if (!substream)
-		return;
-
-	buf = &substream->dma_buffer;
-	if (!buf->area)
-		return;
-
-	dma_free_writecombine(pcm->card->dev, buf->bytes,
-				buf->area, buf->addr);
-	buf->area = NULL;
-}
-
-static u64 tegra_dma_mask = DMA_BIT_MASK(32);
-
 static int tegra_pcm_new(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_card *card = rtd->card->snd_card;
-	struct snd_pcm *pcm = rtd->pcm;
-	int ret = 0;
-
-	if (!card->dev->dma_mask)
-		card->dev->dma_mask = &tegra_dma_mask;
-	if (!card->dev->coherent_dma_mask)
-		card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
-
-	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
-		ret = tegra_pcm_preallocate_dma_buffer(pcm,
-						SNDRV_PCM_STREAM_PLAYBACK);
-		if (ret)
-			goto err;
-	}
-
-	if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {
-		ret = tegra_pcm_preallocate_dma_buffer(pcm,
-						SNDRV_PCM_STREAM_CAPTURE);
-		if (ret)
-			goto err_free_play;
-	}
-
-	return 0;
-
-err_free_play:
-	tegra_pcm_deallocate_dma_buffer(pcm, SNDRV_PCM_STREAM_PLAYBACK);
-err:
-	return ret;
-}
-
-static void tegra_pcm_free(struct snd_pcm *pcm)
-{
-	tegra_pcm_deallocate_dma_buffer(pcm, SNDRV_PCM_STREAM_CAPTURE);
-	tegra_pcm_deallocate_dma_buffer(pcm, SNDRV_PCM_STREAM_PLAYBACK);
+	return snd_soc_pcm_new_writecombine_dma_buffer(rtd,
+			tegra_pcm_hardware.buffer_bytes_max);
 }
 
 static struct snd_soc_platform_driver tegra_pcm_platform = {
 	.ops		= &tegra_pcm_ops,
 	.pcm_new	= tegra_pcm_new,
-	.pcm_free	= tegra_pcm_free,
+	.pcm_free	= snd_soc_pcm_free_writecombine_dma_buffer,
 };
 
 int __devinit tegra_pcm_platform_register(struct device *dev)
-- 
1.7.1.1

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Takashi Iwai]

At Fri, 29 Jun 2012 15:53:15 +0530,
Laxman Dewangan wrote:
> 
> Some of the ARM based soc allocate the writecombine dma buffer for
> pcm substreams. They have the same codes for managing this buffer.
> Moving this to the core/pcm files so that they can use that directly.
> 
> Remove the code from Tegra PCM and use these new library function.
> 
> This is enabled only for ARM specific and can be extended to other
> architecture if they support the writecombine dma buffer.
> 
> This patch is based on detail discussion on patch:
> [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap
> And suggestion from Lars and Takashi.

Looking through your patch, I think an easier integration is just to
add writecombine option to memalloc.c which calls
dma_alloc_writecombine() instead of dma_alloc_coherent().

The addition for mmap is still an open question.  Again, an easier
option so far looks like just add the call of dma_alloc_writecombine()
in snd_pcm_lib_default_mmap().  Alternatively, we can create an
individual mmap pcm_ops as we discussed.

Below is a totally untested patch, but you can imagine what I meant.

After this change, replace with snd_pcm_lib_malloc_pages with
SNDRV_DMA_TYPE_DEV_WC, and that's all you need in the driver side.


Takashi

---
diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h
index c425062..7840340 100644
--- a/include/sound/memalloc.h
+++ b/include/sound/memalloc.h
@@ -52,6 +52,7 @@ struct snd_dma_device {
 #else
 #define SNDRV_DMA_TYPE_DEV_SG	SNDRV_DMA_TYPE_DEV /* no SG-buf support */
 #endif
+#define SNDRV_DMA_TYPE_DEV_WC		4	/* writecombine pages */
 
 /*
  * info for buffer allocation
diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
index 6915692..11108e8 100644
--- a/sound/core/memalloc.c
+++ b/sound/core/memalloc.c
@@ -124,8 +124,12 @@ void snd_free_pages(void *ptr, size_t size)
  */
 
 #ifdef CONFIG_HAS_DMA
-/* allocate the coherent DMA pages */
-static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *dma)
+static void *__snd_malloc_dev_pages(struct device *dev, size_t size,
+				    dma_addr_t *dma,
+				    void *(*func)(struct device *dev,
+						  size_t size,
+						  dma_addr_t *dma_handle,
+						  gfp_t flag))
 {
 	int pg;
 	void *res;
@@ -138,16 +142,18 @@ static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *d
 		| __GFP_COMP	/* compound page lets parts be mapped */
 		| __GFP_NORETRY /* don't trigger OOM-killer */
 		| __GFP_NOWARN; /* no stack trace print - this call is non-critical */
-	res = dma_alloc_coherent(dev, PAGE_SIZE << pg, dma, gfp_flags);
+	res = func(dev, PAGE_SIZE << pg, dma, gfp_flags);
 	if (res != NULL)
 		inc_snd_pages(pg);
 
 	return res;
 }
 
-/* free the coherent DMA pages */
-static void snd_free_dev_pages(struct device *dev, size_t size, void *ptr,
-			       dma_addr_t dma)
+static void __snd_free_dev_pages(struct device *dev, size_t size, void *ptr,
+				 dma_addr_t dma,
+				 void (*func)(struct device *dev,
+					      size_t size, void *vaddr,
+					      dma_addr_t dma_handle))
 {
 	int pg;
 
@@ -155,8 +161,36 @@ static void snd_free_dev_pages(struct device *dev, size_t size, void *ptr,
 		return;
 	pg = get_order(size);
 	dec_snd_pages(pg);
-	dma_free_coherent(dev, PAGE_SIZE << pg, ptr, dma);
+	func(dev, PAGE_SIZE << pg, ptr, dma);
+}
+
+/* allocate the coherent DMA pages */
+static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *dma)
+{
+	return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);
+}
+
+/* free the coherent DMA pages */
+static void snd_free_dev_pages(struct device *dev, size_t size, void *ptr,
+ 			       dma_addr_t dma)
+{
+	__snd_free_dev_pages(dev, ptr, dma, dma_free_coherent);
 }
+
+#ifdef CONFIG_ARM
+/* allocate the writecombine DMA pages */
+static void *snd_malloc_dev_wc_pages(struct device *dev, size_t size, dma_addr_t *dma)
+{
+	return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_writecombine);
+}
+
+/* free the writecombine DMA pages */
+static void snd_free_dev_wc_pages(struct device *dev, size_t size, void *ptr,
+				  dma_addr_t dma)
+{
+	__snd_free_dev_pages(dev, ptr, dma, dma_free_writecombine);
+}
+#endif /* CONFIG_ARM */
 #endif /* CONFIG_HAS_DMA */
 
 /*
@@ -200,6 +234,11 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size,
 	case SNDRV_DMA_TYPE_DEV:
 		dmab->area = snd_malloc_dev_pages(device, size, &dmab->addr);
 		break;
+#ifdef CONFIG_ARM
+	case SNDRV_DMA_TYPE_DEV_WC:
+		dmab->area = snd_malloc_dev_wc_pages(device, size, &dmab->addr);
+		break;
+#endif
 #endif
 #ifdef CONFIG_SND_DMA_SGBUF
 	case SNDRV_DMA_TYPE_DEV_SG:
@@ -272,6 +311,11 @@ void snd_dma_free_pages(struct snd_dma_buffer *dmab)
 	case SNDRV_DMA_TYPE_DEV:
 		snd_free_dev_pages(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
 		break;
+#ifdef CONFIG_ARM
+	case SNDRV_DMA_TYPE_DEV_WC:
+		snd_free_dev_wc_pages(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
+		break;
+#endif
 #endif
 #ifdef CONFIG_SND_DMA_SGBUF
 	case SNDRV_DMA_TYPE_DEV_SG:
@@ -378,7 +422,7 @@ static int snd_mem_proc_read(struct seq_file *seq, void *offset)
 	long pages = snd_allocated_pages >> (PAGE_SHIFT-12);
 	struct snd_mem_list *mem;
 	int devno;
-	static char *types[] = { "UNKNOWN", "CONT", "DEV", "DEV-SG" };
+	static char *types[] = { "UNKNOWN", "CONT", "DEV", "DEV-SG", "WC" };
 
 	mutex_lock(&list_mutex);
 	seq_printf(seq, "pages  : %li bytes (%li pages per %likB)\n",
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 53b5ada..c499791 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3170,6 +3170,14 @@ int snd_pcm_lib_default_mmap(struct snd_pcm_substream *substream,
 			     struct vm_area_struct *area)
 {
 	area->vm_flags |= VM_RESERVED;
+#ifdef CONFIG_ARM
+	if (substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV_WC)
+		return dma_mmap_writecombine(substream->dma_buffer.dev.dev,
+					 area,
+					 substream->runtime->dma_area,
+					 substream->runtime->dma_addr,
+					 area->vm_end - area->vm_start);
+#endif
 #ifdef ARCH_HAS_DMA_MMAP_COHERENT
 	if (!substream->ops->page &&
 	    substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Laxman Dewangan]

Hi Takashi,
Thanks for sample code. It helps lot.

On Friday 29 June 2012 05:43 PM, Takashi Iwai wrote:
> At Fri, 29 Jun 2012 15:53:15 +0530,
> Laxman Dewangan wrote:
>> Some of the ARM based soc allocate the writecombine dma buffer for
>> pcm substreams. They have the same codes for managing this buffer.
>> Moving this to the core/pcm files so that they can use that directly.
>>
>> Remove the code from Tegra PCM and use these new library function.
>>
>> This is enabled only for ARM specific and can be extended to other
>> architecture if they support the writecombine dma buffer.
>>
>> This patch is based on detail discussion on patch:
>> [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap
>> And suggestion from Lars and Takashi.
> Looking through your patch, I think an easier integration is just to
> add writecombine option to memalloc.c which calls
> dma_alloc_writecombine() instead of dma_alloc_coherent().
>
> The addition for mmap is still an open question.  Again, an easier
> option so far looks like just add the call of dma_alloc_writecombine()
> in snd_pcm_lib_default_mmap().  Alternatively, we can create an
> individual mmap pcm_ops as we discussed.
>
> Below is a totally untested patch, but you can imagine what I meant.
>
> After this change, replace with snd_pcm_lib_malloc_pages with
> SNDRV_DMA_TYPE_DEV_WC, and that's all you need in the driver side.

We can not use the snd_pcm_lib_malloc_pages() in the pcm_new callback 
because at this time the substream->runtime is not initialized and it 
leads to the kernel crash.

I used the apis as

int snd_soc_pcm_new_wc_dma_buffer(struct snd_soc_pcm_runtime *rtd,
                 size_t max_bytes)
{
      :::::::::::::
         substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
         if (substream) {
                 substream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_WC;
                 ret = snd_pcm_lib_malloc_pages(substream, max_bytes);
                 if (ret)
                         goto err;
         }
::::::::
}


int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t 
size)
{
         struct snd_pcm_runtime *runtime;
         struct snd_dma_buffer *dmab = NULL;

         if (PCM_RUNTIME_CHECK(substream))
                 return -EINVAL;

         if (snd_BUG_ON(substream->dma_buffer.dev.type ==
                        SNDRV_DMA_TYPE_UNKNOWN))
                 return -EINVAL;

         runtime = substream->runtime;

         if (runtime->dma_buffer_p) {
---------------Kernel crash at this point ----------

So I used the snd_dma_alloc_pages() from the driver to allocate WC memory.


> +/* allocate the coherent DMA pages */
> +static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *dma)
> +{
> +	return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);

This does not get compiled in ARM because dma_alloc_coherant is macro 
defined as

#define dma_alloc_coherent(d, s, h, f) dma_alloc_attrs(d, s, h, f, NULL)

static inline void *dma_alloc_attrs(struct device *dev, size_t size,
                                        dma_addr_t *dma_handle, gfp_t flag,
                                        struct dma_attrs *attrs)


I fixed this by
static void *_dma_alloc_coherent(struct device *dev, size_t size,
                 dma_addr_t *dma_handle, gfp_t flag)
{
         return dma_alloc_coherent(dev, size, dma_handle, flag);
}

/* allocate the coherent DMA pages */
static void *snd_malloc_dev_pages(struct device *dev, size_t size, 
dma_addr_t *dma)
{
         return _snd_malloc_dev_pages(dev, size, dma, _dma_alloc_coherent);
}

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Takashi Iwai]

At Fri, 29 Jun 2012 20:34:24 +0530,
Laxman Dewangan wrote:
> 
> Hi Takashi,
> Thanks for sample code. It helps lot.
> 
> On Friday 29 June 2012 05:43 PM, Takashi Iwai wrote:
> > At Fri, 29 Jun 2012 15:53:15 +0530,
> > Laxman Dewangan wrote:
> >> Some of the ARM based soc allocate the writecombine dma buffer for
> >> pcm substreams. They have the same codes for managing this buffer.
> >> Moving this to the core/pcm files so that they can use that directly.
> >>
> >> Remove the code from Tegra PCM and use these new library function.
> >>
> >> This is enabled only for ARM specific and can be extended to other
> >> architecture if they support the writecombine dma buffer.
> >>
> >> This patch is based on detail discussion on patch:
> >> [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap
> >> And suggestion from Lars and Takashi.
> > Looking through your patch, I think an easier integration is just to
> > add writecombine option to memalloc.c which calls
> > dma_alloc_writecombine() instead of dma_alloc_coherent().
> >
> > The addition for mmap is still an open question.  Again, an easier
> > option so far looks like just add the call of dma_alloc_writecombine()
> > in snd_pcm_lib_default_mmap().  Alternatively, we can create an
> > individual mmap pcm_ops as we discussed.
> >
> > Below is a totally untested patch, but you can imagine what I meant.
> >
> > After this change, replace with snd_pcm_lib_malloc_pages with
> > SNDRV_DMA_TYPE_DEV_WC, and that's all you need in the driver side.
> 
> We can not use the snd_pcm_lib_malloc_pages() in the pcm_new callback 
> because at this time the substream->runtime is not initialized and it 
> leads to the kernel crash.
> 
> I used the apis as
> 
> int snd_soc_pcm_new_wc_dma_buffer(struct snd_soc_pcm_runtime *rtd,
>                  size_t max_bytes)
> {
>       :::::::::::::
>          substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
>          if (substream) {
>                  substream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_WC;
>                  ret = snd_pcm_lib_malloc_pages(substream, max_bytes);
>                  if (ret)
>                          goto err;
>          }
> ::::::::
> }
> 
> 
> int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t 
> size)
> {
>          struct snd_pcm_runtime *runtime;
>          struct snd_dma_buffer *dmab = NULL;
> 
>          if (PCM_RUNTIME_CHECK(substream))
>                  return -EINVAL;
> 
>          if (snd_BUG_ON(substream->dma_buffer.dev.type ==
>                         SNDRV_DMA_TYPE_UNKNOWN))
>                  return -EINVAL;
> 
>          runtime = substream->runtime;
> 
>          if (runtime->dma_buffer_p) {
> ---------------Kernel crash at this point ----------
> 
> So I used the snd_dma_alloc_pages() from the driver to allocate WC memory.

snd_pcm_lib_malloc_pages() should be called only from hw_params.
Prior to that, you need to set up the dam_buffer type by calling
snd_pcm_lib_preallocate_pages() or
snd_pcm_lib_preallocate_pages_for_all() at the time to create a PCM
instance.  The size can be 0 if not necessary to allocate there but
later on demand.

> 
> > +/* allocate the coherent DMA pages */
> > +static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *dma)
> > +{
> > +	return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);
> 
> This does not get compiled in ARM because dma_alloc_coherant is macro 
> defined as
> 
> #define dma_alloc_coherent(d, s, h, f) dma_alloc_attrs(d, s, h, f, NULL)

Ah, OK.


thanks,

Takashi

> static inline void *dma_alloc_attrs(struct device *dev, size_t size,
>                                         dma_addr_t *dma_handle, gfp_t flag,
>                                         struct dma_attrs *attrs)
> 
> 
> I fixed this by
> static void *_dma_alloc_coherent(struct device *dev, size_t size,
>                  dma_addr_t *dma_handle, gfp_t flag)
> {
>          return dma_alloc_coherent(dev, size, dma_handle, flag);
> }
> 
> /* allocate the coherent DMA pages */
> static void *snd_malloc_dev_pages(struct device *dev, size_t size, 
> dma_addr_t *dma)
> {
>          return _snd_malloc_dev_pages(dev, size, dma, _dma_alloc_coherent);
> }
> 

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Laxman Dewangan]

On Friday 29 June 2012 08:52 PM, Takashi Iwai wrote:
> At Fri, 29 Jun 2012 20:34:24 +0530,
> Laxman Dewangan wrote:
>> Hi Takashi,
>> Thanks for sample code. It helps lot.
>>
>> int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t
>> size)
>> {
>>           struct snd_pcm_runtime *runtime;
>>           struct snd_dma_buffer *dmab = NULL;
>>
>>           if (PCM_RUNTIME_CHECK(substream))
>>                   return -EINVAL;
>>
>>           if (snd_BUG_ON(substream->dma_buffer.dev.type ==
>>                          SNDRV_DMA_TYPE_UNKNOWN))
>>                   return -EINVAL;
>>
>>           runtime = substream->runtime;
>>
>>           if (runtime->dma_buffer_p) {
>> ---------------Kernel crash at this point ----------
>>
>> So I used the snd_dma_alloc_pages() from the driver to allocate WC memory.
> snd_pcm_lib_malloc_pages() should be called only from hw_params.
> Prior to that, you need to set up the dam_buffer type by calling
> snd_pcm_lib_preallocate_pages() or
> snd_pcm_lib_preallocate_pages_for_all() at the time to create a PCM
> instance.  The size can be 0 if not necessary to allocate there but
> later on demand.

It worked well. Cool implementation.

I will send the patch of memalloc and for Tegra actually using this feature.

Thanks for suggestion and help.


Laxman

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Lars-Peter Clausen]

On 06/29/2012 05:04 PM, Laxman Dewangan wrote:
> [...]
> 
> 
>> +/* allocate the coherent DMA pages */
>> +static void *snd_malloc_dev_pages(struct device *dev, size_t size,
>> dma_addr_t *dma)
>> +{
>> +    return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);
> 
> This does not get compiled in ARM because dma_alloc_coherant is macro
> defined as
> 
> #define dma_alloc_coherent(d, s, h, f) dma_alloc_attrs(d, s, h, f, NULL)
> 
> static inline void *dma_alloc_attrs(struct device *dev, size_t size,
>                                        dma_addr_t *dma_handle, gfp_t flag,
>                                        struct dma_attrs *attrs)
> 
> 
> I fixed this by
> static void *_dma_alloc_coherent(struct device *dev, size_t size,
>                 dma_addr_t *dma_handle, gfp_t flag)
> {
>         return dma_alloc_coherent(dev, size, dma_handle, flag);
> }
> 
> /* allocate the coherent DMA pages */
> static void *snd_malloc_dev_pages(struct device *dev, size_t size,
> dma_addr_t *dma)
> {
>         return _snd_malloc_dev_pages(dev, size, dma, _dma_alloc_coherent);
> }

I think all dma_alloc_* functions use dma_alloc_attrs internally these days.
I think it might be a better idea just to use dma_alloc_attrs in
__snd_malloc_dev_pages and just pass a dma_attrs struct instead of a function.

Something like:

static void *__snd_malloc_dev_pages(struct device *dev, size_t size,
			    dma_addr_t *dma, struct dma_attrs *attrs)
{
....
	res = dma_alloc_attrs(dev, PAGE_SIZE << pg, dma, gfp_flags, attrs);
....
}


static void *snd_malloc_dev_pages(struct device *dev, size_t size,
	dma_addr_t *dma)
{
	return __snd_malloc_dev_pages(dev, size, dma, NULL);
}

static void *snd_malloc_dev_wc_pages(struct device *dev, size_t size,
	dma_addr_t *dma)
{
	DEFINE_DMA_ATTRS(attrs);
	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &attrs);

	return __snd_malloc_dev_pages(dev, size, dma, &attrs);
}

Same applies for freeing the memory again.

- Lars

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Takashi Iwai]

At Fri, 29 Jun 2012 18:06:52 +0200,
Lars-Peter Clausen wrote:
> 
> On 06/29/2012 05:04 PM, Laxman Dewangan wrote:
> > [...]
> > 
> > 
> >> +/* allocate the coherent DMA pages */
> >> +static void *snd_malloc_dev_pages(struct device *dev, size_t size,
> >> dma_addr_t *dma)
> >> +{
> >> +    return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);
> > 
> > This does not get compiled in ARM because dma_alloc_coherant is macro
> > defined as
> > 
> > #define dma_alloc_coherent(d, s, h, f) dma_alloc_attrs(d, s, h, f, NULL)
> > 
> > static inline void *dma_alloc_attrs(struct device *dev, size_t size,
> >                                        dma_addr_t *dma_handle, gfp_t flag,
> >                                        struct dma_attrs *attrs)
> > 
> > 
> > I fixed this by
> > static void *_dma_alloc_coherent(struct device *dev, size_t size,
> >                 dma_addr_t *dma_handle, gfp_t flag)
> > {
> >         return dma_alloc_coherent(dev, size, dma_handle, flag);
> > }
> > 
> > /* allocate the coherent DMA pages */
> > static void *snd_malloc_dev_pages(struct device *dev, size_t size,
> > dma_addr_t *dma)
> > {
> >         return _snd_malloc_dev_pages(dev, size, dma, _dma_alloc_coherent);
> > }
> 
> I think all dma_alloc_* functions use dma_alloc_attrs internally these days.
> I think it might be a better idea just to use dma_alloc_attrs in
> __snd_malloc_dev_pages and just pass a dma_attrs struct instead of a function.
> 
> Something like:
> 
> static void *__snd_malloc_dev_pages(struct device *dev, size_t size,
> 			    dma_addr_t *dma, struct dma_attrs *attrs)
> {
> ....
> 	res = dma_alloc_attrs(dev, PAGE_SIZE << pg, dma, gfp_flags, attrs);
> ....
> }
> 
> 
> static void *snd_malloc_dev_pages(struct device *dev, size_t size,
> 	dma_addr_t *dma)
> {
> 	return __snd_malloc_dev_pages(dev, size, dma, NULL);
> }
> 
> static void *snd_malloc_dev_wc_pages(struct device *dev, size_t size,
> 	dma_addr_t *dma)
> {
> 	DEFINE_DMA_ATTRS(attrs);
> 	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &attrs);
> 
> 	return __snd_malloc_dev_pages(dev, size, dma, &attrs);
> }
> 
> Same applies for freeing the memory again.

Yes, that'd be cleaner.
But I prefer the code still workable on 3.5-base kernel, which makes
easier to test and merge for 3.6, at first. 

So, let's clean up these indirect calls once after all things are
settled down at 3.6-rc1.  It can be applied even before 3.6-rc2.


Takashi

Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Lars-Peter Clausen]

On 06/29/2012 06:18 PM, Takashi Iwai wrote:
> At Fri, 29 Jun 2012 18:06:52 +0200,
> Lars-Peter Clausen wrote:
>>
>> On 06/29/2012 05:04 PM, Laxman Dewangan wrote:
>>> [...]
>>>
>>>
>>>> +/* allocate the coherent DMA pages */
>>>> +static void *snd_malloc_dev_pages(struct device *dev, size_t size,
>>>> dma_addr_t *dma)
>>>> +{
>>>> +    return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);
>>>
>>> This does not get compiled in ARM because dma_alloc_coherant is macro
>>> defined as
>>>
>>> #define dma_alloc_coherent(d, s, h, f) dma_alloc_attrs(d, s, h, f, NULL)
>>>
>>> static inline void *dma_alloc_attrs(struct device *dev, size_t size,
>>>                                        dma_addr_t *dma_handle, gfp_t flag,
>>>                                        struct dma_attrs *attrs)
>>>
>>>
>>> I fixed this by
>>> static void *_dma_alloc_coherent(struct device *dev, size_t size,
>>>                 dma_addr_t *dma_handle, gfp_t flag)
>>> {
>>>         return dma_alloc_coherent(dev, size, dma_handle, flag);
>>> }
>>>
>>> /* allocate the coherent DMA pages */
>>> static void *snd_malloc_dev_pages(struct device *dev, size_t size,
>>> dma_addr_t *dma)
>>> {
>>>         return _snd_malloc_dev_pages(dev, size, dma, _dma_alloc_coherent);
>>> }
>>
>> I think all dma_alloc_* functions use dma_alloc_attrs internally these days.
>> I think it might be a better idea just to use dma_alloc_attrs in
>> __snd_malloc_dev_pages and just pass a dma_attrs struct instead of a function.
>>
>> Something like:
>>
>> static void *__snd_malloc_dev_pages(struct device *dev, size_t size,
>> 			    dma_addr_t *dma, struct dma_attrs *attrs)
>> {
>> ....
>> 	res = dma_alloc_attrs(dev, PAGE_SIZE << pg, dma, gfp_flags, attrs);
>> ....
>> }
>>
>>
>> static void *snd_malloc_dev_pages(struct device *dev, size_t size,
>> 	dma_addr_t *dma)
>> {
>> 	return __snd_malloc_dev_pages(dev, size, dma, NULL);
>> }
>>
>> static void *snd_malloc_dev_wc_pages(struct device *dev, size_t size,
>> 	dma_addr_t *dma)
>> {
>> 	DEFINE_DMA_ATTRS(attrs);
>> 	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &attrs);
>>
>> 	return __snd_malloc_dev_pages(dev, size, dma, &attrs);
>> }
>>
>> Same applies for freeing the memory again.
> 
> Yes, that'd be cleaner.
> But I prefer the code still workable on 3.5-base kernel, which makes
> easier to test and merge for 3.6, at first. 

Actually that part is already in 3.5. But I've just had a closer look and it
looks as if even for 3.6 not all archs have been converted to
dma_alloc_attrs yet :/