thesofproject
ASoC: SOF: ipc4-topology: Improve playback dai copier in/out format selection
ASoC: SOF: ipc4-topology: Improve playback dai copier in/out format selection
The dai copier configuration for playback and capture needs to be separated because it is not correct to configure the dai copier as part of the input format selection for both direction. The input format is the dai format for capture, but it is not for playback, for playback the dai format is on the output side.
Currently we configure and adjust the params based on the DAI supported formats when configuring the input side of the copier but right after the format has been adjusted we reset it for playback and loose this information.
When using a nocodec passthrough topology (which is a bug) we have SSP blobs supporting 32bit only, copier supporting 16/24/32 bit then on playback the dai and copier will be incorrectly configured: host.copier.in: S16_LE host.copier.out: S16_LE dai.copier.in: S16_LE SSP.blob: S32_LE (we only have S32_LE blobs) dai.copier.out: S16_LE (the dai constraint is ignored)
To handle such case the handling of capture and playback streams must be changed: The input format (no changes to previous implementation): for playback it is the pipeline_params for capture it is the adjusted fe_params
The output format (no change for capture direction): for playback it is the adjusted fe_params for capture it is the fe_params
with this change path format configuration will be correct: host.copier.in: S16_LE host.copier.out: S16_LE dai.copier.in: S16_LE SSP.blob: S32_LE (we only have S32_LE blobs) dai.copier.out: S32_LE
@ranj063, @kv2019i, can you take a look at this and perhaps test it as well? In my testing it does not introduce regression or change in behavior when the topology (and it's blobs, config) is correct.
I'm juggling betweeen whether this is going too far in terms of adding intelligence in the kernel code, or whether this is doing mandatory things, and I'm leaning towards the latter.
Did you test with the BT offload test (with nocodec), I think it would be good to do a sanity test before merging.
Minor nit in comimt message: "When using a nocodec passthrough topology (which is a bug)". I had to read this a few time, what do you mean this is a bug? I guess this whole configuration is invalid... (with current code), but with your PR it actually works correctly, so not so clear is it invalid anymore. NHLT and topology can come via different routes, so I think this needs clarify how we handle if topology has more formats and than the NHLT blob. wether this
I have fixed now the nocodec passthrough topology, but there were a bug that it contained only S32_LE blob and config, but advertised support for S16_LE, S24_LE as well. That did not went well and brought this kernel bug in surface, that we were doing the blob selection wrongly for playback, only the capture was correct and this worked because all of our other topologies were correct.