[PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Nicolin Chen]

Now it's quite common that an SoC contains its on-chip internal memory.
By using this memory space for DMA buffer during audio playback/record,
we can shutdown the voltage for external memory to save power.

Thus add new DEV type with iram malloc()/free() and accordingly modify
current default mmap() for the iram circumstance.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
Changelog
v3->v4:
 * Appropriately placed '#ifdef CONFIG_OF'
 * Tested by adding '#undef CONFIG_OF' to modified files, and passed compiling.
v2->v3:
 * Moved iram specific mmap procedure out of ifdef ARCH_HAS_DMA_MMAP_COHERENT
v1->v2:
 * Added of_node check
 * Dropped noops and unused physical addr in snd_free_dev_iram()

---
 include/sound/memalloc.h |  5 ++++
 sound/core/memalloc.c    | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
 sound/core/pcm_native.c  |  7 +++++
 3 files changed, 83 insertions(+)

diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h
index cf15b82..b53443e 100644
--- a/include/sound/memalloc.h
+++ b/include/sound/memalloc.h
@@ -52,6 +52,11 @@ struct snd_dma_device {
 #else
 #define SNDRV_DMA_TYPE_DEV_SG	SNDRV_DMA_TYPE_DEV /* no SG-buf support */
 #endif
+#ifdef CONFIG_OF
+#define SNDRV_DMA_TYPE_DEV_IRAM		4	/* generic device iram-buffer */
+#else
+#define SNDRV_DMA_TYPE_DEV_IRAM		SNDRV_DMA_TYPE_DEV
+#endif
 
 /*
  * info for buffer allocation
diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
index bdf826f..fc0f27b 100644
--- a/sound/core/memalloc.c
+++ b/sound/core/memalloc.c
@@ -30,6 +30,7 @@
 #include <linux/seq_file.h>
 #include <asm/uaccess.h>
 #include <linux/dma-mapping.h>
+#include <linux/genalloc.h>
 #include <linux/moduleparam.h>
 #include <linux/mutex.h>
 #include <sound/memalloc.h>
@@ -157,6 +158,61 @@ static void snd_free_dev_pages(struct device *dev, size_t size, void *ptr,
 	dec_snd_pages(pg);
 	dma_free_coherent(dev, PAGE_SIZE << pg, ptr, dma);
 }
+
+#ifdef CONFIG_OF
+/**
+ * snd_malloc_dev_iram - allocate memory from on-chip internal memory
+ * @dev: DMA device pointer
+ * @size: number of bytes to allocate from the iram
+ * @dma: dma-view physical address
+ *
+ * Return cpu-view address or NULL indicating allocating failure
+ *
+ * This function requires iram phandle provided via of_node
+ */
+void *snd_malloc_dev_iram(struct device *dev, size_t size, dma_addr_t *dma)
+{
+	struct gen_pool *pool;
+	unsigned long vaddr;
+
+	if (!dev->of_node)
+		return NULL;
+
+	pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
+	if (!pool)
+		return NULL;
+
+	vaddr = gen_pool_alloc(pool, size);
+	if (!vaddr)
+		return NULL;
+
+	*dma = gen_pool_virt_to_phys(pool, vaddr);
+
+	return (void *)vaddr;
+}
+
+/**
+ * snd_free_dev_iram - free allocated specific memory from on-chip internal memory
+ * @dev: DMA device pointer
+ * @size: size in bytes of memory to free
+ * @ptr: cpu-view address returned from snd_malloc_dev_iram
+ *
+ * This function requires iram phandle provided via of_node
+ */
+void snd_free_dev_iram(struct device *dev, size_t size, void *ptr)
+{
+	struct gen_pool *pool;
+
+	if (!dev->of_node)
+		return;
+
+	pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
+	if (!pool)
+		return;
+
+	gen_pool_free(pool, (unsigned long)ptr, size);
+}
+#endif /* CONFIG_OF */
 #endif /* CONFIG_HAS_DMA */
 
 /*
@@ -197,6 +253,16 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size,
 		dmab->addr = 0;
 		break;
 #ifdef CONFIG_HAS_DMA
+#ifdef CONFIG_OF
+	case SNDRV_DMA_TYPE_DEV_IRAM:
+		dmab->area = snd_malloc_dev_iram(device, size, &dmab->addr);
+		if (dmab->area)
+			break;
+		/* Internal memory might have limited size and no enough space,
+		 * so if we fail to malloc, try to fetch memory traditionally.
+		 */
+		dmab->dev.type = SNDRV_DMA_TYPE_DEV;
+#endif
 	case SNDRV_DMA_TYPE_DEV:
 		dmab->area = snd_malloc_dev_pages(device, size, &dmab->addr);
 		break;
@@ -269,6 +335,11 @@ void snd_dma_free_pages(struct snd_dma_buffer *dmab)
 		snd_free_pages(dmab->area, dmab->bytes);
 		break;
 #ifdef CONFIG_HAS_DMA
+#ifdef CONFIG_OF
+	case SNDRV_DMA_TYPE_DEV_IRAM:
+		snd_free_dev_iram(dmab->dev.dev, dmab->bytes, dmab->area);
+			break;
+#endif
 	case SNDRV_DMA_TYPE_DEV:
 		snd_free_dev_pages(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
 		break;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index a68d4c6..74de2a3 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3199,6 +3199,13 @@ int snd_pcm_lib_default_mmap(struct snd_pcm_substream *substream,
 			     struct vm_area_struct *area)
 {
 	area->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+	if (substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV_IRAM &&
+			SNDRV_DMA_TYPE_DEV_IRAM != SNDRV_DMA_TYPE_DEV) {
+		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
+		return remap_pfn_range(area, area->vm_start,
+				substream->dma_buffer.addr >> PAGE_SHIFT,
+				area->vm_end - area->vm_start, area->vm_page_prot);
+	}
 #ifdef ARCH_HAS_DMA_MMAP_COHERENT
 	if (!substream->ops->page &&
 	    substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)
-- 
1.8.4

Re: [PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 624 bytes --]

On Wed, Oct 16, 2013 at 06:53:21PM +0800, Nicolin Chen wrote:

> +	if (!dev->of_node)
> +		return NULL;
> +
> +	pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
> +	if (!pool)
> +		return NULL;

Can you refactor these ifdefs so they just protect this OF specific code
and just have the allocation fail otherwise please?  That way the OF
dependency is minimised - we should be able to support this on other
platforms in the future and the rest of the code will be the same.

Also is iram the best name?  It makes me think of the dedicated fast
instruction RAM that some processors have.  sram perhaps (for static
RAM)?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Chen Guangyu-B42378]

Okay,  I will try to refine it tomorrow.

Thank you for the comments.

Sent by Android device.

Mark Brown <broonie@kernel.org> wrote:


On Wed, Oct 16, 2013 at 06:53:21PM +0800, Nicolin Chen wrote:

> +     if (!dev->of_node)
> +             return NULL;
> +
> +     pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
> +     if (!pool)
> +             return NULL;

Can you refactor these ifdefs so they just protect this OF specific code
and just have the allocation fail otherwise please?  That way the OF
dependency is minimised - we should be able to support this on other
platforms in the future and the rest of the code will be the same.

Also is iram the best name?  It makes me think of the dedicated fast
instruction RAM that some processors have.  sram perhaps (for static
RAM)?

Re: [PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Lars-Peter Clausen]

On 10/16/2013 01:58 PM, Mark Brown wrote:
> On Wed, Oct 16, 2013 at 06:53:21PM +0800, Nicolin Chen wrote:
> 
>> +	if (!dev->of_node)
>> +		return NULL;
>> +
>> +	pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
>> +	if (!pool)
>> +		return NULL;
> 
> Can you refactor these ifdefs so they just protect this OF specific code
> and just have the allocation fail otherwise please?  That way the OF
> dependency is minimised - we should be able to support this on other
> platforms in the future and the rest of the code will be the same.
> 
> Also is iram the best name?  It makes me think of the dedicated fast
> instruction RAM that some processors have.  sram perhaps (for static
> RAM)?

Using IRAM (internal SRAM) for this seems to be quite popular (`grep IRAM
arch/arm -r`). Existing devicetree bindings also seem to use the 'iram'
property to refer to the on-chip SRAM
(Documentation/devicetree/bindings/media/coda.txt). It's probably a good
idea to stay consistent with the existing bindings.

- Lars

Re: [PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 695 bytes --]

On Wed, Oct 16, 2013 at 06:33:52PM +0200, Lars-Peter Clausen wrote:
> On 10/16/2013 01:58 PM, Mark Brown wrote:

> > Also is iram the best name?  It makes me think of the dedicated fast
> > instruction RAM that some processors have.  sram perhaps (for static
> > RAM)?

> Using IRAM (internal SRAM) for this seems to be quite popular (`grep IRAM
> arch/arm -r`). Existing devicetree bindings also seem to use the 'iram'
> property to refer to the on-chip SRAM
> (Documentation/devicetree/bindings/media/coda.txt). It's probably a good
> idea to stay consistent with the existing bindings.

Oh, I wonder when that changed - it always used to get called SRAM (or
TCM when that was what was used).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Nicolin Chen]

On Wed, Oct 16, 2013 at 06:46:58PM +0100, Mark Brown wrote:
> On Wed, Oct 16, 2013 at 06:33:52PM +0200, Lars-Peter Clausen wrote:
> > On 10/16/2013 01:58 PM, Mark Brown wrote:
> 
> > > Also is iram the best name?  It makes me think of the dedicated fast
> > > instruction RAM that some processors have.  sram perhaps (for static
> > > RAM)?
> 
> > Using IRAM (internal SRAM) for this seems to be quite popular (`grep IRAM
> > arch/arm -r`). Existing devicetree bindings also seem to use the 'iram'
> > property to refer to the on-chip SRAM
> > (Documentation/devicetree/bindings/media/coda.txt). It's probably a good
> > idea to stay consistent with the existing bindings.
> 
> Oh, I wonder when that changed - it always used to get called SRAM (or
> TCM when that was what was used).

Do I still need to change the name to SRAM? I think both name make sense to me.
I use IRAM just because using different name for phandle and macro/functions
might confuse people when reviewing the patch. But it looks like IRAM is still
not the perfect choice.

Looking forward to your reply,
Nicolin Chen

Re: [PATCH v4] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 386 bytes --]

On Thu, Oct 17, 2013 at 09:54:32AM +0800, Nicolin Chen wrote:

> Do I still need to change the name to SRAM? I think both name make sense to me.
> I use IRAM just because using different name for phandle and macro/functions
> might confuse people when reviewing the patch. But it looks like IRAM is still
> not the perfect choice.

No, IRAM is fine if that is actually used these days.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]