On Sun, 19 Jan 2020 09:11:17 +0100, Keyon Jie wrote:
On 2020/1/19 下午3:09, Takashi Iwai wrote:
On Sun, 19 Jan 2020 04:52:55 +0100, Keyon Jie wrote:
On 2020/1/17 下午7:12, Takashi Iwai wrote:
On Fri, 17 Jan 2020 11:43:24 +0100, Keyon Jie wrote:
In SOF driver, we don't use kernel config item like CONFIG_SND_HDA_PREALLOC_SIZE for HDA, the code for it is:
snd_pcm_lib_preallocate_pages(pcm->streams[stream].substream, SNDRV_DMA_TYPE_DEV_SG, sdev->dev, le32_to_cpu(caps->buffer_size_min), le32_to_cpu(caps->buffer_size_max));
So the preallocated size is configured via topology file, that is caps->buffer_size_min, no chance for PulseAudio to reconfigure it.
So, it looks like we have to change it to this if we don't change the ALSA core:
snd_pcm_lib_preallocate_pages(pcm->streams[stream].substream, SNDRV_DMA_TYPE_DEV_SG, sdev->dev,
le32_to_cpu(caps->buffer_size_min),
le32_to_cpu(caps->buffer_size_max), le32_to_cpu(caps->buffer_size_max));Yes, passing buffer_size_min for the preallocation sounds already bad. The default value should be sufficient for usual operations, not the cost-cutting minimum. Otherwise there is no merit of preallocation.
Alternatively, we may pass 0 there, indicating no limitation, too. But, this would need a bit other adjustment, e.g. snd_pcm_hardware should have lower buffer_bytes_max.
Thank you Takashi, then let's follow it to pre-allocate with caps->buffer_size_max, as we don't specify any limitations in snd_pcm_hardware today, we want to leave it configurable to each specific topology file for different machines.
How big is caps->buffer_size_max? Passing the value there means actually trying to allocate the given size as default, and it'd be a lot of waste if a too large value (e.g. 32MB) is passed there.
It varies for each stream, most of them are 65536 Bytes only, whereas one for Wake-On-Voice might need a > 4 Seconds buffer could be up to about 1~2MBytes, and another one for deep-buffer playback can be up to about 8MBytes.
Hm, so this varies so much depending on the use case? I thought it comes from the topology file and it's essentially consistent over various purposes.
I think we can go for passing zero as default, which means skipping preallocation. In addition, we may add an upper limit of the total
Just did an experiment and this works for me, I believe we still need to call snd_pcm_set_managed_buffer() though the preallocation is skipped in this, right?
No, snd_pcm_set_managed_buffer() is the new PCM preallocation API. The old snd_pcm_lib_preallocate*() is almost gone.
amount of allocation per card, controlled in pcm_memory.c, for example. This logic can be applied to the legacy HDA, too.
This should be relatively easy, and I'll provide the patch in the next week.
OK, that's fine for me also, thank you.
Below is a quick hack for HDA. We still need the certain amount of preallocation for non-x86 systems that don't support SG-buffers, so a bit of trick is applied to Kconfig.
Totally untested, as usual.
thanks,
Takashi
--- diff --git a/include/sound/core.h b/include/sound/core.h index 0e14b7a3e67b..ac8b692b69b4 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -120,6 +120,9 @@ struct snd_card { int sync_irq; /* assigned irq, used for PCM sync */ wait_queue_head_t remove_sleep;
+ size_t total_pcm_alloc_bytes; /* total amount of allocated buffers */ + struct mutex memory_mutex; /* protection for the above */ + #ifdef CONFIG_PM unsigned int power_state; /* power state */ wait_queue_head_t power_sleep; diff --git a/sound/core/init.c b/sound/core/init.c index faa9f03c01ca..b02a99766351 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -211,6 +211,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid, INIT_LIST_HEAD(&card->ctl_files); spin_lock_init(&card->files_lock); INIT_LIST_HEAD(&card->files_list); + mutex_init(&card->memory_mutex); #ifdef CONFIG_PM init_waitqueue_head(&card->power_sleep); #endif diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c index d4702cc1d376..4883b0ccd475 100644 --- a/sound/core/pcm_memory.c +++ b/sound/core/pcm_memory.c @@ -27,6 +27,37 @@ MODULE_PARM_DESC(maximum_substreams, "Maximum substreams with preallocated DMA m
static const size_t snd_minimum_buffer = 16384;
+static unsigned long max_alloc_per_card = 32UL * 1024UL * 1024UL * 1024UL; +module_param(max_alloc_per_card, ulong, 0644); +MODULE_PARM_DESC(max_alloc_per_card, "Max total allocation bytes per card."); + +static int do_alloc_pages(struct snd_card *card, int type, struct device *dev, + size_t size, struct snd_dma_buffer *dmab) +{ + int err; + + if (card->total_pcm_alloc_bytes + size > max_alloc_per_card) + return -ENOMEM; + err = snd_dma_alloc_pages(type, dev, size, dmab); + if (!err) { + mutex_lock(&card->memory_mutex); + card->total_pcm_alloc_bytes += dmab->bytes; + mutex_unlock(&card->memory_mutex); + } + return err; +} + +static void do_free_pages(struct snd_card *card, struct snd_dma_buffer *dmab) +{ + if (!dmab->area) + return; + mutex_lock(&card->memory_mutex); + WARN_ON(card->total_pcm_alloc_bytes < dmab->bytes); + card->total_pcm_alloc_bytes -= dmab->bytes; + mutex_unlock(&card->memory_mutex); + snd_dma_free_pages(dmab); + dmab->area = NULL; +}
/* * try to allocate as the large pages as possible. @@ -37,16 +68,15 @@ static const size_t snd_minimum_buffer = 16384; static int preallocate_pcm_pages(struct snd_pcm_substream *substream, size_t size) { struct snd_dma_buffer *dmab = &substream->dma_buffer; + struct snd_card *card = substream->pcm->card; size_t orig_size = size; int err;
do { - if ((err = snd_dma_alloc_pages(dmab->dev.type, dmab->dev.dev, - size, dmab)) < 0) { - if (err != -ENOMEM) - return err; /* fatal error */ - } else - return 0; + err = do_alloc_pages(card, dmab->dev.type, dmab->dev.dev, + size, dmab); + if (err != -ENOMEM) + return err; size >>= 1; } while (size >= snd_minimum_buffer); dmab->bytes = 0; /* tell error */ @@ -62,10 +92,7 @@ static int preallocate_pcm_pages(struct snd_pcm_substream *substream, size_t siz */ static void snd_pcm_lib_preallocate_dma_free(struct snd_pcm_substream *substream) { - if (substream->dma_buffer.area == NULL) - return; - snd_dma_free_pages(&substream->dma_buffer); - substream->dma_buffer.area = NULL; + do_free_pages(substream->pcm->card, &substream->dma_buffer); }
/** @@ -130,6 +157,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct snd_pcm_substream *substream = entry->private_data; + struct snd_card *card = substream->pcm->card; char line[64], str[64]; size_t size; struct snd_dma_buffer new_dmab; @@ -150,9 +178,10 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, memset(&new_dmab, 0, sizeof(new_dmab)); new_dmab.dev = substream->dma_buffer.dev; if (size > 0) { - if (snd_dma_alloc_pages(substream->dma_buffer.dev.type, - substream->dma_buffer.dev.dev, - size, &new_dmab) < 0) { + if (do_alloc_pages(card, + substream->dma_buffer.dev.type, + substream->dma_buffer.dev.dev, + size, &new_dmab) < 0) { buffer->error = -ENOMEM; return; } @@ -161,7 +190,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, substream->buffer_bytes_max = UINT_MAX; } if (substream->dma_buffer.area) - snd_dma_free_pages(&substream->dma_buffer); + do_free_pages(card, &substream->dma_buffer); substream->dma_buffer = new_dmab; } else { buffer->error = -EINVAL; @@ -346,6 +375,7 @@ struct page *snd_pcm_sgbuf_ops_page(struct snd_pcm_substream *substream, unsigne */ int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t size) { + struct snd_card *card = substream->pcm->card; struct snd_pcm_runtime *runtime; struct snd_dma_buffer *dmab = NULL;
@@ -374,9 +404,10 @@ int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t size) if (! dmab) return -ENOMEM; dmab->dev = substream->dma_buffer.dev; - if (snd_dma_alloc_pages(substream->dma_buffer.dev.type, - substream->dma_buffer.dev.dev, - size, dmab) < 0) { + if (do_alloc_pages(card, + substream->dma_buffer.dev.type, + substream->dma_buffer.dev.dev, + size, dmab) < 0) { kfree(dmab); return -ENOMEM; } @@ -397,6 +428,7 @@ EXPORT_SYMBOL(snd_pcm_lib_malloc_pages); */ int snd_pcm_lib_free_pages(struct snd_pcm_substream *substream) { + struct snd_card *card = substream->pcm->card; struct snd_pcm_runtime *runtime;
if (PCM_RUNTIME_CHECK(substream)) @@ -406,7 +438,7 @@ int snd_pcm_lib_free_pages(struct snd_pcm_substream *substream) return 0; if (runtime->dma_buffer_p != &substream->dma_buffer) { /* it's a newly allocated buffer. release it now. */ - snd_dma_free_pages(runtime->dma_buffer_p); + do_free_pages(card, runtime->dma_buffer_p); kfree(runtime->dma_buffer_p); } snd_pcm_set_runtime_buffer(substream, NULL); diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig index b0c88fe040ee..4ca6b09056f3 100644 --- a/sound/hda/Kconfig +++ b/sound/hda/Kconfig @@ -21,14 +21,16 @@ config SND_HDA_EXT_CORE select SND_HDA_CORE
config SND_HDA_PREALLOC_SIZE - int "Pre-allocated buffer size for HD-audio driver" + int "Pre-allocated buffer size for HD-audio driver" if !SND_DMA_SGBUF range 0 32768 - default 64 + default 0 if SND_DMA_SGBUF + default 64 if !SND_DMA_SGBUF help Specifies the default pre-allocated buffer-size in kB for the HD-audio driver. A larger buffer (e.g. 2048) is preferred for systems using PulseAudio. The default 64 is chosen just for compatibility reasons. + On x86 systems, the default is zero as we need no preallocation.
Note that the pre-allocation size can be changed dynamically via a proc file (/proc/asound/card*/pcm*/sub*/prealloc), too.