ASoC: SOF: ipc4-topology: Introduce new token for period size #4664
Conversation
Add SOF_TKN_PROCESS_BUFFER_PERIOD_SIZE token, when set in topology, the tuple value of the token will be used as multiplier of process's buffer size. Signed-off-by: Ranjani Sridharan <[email protected]> Signed-off-by: Yong Zhi <[email protected]>
yongzhi1
requested review from
lyakh,
ranj063,
lgirdwood,
plbossart,
kv2019i,
dbaluta,
bardliao,
ujfalusi,
vijendarmukunda,
kuanhsuncheng and
yaochunhung
as code owners
|
Has dependency on thesofproject/sof#8403. |
| @@ -2075,6 +2092,10 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget, | |||
| return output_fmt_index; | |||
| } | |||
|
|
|||
| /* adjust the input/output buffer size based on the period size set in topology */ | |||
| process->base_config.ibs *= process->buffer_period_size; | |||
| process->base_config.obs *= process->buffer_period_size; | |||
There was a problem hiding this comment.
is this correct to multiply the pin_format->buffer_size and ibs/obs? Isn't the pin_format->buffer_size derived from IBS/OBS?
Looks like a double multiplication to me...
There was a problem hiding this comment.
Hi, @RanderWang , could you help to double check above? sorry I am not really familiar this part.
There was a problem hiding this comment.
this change should be made in topology since driver just pass it to fw.
please check audio_format.conf. you can override it for DP module
# math expression for computing input/put buffer sizes
# for 11.025 22.05, 44.1, 88.2 and 176.4khz, we need to round it to ceiling value
ibs "$[($in_channels * ($[($in_rate + 999)] / 1000)) * ($in_bit_depth / 8)]"
obs "$[($out_channels * ($[($out_rate + 999)] / 1000)) * ($out_bit_depth / 8)]"There was a problem hiding this comment.
And if the process->buffer_period_size is used to build IBS or OBS only, we can do it in topology and no need to change driver ?
There was a problem hiding this comment.
Thanks @RanderWang , updated the thesofproject/sof#8403, hope to have same net results :)
There was a problem hiding this comment.
This PR can be closed then?
| @@ -2012,6 +2028,7 @@ sof_ipc4_process_set_pin_formats(struct snd_sof_widget *swidget, int pin_type) | |||
|
|
|||
| if (pin_format_item->pin_index == i - pin_format_offset) { | |||
| *pin_format = *pin_format_item; | |||
| pin_format->buffer_size *= process->buffer_period_size; | |||
There was a problem hiding this comment.
naming is awful. suggest "buffer_size_multiplier". It doesn't help to refer to the period and is't not a buffer or period_size in the first place.
| @@ -473,6 +473,7 @@ struct sof_ipc4_process { | |||
| struct sof_ipc4_msg msg; | |||
| u32 base_config_ext_size; | |||
| u32 init_config; | |||
| u32 buffer_period_size; | |||
There was a problem hiding this comment.
commit message: you're describing a solution without explaining what the issue is. Why do we need this in the first place? In what context does this help?
There was a problem hiding this comment.
This is intended to handle DP module that requires larger buffer than 1ms.
There was a problem hiding this comment.
DP modules scheduling is based on their IBS and OBS
In general:
- the module is ready to run and will be scheduled if it has at least IBS number of bytes available on each input pin and at least OBS number of bytes free on each output pin
please note that each pin may have its own format - number of channels, sampling frequency, etc. so each pin may have its own IBS/OBS
IBS and OBS are as declared in module bind IPC.
- calculation of module period - required for synchronization with LL modules and EDF scheduling - is also based on IBS(IBSes)
special case like variable bitrate: the module may override both 1 and 2 if really needed.
so - for DP modules IBS/OBS values should be precise, it is critical for proper data flow.
example: if a module has 2 input pins, one 16000/16/2, second 48000/16/2 and one output pin 48000/32/4 and is designed to process 10ms data chunks:
in pin1 IBS should be 16(samples/1ms) * 2 (bytes per sample) * 2 (channels) * 10(ms) = 640
in pin2 IBS should be 48(samples/1ms) * 2 (bytes per sample) * 2 (channels) * 10(ms) = 1920
out pin1 OBS should be 48(samples/1ms) * 4 (bytes per sample) * 4 (channels) * 10(ms) = 7680
another note - bigger OBS won't hurt the processing but is a memory waste - so should be avoided.
|
Close for now in light of the updates on topology side, thanks for all the reviews! |
Add SOF_TKN_PROCESS_BUFFER_PERIOD_SIZE token, when set in topology, the tuple value of the token will be used as multiplier of process's buffer size.