Re: [PATCH 2/2] ASoC: fsl: fsl_qmc_audio: Only request completion on last channel

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Hi Hervé,

Le 09/05/2025 à 10:45, Herve Codina a écrit :
> On Fri,  9 May 2025 09:48:45 +0200
> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
>> In non-interleaved mode, several QMC channels are used in sync.
>> More details can be found in commit 188d9cae5438 ("ASoC: fsl:
>> fsl_qmc_audio: Add support for non-interleaved mode.")
>> At the time being, an interrupt is requested on each channel to
>> perform capture/playback completion, allthough the completion is
>> really performed only once all channels have completed their work.
>>
>> This leads to a lot more interrupts than really needed. Looking at
>> /proc/interrupts shows ~3800 interrupts per second when using
>> 4 capture and 4 playback devices with 5ms periods while
>> only 1600 (200 x 4 + 200 x 4) periods are processed during one second.
>>
>> The QMC channels work in sync, the one started first is the one
>> finishing first and the one started last is the one finishing last,
> 
> How can we be sure about that?
> 
> The mapping on the TDM bus has to be taken into account.
> 
> chan 0 -> TDM bits 0..8
> chan 1 -> TDM bits 16..23
> chan 2 -> TDM bits 9..15

In interleaved mode, the QMC will not allow that. You can have 
TS0-TS1-TS2 or TS1-TS2-TS0 but you can't have TS0-TS2-TS1.

In non-interleaved mode we mimic the interleaved mode so I don't expect 
it either.

> 
> In that case chan 1 can finish after chan 2.
> 
> qmc_chan_get_ts_info() could be used to get struct qmc_chan_ts_info
> and [rx,tx]_ts_mask field in the struct give the mapping information.
> 
> The channel that ends last is the one with the highest bit set in the
> mask (rx_tx_mask for capture and tx_ts_mask for playback).

That would be right if the channels were starting all at exactely the 
same time. But qmc_audio_pcm_write_submit() and 
qmc_audio_pcm_read_submit() are calling resp. qmc_chan_write_submit() 
and qmc_chan_read_submit() one by one.

Even if that happens it shouldn't be a problem on its own as there are 
only a few microseconds between each Timeslot (a full cycle is 125 µs). 
And also because calling snd_pcm_period_elapsed() doesn't have any 
destructive effect on ongoing processing.

So I wouldn't make it too complicated. Here the benefit is real and 
worth it.

Thanks,
Christophe

> 
> Best regards,
> Hervé
> 
>> so when the last one finishes it is guaranteed that the other ones are
>> finished as well. Therefore, only request completion processing on the
>> last QMC channel.
>>
>> On my board with the above exemple, on a kernel started with
>> 'threadirqs' option, the QMC irq thread uses 16% CPU time with this
>> patch while it uses 26% CPU time without this patch.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---