topology2: Revisit period and buffer size settings #8925
Conversation
| buffer_size_min 65536 | ||
| buffer_size_max 65536 | ||
| buffer_size_min 384 # period_size_min * 2 | ||
| buffer_size_max -1 |
There was a problem hiding this comment.
maybe use a more reasonable value? It's rather unlikely that the deep buffer will ever be more than 2s, that adds delays in user interaction to change tracks and apply volumes.
Infinity and beyond are not that useful for buffer sizes.
There was a problem hiding this comment.
What is reasonable? 131072? 16777216? 4294967295?
For example alsa conformance test with this command (on 100ms deep buffer):
./alsa_conformance_test/alsa_conformance_test -P hw:0,31 --iterations=1 -d2 -f S32_LE -B4800
---------PRINT PARAMS---------
PCM name: hw:0,31
card: sofhdadsp [sof-hda-dsp]
device: Deepbuffer HDA Analog (*) []
stream: PLAYBACK
merge_threshold_t: 0.000100
merge_threshold_sz: 2
access type: MMAP_INTERLEAVED
format: S32_LE
channels: 2
rate: 48000 fps
period time: 170645 us
period size: 8191 frames
buffer time: 2730333 us
buffer size: 131056 frames
You can force it with -p4800 to take 4800 frames periods:
period time: 100000 us
period size: 4800 frames
buffer time: 1600000 us
buffer size: 76800 frames
It insists to have 16 periods of whatever sizes it picks.
| @@ -88,8 +88,8 @@ Class.PCM."pcm_caps" { | |||
| periods_max 16 | |||
| channels_min 2 | |||
| channels_max 2 | |||
| period_size_min 192 | |||
| period_size_min 192 # Stereo, S16_LE, 48KHz, 1ms | |||
There was a problem hiding this comment.
why would we keep the period_size_max? Is there really any limitation?
There was a problem hiding this comment.
period_size_max in theory can be up to UINT_MAX (it is unsigned 32bit value). 32768?
Fwiw, the period_size_min=192 is a bit problematic for playback as the DMA moves 2ms on start, so two periods.
192 is only safe for S16_LE, stereo, 48KHz capture, everything else should be using something higher.
S16_LE, stereo, 48KHz, capture should use 192, higher min is not optimal.
ujfalusi
force-pushed
the
peter/pr/tplg2_sizes_01
branch
from
b764575 to
766f950
Compare
|
Changes since v1:
|
| period_size_min 192 # Stereo, S16_LE, 48KHz, 1ms | ||
| period_size_max 19200 # Stereo, S16_LE, 48KHz, 100ms | ||
| buffer_size_min 384 # period_size_min * periods_min | ||
| buffer_size_max 307200 # period_size_max * periods_max |
There was a problem hiding this comment.
I would make this size up to 10s, and also account for higher resolutions/number of channels. If we need to support 8ch 192kHz the buffers are going to be quite small in the end,
There was a problem hiding this comment.
For that we need to increase the periods_max.
These are the default parameters, if you override them then you need to update the sizes also.
In the definition we cannot use calculations, so it has to be done individually.
I can increase the periods_max to let's say 255?
There was a problem hiding this comment.
or the other way around, start from a buffer size that gives you up to 10s, and divide down the max_period_size and max_periods.
There was a problem hiding this comment.
10s for all PCMs? You still need to select either the number of periods or the period size max.
These are the default PCM parameters, not the DB ones.
We can have UINT_MAX for both buffer and period size max and call it a day. 1024 max periods and all config should work.
But we likely want to specify some numbers, right? 1ms min for capture, 2ms for playback, 100ms as max period size as default. Number of periods? 255 should be enough.
Then you re-calculate it for places where the default is changed (format, channels, rate)
DeeBuffer is a special one, we need 100ms as minimum (the kernel needs to help here), but what max period size we want? 1s? 500ms? 10s max buffer is fine, I guess but I don't want to over complicate things.
There is no technical reason to limit how many periods applications use. Generic sound cards tends to allow 32768 periods (period size range is 16 - 524288) but that is an overshot, let's raise the limit to 1024. Signed-off-by: Peter Ujfalusi <[email protected]>
Increase the default period_size_max to a much larger value which is commonly used by sound cards from 16384 to 2097152. Adjust the buffer_size limits to minimum 384 (2 * period_size_min) and the maximum to 4194304 (2 * period_size_max). Applications do not need to use these large buffers, but there is no technical reason to not allow them, like they available on other sound hardware. Signed-off-by: Peter Ujfalusi <[email protected]>
There is no benefit on changing the period/buffer size_max as it is set by default to a reasonably large value already. What matters is the buffer_size_min: it needs to be larger than the amount of data that one DMA burst will transfer, which is equal to the DeepBuffer size. For safety reasons the minimum buffer size has to be larger than this. Signed-off-by: Peter Ujfalusi <[email protected]>
ujfalusi
force-pushed
the
peter/pr/tplg2_sizes_01
branch
from
766f950 to
402ae25
Compare
|
Changes since v2:
|
topology2: Revisit period and buffer size settings
To be pragmatic about the period/buffer size values:
period_size_min = 192 is for Stereo, S16_le, 48Khz, 1ms
period_size_max = 2097152 common value among other sound cards
The buffer size:
buffer_size_min = 384 # period_size_min * periods_min
buffer_size_max = 4194304 # period_size_max * 2
The deep buffer PCM only changes the buffer_size_min to (S16_LE, Stereo, 48KHz, DEEPBUFFER_FW_DMA_MS) * 2