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

[PATCH V2 0/2] 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 memory librray so that they can use that
directly.

Remove the code from Tegra PCM and use these new feature from 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.

Changes from V1:
Takashi had send the sample code to integrate the writecombine with
dma coherant and this is based on that code.
Also change the tegra-pcm driver accordingly.


Laxman Dewangan (2):
  ALSA: Support writecombine DMA buffer memory page alloc/free
  ASoC: tegra: use sound pcm library for substream dma buffer

 include/sound/memalloc.h    |    1 +
 sound/core/memalloc.c       |   70 ++++++++++++++++++++++++---
 sound/core/pcm_native.c     |   10 ++++
 sound/soc/tegra/tegra_pcm.c |  112 ++++++++++--------------------------------
 4 files changed, 100 insertions(+), 93 deletions(-)

[PATCH V2 1/2] ALSA: Support writecombine DMA buffer memory page alloc/free

[Laxman Dewangan]

Some of architecture like ARM support the writecombine DMA
buffer which is used for pcm substream DMA buffer.

Supporting writecombine DMA buffer allocation/free/mmap
through the sound library memory management.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
Integrate the changes from Takashi's sample code for supporting
writecombine.

 include/sound/memalloc.h |    1 +
 sound/core/memalloc.c    |   74 +++++++++++++++++++++++++++++++++++++++++-----
 sound/core/pcm_native.c  |   10 ++++++
 3 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h
index c425062..0328e37 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 page */
 
 /*
  * info for buffer allocation
diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
index 6915692..7468251 100644
--- a/sound/core/memalloc.c
+++ b/sound/core/memalloc.c
@@ -124,8 +124,10 @@ 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 *(*alloc_func)(struct device *dev, size_t size,
+			dma_addr_t *dma, gfp_t gfp_flags))
 {
 	int pg;
 	void *res;
@@ -138,16 +140,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 = alloc_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 (*free_func)(struct device *dev,
+					size_t size, void *vaddr,
+					dma_addr_t dma_handle))
 {
 	int pg;
 
@@ -155,8 +159,51 @@ 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);
+	free_func(dev, PAGE_SIZE << pg, ptr, dma);
+}
+
+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);
+}
+
+static void _dma_free_coherent(struct device *dev, size_t size,
+			void *vaddr, dma_addr_t dma_handle)
+{
+	dma_free_coherent(dev, size, vaddr, dma_handle);
+}
+
+
+/* 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, size, 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, size, ptr, dma, dma_free_writecombine);
+}
+#endif
 #endif /* CONFIG_HAS_DMA */
 
 /*
@@ -200,6 +247,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 +324,12 @@ 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 +436,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..28d949c 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3170,6 +3170,16 @@ 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)
-- 
1.7.1.1

[PATCH 2/2] ASoC: tegra: use sound pcm library for substream dma buffer

[Laxman Dewangan]

The sound pcm library support the writecombine DMA buffer
allocation/deallocation/mapping and hence using the APIs
form sound pcm library inplace of implementing the same
locally.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
Changes done to use the existing APIs for sound pcm memory management
which is now supporting writecombine.

 sound/soc/tegra/tegra_pcm.c |  112 ++++++++++--------------------------------
 1 files changed, 27 insertions(+), 85 deletions(-)

diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c
index 127348d..ec1fa25 100644
--- a/sound/soc/tegra/tegra_pcm.c
+++ b/sound/soc/tegra/tegra_pcm.c
@@ -202,8 +202,14 @@ static int tegra_pcm_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct tegra_runtime_data *prtd = runtime->private_data;
+	int ret;
 
-	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+	ret = snd_pcm_lib_malloc_pages(substream,
+				params_buffer_bytes(params));
+	if (ret < 0) {
+		pr_err("page allocation failed, err %d\n", ret);
+		return ret;
+	}
 
 	prtd->dma_req[0].size = params_period_bytes(params);
 	prtd->dma_req[1].size = prtd->dma_req[0].size;
@@ -213,9 +219,7 @@ static int tegra_pcm_hw_params(struct snd_pcm_substream *substream,
 
 static int tegra_pcm_hw_free(struct snd_pcm_substream *substream)
 {
-	snd_pcm_set_runtime_buffer(substream, NULL);
-
-	return 0;
+	return snd_pcm_lib_free_pages(substream);
 }
 
 static int tegra_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
@@ -263,18 +267,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,87 +275,37 @@ 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_default_mmap,
 };
 
-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);
-
+#define TEGRA_PCM_PREALLOC_BUFFER	(32 * 1024)
+#define TEGRA_PCM_PREALLOC_BUFFER_MAX	(32 * 1024)
 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;
+	struct snd_card *card = rtd->card->snd_card;
+	int retval = 0;
+
+	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream ||
+			pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {
+		retval =  snd_pcm_lib_preallocate_pages_for_all(pcm,
+				SNDRV_DMA_TYPE_DEV_WC,
+				card->dev,
+				TEGRA_PCM_PREALLOC_BUFFER,
+				TEGRA_PCM_PREALLOC_BUFFER_MAX);
+		if (retval < 0)
+			dev_err(card->dev,
+				"dma buffer pre-alloc failied %d\n", retval);
 	}
-
-	return 0;
-
-err_free_play:
-	tegra_pcm_deallocate_dma_buffer(pcm, SNDRV_PCM_STREAM_PLAYBACK);
-err:
-	return ret;
+	return retval;
 }
 
 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);
+	snd_pcm_lib_preallocate_free_for_all(pcm);
 }
 
+
 static struct snd_soc_platform_driver tegra_pcm_platform = {
 	.ops		= &tegra_pcm_ops,
 	.pcm_new	= tegra_pcm_new,
-- 
1.7.1.1

Re: [PATCH V2 1/2] ALSA: Support writecombine DMA buffer memory page alloc/free

[Takashi Iwai]

At Mon, 2 Jul 2012 14:24:32 +0530,
Laxman Dewangan wrote:
> 
> Some of architecture like ARM support the writecombine DMA
> buffer which is used for pcm substream DMA buffer.
> 
> Supporting writecombine DMA buffer allocation/free/mmap
> through the sound library memory management.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> Changes from V1:
> Integrate the changes from Takashi's sample code for supporting
> writecombine.

Could you give a short comment in each file that ifdef CONFIG_ARM is a
workaround?


thanks,

Takashi

> 
>  include/sound/memalloc.h |    1 +
>  sound/core/memalloc.c    |   74 +++++++++++++++++++++++++++++++++++++++++-----
>  sound/core/pcm_native.c  |   10 ++++++
>  3 files changed, 77 insertions(+), 8 deletions(-)
> 
> diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h
> index c425062..0328e37 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 page */
>  
>  /*
>   * info for buffer allocation
> diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
> index 6915692..7468251 100644
> --- a/sound/core/memalloc.c
> +++ b/sound/core/memalloc.c
> @@ -124,8 +124,10 @@ 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 *(*alloc_func)(struct device *dev, size_t size,
> +			dma_addr_t *dma, gfp_t gfp_flags))
>  {
>  	int pg;
>  	void *res;
> @@ -138,16 +140,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 = alloc_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 (*free_func)(struct device *dev,
> +					size_t size, void *vaddr,
> +					dma_addr_t dma_handle))
>  {
>  	int pg;
>  
> @@ -155,8 +159,51 @@ 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);
> +	free_func(dev, PAGE_SIZE << pg, ptr, dma);
> +}
> +
> +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);
> +}
> +
> +static void _dma_free_coherent(struct device *dev, size_t size,
> +			void *vaddr, dma_addr_t dma_handle)
> +{
> +	dma_free_coherent(dev, size, vaddr, dma_handle);
> +}
> +
> +
> +/* 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, size, 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, size, ptr, dma, dma_free_writecombine);
> +}
> +#endif
>  #endif /* CONFIG_HAS_DMA */
>  
>  /*
> @@ -200,6 +247,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 +324,12 @@ 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 +436,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..28d949c 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -3170,6 +3170,16 @@ 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)
> -- 
> 1.7.1.1
> 

Re: [PATCH 2/2] ASoC: tegra: use sound pcm library for substream dma buffer

[Takashi Iwai]

At Mon, 2 Jul 2012 14:24:33 +0530,
Laxman Dewangan wrote:
> 
> The sound pcm library support the writecombine DMA buffer
> allocation/deallocation/mapping and hence using the APIs
> form sound pcm library inplace of implementing the same
> locally.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> Changes from V1:
> Changes done to use the existing APIs for sound pcm memory management
> which is now supporting writecombine.
> 
>  sound/soc/tegra/tegra_pcm.c |  112 ++++++++++--------------------------------
>  1 files changed, 27 insertions(+), 85 deletions(-)
> 
> diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c
> index 127348d..ec1fa25 100644
> --- a/sound/soc/tegra/tegra_pcm.c
> +++ b/sound/soc/tegra/tegra_pcm.c
> @@ -202,8 +202,14 @@ static int tegra_pcm_hw_params(struct snd_pcm_substream *substream,
>  {
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct tegra_runtime_data *prtd = runtime->private_data;
> +	int ret;
>  
> -	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
> +	ret = snd_pcm_lib_malloc_pages(substream,
> +				params_buffer_bytes(params));
> +	if (ret < 0) {
> +		pr_err("page allocation failed, err %d\n", ret);

Do you really want to spew error messages at each time?

> +		return ret;
> +	}
>  
>  	prtd->dma_req[0].size = params_period_bytes(params);
>  	prtd->dma_req[1].size = prtd->dma_req[0].size;
> @@ -213,9 +219,7 @@ static int tegra_pcm_hw_params(struct snd_pcm_substream *substream,
>  
>  static int tegra_pcm_hw_free(struct snd_pcm_substream *substream)
>  {
> -	snd_pcm_set_runtime_buffer(substream, NULL);
> -
> -	return 0;
> +	return snd_pcm_lib_free_pages(substream);
>  }
>  
>  static int tegra_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> @@ -263,18 +267,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,87 +275,37 @@ 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_default_mmap,

You can simply remove .mmap definition.  NULL falls back to the
default mmap handler.

>  };
>  
> -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);
> -
> +#define TEGRA_PCM_PREALLOC_BUFFER	(32 * 1024)
> +#define TEGRA_PCM_PREALLOC_BUFFER_MAX	(32 * 1024)
>  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);

The mask setup is still required even if you use the preallocation
helper.

> -	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;
> +	struct snd_card *card = rtd->card->snd_card;
> +	int retval = 0;
> +
> +	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream ||
> +			pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {

You can omit this check.  snd_pcm_lib_preallocate_pages_for_all() will
ignore the streams that have no substreams assigned.


Takashi


> +		retval =  snd_pcm_lib_preallocate_pages_for_all(pcm,
> +				SNDRV_DMA_TYPE_DEV_WC,
> +				card->dev,
> +				TEGRA_PCM_PREALLOC_BUFFER,
> +				TEGRA_PCM_PREALLOC_BUFFER_MAX);
> +		if (retval < 0)
> +			dev_err(card->dev,
> +				"dma buffer pre-alloc failied %d\n", retval);
>  	}
> -
> -	return 0;
> -
> -err_free_play:
> -	tegra_pcm_deallocate_dma_buffer(pcm, SNDRV_PCM_STREAM_PLAYBACK);
> -err:
> -	return ret;
> +	return retval;
>  }
>  
>  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);
> +	snd_pcm_lib_preallocate_free_for_all(pcm);
>  }
>  
> +
>  static struct snd_soc_platform_driver tegra_pcm_platform = {
>  	.ops		= &tegra_pcm_ops,
>  	.pcm_new	= tegra_pcm_new,
> -- 
> 1.7.1.1
> 

Re: [PATCH 2/2] ASoC: tegra: use sound pcm library for substream dma buffer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

On Mon, Jul 02, 2012 at 11:14:55AM +0200, Takashi Iwai wrote:
> Laxman Dewangan wrote:

> > -	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);

> The mask setup is still required even if you use the preallocation
> helper.

Though that code is at least notionally buggy anyway, it should be the
DMA controller device that's used to obtain the DMA mask rather than the
card device which is basically unconnected with DMA.

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