[PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Pierre-Louis Bossart]

Default of 2MB of physically-contiguous memory can be
too high for embedded solutions, make the size configurable.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/drivers/Kconfig |   15 +++++++++++++++
 sound/drivers/aloop.c |    6 +++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/sound/drivers/Kconfig b/sound/drivers/Kconfig
index c896116..ad51067 100644
--- a/sound/drivers/Kconfig
+++ b/sound/drivers/Kconfig
@@ -93,6 +93,21 @@ config SND_ALOOP
 	  To compile this driver as a module, choose M here: the module
 	  will be called snd-aloop.
 
+if SND_ALOOP
+config SND_ALOOP_PREALLOC_SIZE
+	int "Pre-allocated buffer size for aloop driver"
+	range 0 32768
+	default 2048
+	help
+	  Specifies the default pre-allocated buffer-size in kB for the
+	  snd-aloop driver. The default 2048 is chosen just
+	  for compatibility reasons. Some distributions may want to
+	  use a smaller size when only one stream is used.
+
+	  Note that the pre-allocation size can be changed dynamically
+	  via a proc file (/proc/asound/card*/pcm*/sub*/prealloc), too.
+endif
+
 config SND_VIRMIDI
 	tristate "Virtual MIDI soundcard"
 	depends on SND_SEQUENCER
diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index a0da775..f945573 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -556,11 +556,11 @@ static struct snd_pcm_hardware loopback_pcm_hardware =
 	.rate_max =		192000,
 	.channels_min =		1,
 	.channels_max =		32,
-	.buffer_bytes_max =	2 * 1024 * 1024,
+	.buffer_bytes_max =	CONFIG_SND_ALOOP_PREALLOC_SIZE * 1024,
 	.period_bytes_min =	64,
 	/* note check overflow in frac_pos() using pcm_rate_shift before
 	   changing period_bytes_max value */
-	.period_bytes_max =	1024 * 1024,
+	.period_bytes_max =     CONFIG_SND_ALOOP_PREALLOC_SIZE * 1024,
 	.periods_min =		1,
 	.periods_max =		1024,
 	.fifo_size =		0,
@@ -774,7 +774,7 @@ static int __devinit loopback_pcm_new(struct loopback *loopback,
 
 	snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_CONTINUOUS,
 			snd_dma_continuous_data(GFP_KERNEL),
-			0, 2 * 1024 * 1024);
+			0, CONFIG_SND_ALOOP_PREALLOC_SIZE * 1024);
 	return 0;
 }
 
-- 
1.7.6

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Takashi Iwai]

At Tue, 20 Sep 2011 14:02:12 -0500,
Pierre-Louis Bossart wrote:
> 
> Default of 2MB of physically-contiguous memory can be
> too high for embedded solutions, make the size configurable.

Hm, maybe better to have an option for global limit of preallocation?
The problem is not necessarily specific to snd-aloop.


thanks,

Takashi

> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/drivers/Kconfig |   15 +++++++++++++++
>  sound/drivers/aloop.c |    6 +++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/drivers/Kconfig b/sound/drivers/Kconfig
> index c896116..ad51067 100644
> --- a/sound/drivers/Kconfig
> +++ b/sound/drivers/Kconfig
> @@ -93,6 +93,21 @@ config SND_ALOOP
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called snd-aloop.
>  
> +if SND_ALOOP
> +config SND_ALOOP_PREALLOC_SIZE
> +	int "Pre-allocated buffer size for aloop driver"
> +	range 0 32768
> +	default 2048
> +	help
> +	  Specifies the default pre-allocated buffer-size in kB for the
> +	  snd-aloop driver. The default 2048 is chosen just
> +	  for compatibility reasons. Some distributions may want to
> +	  use a smaller size when only one stream is used.
> +
> +	  Note that the pre-allocation size can be changed dynamically
> +	  via a proc file (/proc/asound/card*/pcm*/sub*/prealloc), too.
> +endif
> +
>  config SND_VIRMIDI
>  	tristate "Virtual MIDI soundcard"
>  	depends on SND_SEQUENCER
> diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
> index a0da775..f945573 100644
> --- a/sound/drivers/aloop.c
> +++ b/sound/drivers/aloop.c
> @@ -556,11 +556,11 @@ static struct snd_pcm_hardware loopback_pcm_hardware =
>  	.rate_max =		192000,
>  	.channels_min =		1,
>  	.channels_max =		32,
> -	.buffer_bytes_max =	2 * 1024 * 1024,
> +	.buffer_bytes_max =	CONFIG_SND_ALOOP_PREALLOC_SIZE * 1024,
>  	.period_bytes_min =	64,
>  	/* note check overflow in frac_pos() using pcm_rate_shift before
>  	   changing period_bytes_max value */
> -	.period_bytes_max =	1024 * 1024,
> +	.period_bytes_max =     CONFIG_SND_ALOOP_PREALLOC_SIZE * 1024,
>  	.periods_min =		1,
>  	.periods_max =		1024,
>  	.fifo_size =		0,
> @@ -774,7 +774,7 @@ static int __devinit loopback_pcm_new(struct loopback *loopback,
>  
>  	snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_CONTINUOUS,
>  			snd_dma_continuous_data(GFP_KERNEL),
> -			0, 2 * 1024 * 1024);
> +			0, CONFIG_SND_ALOOP_PREALLOC_SIZE * 1024);
>  	return 0;
>  }
>  
> -- 
> 1.7.6
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Pierre-Louis Bossart]

> > Default of 2MB of physically-contiguous memory can be
> > too high for embedded solutions, make the size configurable.
> 
> Hm, maybe better to have an option for global limit of preallocation?
> The problem is not necessarily specific to snd-aloop.

Not sure I understand the comment. This code is essentially the same as the
configuration option for HDAudio, and the parameter value defines the amount
of preallocated memory, with a limit to 32MB.
-Pierre

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Takashi Iwai]

At Wed, 21 Sep 2011 08:26:27 -0500,
Pierre-Louis Bossart wrote:
> 
> > > Default of 2MB of physically-contiguous memory can be
> > > too high for embedded solutions, make the size configurable.
> > 
> > Hm, maybe better to have an option for global limit of preallocation?
> > The problem is not necessarily specific to snd-aloop.
> 
> Not sure I understand the comment. This code is essentially the same as the
> configuration option for HDAudio, and the parameter value defines the amount
> of preallocated memory, with a limit to 32MB.

I meant to have a kconfig option to limit the upper-bound of pre-alloc
buffer sizes for all drivers.  That is, a patch like below (untested)


Takashi

---
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 475455c..525ed28 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -204,6 +204,15 @@ config SND_PCM_XRUN_DEBUG
 	  sound clicking when system is loaded, it may help to determine
 	  the process or driver which causes the scheduling gaps.
 
+config SND_PCM_MAX_PREALLOC_SIZE
+	int "Maximum pre-allocated buffer size"
+	range 0 32768
+	default 0
+	depends on EMBEDDED
+	help
+	  Specifies the default upper-limit of PCM pre-allocation
+	  buffer sizes in kB.  Zero means unlimited.
+
 config SND_VMASTER
 	bool
 
diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index 150cb7e..a09425c 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -270,6 +270,9 @@ int snd_pcm_lib_preallocate_pages(struct snd_pcm_substream *substream,
 				  int type, struct device *data,
 				  size_t size, size_t max)
 {
+	if (CONFIG_SND_PCM_MAX_PREALLOC_SIZE &&
+	    max > CONFIG_PCM_SND_MAX_PREALLOC_SIZE * 1024)
+		max = CONFIG_SND_PCM_PREALLOC_SIZE * 1024;
 	substream->dma_buffer.dev.type = type;
 	substream->dma_buffer.dev.dev = data;
 	return snd_pcm_lib_preallocate_pages1(substream, size, max);

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Pierre-Louis Bossart]

> I meant to have a kconfig option to limit the upper-bound of pre-alloc
> buffer sizes for all drivers.  That is, a patch like below (untested)

That's a good idea, but the two patches aren't necessarily exclusive. In an
embedded solution you may want a 2s PCM buffer for low-power playback and a
64k buffer for snd-aloop as it is only used with small periods. Also one
would need to check the buffer/period sizes in each driver, and make sure
they are smaller than this global max.
-Pierre

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Takashi Iwai]

At Wed, 21 Sep 2011 10:02:27 -0500,
Pierre-Louis Bossart wrote:
> 
> > I meant to have a kconfig option to limit the upper-bound of pre-alloc
> > buffer sizes for all drivers.  That is, a patch like below (untested)
> 
> That's a good idea, but the two patches aren't necessarily exclusive. In an
> embedded solution you may want a 2s PCM buffer for low-power playback and a
> 64k buffer for snd-aloop as it is only used with small periods. Also one
> would need to check the buffer/period sizes in each driver, and make sure
> they are smaller than this global max.

Well, if it's only about snd-aloop, another option would be to use
vmalloc'ed buffer for aloop.  Since it's no hardware buffer, it's
more system-friendly, and you don't need a pre-allocation.
Again, an untested patch is below.


thanks,

Takashi

---
diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index a0da775..4067f15 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -575,7 +575,8 @@ static void loopback_runtime_free(struct snd_pcm_runtime *runtime)
 static int loopback_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
 {
-	return snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(params));
 }
 
 static int loopback_hw_free(struct snd_pcm_substream *substream)
@@ -587,7 +588,7 @@ static int loopback_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&dpcm->loopback->cable_lock);
 	cable->valid &= ~(1 << substream->stream);
 	mutex_unlock(&dpcm->loopback->cable_lock);
-	return snd_pcm_lib_free_pages(substream);
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
 }
 
 static unsigned int get_cable_index(struct snd_pcm_substream *substream)
@@ -740,6 +741,8 @@ static struct snd_pcm_ops loopback_playback_ops = {
 	.prepare =	loopback_prepare,
 	.trigger =	loopback_trigger,
 	.pointer =	loopback_pointer,
+	.page =		snd_pcm_lib_get_vmalloc_page,
+	.mmap =		snd_pcm_lib_mmap_vmalloc,
 };
 
 static struct snd_pcm_ops loopback_capture_ops = {
@@ -751,6 +754,8 @@ static struct snd_pcm_ops loopback_capture_ops = {
 	.prepare =	loopback_prepare,
 	.trigger =	loopback_trigger,
 	.pointer =	loopback_pointer,
+	.page =		snd_pcm_lib_get_vmalloc_page,
+	.mmap =		snd_pcm_lib_mmap_vmalloc,
 };
 
 static int __devinit loopback_pcm_new(struct loopback *loopback,
@@ -771,10 +776,6 @@ static int __devinit loopback_pcm_new(struct loopback *loopback,
 	strcpy(pcm->name, "Loopback PCM");
 
 	loopback->pcm[device] = pcm;
-
-	snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_CONTINUOUS,
-			snd_dma_continuous_data(GFP_KERNEL),
-			0, 2 * 1024 * 1024);
 	return 0;
 }
 

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Pierre-Louis Bossart]

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


> Well, if it's only about snd-aloop, another option would be to use
> vmalloc'ed buffer for aloop.  Since it's no hardware buffer, it's
> more system-friendly, and you don't need a pre-allocation.
> Again, an untested patch is below.

I wasn't sure why physically-contiguous memory would be required, just
reduced it, but it makes sense to remove it altogether.

I used a double loopback, one in alsa and one in pulseaudio. Works fine.
$ pactl load-module module-loopback source=alsa_input.1.analog-stereo
sink=alsa_output.pci-0000_00_1b.0.analog-stereo
$ aplay -Dhw:1,1 viol-1mn.wav 


For some reason I couldn't apply the patch as is, here's what I used
Thanks!
-Pierre

[-- Attachment #2: aloop.patch --]
[-- Type: text/x-patch, Size: 1893 bytes --]

diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index a0da775..4e40a40 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -575,7 +575,8 @@ static void loopback_runtime_free(struct snd_pcm_runtime *runtime)
 static int loopback_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
 {
-	return snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(params));
 }
 
 static int loopback_hw_free(struct snd_pcm_substream *substream)
@@ -587,7 +588,7 @@ static int loopback_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&dpcm->loopback->cable_lock);
 	cable->valid &= ~(1 << substream->stream);
 	mutex_unlock(&dpcm->loopback->cable_lock);
-	return snd_pcm_lib_free_pages(substream);
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
 }
 
 static unsigned int get_cable_index(struct snd_pcm_substream *substream)
@@ -740,6 +741,8 @@ static struct snd_pcm_ops loopback_playback_ops = {
 	.prepare =	loopback_prepare,
 	.trigger =	loopback_trigger,
 	.pointer =	loopback_pointer,
+	.page =         snd_pcm_lib_get_vmalloc_page,
+	.mmap =         snd_pcm_lib_mmap_vmalloc
 };
 
 static struct snd_pcm_ops loopback_capture_ops = {
@@ -751,6 +754,8 @@ static struct snd_pcm_ops loopback_capture_ops = {
 	.prepare =	loopback_prepare,
 	.trigger =	loopback_trigger,
 	.pointer =	loopback_pointer,
+	.page =         snd_pcm_lib_get_vmalloc_page,
+	.mmap =         snd_pcm_lib_mmap_vmalloc
 };
 
 static int __devinit loopback_pcm_new(struct loopback *loopback,
@@ -772,9 +777,6 @@ static int __devinit loopback_pcm_new(struct loopback *loopback,
 
 	loopback->pcm[device] = pcm;
 
-	snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_CONTINUOUS,
-			snd_dma_continuous_data(GFP_KERNEL),
-			0, 2 * 1024 * 1024);
 	return 0;
 }
 

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

Re: [PATCH] ALSA: snd-aloop: make preallocated buffer size configurable

[Takashi Iwai]

At Fri, 23 Sep 2011 16:03:31 -0500,
Pierre-Louis Bossart wrote:
> 
> > Well, if it's only about snd-aloop, another option would be to use
> > vmalloc'ed buffer for aloop.  Since it's no hardware buffer, it's
> > more system-friendly, and you don't need a pre-allocation.
> > Again, an untested patch is below.
> 
> I wasn't sure why physically-contiguous memory would be required, just
> reduced it, but it makes sense to remove it altogether.
> 
> I used a double loopback, one in alsa and one in pulseaudio. Works fine.
> $ pactl load-module module-loopback source=alsa_input.1.analog-stereo
> sink=alsa_output.pci-0000_00_1b.0.analog-stereo
> $ aplay -Dhw:1,1 viol-1mn.wav 
> 
> 
> For some reason I couldn't apply the patch as is, here's what I used
> Thanks!

OK, I applied the patch now to sound git tree.


thanks,

Takashi