On Thu, 2020-01-16 at 11:27 +0100, Takashi Iwai wrote:
> On Thu, 16 Jan 2020 10:50:33 +0100,
>
> Oh, you're right, and I completely misread the patch.
>
> Now I took a coffee and can tell you the story behind the scene.
>
> I believe the current code is intentionally limiting the size to the
> preallocated size. This limitation was brought for not trying to
> allocate a larger buffer when the buffer has been preallocated. In
> the past, most hardware allocated the continuous pages for a buffer
> and the allocation of a large buffer fails quite likely. This was
> the
> reason of the buffer preallocation. So, the driver wanted to tell
> the
> user-space the limit. If user needs to have an extra large buffer,
> they are supposed to fiddle with prealloc procfs (either setting zero
> to clear the preallocation or setting a large enough buffer
> beforehand).
Thank you for the sharing, it is interesting and knowledge learned to
me.
>
> For SG-buffers, though, limitation makes less sense than continuous
> pages. e.g. a patch below removes the limitation for SG-buffers.
> But changing this would definitely cause the behavior difference, and
> I don't know whether it's a reasonable move -- I'm afraid that apps
> would start hogging too much memory if the limitation is gone.
I just went through all invoking to snd_pcm_lib_preallocate_pages*(),
for those SNDRV_DMA_TYPE_DEV, some of them set the *size* equal to the
*max*, some set the *max* several times to the *size*, IMHO, the *max*s
are matched to those hardware's limiatation, comparing to the *size*s,
aren't they?
In this case, I still think my patch hanle all
TYPE_DEV/SNDRV_DMA_TYPE_DEV/TYPE_SG/SNDRV_DMA_TYPE_DEV cases more
gracefully, we will still take the limitation from the specific driver
set, from the *max* param, and the test results looks very nice here,
we will take what the user space wanted for buffer-bytes via aply
exactly, as long as it is suitable for the interval and constraints.
What's your opinion about it?
>
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
> index d4702cc1d376..6a6c3469bbcd 100644
> --- a/sound/core/pcm_memory.c
> +++ b/sound/core/pcm_memory.c
> @@ -96,6 +96,29 @@ void snd_pcm_lib_preallocate_free_for_all(struct
> snd_pcm *pcm)
> }
> EXPORT_SYMBOL(snd_pcm_lib_preallocate_free_for_all);
>
> +/* set up substream->buffer_bytes_max, which is used in
> hw_constraint */
> +static void set_buffer_bytes_max(struct snd_pcm_substream
> *substream,
> + size_t size)
> +{
> + substream->buffer_bytes_max = UINT_MAX;
> +
> + if (!size)
> + return; /* no preallocation */
> +
> + /* for SG-buffers, no limitation is needed */
> + switch (substream->dma_buffer.dev.type) {
> +#ifdef CONFIG_SND_DMA_SGBUF
> + case SNDRV_DMA_TYPE_DEV_SG:
> + case SNDRV_DMA_TYPE_DEV_UC_SG:
> +#endif
> + case SNDRV_DMA_TYPE_VMALLOC:
> + return;
> + }
> +
> + /* for continuous buffers, limit to the preallocated size */
> + substream->buffer_bytes_max = size;
> +}
> +
> #ifdef CONFIG_SND_VERBOSE_PROCFS
> /*
> * read callback for prealloc proc file
> @@ -156,10 +179,8 @@ static void
> snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry,
> buffer->error = -ENOMEM;
if we won't take this change from user's fiddling for SG buffer, we
should not reallocate dma pages here also?
Thanks,
~Keyon
> return;
> }
> - substream->buffer_bytes_max = size;
> - } else {
> - substream->buffer_bytes_max = UINT_MAX;
> }
> + set_buffer_bytes_max(substream, size);
> if (substream->dma_buffer.area)
> snd_dma_free_pages(&substream->dma_buffer);
> substream->dma_buffer = new_dmab;
> @@ -206,10 +227,8 @@ static void preallocate_pages(struct
> snd_pcm_substream *substream,
>
> if (size > 0 && preallocate_dma && substream->number <
> maximum_substreams)
> preallocate_pcm_pages(substream, size);
> -
> - if (substream->dma_buffer.bytes > 0)
> - substream->buffer_bytes_max = substream-
> >dma_buffer.bytes;
> substream->dma_max = max;
> + set_buffer_bytes_max(substream, substream->dma_buffer.bytes);
> if (max > 0)
> preallocate_info_init(substream);
> if (managed)
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@xxxxxxxxxxxxxxxx
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
