topology2: force all SoundWire link-side copiers to use S24_LE

[plbossart]

Using S32_LE wastes bandwdith for no good reason, we should use 24 bits on the link to maximize bus efficiency with the 9.6 MHz bus clock.

~~FIXME: it's likely we need a kernel-side change as well to configure the hw_params to S24_LE, currently the stream/port is configured with~~

~~sconfig.bps = snd_pcm_format_width(params_format(params));~~

Link: https://github.com/thesofproject/sof/issues/8960

[plbossart]

looks like this topology change is enough, according to kernel logs this changes the format

[  167.550540] snd_sof:sof_ipc4_pcm_dai_link_fixup: sof-audio-pci-intel-mtl 0000:00:1f.3: Set SDW0-Playback to 24 bit format
[  167.748407] snd_sof:sof_ipc4_pcm_dai_link_fixup: sof-audio-pci-intel-mtl 0000:00:1f.3: Set SDW1-Playback to 24 bit format
[  167.770560] snd_sof:sof_ipc4_pcm_dai_link_fixup: sof-audio-pci-intel-mtl 0000:00:1f.3: Set SDW0-Capture to 24 bit format
````

[bardliao]

looks like this topology change is enough, according to kernel logs this changes the format

[  167.550540] snd_sof:sof_ipc4_pcm_dai_link_fixup: sof-audio-pci-intel-mtl 0000:00:1f.3: Set SDW0-Playback to 24 bit format
[  167.748407] snd_sof:sof_ipc4_pcm_dai_link_fixup: sof-audio-pci-intel-mtl 0000:00:1f.3: Set SDW1-Playback to 24 bit format
[  167.770560] snd_sof:sof_ipc4_pcm_dai_link_fixup: sof-audio-pci-intel-mtl 0000:00:1f.3: Set SDW0-Capture to 24 bit format

Yes, we did it at https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/ipc4-pcm.c#L713.

[plbossart]

@bardliao I vaguely recall that the RT713/1713 configuration would give us one playback stream for the heaphone and two capture streams for headset and mic. Did we have any other cases where we ran out of bandwidth with a 4.8MHz clock? I guess for the Rt722 we will have to use 9.6 MHz or two lanes, there's potentially 4 streams and that will not fly at 4.8/single lane.

[plbossart]

Not sure what I did but now I have zero valid bits haha Pin index #0: 0Hz, 0bit, 0ch

https://sof-ci.01.org/sofpr/PR9282/build6224/devicetest/index.html?model=LNLM_SDW_AIOC&testcase=check-capture-10sec

[  245.072209] kernel: snd_sof:sof_ipc4_init_input_audio_fmt: sof-audio-pci-intel-lnl 0000:00:1f.3: Init input audio formats for alh-copier.SDW0-Capture.0
[  245.072213] kernel: snd_sof:sof_ipc4_dbg_audio_format: sof-audio-pci-intel-lnl 0000:00:1f.3: Pin index #0: 48000Hz, 32bit, 2ch (ch_map 0xffffff10 ch_cfg 1 interleaving_style 0 fmt_cfg 0x1802) buffer size 384
[  245.072219] kernel: snd_sof:sof_ipc4_prepare_copier_module: sof-audio-pci-intel-lnl 0000:00:1f.3: copier alh-copier.SDW0-Capture.0: reference output rate 48000, channels 2 valid_bits 0
[  245.072223] kernel: snd_sof:sof_ipc4_prepare_copier_module: sof-audio-pci-intel-lnl 0000:00:1f.3: Output audio format for alh-copier.SDW0-Capture.0
[  245.072225] kernel: snd_sof:sof_ipc4_dbg_audio_format: sof-audio-pci-intel-lnl 0000:00:1f.3: Pin index #0: 0Hz, 0bit, 0ch (ch_map 0x0 ch_cfg 0 interleaving_style 0 fmt_cfg 0x0) buffer size 0
[  245.072235] kernel: sof-audio-pci-intel-lnl 0000:00:1f.3: invalid PCM valid_bits 0
[  245.072331] kernel: sof-audio-pci-intel-lnl 0000:00:1f.3: failed to prepare widget alh-copier.SDW0-Capture.0

[plbossart]

v2: fixed the jack capture. the output format was missing.

[bardliao]

@bardliao I vaguely recall that the RT713/1713 configuration would give us one playback stream for the heaphone and two capture streams for headset and mic. Did we have any other cases where we ran out of bandwidth with a 4.8MHz clock? I guess for the Rt722 we will have to use 9.6 MHz or two lanes, there's potentially 4 streams and that will not fly at 4.8/single lane.

rt713/1713 uses a separated SDW interfaces for mic which is 1713. And rt713 is headset only. rt712 is rt713 + speaker playback. So, rt712 has up to 2 playback streams (headphone and speaker) and 1 capture stream (headset). IOW, the 4.8MHz bandwidth is enough for rt712/713 series. However, rt722 uses the same SDW interface for mic, and has up to 2 playback streams (headphone and speaker) and 2 capture streams (headset and mic). So, rt722 needs 9.6MHz or multilane. Same as rt712-VB. @shumingfan Please correct me if I am wrong.

[plbossart]

Just to be clear @bardliao "the 4.8MHz bandwidth is enough for rt712/713 series" is only true with this topology modification. If we use 32-bits, then we don't have enough bandwidth for 3 streams at 4.8 MHz with a 50x4 frame shape.

[bardliao]

Just to be clear @bardliao "the 4.8MHz bandwidth is enough for rt712/713 series" is only true with this topology modification. If we use 32-bits, then we don't have enough bandwidth for 3 streams at 4.8 MHz with a 50x4 frame shape.

Oh, yes, that's right. I assume 24 bit format is used.

[plbossart]

I am having second-thoughts on the PR, maybe we should avoid hard-coding the valid bits to 24, e.g. if we use 12.288 MHz 32 bits would be just fine since we'd have 64 rows. Probably a macro would be better.

[bardliao]

I am having second-thoughts on the PR, maybe we should avoid hard-coding the valid bits to 24, e.g. if we use 12.288 MHz 32 bits would be just fine since we'd have 64 rows. Probably a macro would be better.

Then we just need to set SDW_AMP_FMT_24 and SDW_JACK_FMT_24 = true by default.

[plbossart]

Then we just need to set SDW_AMP_FMT_24 and SDW_JACK_FMT_24 = true by default.

Humm... the configuration does not really depend on the type of function but rather on the bus/link configuration. it's really SDW_LINK_BITS or something, and we want this to be a value not a boolean to make the code simpler.

[bardliao]

Then we just need to set SDW_AMP_FMT_24 and SDW_JACK_FMT_24 = true by default.

Humm... the configuration does not really depend on the type of function but rather on the bus/link configuration. it's really SDW_LINK_BITS or something, and we want this to be a value not a boolean to make the code simpler.

Be a value is definitely better. I just don't know how to do it. :)

[plbossart]

v3: added SDW_LINK_VALID_BITS macro and fixed bad edit for output format.

[bardliao]

Do we need to consider about in_bit_depth and out_bit_depth, too? For example, in_bit_depth and out_bit_depth can be 16 if the valid bit is 16.

[plbossart]

The in_bit_depth is always 32 for the capture dai copier and the output_bit_depth is always 32 for the playback dai copier, isn't it? The options for 16 bits are only on the host-copier side

[bardliao]

The in_bit_depth is always 32 for the capture dai copier and the output_bit_depth is always 32 for the playback dai copier, isn't it? The options for 16 bits are only on the host-copier side

I am not sure about it. We have for example out_bit_depth 16 in current sdw-amp-generic.conf.

[plbossart]

The in_bit_depth is always 32 for the capture dai copier and the output_bit_depth is always 32 for the playback dai copier, isn't it? The options for 16 bits are only on the host-copier side

I am not sure about it. We have for example out_bit_depth 16 in current sdw-amp-generic.conf.

I removed all this to have a single bit depth on the link side. This configuration of 16 bits in 16 bit words makes no sense for SoundWire, the FIFOs only work for 32 bits. The only variable is the word length i.e. the number of bits pushed on the bus.

[plbossart]

@kv2019i @lgirdwood this PR is not urgent, it should only be merged after re-testing all the SoundWire devices we have in the daily tests.

[lgirdwood]

@kv2019i @lgirdwood this PR is not urgent, it should only be merged after re-testing all the SoundWire devices we have in the daily tests.

@ssavati @fredoh9 can someone help here with soundwire testing using this PR ?

[ssavati]

@ssavati @fredoh9 can someone help here with soundwire testing using this PR ?

Sure @lgirdwood I will run more test on SDW

[ssavati]

@ssavati @fredoh9 can someone help here with soundwire testing using this PR ?

@lgirdwood I have completed soundwire functional and stress tests for MTL and LNL . no new regression observed. For more details look into test report ID 43727 and 43732

[plbossart]

Thanks @ssavati, all good to merge then.