From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
[ Upstream commit 6ad299a9b968e1c63988e2a327295e522cf6bbf5 ]
We can have modules in path which can change the number of channels and in this case the BE params needs to be adjusted to configure the DAI according to the copier configuration.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Liam Girdwood liam.r.girdwood@intel.com Message-ID: 20250829105305.31818-2-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Why this is a bugfix that helps users - Problem: When a pipeline contains modules that change channel count (e.g., upmix/downmix), the BE DAI was still being configured from FE channels, which can misconfigure the DAI and cause failures or corrupted audio. The commit message explicitly states this mismatch and the need to adjust BE params to the copier configuration. - Fix: Adds a channel “fixup” that mirrors the already-existing rate fixup so the BE DAI is configured with the correct, unambiguous channel count derived from the DAI copier’s available formats.
What changed (key code points) - New function: `sof_ipc4_pcm_dai_link_fixup_channels()` selects BE channels from the DAI copier’s input pin formats and constrains the BE params when FE/BE channels differ. - Definition: sound/soc/sof/ipc4-pcm.c:730 - Logic: - Reads `ipc4_copier->available_fmt.input_pin_fmts` and extracts channels via `SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt_cfg)`. - If FE channels match any BE input format, do nothing. - If FE channels don’t match, require the BE to define a single (unambiguous) channel count and then constrain `SNDRV_PCM_HW_PARAM_CHANNELS` to exactly that value (min=max). If ambiguous, error out early to avoid wrong configuration. - This mirrors the existing rate fixup logic in `sof_ipc4_pcm_dai_link_fixup_rate()` (sound/soc/sof/ipc4-pcm.c:678), keeping behavior consistent for channels and rate. - Integration: The new channel fixup is invoked in the main BE link fixup sequence right after rate fixup: - Calls at sound/soc/sof/ipc4-pcm.c:841 (rate) and sound/soc/sof/ipc4-pcm.c:845 (channels). - Ensures that subsequent hardware config selection (e.g., `ipc4_ssp_dai_config_pcm_params_match()` using `params_channels(params)` and `params_rate(params)`) sees the corrected constraints. See call at sound/soc/sof/ipc4-pcm.c:870.
Why it’s suitable for stable backport - Fixes a real user-visible bug: prevents misconfigured DAI when channel-changing modules are present, avoiding playback/capture failures or corrupted audio. - Small and contained: Changes only `sound/soc/sof/ipc4-pcm.c` with a single helper and one new call in the existing fixup path; no ABI or architectural changes. - Conservative behavior: - Adjusts BE channels only if FE/BE differ and the BE exposes a single, unambiguous channel count. Otherwise, it leaves FE/BE unchanged or fails fast to avoid silently choosing an arbitrary configuration. - Uses the same safe pattern as the existing rate fixup, reducing regression risk. - Dependencies are already present in IPC4 code: - Uses existing IPC4 types/fields (`struct sof_ipc4_copier`, `available_fmt`, `fmt_cfg`) and macros (`SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT` in include/sound/sof/ipc4/header.h:285). - Impacted scope: Limited to SOF IPC4 ASoC path and only at BE parameter fixup time; this is not core kernel or broad subsystem behavior.
Risk assessment - Low risk: The change only constrains BE channel interval in the specific mismatch case when BE channels are unambiguous; otherwise behavior is unchanged. - Fails early in ambiguous configurations (which would previously risk misprogramming the DAI), improving robustness rather than introducing silent changes.
Conclusion - This is a clear, minimal, and robust bugfix that aligns BE configuration with the DAI copier’s declared formats when channel counts differ. It should be backported to stable trees that include SOF IPC4, as it improves correctness with minimal regression risk.
sound/soc/sof/ipc4-pcm.c | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 37d72a50c1272..9542c428daa4a 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -738,6 +738,58 @@ static int sof_ipc4_pcm_dai_link_fixup_rate(struct snd_sof_dev *sdev, return 0; }
+static int sof_ipc4_pcm_dai_link_fixup_channels(struct snd_sof_dev *sdev, + struct snd_pcm_hw_params *params, + struct sof_ipc4_copier *ipc4_copier) +{ + struct sof_ipc4_pin_format *pin_fmts = ipc4_copier->available_fmt.input_pin_fmts; + struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); + int num_input_formats = ipc4_copier->available_fmt.num_input_formats; + unsigned int fe_channels = params_channels(params); + bool fe_be_match = false; + bool single_be_channels = true; + unsigned int be_channels, val; + int i; + + if (WARN_ON_ONCE(!num_input_formats)) + return -EINVAL; + + /* + * Copier does not change channels, so we + * need to only consider the input pin information. + */ + be_channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(pin_fmts[0].audio_fmt.fmt_cfg); + for (i = 0; i < num_input_formats; i++) { + val = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(pin_fmts[i].audio_fmt.fmt_cfg); + + if (val != be_channels) + single_be_channels = false; + + if (val == fe_channels) { + fe_be_match = true; + break; + } + } + + /* + * If channels is different than FE channels, topology must contain a + * module which can change the number of channels. But we do require + * topology to define a single channels in the DAI copier config in + * this case (FE channels may be variable). + */ + if (!fe_be_match) { + if (!single_be_channels) { + dev_err(sdev->dev, "Unable to select channels for DAI link\n"); + return -EINVAL; + } + + channels->min = be_channels; + channels->max = be_channels; + } + + return 0; +} + static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params) { @@ -801,6 +853,10 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, if (ret) return ret;
+ ret = sof_ipc4_pcm_dai_link_fixup_channels(sdev, params, ipc4_copier); + if (ret) + return ret; + if (single_bitdepth) { snd_mask_none(fmt); valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(ipc4_fmt->fmt_cfg);