[PATCH v5 0/5] ASoC: makes CPU/Codec channel connection map more generic

[PATCH v5 0/5] ASoC: makes CPU/Codec channel connection map more generic

[Kuninori Morimoto]

Hi Mark
Cc Bard, Pierre-Louis, Jerome, DT-ML

This is v5 patch-set.

Current ASoC is supporting CPU/Codec = N:M (N < M) connection by using
ch_map idea. This patch-set expands it that all connection uses this idea,
and no longer N < M limit [1][2].

Link: https://lore.kernel.org/r/87fs6wuszr.wl-kuninori.morimoto.gx@renesas.com [1]
Link: https://lore.kernel.org/r/878r7yqeo4.wl-kuninori.morimoto.gx@renesas.com [2]

This patch is tested on Audio-Graph-Card2 with sample dtsi,
but needs Tested-by, at least from Intel.

[PATCH 1/5] patch got Tested-by from Pierre-Louis / Jerome before,
but v5 is using different idea from v4. Thus I didn't add below tag.

	Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
	Tested-by: Jerome Brunet <jbrunet@baylibre.com>


v4 -> v5
	- use cpu/codec index on ch_maps
	- separate card2 sample DT patch into prepare and new feature
	- ch-maps -> ch-map-idx on DT

v3 -> v4
	- add Jerome on To
	- add "description" on "ch-maps"

v2 -> v3
	- tidyup comment
	- use more clear connection image on DT
	- "ch_maps" -> "ch-maps" on DT
	- Add DT maintainer on "To:" for all patches

v1 -> v2
	- makes CPU/Codec connection relation clear on comment/sample
	- fixup type "connction" -> "connection"
	- makes error message clear

Kuninori Morimoto (4):
  ASoC: makes CPU/Codec channel connection map more generic
  ASoC: audio-graph-card2: add CPU:Codec = N:M support
  ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample
  dt-bindings: audio-graph-port: add ch-maps property


Kuninori Morimoto (5):
  ASoC: makes CPU/Codec channel connection map more generic
  ASoC: audio-graph-card2: add CPU:Codec = N:M support
  ASoC: audio-graph-card2-custom-sample: tidyup comment / numbering
  ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample
  dt-bindings: audio-graph-port: add ch-map-idx property

 .../bindings/sound/audio-graph-port.yaml      |   7 +-
 include/sound/soc.h                           |  55 ++++++-
 .../audio-graph-card2-custom-sample.dtsi      | 136 +++++++++++++++---
 sound/soc/generic/audio-graph-card2.c         |  49 +++++++
 sound/soc/intel/boards/sof_sdw.c              |  28 ++--
 sound/soc/soc-core.c                          |  97 ++++++++++++-
 sound/soc/soc-dapm.c                          |  45 ++----
 sound/soc/soc-pcm.c                           |  44 ++----
 8 files changed, 360 insertions(+), 101 deletions(-)

-- 
2.25.1

[PATCH v5 1/5] ASoC: makes CPU/Codec channel connection map more generic

[Kuninori Morimoto]

Current ASoC CPU:Codec = N:M connection is using connection mapping idea,
but it is used for N < M case only. We want to use it for any case.

By this patch, not only N:M connection, but all existing connection
(1:1, 1:N, N:N) will use same connection mapping. Then, because it will
use default mapping, no conversion patch is needed to exising drivers.

More over, CPU:Codec = N:M (N > M) also supported in the same time.

ch_maps array will has CPU/Codec index by this patch.

Image
	CPU0 <---> Codec0
	CPU1 <-+-> Codec1
	CPU2 <-/

ch_map
	ch_map[0].cpu = 0	ch_map[0].codec = 0
	ch_map[1].cpu = 1	ch_map[1].codec = 1
	ch_map[2].cpu = 2	ch_map[2].codec = 1

Link: https://lore.kernel.org/r/87fs6wuszr.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/878r7yqeo4.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h              | 55 +++++++++++++++++-
 sound/soc/intel/boards/sof_sdw.c | 28 ++++-----
 sound/soc/soc-core.c             | 97 +++++++++++++++++++++++++++++++-
 sound/soc/soc-dapm.c             | 45 ++++-----------
 sound/soc/soc-pcm.c              | 44 +++++----------
 5 files changed, 186 insertions(+), 83 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 63b57f58cc56..2eb93e357789 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -655,8 +655,45 @@ struct snd_soc_dai_link_component {
 	struct of_phandle_args *dai_args;
 };
 
-struct snd_soc_dai_link_codec_ch_map {
-	unsigned int connected_cpu_id;
+/*
+ * [dai_link->ch_maps Image sample]
+ *
+ *-------------------------
+ * CPU0 <---> Codec0
+ *
+ * ch-map[0].cpu = 0	ch-map[0].codec = 0
+ *
+ *-------------------------
+ * CPU0 <---> Codec0
+ * CPU1 <---> Codec1
+ * CPU2 <---> Codec2
+ *
+ * ch-map[0].cpu = 0	ch-map[0].codec = 0
+ * ch-map[1].cpu = 1	ch-map[1].codec = 1
+ * ch-map[2].cpu = 2	ch-map[2].codec = 2
+ *
+ *-------------------------
+ * CPU0 <---> Codec0
+ * CPU1 <-+-> Codec1
+ * CPU2 <-/
+ *
+ * ch-map[0].cpu = 0	ch-map[0].codec = 0
+ * ch-map[1].cpu = 1	ch-map[1].codec = 1
+ * ch-map[2].cpu = 2	ch-map[2].codec = 1
+ *
+ *-------------------------
+ * CPU0 <---> Codec0
+ * CPU1 <-+-> Codec1
+ *	  \-> Codec2
+ *
+ * ch-map[0].cpu = 0	ch-map[0].codec = 0
+ * ch-map[1].cpu = 1	ch-map[1].codec = 1
+ * ch-map[2].cpu = 1	ch-map[2].codec = 2
+ *
+ */
+struct snd_soc_dai_link_ch_map {
+	unsigned int cpu;
+	unsigned int codec;
 	unsigned int ch_mask;
 };
 
@@ -688,7 +725,9 @@ struct snd_soc_dai_link {
 	struct snd_soc_dai_link_component *codecs;
 	unsigned int num_codecs;
 
-	struct snd_soc_dai_link_codec_ch_map *codec_ch_maps;
+	/* num_ch_maps = max(num_cpu, num_codecs) */
+	struct snd_soc_dai_link_ch_map *ch_maps;
+
 	/*
 	 * You MAY specify the link's platform/PCM/DMA driver, either by
 	 * device name, or by DT/OF node, but not both. Some forms of link
@@ -775,6 +814,10 @@ struct snd_soc_dai_link {
 #endif
 };
 
+static inline int snd_soc_link_num_ch_map(struct snd_soc_dai_link *link) {
+	return max(link->num_cpus, link->num_codecs);
+}
+
 static inline struct snd_soc_dai_link_component*
 snd_soc_link_to_cpu(struct snd_soc_dai_link *link, int n) {
 	return &(link)->cpus[n];
@@ -808,6 +851,12 @@ snd_soc_link_to_platform(struct snd_soc_dai_link *link, int n) {
 		     ((cpu) = snd_soc_link_to_cpu(link, i));		\
 	     (i)++)
 
+#define for_each_link_ch_maps(link, i, ch_map)			\
+	for ((i) = 0;						\
+	     ((i) < snd_soc_link_num_ch_map(link) &&		\
+		      ((ch_map) = link->ch_maps + i));		\
+	     (i)++)
+
 /*
  * Sample 1 : Single CPU/Codec/Platform
  *
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 226a74a4c340..d62b3b46297f 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -570,16 +570,14 @@ int sdw_hw_params(struct snd_pcm_substream *substream,
 		  struct snd_pcm_hw_params *params)
 {
 	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);
-	struct snd_soc_dai *codec_dai;
-	struct snd_soc_dai *cpu_dai;
 	unsigned int ch_mask;
 	int num_codecs;
 	int step;
 	int i;
-	int j;
 
-	if (!rtd->dai_link->codec_ch_maps)
+	if (!rtd->dai_link->ch_maps)
 		return 0;
 
 	/* Identical data will be sent to all codecs in playback */
@@ -605,13 +603,9 @@ int sdw_hw_params(struct snd_pcm_substream *substream,
 	 * link has more than one codec DAIs. Set codec channel mask and
 	 * ASoC will set the corresponding channel numbers for each cpu dai.
 	 */
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		for_each_rtd_codec_dais(rtd, j, codec_dai) {
-			if (rtd->dai_link->codec_ch_maps[j].connected_cpu_id != i)
-				continue;
-			rtd->dai_link->codec_ch_maps[j].ch_mask = ch_mask << (j * step);
-		}
-	}
+	for_each_link_ch_maps(rtd->dai_link, i, ch_maps)
+		ch_maps->ch_mask = ch_mask << (i * step);
+
 	return 0;
 }
 
@@ -1350,15 +1344,17 @@ static int get_slave_info(const struct snd_soc_acpi_link_adr *adr_link,
 	return 0;
 }
 
-static void set_dailink_map(struct snd_soc_dai_link_codec_ch_map *sdw_codec_ch_maps,
+static void set_dailink_map(struct snd_soc_dai_link_ch_map *sdw_codec_ch_maps,
 			    int codec_num, int cpu_num)
 {
 	int step;
 	int i;
 
 	step = codec_num / cpu_num;
-	for (i = 0; i < codec_num; i++)
-		sdw_codec_ch_maps[i].connected_cpu_id = i / step;
+	for (i = 0; i < codec_num; i++) {
+		sdw_codec_ch_maps[i].cpu	= i / step;
+		sdw_codec_ch_maps[i].codec	= i;
+	}
 }
 
 static const char * const type_strings[] = {"SimpleJack", "SmartAmp", "SmartMic"};
@@ -1453,7 +1449,7 @@ static int create_sdw_dailink(struct snd_soc_card *card, int *link_index,
 		*ignore_pch_dmic = true;
 
 	for_each_pcm_streams(stream) {
-		struct snd_soc_dai_link_codec_ch_map *sdw_codec_ch_maps;
+		struct snd_soc_dai_link_ch_map *sdw_codec_ch_maps;
 		char *name, *cpu_name;
 		int playback, capture;
 		static const char * const sdw_stream_name[] = {
@@ -1530,7 +1526,7 @@ static int create_sdw_dailink(struct snd_soc_card *card, int *link_index,
 		dai_links[*link_index].nonatomic = true;
 
 		set_dailink_map(sdw_codec_ch_maps, codec_num, cpu_dai_num);
-		dai_links[*link_index].codec_ch_maps = sdw_codec_ch_maps;
+		dai_links[*link_index].ch_maps = sdw_codec_ch_maps;
 		ret = set_codec_init_func(card, adr_link, dai_links + (*link_index)++,
 					  playback, group_id, adr_index, dai_index);
 		if (ret < 0) {
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c305e94762c3..a55fea23f871 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1015,6 +1015,96 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
 	return -EINVAL;
 }
 
+#define MAX_DEFAULT_CH_MAP_SIZE 7
+static struct snd_soc_dai_link_ch_map default_ch_map_sync[MAX_DEFAULT_CH_MAP_SIZE] = {
+	{ .cpu = 0, .codec = 0 },
+	{ .cpu = 1, .codec = 1 },
+	{ .cpu = 2, .codec = 2 },
+	{ .cpu = 3, .codec = 3 },
+	{ .cpu = 4, .codec = 4 },
+	{ .cpu = 5, .codec = 5 },
+	{ .cpu = 6, .codec = 6 },
+};
+static struct snd_soc_dai_link_ch_map default_ch_map_1cpu[MAX_DEFAULT_CH_MAP_SIZE] = {
+	{ .cpu = 0, .codec = 0 },
+	{ .cpu = 0, .codec = 1 },
+	{ .cpu = 0, .codec = 2 },
+	{ .cpu = 0, .codec = 3 },
+	{ .cpu = 0, .codec = 4 },
+	{ .cpu = 0, .codec = 5 },
+	{ .cpu = 0, .codec = 6 },
+};
+static struct snd_soc_dai_link_ch_map default_ch_map_1codec[MAX_DEFAULT_CH_MAP_SIZE] = {
+	{ .cpu = 0, .codec = 0 },
+	{ .cpu = 1, .codec = 0 },
+	{ .cpu = 2, .codec = 0 },
+	{ .cpu = 3, .codec = 0 },
+	{ .cpu = 4, .codec = 0 },
+	{ .cpu = 5, .codec = 0 },
+	{ .cpu = 6, .codec = 0 },
+};
+static int snd_soc_compensate_channel_connection_map(struct snd_soc_card *card,
+						     struct snd_soc_dai_link *dai_link)
+{
+	struct snd_soc_dai_link_ch_map *ch_maps;
+	int i, max;
+
+	/*
+	 * dai_link->ch_maps indicates how CPU/Codec are connected.
+	 * It will be a map seen from a larger number of DAI.
+	 * see
+	 *	soc.h :: [dai_link->ch_maps Image sample]
+	 */
+
+	/* it should have ch_maps if connection was N:M */
+	if (dai_link->num_cpus > 1 && dai_link->num_codecs > 1 &&
+	    dai_link->num_cpus != dai_link->num_codecs && !dai_link->ch_maps) {
+		dev_err(card->dev, "need to have ch_maps when N:M connction (%s)",
+			dai_link->name);
+		return -EINVAL;
+	}
+
+	/* do nothing if it has own maps */
+	if (dai_link->ch_maps)
+		goto sanity_check;
+
+	/* check default map size */
+	if (dai_link->num_cpus   > MAX_DEFAULT_CH_MAP_SIZE ||
+	    dai_link->num_codecs > MAX_DEFAULT_CH_MAP_SIZE) {
+		dev_err(card->dev, "soc-core.c needs update default_connection_maps");
+		return -EINVAL;
+	}
+
+	/* Compensate missing map for ... */
+	if (dai_link->num_cpus == dai_link->num_codecs)
+		dai_link->ch_maps = default_ch_map_sync;	/* for 1:1 or N:N */
+	else if (dai_link->num_cpus <  dai_link->num_codecs)
+		dai_link->ch_maps = default_ch_map_1cpu;	/* for 1:N */
+	else
+		dai_link->ch_maps = default_ch_map_1codec;	/* for N:1 */
+
+sanity_check:
+	max = min(dai_link->num_cpus, dai_link->num_codecs);
+
+	dev_dbg(card->dev, "dai_link %s\n", dai_link->stream_name);
+	for_each_link_ch_maps(dai_link, i, ch_maps) {
+		if ((ch_maps->cpu   >= dai_link->num_cpus) ||
+		    (ch_maps->codec >= dai_link->num_codecs)) {
+			dev_err(card->dev,
+				"unexpected dai_link->ch_maps[%d] index (cpu(%d/%d) codec(%d/%d))",
+				i,
+				ch_maps->cpu,	dai_link->num_cpus,
+				ch_maps->codec,	dai_link->num_codecs);
+			return -EINVAL;
+		}
+
+		dev_dbg(card->dev, "  [%d] cpu%d <-> codec%d\n",
+			i, ch_maps->cpu, ch_maps->codec);
+	}
+
+	return 0;
+}
+
 /**
  * snd_soc_remove_pcm_runtime - Remove a pcm_runtime from card
  * @card: The ASoC card to which the pcm_runtime has
@@ -1121,8 +1211,13 @@ int snd_soc_add_pcm_runtimes(struct snd_soc_card *card,
 			     int num_dai_link)
 {
 	for (int i = 0; i < num_dai_link; i++) {
-		int ret = snd_soc_add_pcm_runtime(card, dai_link + i);
+		int ret;
+
+		ret = snd_soc_compensate_channel_connection_map(card, dai_link + i);
+		if (ret < 0)
+			return ret;
 
+		ret = snd_soc_add_pcm_runtime(card, dai_link + i);
 		if (ret < 0)
 			return ret;
 	}
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 2512aadf95f7..770768f4a027 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -4426,11 +4426,14 @@ static void soc_dapm_dai_stream_event(struct snd_soc_dai *dai, int stream,
 void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
 {
 	struct snd_soc_pcm_runtime *rtd;
+	struct snd_soc_dai *cpu_dai;
 	struct snd_soc_dai *codec_dai;
-	int i;
 
 	/* for each BE DAI link... */
 	for_each_card_rtds(card, rtd)  {
+		struct snd_soc_dai_link_ch_map *ch_maps;
+		int i;
+
 		/*
 		 * dynamic FE links have no fixed DAI mapping.
 		 * CODEC<->CODEC links have no direct connection.
@@ -4438,39 +4441,15 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
 		if (rtd->dai_link->dynamic)
 			continue;
 
-		if (rtd->dai_link->num_cpus == 1) {
-			for_each_rtd_codec_dais(rtd, i, codec_dai)
-				dapm_connect_dai_pair(card, rtd, codec_dai,
-						      snd_soc_rtd_to_cpu(rtd, 0));
-		} else if (rtd->dai_link->num_codecs == rtd->dai_link->num_cpus) {
-			for_each_rtd_codec_dais(rtd, i, codec_dai)
-				dapm_connect_dai_pair(card, rtd, codec_dai,
-						      snd_soc_rtd_to_cpu(rtd, i));
-		} else if (rtd->dai_link->num_codecs > rtd->dai_link->num_cpus) {
-			int cpu_id;
-
-			if (!rtd->dai_link->codec_ch_maps) {
-				dev_err(card->dev, "%s: no codec channel mapping table provided\n",
-					__func__);
-				continue;
-			}
+		/*
+		 * see
+		 *	soc.h :: [dai_link->ch_maps Image sample]
+		 */
+		for_each_link_ch_maps(rtd->dai_link, i, ch_maps) {
+			cpu_dai   = snd_soc_rtd_to_cpu(rtd,   ch_maps->cpu);
+			codec_dai = snd_soc_rtd_to_codec(rtd, ch_maps->codec);
 
-			for_each_rtd_codec_dais(rtd, i, codec_dai) {
-				cpu_id = rtd->dai_link->codec_ch_maps[i].connected_cpu_id;
-				if (cpu_id >= rtd->dai_link->num_cpus) {
-					dev_err(card->dev,
-						"%s: dai_link %s cpu_id %d too large, num_cpus is %d\n",
-						__func__, rtd->dai_link->name, cpu_id,
-						rtd->dai_link->num_cpus);
-					continue;
-				}
-				dapm_connect_dai_pair(card, rtd, codec_dai,
-						      snd_soc_rtd_to_cpu(rtd, cpu_id));
-			}
-		} else {
-			dev_err(card->dev,
-				"%s: codec number %d < cpu number %d is not supported\n",
-				__func__, rtd->dai_link->num_codecs, rtd->dai_link->num_cpus);
+			dapm_connect_dai_pair(card, rtd, codec_dai, cpu_dai);
 		}
 	}
 }
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 8c168dc553f6..c434fc19e253 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1042,6 +1042,7 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
 	}
 
 	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
+		struct snd_soc_dai_link_ch_map *ch_maps;
 		unsigned int ch_mask = 0;
 		int j;
 
@@ -1055,22 +1056,20 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
 		/* copy params for each cpu */
 		tmp_params = *params;
 
-		if (!rtd->dai_link->codec_ch_maps)
-			goto hw_params;
 		/*
 		 * construct cpu channel mask by combining ch_mask of each
 		 * codec which maps to the cpu.
+		 * see
+		 *	soc.h :: [dai_link->ch_maps Image sample]
 		 */
-		for_each_rtd_codec_dais(rtd, j, codec_dai) {
-			if (rtd->dai_link->codec_ch_maps[j].connected_cpu_id == i)
-				ch_mask |= rtd->dai_link->codec_ch_maps[j].ch_mask;
-		}
+		for_each_link_ch_maps(rtd->dai_link, j, ch_maps)
+			if (ch_maps->cpu == i)
+				ch_mask |= ch_maps->ch_mask;
 
 		/* fixup cpu channel number */
 		if (ch_mask)
 			soc_pcm_codec_params_fixup(&tmp_params, ch_mask);
 
-hw_params:
 		ret = snd_soc_dai_hw_params(cpu_dai, substream, &tmp_params);
 		if (ret < 0)
 			goto out;
@@ -2818,35 +2817,20 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
 			}
 		}
 	} else {
+		struct snd_soc_dai_link_ch_map *ch_maps;
 		struct snd_soc_dai *codec_dai;
 
 		/* Adapt stream for codec2codec links */
 		int cpu_capture  = snd_soc_get_stream_cpu(dai_link, SNDRV_PCM_STREAM_CAPTURE);
 		int cpu_playback = snd_soc_get_stream_cpu(dai_link, SNDRV_PCM_STREAM_PLAYBACK);
 
-		for_each_rtd_codec_dais(rtd, i, codec_dai) {
-			if (dai_link->num_cpus == 1) {
-				cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
-			} else if (dai_link->num_cpus == dai_link->num_codecs) {
-				cpu_dai = snd_soc_rtd_to_cpu(rtd, i);
-			} else if (rtd->dai_link->num_codecs > rtd->dai_link->num_cpus) {
-				int cpu_id;
-
-				if (!rtd->dai_link->codec_ch_maps) {
-					dev_err(rtd->card->dev, "%s: no codec channel mapping table provided\n",
-						__func__);
-					return -EINVAL;
-				}
-
-				cpu_id = rtd->dai_link->codec_ch_maps[i].connected_cpu_id;
-				cpu_dai = snd_soc_rtd_to_cpu(rtd, cpu_id);
-			} else {
-				dev_err(rtd->card->dev,
-					"%s codec number %d < cpu number %d is not supported\n",
-					__func__, rtd->dai_link->num_codecs,
-					rtd->dai_link->num_cpus);
-				return -EINVAL;
-			}
+		/*
+		 * see
+		 *	soc.h :: [dai_link->ch_maps Image sample]
+		 */
+		for_each_link_ch_maps(dai_link, i, ch_maps) {
+			cpu_dai	  = snd_soc_rtd_to_cpu(rtd,   ch_maps->cpu);
+			codec_dai = snd_soc_rtd_to_codec(rtd, ch_maps->codec);
 
 			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
 			    snd_soc_dai_stream_valid(cpu_dai,   cpu_playback))
-- 
2.25.1

Re: [PATCH v5 1/5] ASoC: makes CPU/Codec channel connection map more generic

[Pierre-Louis Bossart]

On 10/23/23 00:35, Kuninori Morimoto wrote:
> Current ASoC CPU:Codec = N:M connection is using connection mapping idea,
> but it is used for N < M case only. We want to use it for any case.
> 
> By this patch, not only N:M connection, but all existing connection
> (1:1, 1:N, N:N) will use same connection mapping. Then, because it will
> use default mapping, no conversion patch is needed to exising drivers.
> 
> More over, CPU:Codec = N:M (N > M) also supported in the same time.
> 
> ch_maps array will has CPU/Codec index by this patch.
> 
> Image
> 	CPU0 <---> Codec0
> 	CPU1 <-+-> Codec1
> 	CPU2 <-/
> 
> ch_map
> 	ch_map[0].cpu = 0	ch_map[0].codec = 0
> 	ch_map[1].cpu = 1	ch_map[1].codec = 1
> 	ch_map[2].cpu = 2	ch_map[2].codec = 1
> 
> Link: https://lore.kernel.org/r/87fs6wuszr.wl-kuninori.morimoto.gx@renesas.com
> Link: https://lore.kernel.org/r/878r7yqeo4.wl-kuninori.morimoto.gx@renesas.com
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

The Intel CI did not detect any issues with this patch, see
https://github.com/thesofproject/linux/pull/4632, so

Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Note however the -W1 error below


> +static int snd_soc_compensate_channel_connection_map(struct snd_soc_card *card,
> +						     struct snd_soc_dai_link *dai_link)
> +{
> +	struct snd_soc_dai_link_ch_map *ch_maps;
> +	int i, max;

sound/soc/soc-core.c: In function
‘snd_soc_compensate_channel_connection_map’:
sound/soc/soc-core.c:1050:16: error: variable ‘max’ set but not used
[-Werror=unused-but-set-variable]
 1050 |         int i, max;
      |                ^~~

> +	/*
> +	 * dai_link->ch_maps indicates how CPU/Codec are connected.
> +	 * It will be a map seen from a larger number of DAI.
> +	 * see
> +	 *	soc.h :: [dai_link->ch_maps Image sample]
> +	 */
> +
> +	/* it should have ch_maps if connection was N:M */
> +	if (dai_link->num_cpus > 1 && dai_link->num_codecs > 1 &&
> +	    dai_link->num_cpus != dai_link->num_codecs && !dai_link->ch_maps) {
> +		dev_err(card->dev, "need to have ch_maps when N:M connction (%s)",
> +			dai_link->name);
> +		return -EINVAL;
> +	}
> +
> +	/* do nothing if it has own maps */
> +	if (dai_link->ch_maps)
> +		goto sanity_check;
> +
> +	/* check default map size */
> +	if (dai_link->num_cpus   > MAX_DEFAULT_CH_MAP_SIZE ||
> +	    dai_link->num_codecs > MAX_DEFAULT_CH_MAP_SIZE) {
> +		dev_err(card->dev, "soc-core.c needs update default_connection_maps");
> +		return -EINVAL;
> +	}
> +
> +	/* Compensate missing map for ... */
> +	if (dai_link->num_cpus == dai_link->num_codecs)
> +		dai_link->ch_maps = default_ch_map_sync;	/* for 1:1 or N:N */
> +	else if (dai_link->num_cpus <  dai_link->num_codecs)
> +		dai_link->ch_maps = default_ch_map_1cpu;	/* for 1:N */
> +	else
> +		dai_link->ch_maps = default_ch_map_1codec;	/* for N:1 */
> +
> +sanity_check:
> +	max = min(dai_link->num_cpus, dai_link->num_codecs);

sound/soc/soc-core.c: In function
‘snd_soc_compensate_channel_connection_map’:
sound/soc/soc-core.c:1050:16: error: variable ‘max’ set but not used
[-Werror=unused-but-set-variable]
 1050 |         int i, max;
      |                ^~~
> +
> +	dev_dbg(card->dev, "dai_link %s\n", dai_link->stream_name);
> +	for_each_link_ch_maps(dai_link, i, ch_maps) {
> +		if ((ch_maps->cpu   >= dai_link->num_cpus) ||
> +		    (ch_maps->codec >= dai_link->num_codecs)) {
> +			dev_err(card->dev,
> +				"unexpected dai_link->ch_maps[%d] index (cpu(%d/%d) codec(%d/%d))",
> +				i,
> +				ch_maps->cpu,	dai_link->num_cpus,
> +				ch_maps->codec,	dai_link->num_codecs);
> +			return -EINVAL;
> +		}
> +
> +		dev_dbg(card->dev, "  [%d] cpu%d <-> codec%d\n",
> +			i, ch_maps->cpu, ch_maps->codec);
> +	}
> +
> +	return 0;
> +}

Re: [PATCH v5 1/5] ASoC: makes CPU/Codec channel connection map more generic

[Kuninori Morimoto]

Hi Pierre-Louis

> The Intel CI did not detect any issues with this patch, see
(snip)
> Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Thank you for your test, again !

> Note however the -W1 error below

Thanks. Will fix in v6


Thank you for your help !!

Best regards
---
Kuninori Morimoto

[PATCH v5 2/5] ASoC: audio-graph-card2: add CPU:Codec = N:M support

[Kuninori Morimoto]

Now ASoC is supporting CPU:Codec = N:M support.
This patch enables it on Audio Graph Card2.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card2.c | 49 +++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index 5d856942bcae..bef7ef5e408c 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -506,6 +506,7 @@ static int __graph_parse_node(struct simple_util_priv *priv,
 	return 0;
 }
 
+#define MAX_PROP 7
 static int graph_parse_node(struct simple_util_priv *priv,
 			    enum graph_type gtype,
 			    struct device_node *port,
@@ -515,10 +516,31 @@ static int graph_parse_node(struct simple_util_priv *priv,
 	int ret = 0;
 
 	if (graph_lnk_is_multi(port)) {
+		struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
+		struct device_node *ports = of_get_parent(port);
+		struct device *dev = simple_priv_to_dev(priv);
 		int idx;
+		int num;
 
 		of_node_get(port);
 
+		/*
+		 * create ch_maps if CPU:Codec = N:M
+		 * DPCM is out of scope
+		 */
+		if (gtype != GRAPH_DPCM && !dai_link->ch_maps &&
+		    dai_link->num_cpus > 1 && dai_link->num_codecs > 1 &&
+		    dai_link->num_cpus != dai_link->num_codecs) {
+			num = max (dai_link->num_cpus, dai_link->num_codecs);
+
+			dai_link->ch_maps = devm_kcalloc(dev, num,
+						sizeof(struct snd_soc_dai_link_ch_map), GFP_KERNEL);
+			if (!dai_link->ch_maps) {
+				ret = -ENOMEM;
+				goto multi_end;
+			}
+		}
+
 		for (idx = 0;; idx++) {
 			ep = graph_get_next_multi_ep(&port);
 			if (!ep)
@@ -529,7 +551,34 @@ static int graph_parse_node(struct simple_util_priv *priv,
 			of_node_put(ep);
 			if (ret < 0)
 				break;
+
+			/* CPU:Codec = N:M */
+			if (dai_link->ch_maps) {
+				const char *props = "ch-map-idx";
+				u32 num_array[MAX_PROP];
+				int i;
+
+				num = of_property_count_elems_of_size(ep, props, sizeof(u32));
+				if (num > MAX_PROP) {
+					dev_err(dev, "need update MAX_PROP (%d)\n", num);
+					ret = -EINVAL;
+					goto multi_end;
+				}
+
+				ret = of_property_read_u32_array(ep, props, num_array, num);
+				if (ret < 0)
+					goto multi_end;
+
+				for (i = 0; i < num; i++) {
+					if (is_cpu)
+						dai_link->ch_maps[num_array[i]].cpu = idx;
+					else
+						dai_link->ch_maps[num_array[i]].codec = idx;
+				}
+			}
 		}
+multi_end:
+		of_node_put(ports);
 	} else {
 		/* Single CPU / Codec */
 		ep = port_to_endpoint(port);
-- 
2.25.1

[PATCH v5 3/5] ASoC: audio-graph-card2-custom-sample: tidyup comment / numbering

[Kuninori Morimoto]

Some comment doesn't match other styles, this patch tidyup it.
To prepare Multi CPU/Codec, this patch fixup some CPU/Codec numbering.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../audio-graph-card2-custom-sample.dtsi      | 37 ++++++++++---------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card2-custom-sample.dtsi b/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
index 8acaa2ddb335..736eca553d7c 100644
--- a/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
+++ b/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
@@ -58,7 +58,7 @@ / {
 	 *			| |-> codec13
 	 *			+-+
 	 *
-	 * [Multi-CPU/Codec]
+	 * [Multi-CPU/Codec-0]
 	 *		+-+		+-+
 	 *	cpu1 <--| |<-@--------->| |-> codec1
 	 *	cpu2 <--| |		| |-> codec2
@@ -144,11 +144,14 @@ audio-graph-card2-custom-sample {
 			 */
 			 &cpu0
 
-			/* [Semi-Multi] */
+			/*
+			 * [Semi-Multi]
+			 * cpu7/codec12/codec13
+			 */
 			&sm0
 
 			/*
-			 * [Multi-CPU/Codec]: cpu side only
+			 * [Multi-CPU/Codec-0]: cpu side only
 			 * cpu1/cpu2/codec1/codec2
 			 */
 			 &mcpu0
@@ -182,24 +185,24 @@ multi {
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			/* [Multi-CPU-0] */
 			ports@0 {
 				reg = <0>;
 				#address-cells = <1>;
 				#size-cells = <0>;
-			/* [Multi-CPU] */
-			mcpu0:	port@0 { reg = <0>; mcpu0_ep: endpoint { remote-endpoint = <&mcodec0_ep>; }; };
-				port@1 { reg = <1>; mcpu1_ep: endpoint { remote-endpoint = <&cpu1_ep>;    }; };
-				port@2 { reg = <2>; mcpu2_ep: endpoint { remote-endpoint = <&cpu2_ep>;    }; };
+			mcpu0:	port@0 { reg = <0>; mcpu00_ep: endpoint { remote-endpoint = <&mcodec00_ep>; }; };
+				port@1 { reg = <1>; mcpu01_ep: endpoint { remote-endpoint = <&cpu1_ep>;     }; };
+				port@2 { reg = <2>; mcpu02_ep: endpoint { remote-endpoint = <&cpu2_ep>;     }; };
 			};
 
-			/* [Multi-Codec] */
+			/* [Multi-Codec-0] */
 			ports@1 {
 				reg = <1>;
 				#address-cells = <1>;
 				#size-cells = <0>;
-				port@0 { reg = <0>; mcodec0_ep: endpoint { remote-endpoint = <&mcpu0_ep>;  }; };
-				port@1 { reg = <1>; mcodec1_ep: endpoint { remote-endpoint = <&codec1_ep>; }; };
-				port@2 { reg = <2>; mcodec2_ep: endpoint { remote-endpoint = <&codec2_ep>; }; };
+				port@0 { reg = <0>; mcodec00_ep: endpoint { remote-endpoint = <&mcpu00_ep>;  }; };
+				port@1 { reg = <1>; mcodec01_ep: endpoint { remote-endpoint = <&codec1_ep>;  }; };
+				port@2 { reg = <2>; mcodec02_ep: endpoint { remote-endpoint = <&codec2_ep>;  }; };
 			};
 
 			/* [DPCM-Multi]::BE */
@@ -323,9 +326,9 @@ ports {
 			/* [Normal] */
 			cpu0: port@0 { reg = <0>; cpu0_ep: endpoint { remote-endpoint = <&codec0_ep>; }; };
 
-			/* [Multi-CPU] */
-			      port@1 { reg = <1>; cpu1_ep: endpoint { remote-endpoint = <&mcpu1_ep>; }; };
-			      port@2 { reg = <2>; cpu2_ep: endpoint { remote-endpoint = <&mcpu2_ep>; }; };
+			/* [Multi-CPU-0] */
+			      port@1 { reg = <1>; cpu1_ep: endpoint { remote-endpoint = <&mcpu01_ep>; }; };
+			      port@2 { reg = <2>; cpu2_ep: endpoint { remote-endpoint = <&mcpu02_ep>; }; };
 
 			/* [DPCM]::FE */
 			      port@3 { reg = <3>; cpu3_ep: endpoint { remote-endpoint = <&fe00_ep>; }; };
@@ -363,9 +366,9 @@ ports {
 			/* [Normal] */
 			port@0  { reg = <0>; codec0_ep:  endpoint { remote-endpoint = <&cpu0_ep>; }; };
 
-			/* [Multi-Codec] */
-			port@1  { reg = <1>; codec1_ep:  endpoint { remote-endpoint = <&mcodec1_ep>; }; };
-			port@2  { reg = <2>; codec2_ep:  endpoint { remote-endpoint = <&mcodec2_ep>; }; };
+			/* [Multi-Codec-0] */
+			port@1  { reg = <1>; codec1_ep:  endpoint { remote-endpoint = <&mcodec01_ep>; }; };
+			port@2  { reg = <2>; codec2_ep:  endpoint { remote-endpoint = <&mcodec02_ep>; }; };
 
 			/* [DPCM]::BE */
 			port@3  {
-- 
2.25.1

[PATCH v5 4/5] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample

[Kuninori Morimoto]

Now ASoC is supporting CPU/Codec = N:M connection.
This patch adds its sample settings.

But One note here is that it has many type of samples, it reached to
maximum of sound minor number. Therefore, new sample is disabled so far.
If you want to try it, you need to disable some other one instead.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../audio-graph-card2-custom-sample.dtsi      | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/sound/soc/generic/audio-graph-card2-custom-sample.dtsi b/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
index 736eca553d7c..fbbc203edee1 100644
--- a/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
+++ b/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
@@ -64,6 +64,26 @@ / {
 	 *	cpu2 <--| |		| |-> codec2
 	 *		+-+		+-+
 	 *
+	 * [Multi-CPU/Codec-1]
+	 * About ch-map / ch-map-idx (*), see
+	 *	soc.h :: [dai_link->ch_maps Image sample]
+	 *
+	 *		+-+		+-+		ch-map (*)
+	 *	cpu8 <--| |<-@--------->| |-> codec14	ch-map[0].cpu = cpu8	ch-map[0].codec = codec14
+	 *	cpu9 <--| |		| |-> codec15	ch-map[1].cpu = cpu9	ch-map[1].codec = codec15
+	 *		+-+		| |-> codec16	ch-map[2].cpu = cpu9	ch-map[2].codec = codec16
+	 *				+-+
+	 *
+	 * [Multi-CPU/Codec-2]
+	 * About ch-map / ch-map-idx (*), see
+	 *	soc.h :: [dai_link->ch_maps Image sample]
+	 *
+	 *		+-+		+-+		ch-map (*)
+	 *	cpu10 <-| |<-@--------->| |-> codec17	ch-map[0].cpu = cpu10	ch-map[0].codec = codec17
+	 *	cpu11 <-| |		| |-> codec18	ch-map[1].cpu = cpu11	ch-map[1].codec = codec18
+	 *	cpu12 <-| |		+-+		ch-map[2].cpu = cpu12	ch-map[2].codec = codec18
+	 *		+-+
+	 *
 	 * [DPCM]
 	 *
 	 *	CPU3/CPU4 are converting rate to 44100
@@ -156,6 +176,26 @@ &sm0
 			 */
 			 &mcpu0
 
+			/*
+			 * [Multi-CPU/Codec-1]: cpu side only
+			 * cpu8/cpu9/codec14/codec15/codec16
+			 *
+			 * Because it will reach to the maximum of sound minor number,
+			 * disable it so far.
+			 * If you want to try it, please disable some other one instead.
+			 */
+			//&mcpu1
+
+			/*
+			 * [Multi-CPU/Codec-2]: cpu side only
+			 * cpu10/cpu11/cpu12/codec17/codec18
+			 *
+			 * Because it will reach to the maximum of sound minor number,
+			 * disable it so far.
+			 * If you want to try it, please disable some other one instead.
+			 */
+			//&mcpu2
+
 			/*
 			 * [DPCM]: both FE / BE
 			 * cpu3/cpu4/codec3
@@ -244,6 +284,48 @@ ports@5 {
 				port@1 { reg = <1>; smcodec1_ep: endpoint { remote-endpoint = <&codec12_ep>; }; };
 				port@2 { reg = <2>; smcodec2_ep: endpoint { remote-endpoint = <&codec13_ep>; }; };
 			};
+
+			/* [Multi-CPU-1] */
+			ports@6 {
+				reg = <6>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			mcpu1:	port@0 { reg = <0>; mcpu10_ep: endpoint { remote-endpoint = <&mcodec10_ep>; }; };
+				port@1 { reg = <1>; mcpu11_ep: endpoint { remote-endpoint = <&cpu8_ep>;     }; };
+				port@2 { reg = <2>; mcpu12_ep: endpoint { remote-endpoint = <&cpu9_ep>;     }; };
+			};
+
+			/* [Multi-Codec-1] */
+			ports@7 {
+				reg = <7>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 { reg = <0>; mcodec10_ep: endpoint { remote-endpoint = <&mcpu10_ep>;  }; };
+				port@1 { reg = <1>; mcodec11_ep: endpoint { remote-endpoint = <&codec14_ep>; }; };
+				port@2 { reg = <2>; mcodec12_ep: endpoint { remote-endpoint = <&codec15_ep>; }; };
+				port@3 { reg = <3>; mcodec13_ep: endpoint { remote-endpoint = <&codec16_ep>; }; };
+			};
+
+			/* [Multi-CPU-2] */
+			ports@8 {
+				reg = <8>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			mcpu2:	port@0 { reg = <0>; mcpu20_ep: endpoint { remote-endpoint = <&mcodec20_ep>; }; };
+				port@1 { reg = <1>; mcpu21_ep: endpoint { remote-endpoint = <&cpu10_ep>;    }; };
+				port@2 { reg = <2>; mcpu22_ep: endpoint { remote-endpoint = <&cpu11_ep>;    }; };
+				port@3 { reg = <3>; mcpu23_ep: endpoint { remote-endpoint = <&cpu12_ep>;    }; };
+			};
+
+			/* [Multi-Codec-2] */
+			ports@9 {
+				reg = <9>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 { reg = <0>; mcodec20_ep: endpoint { remote-endpoint = <&mcpu20_ep>;  }; };
+				port@1 { reg = <1>; mcodec21_ep: endpoint { remote-endpoint = <&codec17_ep>; }; };
+				port@2 { reg = <2>; mcodec22_ep: endpoint { remote-endpoint = <&codec18_ep>; }; };
+			};
 		};
 
 		dpcm {
@@ -340,6 +422,15 @@ ports {
 
 			/* [Semi-Multi] */
 			sm0:  port@7 { reg = <7>; cpu7_ep: endpoint { remote-endpoint = <&smcodec0_ep>; }; };
+
+			/* [Multi-CPU-1] */
+			      port@8 { reg = <8>; cpu8_ep: endpoint { remote-endpoint = <&mcpu11_ep>; ch-map-idx = <0>;   }; };
+			      port@9 { reg = <9>; cpu9_ep: endpoint { remote-endpoint = <&mcpu12_ep>; ch-map-idx = <1 2>; }; };
+
+			/* [Multi-CPU-2] */
+			      port@a { reg = <10>; cpu10_ep: endpoint { remote-endpoint = <&mcpu21_ep>; ch-map-idx = <0>; }; };
+			      port@b { reg = <11>; cpu11_ep: endpoint { remote-endpoint = <&mcpu22_ep>; ch-map-idx = <1>; }; };
+			      port@c { reg = <12>; cpu12_ep: endpoint { remote-endpoint = <&mcpu23_ep>; ch-map-idx = <2>; }; };
 		};
 	};
 
@@ -398,6 +489,14 @@ port@3  {
 			port@c { reg = <12>; codec12_ep: endpoint { remote-endpoint = <&smcodec1_ep>; }; };
 			port@d { reg = <13>; codec13_ep: endpoint { remote-endpoint = <&smcodec2_ep>; }; };
 
+			/* [Multi-Codec-1] */
+			port@e  { reg = <14>; codec14_ep:  endpoint { remote-endpoint = <&mcodec11_ep>; ch-map-idx = <0>;}; };
+			port@f  { reg = <15>; codec15_ep:  endpoint { remote-endpoint = <&mcodec12_ep>; ch-map-idx = <1>;}; };
+			port@10 { reg = <16>; codec16_ep:  endpoint { remote-endpoint = <&mcodec13_ep>; ch-map-idx = <2>;}; };
+
+			/* [Multi-Codec-2] */
+			port@11 { reg = <17>; codec17_ep:  endpoint { remote-endpoint = <&mcodec21_ep>; ch-map-idx = <0>;  }; };
+			port@12 { reg = <18>; codec18_ep:  endpoint { remote-endpoint = <&mcodec22_ep>; ch-map-idx = <1 2>;}; };
 		};
 	};
 };
-- 
2.25.1

[PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Kuninori Morimoto]

This patch adds ch-maps property to enable handling CPU:Codec = N:M
connection.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../devicetree/bindings/sound/audio-graph-port.yaml        | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
index 60b5e3fd1115..47f04cdd6670 100644
--- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
+++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
@@ -19,7 +19,12 @@ definitions:
     properties:
       mclk-fs:
         $ref: simple-card.yaml#/definitions/mclk-fs
-
+      ch-map-idx:
+        description: It indicates index of ch_maps array for CPU / Codec if number of
+          CPU(N) / Codec(M) DAIs were not same in one dai-link. ch-map-idx is not needed if the
+          numbers were 1:M or N:1 or N=M. see soc.h::[dai_link->ch_maps Image sample] and
+          ${LINUX}/sound/soc/generic/audio-graph-card2-custom-sample.dtsi. It is good sample.
+        $ref: /schemas/types.yaml#/definitions/uint32-array
   endpoint-base:
     allOf:
       - $ref: /schemas/graph.yaml#/$defs/endpoint-base
-- 
2.25.1

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2383 bytes --]

Yo,

On Mon, Oct 23, 2023 at 05:36:09AM +0000, Kuninori Morimoto wrote:
> This patch adds ch-maps property to enable handling CPU:Codec = N:M
> connection.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../devicetree/bindings/sound/audio-graph-port.yaml        | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> index 60b5e3fd1115..47f04cdd6670 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> @@ -19,7 +19,12 @@ definitions:
>      properties:
>        mclk-fs:
>          $ref: simple-card.yaml#/definitions/mclk-fs
> -

Why have you removed the blank line here?

> +      ch-map-idx:

I would rather this be spelt out as "channel-map-index" - although I
don't know if that is the best name for the property, as it seems very
tied to a single operating systems variable names.
I'll leave it to Mark as to whether there is a less linux implementation
coupled name for this property.

> +        description: It indicates index of ch_maps array for CPU / Codec if number of

From a bindings perspective, "ch_maps array" is meaningless, as it is
(AFAICT) a linux driver variable name, whereas the property description
needs to describe the hardware alone.

> +          CPU(N) / Codec(M) DAIs were not same in one dai-link. ch-map-idx is not needed if the
> +          numbers were 1:M or N:1 or N=M. see soc.h::[dai_link->ch_maps Image sample] and

Again, relying on header files in an operating system to explain the
property is not a runner. You need to explain how to populate this
property in an operating system independent manner.

> +          ${LINUX}/sound/soc/generic/audio-graph-card2-custom-sample.dtsi. It is good sample.

I'd much rather you added an example to this dt-binding, rather than
pointing off to another location. A proper example will also be able to
be validated by dt-binding-check.

> +        $ref: /schemas/types.yaml#/definitions/uint32-array

Blank line here please.

Cheers,
Conor.

>    endpoint-base:
>      allOf:
>        - $ref: /schemas/graph.yaml#/$defs/endpoint-base
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

On Mon, Oct 23, 2023 at 05:50:42PM +0100, Conor Dooley wrote:
> On Mon, Oct 23, 2023 at 05:36:09AM +0000, Kuninori Morimoto wrote:

> > +      ch-map-idx:

> I would rather this be spelt out as "channel-map-index" - although I
> don't know if that is the best name for the property, as it seems very
> tied to a single operating systems variable names.
> I'll leave it to Mark as to whether there is a less linux implementation
> coupled name for this property.

It's not particularly Linux coupled, this is a fairly general concept.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On Mon, Oct 23, 2023 at 07:47:09PM +0100, Mark Brown wrote:
> On Mon, Oct 23, 2023 at 05:50:42PM +0100, Conor Dooley wrote:
> > On Mon, Oct 23, 2023 at 05:36:09AM +0000, Kuninori Morimoto wrote:
> 
> > > +      ch-map-idx:
> 
> > I would rather this be spelt out as "channel-map-index" - although I
> > don't know if that is the best name for the property, as it seems very
> > tied to a single operating systems variable names.
> > I'll leave it to Mark as to whether there is a less linux implementation
> > coupled name for this property.
> 
> It's not particularly Linux coupled, this is a fairly general concept.

You'd know better than I, it just seemed like a rip from the variable
name :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Kuninori Morimoto]

Hi Mark, Conor

Thank you for your feedbacks.

> > > > +      ch-map-idx:
> > 
> > > I would rather this be spelt out as "channel-map-index" - although I
> > > don't know if that is the best name for the property, as it seems very
> > > tied to a single operating systems variable names.
> > > I'll leave it to Mark as to whether there is a less linux implementation
> > > coupled name for this property.
> > 
> > It's not particularly Linux coupled, this is a fairly general concept.
> 
> You'd know better than I, it just seemed like a rip from the variable
> name :)

I have no special opinion about this, but let's use more generic naming.
v6 will use "channel-map-index" for it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Kuninori Morimoto]

Hi Conor


> > +          CPU(N) / Codec(M) DAIs were not same in one dai-link. ch-map-idx is not needed if the
> > +          numbers were 1:M or N:1 or N=M. see soc.h::[dai_link->ch_maps Image sample] and
> 
> Again, relying on header files in an operating system to explain the
> property is not a runner. You need to explain how to populate this
> property in an operating system independent manner.

Sample is not mandatory here, I will remove Linux header pointer from here in v6.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Mon, Oct 23, 2023 at 10:58:28PM +0000, Kuninori Morimoto wrote:
> 
> Hi Conor
> 
> 
> > > +          CPU(N) / Codec(M) DAIs were not same in one dai-link. ch-map-idx is not needed if the
> > > +          numbers were 1:M or N:1 or N=M. see soc.h::[dai_link->ch_maps Image sample] and
> > 
> > Again, relying on header files in an operating system to explain the
> > property is not a runner. You need to explain how to populate this
> > property in an operating system independent manner.
> 
> Sample is not mandatory here, I will remove Linux header pointer from here in v6.

Please don't just remove the reference to the header file, and actually
explain the property instead.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Kuninori Morimoto]

Hi Conor

> > > Again, relying on header files in an operating system to explain the
> > > property is not a runner. You need to explain how to populate this
> > > property in an operating system independent manner.
> > 
> > Sample is not mandatory here, I will remove Linux header pointer from here in v6.
> 
> Please don't just remove the reference to the header file, and actually
> explain the property instead.

Yes, I will try my best.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Rob Herring]

On Mon, Oct 23, 2023 at 05:36:09AM +0000, Kuninori Morimoto wrote:
> This patch adds ch-maps property to enable handling CPU:Codec = N:M
> connection.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../devicetree/bindings/sound/audio-graph-port.yaml        | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> index 60b5e3fd1115..47f04cdd6670 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> @@ -19,7 +19,12 @@ definitions:
>      properties:
>        mclk-fs:
>          $ref: simple-card.yaml#/definitions/mclk-fs
> -
> +      ch-map-idx:
> +        description: It indicates index of ch_maps array for CPU / Codec if number of
> +          CPU(N) / Codec(M) DAIs were not same in one dai-link. ch-map-idx is not needed if the
> +          numbers were 1:M or N:1 or N=M. see soc.h::[dai_link->ch_maps Image sample] and
> +          ${LINUX}/sound/soc/generic/audio-graph-card2-custom-sample.dtsi. It is good sample.

Why do we have a dtsi file hidden away here?

We have examples and actual users. Do we really need a 3rd way?

Rob

Re: [PATCH v5 5/5] dt-bindings: audio-graph-port: add ch-map-idx property

[Kuninori Morimoto]

Hi Rob

> > +      ch-map-idx:
> > +        description: It indicates index of ch_maps array for CPU / Codec if number of
> > +          CPU(N) / Codec(M) DAIs were not same in one dai-link. ch-map-idx is not needed if the
> > +          numbers were 1:M or N:1 or N=M. see soc.h::[dai_link->ch_maps Image sample] and
> > +          ${LINUX}/sound/soc/generic/audio-graph-card2-custom-sample.dtsi. It is good sample.
> 
> Why do we have a dtsi file hidden away here?
> 
> We have examples and actual users. Do we really need a 3rd way?

ASoC is supporting many type of (complex) connections, and Audio Graph Card2 is
supporting all of them. There is no actual user who is using all type of
connections. Thus there is no good sample for it.

Above is using all type of connections. And I'm using it for Audio Graph
Card2 test purpose.

Thank you for your help !!

Best regards
---
Kuninori Morimoto