devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic
@ 2023-10-25  2:18 Kuninori Morimoto
  2023-10-25  2:18 ` [PATCH v6 1/5] " Kuninori Morimoto
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-25  2:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree


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

This is v6 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]

v5 have changed basic idea, and Intel might got some effect, but was Tested-by
Pierre-Louis.  Jerome's environment might got some effect, but it is less than
Intel and similar environment was already tested by Audio-Graph-Card2 and its
test dtsi. Thus, I added their Tested-by on v5 patch.

v5 -> v6
	- tidyup some warnings
	- rename "ch-map-idx" -> "channel-map-index"
	- Update "channel-map-index" description
	- add Tested-by from Pierre-Louis / Jerome

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 (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 channel-map-index property

 .../bindings/sound/audio-graph-port.yaml      |  13 ++
 include/sound/soc.h                           |  56 +++++++-
 .../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                          |  95 +++++++++++-
 sound/soc/soc-dapm.c                          |  45 ++----
 sound/soc/soc-pcm.c                           |  44 ++----
 8 files changed, 366 insertions(+), 100 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v6 1/5] ASoC: makes CPU/Codec channel connection map more generic
  2023-10-25  2:18 [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
@ 2023-10-25  2:18 ` Kuninori Morimoto
  2023-10-25  5:33   ` Liao, Bard
  2023-10-25  2:18 ` [PATCH v6 2/5] ASoC: audio-graph-card2: add CPU:Codec = N:M support Kuninori Morimoto
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-25  2:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree

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>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Jerome Brunet <jbrunet@baylibre.com>
---
 include/sound/soc.h              | 56 ++++++++++++++++++-
 sound/soc/intel/boards/sof_sdw.c | 28 ++++------
 sound/soc/soc-core.c             | 95 +++++++++++++++++++++++++++++++-
 sound/soc/soc-dapm.c             | 45 ++++-----------
 sound/soc/soc-pcm.c              | 44 +++++----------
 5 files changed, 185 insertions(+), 83 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 7792c393e238..f3803c2dc349 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
  *
@@ -1163,6 +1212,7 @@ struct snd_soc_pcm_runtime {
 	     ((i) < (rtd)->dai_link->num_cpus + (rtd)->dai_link->num_codecs) &&	\
 		     ((dai) = (rtd)->dais[i]);				\
 	     (i)++)
+#define for_each_rtd_ch_maps(rtd, i, ch_maps) for_each_link_ch_maps(rtd->dai_link, i, ch_maps)
 
 void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);
 
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 3312ad8a563b..2faf7372bad0 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 b2bd45e87bc3..4ca3319a8e19 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1015,6 +1015,94 @@ 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;
+
+	/*
+	 * 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:
+	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 +1209,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 4e2beda6f9bf..233ebc74c313 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -4438,11 +4438,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.
@@ -4450,39 +4453,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_rtd_ch_maps(rtd, 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..7198f017c167 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_rtd_ch_maps(rtd, 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_rtd_ch_maps(rtd, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 2/5] ASoC: audio-graph-card2: add CPU:Codec = N:M support
  2023-10-25  2:18 [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
  2023-10-25  2:18 ` [PATCH v6 1/5] " Kuninori Morimoto
@ 2023-10-25  2:18 ` Kuninori Morimoto
  2023-10-25  2:18 ` [PATCH v6 3/5] ASoC: audio-graph-card2-custom-sample: tidyup comment / numbering Kuninori Morimoto
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-25  2:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree

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 7146611df730..25a1c7d40be9 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -504,6 +504,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,
@@ -513,10 +514,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)
@@ -527,7 +549,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 = "channel-map-index";
+				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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 3/5] ASoC: audio-graph-card2-custom-sample: tidyup comment / numbering
  2023-10-25  2:18 [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
  2023-10-25  2:18 ` [PATCH v6 1/5] " Kuninori Morimoto
  2023-10-25  2:18 ` [PATCH v6 2/5] ASoC: audio-graph-card2: add CPU:Codec = N:M support Kuninori Morimoto
@ 2023-10-25  2:18 ` Kuninori Morimoto
  2023-10-25  2:18 ` [PATCH v6 4/5] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample Kuninori Morimoto
  2023-10-25  2:19 ` [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property Kuninori Morimoto
  4 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-25  2:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 4/5] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample
  2023-10-25  2:18 [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2023-10-25  2:18 ` [PATCH v6 3/5] ASoC: audio-graph-card2-custom-sample: tidyup comment / numbering Kuninori Morimoto
@ 2023-10-25  2:18 ` Kuninori Morimoto
  2023-10-25  2:19 ` [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property Kuninori Morimoto
  4 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-25  2:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree

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..f792ada69f02 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 / channel-map-index (*), 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
+	 *				+-+		       ^-- channel-map-index --^
+	 *
+	 * [Multi-CPU/Codec-2]
+	 * About ch-map / channel-map-index (*), 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
+	 *		+-+				       ^-- channel-map-index --^
+	 *
 	 * [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>; channel-map-index = <0>;   }; };
+			      port@9 { reg = <9>; cpu9_ep: endpoint { remote-endpoint = <&mcpu12_ep>; channel-map-index = <1 2>; }; };
+
+			/* [Multi-CPU-2] */
+			      port@a { reg = <10>; cpu10_ep: endpoint { remote-endpoint = <&mcpu21_ep>; channel-map-index = <0>; }; };
+			      port@b { reg = <11>; cpu11_ep: endpoint { remote-endpoint = <&mcpu22_ep>; channel-map-index = <1>; }; };
+			      port@c { reg = <12>; cpu12_ep: endpoint { remote-endpoint = <&mcpu23_ep>; channel-map-index = <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>; channel-map-index = <0>;}; };
+			port@f  { reg = <15>; codec15_ep:  endpoint { remote-endpoint = <&mcodec12_ep>; channel-map-index = <1>;}; };
+			port@10 { reg = <16>; codec16_ep:  endpoint { remote-endpoint = <&mcodec13_ep>; channel-map-index = <2>;}; };
+
+			/* [Multi-Codec-2] */
+			port@11 { reg = <17>; codec17_ep:  endpoint { remote-endpoint = <&mcodec21_ep>; channel-map-index = <0>;  }; };
+			port@12 { reg = <18>; codec18_ep:  endpoint { remote-endpoint = <&mcodec22_ep>; channel-map-index = <1 2>;}; };
 		};
 	};
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property
  2023-10-25  2:18 [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2023-10-25  2:18 ` [PATCH v6 4/5] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample Kuninori Morimoto
@ 2023-10-25  2:19 ` Kuninori Morimoto
  2023-10-26 21:22   ` Rob Herring
  4 siblings, 1 reply; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-25  2:19 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree

This patch adds channel-map-index 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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
index 60b5e3fd1115..2a7e0784d591 100644
--- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
+++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
@@ -93,6 +93,19 @@ definitions:
               minimum: 1
               maximum: 64
 
+      channel-map-index:
+        description: It indicates CPU/Codec DAIs channel mapping index if number of
+          CPU(N) / Codec(M) were not same in one dai-link. channel-map-index is not
+          needed if the numbers were 1:M or N:1 or N==M. Same indexed CPU <-> Codec
+          will be paired. This is CPUx2 <-> Codecx3 sample.
+                                   CPUA   { ... .channel-map-index = <0>;   }
+          [0] CPUA <---> CodecA    CPUB   { ... .channel-map-index = <1 2>; }
+          [1] CPUB <-+-> CodecB
+          [2]        \-> CodecC    CodecA { ... .channel-map-index = <0>; }
+                                   CodecB { ... .channel-map-index = <1>; }
+                                   CodecC { ... .channel-map-index = <2>; }
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+
   ports:
     $ref: "#/definitions/port-base"
     unevaluatedProperties: false
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 1/5] ASoC: makes CPU/Codec channel connection map more generic
  2023-10-25  2:18 ` [PATCH v6 1/5] " Kuninori Morimoto
@ 2023-10-25  5:33   ` Liao, Bard
  0 siblings, 0 replies; 9+ messages in thread
From: Liao, Bard @ 2023-10-25  5:33 UTC (permalink / raw)
  To: Kuninori Morimoto, Pierre-Louis Bossart, Mark Brown, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, Jerome Brunet
  Cc: alsa-devel, devicetree


On 2023/10/25 上午 10:18, 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>
> Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Tested-by: Jerome Brunet <jbrunet@baylibre.com>


The change looks good to me.

Reviewed-by:  Bard Liao <yung-chuan.liao@linux.intel.com>


> ---
>   include/sound/soc.h              | 56 ++++++++++++++++++-
>   sound/soc/intel/boards/sof_sdw.c | 28 ++++------
>   sound/soc/soc-core.c             | 95 +++++++++++++++++++++++++++++++-
>   sound/soc/soc-dapm.c             | 45 ++++-----------
>   sound/soc/soc-pcm.c              | 44 +++++----------
>   5 files changed, 185 insertions(+), 83 deletions(-)
>
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 7792c393e238..f3803c2dc349 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
>    *
> @@ -1163,6 +1212,7 @@ struct snd_soc_pcm_runtime {
>   	     ((i) < (rtd)->dai_link->num_cpus + (rtd)->dai_link->num_codecs) &&	\
>   		     ((dai) = (rtd)->dais[i]);				\
>   	     (i)++)
> +#define for_each_rtd_ch_maps(rtd, i, ch_maps) for_each_link_ch_maps(rtd->dai_link, i, ch_maps)
>   
>   void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);
>   
> diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
> index 3312ad8a563b..2faf7372bad0 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 b2bd45e87bc3..4ca3319a8e19 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1015,6 +1015,94 @@ 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;
> +
> +	/*
> +	 * 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:
> +	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 +1209,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 4e2beda6f9bf..233ebc74c313 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -4438,11 +4438,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.
> @@ -4450,39 +4453,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_rtd_ch_maps(rtd, 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..7198f017c167 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_rtd_ch_maps(rtd, 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_rtd_ch_maps(rtd, 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))

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property
  2023-10-25  2:19 ` [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property Kuninori Morimoto
@ 2023-10-26 21:22   ` Rob Herring
  2023-10-26 23:35     ` Kuninori Morimoto
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2023-10-26 21:22 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Jerome Brunet, alsa-devel,
	devicetree

On Wed, Oct 25, 2023 at 02:19:13AM +0000, Kuninori Morimoto wrote:
> This patch adds channel-map-index property to enable handling

Don't write commit messages with 'This patch' or 'This commit'. See the 
documentation on writing patches.

> CPU:Codec = N:M connection.

This is not answering Why?

> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../devicetree/bindings/sound/audio-graph-port.yaml | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> index 60b5e3fd1115..2a7e0784d591 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> @@ -93,6 +93,19 @@ definitions:
>                minimum: 1
>                maximum: 64
>  
> +      channel-map-index:
> +        description: It indicates CPU/Codec DAIs channel mapping index if number of
> +          CPU(N) / Codec(M) were not same in one dai-link. channel-map-index is not
> +          needed if the numbers were 1:M or N:1 or N==M. Same indexed CPU <-> Codec
> +          will be paired. This is CPUx2 <-> Codecx3 sample.
> +                                   CPUA   { ... .channel-map-index = <0>;   }
> +          [0] CPUA <---> CodecA    CPUB   { ... .channel-map-index = <1 2>; }
> +          [1] CPUB <-+-> CodecB
> +          [2]        \-> CodecC    CodecA { ... .channel-map-index = <0>; }
> +                                   CodecB { ... .channel-map-index = <1>; }
> +                                   CodecC { ... .channel-map-index = <2>; }

We have 2 different meanings for channel-map-index here. We have the 
codecs defining "I am index N" and then the CPUs defining "I'm connected 
to codec N". That's confusing to start with. Made-up indices are 
something we try to avoid in DT. Are the numbers here (0, 1, 2) 
significant? The normal way we link from one node to another is 
phandles. Why not use phandles here:

CPUA   { ... .channel-map-index = <&CodecA>;   }
CPUB   { ... .channel-map-index = <&CodecB &CodecC>; }


However, we also have OF graph to define complex topologies/connections 
AND we're already using it for this binding. So why not here? You can 
always have more than 1 port and/or endpoint. Generally, multiple ports 
are independent/simultaneous data connections and multiple endpoints are 
either 1:N fanout or N:1 muxed connections.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property
  2023-10-26 21:22   ` Rob Herring
@ 2023-10-26 23:35     ` Kuninori Morimoto
  0 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-26 23:35 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pierre-Louis Bossart, Mark Brown, Bard Liao, bard.liao,
	Conor Dooley, Krzysztof Kozlowski, Jerome Brunet, alsa-devel,
	devicetree


Hi Rob

> We have 2 different meanings for channel-map-index here. We have the 
> codecs defining "I am index N" and then the CPUs defining "I'm connected 
> to codec N". That's confusing to start with. Made-up indices are 
> something we try to avoid in DT. Are the numbers here (0, 1, 2) 
> significant? The normal way we link from one node to another is 
> phandles. Why not use phandles here:
> 
> CPUA   { ... .channel-map-index = <&CodecA>;   }
> CPUB   { ... .channel-map-index = <&CodecB &CodecC>; }
> 
> 
> However, we also have OF graph to define complex topologies/connections 
> AND we're already using it for this binding. So why not here? You can 
> always have more than 1 port and/or endpoint. Generally, multiple ports 
> are independent/simultaneous data connections and multiple endpoints are 
> either 1:N fanout or N:1 muxed connections.

Hmm... I will re-consider about it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-10-26 23:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-25  2:18 [PATCH v6 0/5] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
2023-10-25  2:18 ` [PATCH v6 1/5] " Kuninori Morimoto
2023-10-25  5:33   ` Liao, Bard
2023-10-25  2:18 ` [PATCH v6 2/5] ASoC: audio-graph-card2: add CPU:Codec = N:M support Kuninori Morimoto
2023-10-25  2:18 ` [PATCH v6 3/5] ASoC: audio-graph-card2-custom-sample: tidyup comment / numbering Kuninori Morimoto
2023-10-25  2:18 ` [PATCH v6 4/5] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample Kuninori Morimoto
2023-10-25  2:19 ` [PATCH v6 5/5] dt-bindings: audio-graph-port: add channel-map-index property Kuninori Morimoto
2023-10-26 21:22   ` Rob Herring
2023-10-26 23:35     ` Kuninori Morimoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).