copier: pipeline period recalculation

[tmleman]

In case of a deep buffering pipelines don't require scheduling on every millisecond. We can calculate period based on the gateway buffer size.

At pipeline creation we assume that it can work on longer periods, period value is set to zero to be later adjust base on the pipeline components.

[lyakh]

just as an example to clarify why this won't quite calculate correct results and why you need to round up line 114:

one_s = 8090
ten_ms = 80
dma_buffer_size = 8000
chunk_count = 100

that will tell you, that you can fit 100 10ms long chunks in the buffer, so you could conclude that a full second of audio fits into the buffer, whereas it doesn't - 90 bytes don't fit. Whereas if you round up the first division in line 114, you get

one_s = 8090
ten_ms = 81
dma_buffer_size = 8000
chunk_count = 98

so you conclude, that 980ms of audio will fit into the buffer, which is correct - they will fit there and will occupy 0.98*8090 < 7983 < 8000 bytes.

[btian1]

may need enumerate all the possible freqs and then divide can be avoided by look up table.
like is only support 44100 and 48000 input, then two divide can be avoided.

[tmleman]

I assumed that rounding is not needed because none of the formats we use will result in a value that is not divisible by 100. I added rounding anyway.

[lgirdwood]

Yeah, I dont think there are any rates that are not multiples of 100.

[RanderWang]

so we will not support 11.025 khz and 22.05 khz ?

enum ipc4_sampling_frequency {
	IPC4_FS_8000HZ = 8000,
	IPC4_FS_11025HZ = 11025,
	IPC4_FS_12000HZ = 12000, /**< Mp3, AAC, SRC only */
	IPC4_FS_16000HZ = 16000,
	IPC4_FS_18900HZ = 18900, /**< SRC only for 44100 */
	IPC4_FS_22050HZ = 22050,
	IPC4_FS_24000HZ = 24000, /**< Mp3, AAC, SRC only */
	IPC4_FS_32000HZ = 32000,
	IPC4_FS_37800HZ = 37800, /**< SRC only for 44100 */
	IPC4_FS_44100HZ = 44100,
	IPC4_FS_48000HZ = 48000, /**< Default */
	IPC4_FS_64000HZ = 64000, /**< AAC, SRC only */
	IPC4_FS_88200HZ = 88200, /**< AAC, SRC only */
	IPC4_FS_96000HZ = 96000, /**< AAC, SRC only */
	IPC4_FS_176400HZ = 176400, /**< SRC only */
	IPC4_FS_192000HZ = 192000, /**< SRC only */
	IPC4_FS_INVALID
};

[lgirdwood]

oh, good point @RanderWang - these are rare but in IPC4 and ALSA.
@tmleman looks like we need a switch here to deal with the non divisible corner cases.

[lyakh]

so in my above example if buffer size is 80 bytes, you'll calculate chunk_count == 1 but in fact it won't fit full 10ms of audio

[lgirdwood]

Isn't buffer size validated prior to this logic ? @tmleman do we validate copier->base.audio_fmt.sampling_frequency, copier->base.audio_fmt.depth or copier->base.audio_fmt.channels_count to be non zero here otherwise we can divide by 0.

[kv2019i]

I was thinking it would be ncie to have a log entry to catch if topology has an incorrect configuration. But as this is called also in normal case, addign a INF level log is probably not a good idea here. It would be good to have a note/warning if host is asking for a combiation with copier configured with deep buffers for DMA, but the pipeline is not compatible.

[kv2019i]

With so many inputs (both from runtime PCM config as well as topology), I think this warrants printing out the modified period size to FW log.

[lgirdwood]

ack, would be handy for debug.

[btian1]

may need enumerate all the possible freqs and then divide can be avoided by look up table.
like is only support 44100 and 48000 input, then two divide can be avoided.

[btian1]

dev->pipeline->long_period same meaning with dev->deep_buffering

[lgirdwood]

@tmleman This should be a pipeline 2.0 API i.e.

int pipeline_sleep(pipeline, ms);

And this could be called by copiers during process() when conditions allow the pipe to skip schedling ticks. It should also be a feature that can be enabled/disabled by topology flag (i. assume there is an IPC4 flag to enable it ?)

[tmleman]

23ww31.3 Update(1/2): resolving some issues from review comments.

[tmleman]

23ww31.3 Update(2/2): rebase

[lgirdwood]

@tmleman @ranj063 I assume we have a CI test or topology that can test 10ms pipelines ? If so, we should make sure its run since we are optimizing non 1ms pipelines now.

[lgirdwood]

@tmleman sorry to ask, are you able to rebase again on HEAD as a build failure was introduced with VMH and was fixed late yesterday. Thanks !

[tmleman]

23ww31.4 Update(1/2):

• renaming variable long_period to deep_buffering,
• replacing division by 8 with bit shift for medieval compilers.

[tmleman]

23ww31.4 Update(2/2): rebase

[lgirdwood]

Yeah, I dont think there are any rates that are not multiples of 100.

[lgirdwood]

Isn't buffer size validated prior to this logic ? @tmleman do we validate copier->base.audio_fmt.sampling_frequency, copier->base.audio_fmt.depth or copier->base.audio_fmt.channels_count to be non zero here otherwise we can divide by 0.

[lgirdwood]

ack, would be handy for debug.

[lgirdwood]

I would pipe_info("setting pipe period to %d us", LL_TIMER_PERIOD_US) as this will catch any changes to LL period macro.

[btian1]

@tmleman , I would suggest split this PR to two patches if you need use two deep_buffering, one is for dev->deep_buffering adding, another is for pipeline->deep_buffering adding, this will make the PR more easy to read and understand.

[lgirdwood]

@tmleman does the latest update resolve the review comments ?

[tmleman]

@abonislawski @marcinszkudlinski request for a review.

[lyakh]

are we sure, that ipc_comp cannot be NULL here? pipe_id comes from an IPC... BTW, looks like ipc4_add_comp_dev() can be static

[tmleman]

23ww40.3 Update: rebase

[lgirdwood]

23ww40.3 Update: rebase

@tmleman CI showing some issues. We probably want to rebase and retest to give confidence the calculations are correct.

[tmleman]

I'm closing this PR. I don't have time to work on it at the moment.