-----Original Message----- From: Jie, Yang Sent: Thursday, January 16, 2020 10:14 PM To: 'Takashi Iwai' tiwai@suse.de; Keyon Jie yang.jie@linux.intel.com Cc: alsa-devel@alsa-project.org Subject: RE: [alsa-devel] [PATCH] ALSA: pcm: fix buffer_bytes max constrained by preallocated bytes issue
-----Original Message----- From: Alsa-devel alsa-devel-bounces@alsa-project.org On Behalf Of Takashi Iwai Sent: Thursday, January 16, 2020 7:51 PM To: Keyon Jie yang.jie@linux.intel.com Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: pcm: fix buffer_bytes max constrained by preallocated bytes issue
On Thu, 16 Jan 2020 12:25:38 +0100, Keyon Jie wrote:
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.
Well, I have a mixed feeling. Certainly we'd need some better way to allow a larger buffer allocation, especially for HDA. OTOH, if the buffer was preallocated, it's meant to be used actually. That's the point of the hw_constraint setup.
So if the buffer was preallocated, it won't be re-allocated at hw_params() stage, is this conflict with the re-allocate logic in hw_params()?
And now thinking again after another cup of coffee, I wonder why we do preallocate for HDA at all. For HD-audio, the allocation of any large buffer would succeed very likely because of SG-buffer.
So, just setting 0 to the preallocation size (but keeping else) would work,
e.g.
something like below? The help text needs adjustment, but you can see the rough idea.
So, do you suggest not doing preallocation(or calling it with 0 size) for all driver with TYPE_SG? I am fine if this is the recommended method, I can try this on SOF I2S platform to see if it can work as we required for very large buffer size.
Tried and found setting 0 size for preallocation doesn't work for me, I have even tried to setting the size as big as the max(which the user space may require for buffer-bytes), it still doesn't work for me.
Thanks, ~Keyon
Thanks, ~Keyon