ASoC: Fix missing channel fixup for codec end of ch_map

Richard Fitzgerald <rf@opensource.cirrus.com> says:

Struct snd_soc_dai_link_ch_map had a single mask member to set the
CPU channel masks. But no fixup was done to the codec end of the link.

For example if a 4-channel CPU capture DAI was made from two codecs both
supplying 2 channels, the hw_params() of the codec would be passed a
channel count of 4.

On SoundWire this could cause multiple codecs to send data in the same
bits of a frame because the unused channels were not disabled.

The changes in this series are:
- Separate channel masks for CPU and codec in struct
  snd_soc_dai_link_ch_map .

- Apply the codec channel mask as a channel count fixup if the machine
  drive has not set a TDM mask.

- Set the codec channel mask in the SoundWire machine driver.

- Remove the workaround from the cs_amp machine driver.

Link: https://patch.msgid.link/20260910114500.1586637-1-rf@opensource.cirrus.com

5 files changed, 29 insertions, 64 deletions

diff --git a/include/sound/soc.h b/include/sound/soc.h
index f46b2bc2a022..5afc34b147b5 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -699,7 +699,8 @@ struct snd_soc_dai_link_component {
 struct snd_soc_dai_link_ch_map {
 	unsigned int cpu;
 	unsigned int codec;
-	unsigned int ch_mask;
+	unsigned int cpu_ch_mask;
+	unsigned int codec_ch_mask;
 };
 
 struct snd_soc_dai_link {
diff --git a/include/sound/soc_sdw_utils.h b/include/sound/soc_sdw_utils.h
index 9b28e9aef4f1..9fbb69b9052d 100644
--- a/include/sound/soc_sdw_utils.h
+++ b/include/sound/soc_sdw_utils.h
@@ -250,8 +250,6 @@ int asoc_sdw_cs_amp_init(struct snd_soc_card *card,
 			 struct snd_soc_dai_link *dai_links,
 			 struct asoc_sdw_codec_info *info,
 			 bool playback);
-int asoc_sdw_cs_spk_feedback_rtd_init(struct snd_soc_pcm_runtime *rtd,
-				      struct snd_soc_dai *dai);
 int asoc_sdw_cs35l56_volume_limit(struct snd_soc_card *card, const char *name_prefix);
 
 /* MAXIM codec support */
diff --git a/sound/soc/sdw_utils/soc_sdw_cs_amp.c b/sound/soc/sdw_utils/soc_sdw_cs_amp.c
index 325ab7230481..6e21ef8f87e2 100644
--- a/sound/soc/sdw_utils/soc_sdw_cs_amp.c
+++ b/sound/soc/sdw_utils/soc_sdw_cs_amp.c
@@ -14,7 +14,6 @@
 #include <sound/soc-dai.h>
 #include <sound/soc_sdw_utils.h>
 
-#define CS_AMP_CHANNELS_PER_AMP	4
 #define CS35L56_SPK_VOLUME_0DB 400 /* 0dB Max */
 
 int asoc_sdw_cs35l56_volume_limit(struct snd_soc_card *card, const char *name_prefix)
@@ -64,51 +63,6 @@ int asoc_sdw_cs_spk_rtd_init(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai
 }
 EXPORT_SYMBOL_NS(asoc_sdw_cs_spk_rtd_init, "SND_SOC_SDW_UTILS");
 
-int asoc_sdw_cs_spk_feedback_rtd_init(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *dai)
-{
-	const struct snd_soc_dai_link *dai_link = rtd->dai_link;
-	const struct snd_soc_dai_link_ch_map *ch_map;
-	const struct snd_soc_dai_link_component *codec_dlc;
-	struct snd_soc_dai *codec_dai;
-	u8 ch_slot[8] = {};
-	unsigned int amps_per_bus, ch_per_amp, mask;
-	int i, ret;
-
-	WARN_ON(dai_link->num_cpus > ARRAY_SIZE(ch_slot));
-
-	/*
-	 * CS35L56 has 4 TX channels. When the capture is aggregated the
-	 * same bus slots will be allocated to all the amps on a bus. Only
-	 * one amp on that bus can be transmitting in each slot so divide
-	 * the available 4 slots between all the amps on a bus.
-	 */
-	amps_per_bus = dai_link->num_codecs / dai_link->num_cpus;
-	if ((amps_per_bus == 0) || (amps_per_bus > CS_AMP_CHANNELS_PER_AMP)) {
-		dev_err(rtd->card->dev, "Illegal num_codecs:%u / num_cpus:%u\n",
-			dai_link->num_codecs, dai_link->num_cpus);
-		return -EINVAL;
-	}
-
-	ch_per_amp = CS_AMP_CHANNELS_PER_AMP / amps_per_bus;
-
-	for_each_rtd_ch_maps(rtd, i, ch_map) {
-		codec_dlc = snd_soc_link_to_codec(rtd->dai_link, i);
-		codec_dai = snd_soc_find_dai(codec_dlc);
-		mask = GENMASK(ch_per_amp - 1, 0) << ch_slot[ch_map->cpu];
-
-		ret = snd_soc_dai_set_tdm_slot(codec_dai, 0, mask, 4, 32);
-		if (ret < 0) {
-			dev_err(rtd->card->dev, "Failed to set TDM slot:%d\n", ret);
-			return ret;
-		}
-
-		ch_slot[ch_map->cpu] += ch_per_amp;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_NS(asoc_sdw_cs_spk_feedback_rtd_init, "SND_SOC_SDW_UTILS");
-
 int asoc_sdw_cs_amp_init(struct snd_soc_card *card,
 			 struct snd_soc_dai_link *dai_links,
 			 struct asoc_sdw_codec_info *info,
diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c b/sound/soc/sdw_utils/soc_sdw_utils.c
index a66dcc02fb59..d2eeef4931c6 100644
--- a/sound/soc/sdw_utils/soc_sdw_utils.c
+++ b/sound/soc/sdw_utils/soc_sdw_utils.c
@@ -811,7 +811,6 @@ struct asoc_sdw_codec_info codec_info_list[] = {
 				.dai_name = "cs35l56-sdw1c",
 				.dai_type = SOC_SDW_DAI_TYPE_AMP,
 				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID},
-				.rtd_init = asoc_sdw_cs_spk_feedback_rtd_init,
 			},
 		},
 		.dai_num = 2,
@@ -840,7 +839,6 @@ struct asoc_sdw_codec_info codec_info_list[] = {
 				.dai_name = "cs35l56-sdw1c",
 				.dai_type = SOC_SDW_DAI_TYPE_AMP,
 				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID},
-				.rtd_init = asoc_sdw_cs_spk_feedback_rtd_init,
 			},
 		},
 		.dai_num = 2,
@@ -869,7 +867,6 @@ struct asoc_sdw_codec_info codec_info_list[] = {
 				.dai_name = "cs35l56-sdw1c",
 				.dai_type = SOC_SDW_DAI_TYPE_AMP,
 				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID},
-				.rtd_init = asoc_sdw_cs_spk_feedback_rtd_init,
 			},
 		},
 		.dai_num = 2,
@@ -898,7 +895,6 @@ struct asoc_sdw_codec_info codec_info_list[] = {
 				.dai_name = "cs35l56-sdw1c",
 				.dai_type = SOC_SDW_DAI_TYPE_AMP,
 				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID},
-				.rtd_init = asoc_sdw_cs_spk_feedback_rtd_init,
 			},
 		},
 		.dai_num = 2,
@@ -1565,7 +1561,7 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct snd_soc_dai_link_ch_map *ch_maps;
 	int ch = params_channels(params);
-	unsigned int ch_mask;
+	unsigned int cpu_ch_mask, codec_ch_mask;
 	int num_codecs;
 	int step;
 	int i;
@@ -1575,8 +1571,9 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream,
 
 	/* Identical data will be sent to all codecs in playback */
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		ch_mask = GENMASK(ch - 1, 0);
+		cpu_ch_mask = GENMASK(ch - 1, 0);
 		step = 0;
+		codec_ch_mask = 0;
 	} else {
 		num_codecs = rtd->dai_link->num_codecs;
 
@@ -1586,17 +1583,24 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream,
 			return -EINVAL;
 		}
 
-		ch_mask = GENMASK(ch / num_codecs - 1, 0);
-		step = hweight_long(ch_mask);
+		cpu_ch_mask = GENMASK(ch / num_codecs - 1, 0);
+		step = hweight_long(cpu_ch_mask);
+		codec_ch_mask = cpu_ch_mask;
 	}
 
 	/*
 	 * The captured data will be combined from each cpu DAI if the dai
 	 * link has more than one codec DAIs. Set codec channel mask and
 	 * ASoC will set the corresponding channel numbers for each cpu dai.
+	 *
+	 * sdw_stream_add_slave() assigns different payload offsets to each
+	 * codec in a capture stream, so that the same channels on each
+	 * codec map to different channels on the CPU.
 	 */
-	for_each_link_ch_maps(rtd->dai_link, i, ch_maps)
-		ch_maps->ch_mask = ch_mask << (i * step);
+	for_each_link_ch_maps(rtd->dai_link, i, ch_maps) {
+		ch_maps->cpu_ch_mask = cpu_ch_mask << (i * step);
+		ch_maps->codec_ch_mask = codec_ch_mask;
+	}
 
 	return 0;
 }
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 0e49290a8c90..3137c091bdb8 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1206,7 +1206,9 @@ static int __soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		goto out;
 
 	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		unsigned int tdm_mask = snd_soc_dai_tdm_mask_get(codec_dai, substream->stream);
+		unsigned int ch_mask = snd_soc_dai_tdm_mask_get(codec_dai, substream->stream);
+		struct snd_soc_dai_link_ch_map *ch_maps;
+		int j;
 
 		/*
 		 * Skip CODECs which don't support the current stream type,
@@ -1228,9 +1230,15 @@ static int __soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		/* copy params for each codec */
 		tmp_params = *params;
 
-		/* fixup params based on TDM slot masks */
-		if (tdm_mask)
-			soc_pcm_codec_params_fixup(&tmp_params, tdm_mask);
+		/* fixup params based on TDM or ch_map masks */
+		if (!ch_mask) {
+			for_each_rtd_ch_maps(rtd, j, ch_maps)
+				if (ch_maps->codec == i)
+					ch_mask |= ch_maps->codec_ch_mask;
+		}
+
+		if (ch_mask)
+			soc_pcm_codec_params_fixup(&tmp_params, ch_mask);
 
 		ret = snd_soc_dai_hw_params(codec_dai, substream,
 					    &tmp_params);
@@ -1264,7 +1272,7 @@ static int __soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		 */
 		for_each_rtd_ch_maps(rtd, j, ch_maps)
 			if (ch_maps->cpu == i)
-				ch_mask |= ch_maps->ch_mask;
+				ch_mask |= ch_maps->cpu_ch_mask;
 
 		/* fixup cpu channel number */
 		if (ch_mask)