Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic
@ 2023-10-10  1:20 Kuninori Morimoto
  2023-10-10  1:21 ` [PATCH 1/4] " Kuninori Morimoto
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-10  1:20 UTC (permalink / raw)
  To: Mark Brown, Bard Liao, Pierre-Louis Bossart, bard.liao; +Cc: Linux-ALSA


Hi Mark
Cc Bard, Pierre-Louis

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

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

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

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: ch_maps property

 .../bindings/sound/audio-graph-port.yaml      |   2 +
 include/sound/soc.h                           |  48 +++++-
 .../audio-graph-card2-custom-sample.dtsi      | 138 +++++++++++++++---
 sound/soc/generic/audio-graph-card2.c         |  29 ++++
 sound/soc/intel/boards/sof_sdw.c              |  14 +-
 sound/soc/soc-core.c                          |  83 +++++++++++
 sound/soc/soc-dapm.c                          |  47 +++---
 sound/soc/soc-pcm.c                           |  73 ++++-----
 8 files changed, 343 insertions(+), 91 deletions(-)

-- 
2.25.1


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

* [PATCH 1/4] ASoC: makes CPU/Codec channel connection map more generic
  2023-10-10  1:20 [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
@ 2023-10-10  1:21 ` Kuninori Morimoto
  2023-10-10 13:40   ` Pierre-Louis Bossart
  2023-10-10  1:21 ` [PATCH 2/4] 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-10  1:21 UTC (permalink / raw)
  To: Mark Brown, Bard Liao, Pierre-Louis Bossart, bard.liao; +Cc: Linux-ALSA

Current ASoC CPU:Codec = N:M connection is using connection mapping idea,
but it is used for CPU < Codec 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.
Because it will use default mapping, no conversion patch is needed
to exising CPU/Codec drivers.

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

Link: https://lore.kernel.org/r/87fs6wuszr.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h              | 48 ++++++++++++++++--
 sound/soc/intel/boards/sof_sdw.c | 14 +++---
 sound/soc/soc-core.c             | 83 ++++++++++++++++++++++++++++++++
 sound/soc/soc-dapm.c             | 47 +++++++-----------
 sound/soc/soc-pcm.c              | 73 +++++++++++++++-------------
 5 files changed, 191 insertions(+), 74 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 63b57f58cc569..13f1158df2b1e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -655,8 +655,50 @@ 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_maps is from CPU
+ *
+ *	.num_cpus   = 1;
+ *	.num_codecs = 1;
+ *	.connected_node = [0];
+ *
+ * CPU0 <---> Codec_x
+ * CPU1 <---> Codec_y
+ * CPU2 <---> Codec_z
+ *
+ *	.ch_maps is from CPU
+ *
+ *	.num_cpus   = 3;
+ *	.num_codecs = 3;
+ *	.connected_node = [x, y, z];
+ *
+ * CPU0 <---> Codec_x
+ * CPU1 <-+-> Codec_y
+ * CPU2 <-/
+ *
+ *	.ch_maps is from CPU
+ *
+ *	.num_cpus   = 3;
+ *	.num_codecs = 2;
+ *	.connected_node = [x, y, y];
+ *
+ *
+ * CPU_x <---> Codec0
+ * CPU_y <-+-> Codec1
+ *	   \-> Codec2
+ *
+ *	.ch_maps is from Codec
+ *
+ *	.num_cpus   = 2;
+ *	.num_codecs = 3;
+ *	.connected_node = [x, y, y];
+ */
+struct snd_soc_dai_link_ch_map {
+	unsigned int connected_node;
 	unsigned int ch_mask;
 };
 
@@ -688,7 +730,7 @@ 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;
+	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
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 226a74a4c340f..7927b729866d9 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -579,7 +579,7 @@ int sdw_hw_params(struct snd_pcm_substream *substream,
 	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 */
@@ -607,9 +607,9 @@ int sdw_hw_params(struct snd_pcm_substream *substream,
 	 */
 	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)
+			if (rtd->dai_link->ch_maps[j].connected_node != i)
 				continue;
-			rtd->dai_link->codec_ch_maps[j].ch_mask = ch_mask << (j * step);
+			rtd->dai_link->ch_maps[j].ch_mask = ch_mask << (j * step);
 		}
 	}
 	return 0;
@@ -1350,7 +1350,7 @@ 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;
@@ -1358,7 +1358,7 @@ static void set_dailink_map(struct snd_soc_dai_link_codec_ch_map *sdw_codec_ch_m
 
 	step = codec_num / cpu_num;
 	for (i = 0; i < codec_num; i++)
-		sdw_codec_ch_maps[i].connected_cpu_id = i / step;
+		sdw_codec_ch_maps[i].connected_node = i / step;
 }
 
 static const char * const type_strings[] = {"SimpleJack", "SmartAmp", "SmartMic"};
@@ -1453,7 +1453,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 +1530,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 c305e94762c39..a4bb4c29331cf 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1824,6 +1824,84 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const char *flavour)
 EXPORT_SYMBOL_GPL(snd_soc_set_dmi_name);
 #endif /* CONFIG_DMI */
 
+#define MAX_DEFAULT_CONNECTION_MAP_SIZE 7
+static struct snd_soc_dai_link_ch_map default_connction_map1[MAX_DEFAULT_CONNECTION_MAP_SIZE] = {
+	{ .connected_node = 0 },
+	{ .connected_node = 1 },
+	{ .connected_node = 2 },
+	{ .connected_node = 3 },
+	{ .connected_node = 4 },
+	{ .connected_node = 5 },
+	{ .connected_node = 6 },
+};
+static struct snd_soc_dai_link_ch_map default_connction_map2[MAX_DEFAULT_CONNECTION_MAP_SIZE] = {
+	{ .connected_node = 0 },
+	{ .connected_node = 0 },
+	{ .connected_node = 0 },
+	{ .connected_node = 0 },
+	{ .connected_node = 0 },
+	{ .connected_node = 0 },
+	{ .connected_node = 0 },
+};
+
+static int snd_soc_compensate_connection_map(struct snd_soc_card *card)
+{
+	struct snd_soc_dai_link *dai_link;
+	int i, j, n, 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]
+	 */
+	for_each_card_prelinks(card, i, dai_link) {
+
+		/* 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_CONNECTION_MAP_SIZE ||
+		    dai_link->num_codecs > MAX_DEFAULT_CONNECTION_MAP_SIZE) {
+			dev_err(card->dev, "soc-core.c needs update default_connction_maps");
+			return -EINVAL;
+		}
+
+		/* Compensate missing map for ... */
+		if (dai_link->num_cpus == dai_link->num_codecs)
+			dai_link->ch_maps = default_connction_map1; /* for 1:1 or N:N */
+		else
+			dai_link->ch_maps = default_connction_map2; /* for 1:N or N:1 */
+
+sanity_check:
+		if (dai_link->num_cpus >= dai_link->num_codecs) {
+			n   = dai_link->num_cpus;
+			max = dai_link->num_codecs;
+		} else {
+			n   = dai_link->num_codecs;
+			max = dai_link->num_cpus;
+		}
+
+		for (j = 0; j < n; j++)
+			if (dai_link->ch_maps[j].connected_node >= max) {
+				dev_err(card->dev, "strange connected_node (%d) was added to ch_maps",
+					dai_link->ch_maps[j].connected_node);
+				return -EINVAL;
+			}
+	}
+
+	return 0;
+}
+
 static void soc_check_tplg_fes(struct snd_soc_card *card)
 {
 	struct snd_soc_component *component;
@@ -2030,6 +2108,11 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 
 	snd_soc_dapm_init(&card->dapm, card, NULL);
 
+	/* for keeping compatibility */
+	ret = snd_soc_compensate_connection_map(card);
+	if (ret < 0)
+		goto probe_end;
+
 	/* check whether any platform is ignore machine FE and using topology */
 	soc_check_tplg_fes(card);
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 2512aadf95f71..3c7c2b16bd64a 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -4426,6 +4426,7 @@ 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;
 
@@ -4438,39 +4439,25 @@ 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]
+		 */
+		/* .ch_map is from CPU */
+		if (rtd->dai_link->num_cpus >= rtd->dai_link->num_codecs) {
+			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
+				codec_dai = snd_soc_rtd_to_codec(rtd, rtd->dai_link->ch_maps[i].connected_node);
 
+				dapm_connect_dai_pair(card, rtd, codec_dai, cpu_dai);
+			}
+		}
+		/* .ch_map is from Codec */
+		else {
 			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));
+				cpu_dai = snd_soc_rtd_to_cpu(rtd, rtd->dai_link->ch_maps[i].connected_node);
+
+				dapm_connect_dai_pair(card, rtd, codec_dai, cpu_dai);
 			}
-		} 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);
 		}
 	}
 }
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 8c168dc553f65..0bfff2ea111d7 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1043,7 +1043,6 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
 
 	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
 		unsigned int ch_mask = 0;
-		int j;
 
 		/*
 		 * Skip CPUs which don't support the current stream
@@ -1055,22 +1054,28 @@ 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;
+		if (rtd->dai_link->num_cpus >= rtd->dai_link->num_codecs) {
+			/* .ch_map is from CPU */
+			ch_mask = rtd->dai_link->ch_maps[i].ch_mask;
+		} else {
+			int j;
+
+			/* .ch_map is from Codec */
+			for_each_rtd_codec_dais(rtd, j, codec_dai)
+				if (rtd->dai_link->ch_maps[j].connected_node == i)
+					ch_mask |= rtd->dai_link->ch_maps[j].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;
@@ -2824,36 +2829,36 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
 		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;
-				}
+		/*
+		 * see
+		 *	soc.h :: [dai_link->ch_maps Image sample]
+		 */
+		/* .ch_map is from CPU */
+		if (dai_link->num_cpus >= dai_link->num_codecs) {
+			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
+				codec_dai = snd_soc_rtd_to_codec(rtd, dai_link->ch_maps[i].connected_node);
 
-				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;
+				if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
+				    snd_soc_dai_stream_valid(cpu_dai,   cpu_playback))
+					has_playback = 1;
+				if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) &&
+				    snd_soc_dai_stream_valid(cpu_dai,   cpu_capture))
+					has_capture = 1;
 			}
+		}
+		/* .ch_map is from Codec */
+		else {
+			for_each_rtd_codec_dais(rtd, i, codec_dai) {
+				cpu_dai = snd_soc_rtd_to_cpu(rtd, dai_link->ch_maps[i].connected_node);
+
+				if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
+				    snd_soc_dai_stream_valid(cpu_dai,   cpu_playback))
+					has_playback = 1;
+				if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) &&
+				    snd_soc_dai_stream_valid(cpu_dai,   cpu_capture))
+					has_capture = 1;
 
-			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
-			    snd_soc_dai_stream_valid(cpu_dai,   cpu_playback))
-				has_playback = 1;
-			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) &&
-			    snd_soc_dai_stream_valid(cpu_dai,   cpu_capture))
-				has_capture = 1;
+			}
 		}
 	}
 
-- 
2.25.1


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

* [PATCH 2/4] ASoC: audio-graph-card2: add CPU:Codec = N:M support
  2023-10-10  1:20 [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
  2023-10-10  1:21 ` [PATCH 1/4] " Kuninori Morimoto
@ 2023-10-10  1:21 ` Kuninori Morimoto
  2023-10-10  1:21 ` [PATCH 3/4] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample Kuninori Morimoto
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-10  1:21 UTC (permalink / raw)
  To: Mark Brown, Bard Liao, Pierre-Louis Bossart, bard.liao; +Cc: Linux-ALSA

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 | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index 5d856942bcaee..009fef88d443a 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -515,7 +515,10 @@ static int graph_parse_node(struct simple_util_priv *priv,
 	int ret = 0;
 
 	if (graph_lnk_is_multi(port)) {
+		struct device_node *ports = of_get_parent(port);
 		int idx;
+		int num;
+		char *props = "ch_maps";
 
 		of_node_get(port);
 
@@ -530,6 +533,32 @@ static int graph_parse_node(struct simple_util_priv *priv,
 			if (ret < 0)
 				break;
 		}
+
+		/* read nm_ch_maps if exist */
+		num = of_property_count_elems_of_size(ports, props, sizeof(u32));
+		if (num > 0) {
+			struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
+			struct device *dev = simple_priv_to_dev(priv);
+			struct snd_soc_dai_link_ch_map *ch_maps = devm_kcalloc(dev, num, sizeof(*ch_maps), GFP_KERNEL);
+			int *temp = devm_kcalloc(dev, num, sizeof(int), GFP_KERNEL);
+			int i;
+
+			if (!ch_maps || !temp) {
+				ret = -ENOMEM;
+				goto multi_end;
+			}
+
+			ret = of_property_read_u32_array(ports, props, temp, num);
+			if (ret < 0)
+				goto multi_end;
+
+			dai_link->ch_maps = ch_maps;
+			for (i = 0; i < num; i++)
+				dai_link->ch_maps[i].connected_node = temp[i];
+multi_end:
+			devm_kfree(dev, temp);
+		}
+		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 3/4] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample
  2023-10-10  1:20 [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
  2023-10-10  1:21 ` [PATCH 1/4] " Kuninori Morimoto
  2023-10-10  1:21 ` [PATCH 2/4] ASoC: audio-graph-card2: add CPU:Codec = N:M support Kuninori Morimoto
@ 2023-10-10  1:21 ` Kuninori Morimoto
  2023-10-10  1:21 ` [PATCH 4/4] dt-bindings: audio-graph-port: ch_maps property Kuninori Morimoto
  2023-10-10 13:43 ` [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Pierre-Louis Bossart
  4 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-10  1:21 UTC (permalink / raw)
  To: Mark Brown, Bard Liao, Pierre-Louis Bossart, bard.liao; +Cc: Linux-ALSA

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      | 138 +++++++++++++++---
 1 file changed, 121 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 8acaa2ddb335f..d43a4d2cdaa5f 100644
--- a/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
+++ b/sound/soc/generic/audio-graph-card2-custom-sample.dtsi
@@ -58,12 +58,32 @@ / {
 	 *			| |-> codec13
 	 *			+-+
 	 *
-	 * [Multi-CPU/Codec]
+	 * [Multi-CPU/Codec-0]
 	 *		+-+		+-+
 	 *	cpu1 <--| |<-@--------->| |-> codec1
 	 *	cpu2 <--| |		| |-> codec2
 	 *		+-+		+-+
 	 *
+	 * [Multi-CPU/Codec-1]
+	 * for ch_map (*), see
+	 *	soc-core.c :: [dai_link->ch_maps Image sample]
+	 *
+	 *		+-+		+-+
+	 *	cpu8 <--| |<-@--------->| |-> codec14		cpu8 <---> codec14
+	 *	cpu9 <--| |		| |-> codec15		cpu9 <-+-> codec15
+	 *		+-+		| |-> codec16		       \-> codec16
+	 *				+-+		    (*) ch_map = [0, 1, 1]
+	 *
+	 * [Multi-CPU/Codec-2]
+	 * for ch_map (*), see
+	 *	soc-core.c :: [dai_link->ch_maps Image sample]
+	 *
+	 *		+-+		+-+
+	 *	cpu10 <-| |<-@--------->| |-> codec17		cpu11 <---> codec17
+	 *	cpu11 <-| |		| |-> codec18		cpu10 <-+-> codec18
+	 *	cpu12 <-| |		+-+			cpu12 <-/
+	 *		+-+				    (*) ch_map = [1, 0, 1]
+	 *
 	 * [DPCM]
 	 *
 	 *	CPU3/CPU4 are converting rate to 44100
@@ -144,15 +164,38 @@ 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
 
+			/*
+			 * [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
@@ -182,24 +225,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 */
@@ -241,6 +284,50 @@ 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>;
+				ch_maps = <0 1 1>; /* see [Multi-CPU/Codec-1] */
+				#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>;
+				ch_maps = <1 0 1>; /* see [Multi-CPU/Codec-2] */
+				#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 {
@@ -323,9 +410,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>; }; };
@@ -337,6 +424,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>; }; };
+			      port@9 { reg = <9>; cpu9_ep: endpoint { remote-endpoint = <&mcpu12_ep>; }; };
+
+			/* [Multi-CPU-2] */
+			      port@a { reg = <10>; cpu10_ep: endpoint { remote-endpoint = <&mcpu21_ep>; }; };
+			      port@b { reg = <11>; cpu11_ep: endpoint { remote-endpoint = <&mcpu22_ep>; }; };
+			      port@c { reg = <12>; cpu12_ep: endpoint { remote-endpoint = <&mcpu23_ep>; }; };
 		};
 	};
 
@@ -363,9 +459,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  {
@@ -395,6 +491,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>; }; };
+			port@f  { reg = <15>; codec15_ep:  endpoint { remote-endpoint = <&mcodec12_ep>; }; };
+			port@10 { reg = <16>; codec16_ep:  endpoint { remote-endpoint = <&mcodec13_ep>; }; };
+
+			/* [Multi-Codec-2] */
+			port@11 { reg = <17>; codec17_ep:  endpoint { remote-endpoint = <&mcodec21_ep>; }; };
+			port@12 { reg = <18>; codec18_ep:  endpoint { remote-endpoint = <&mcodec22_ep>; }; };
 		};
 	};
 };
-- 
2.25.1


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

* [PATCH 4/4] dt-bindings: audio-graph-port: ch_maps property
  2023-10-10  1:20 [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2023-10-10  1:21 ` [PATCH 3/4] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample Kuninori Morimoto
@ 2023-10-10  1:21 ` Kuninori Morimoto
  2023-10-11 14:20   ` Krzysztof Kozlowski
  2023-10-10 13:43 ` [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Pierre-Louis Bossart
  4 siblings, 1 reply; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-10  1:21 UTC (permalink / raw)
  To: Mark Brown, Bard Liao, Pierre-Louis Bossart, bard.liao; +Cc: Linux-ALSA

This patch adds ch_maps property to enable handling CPU:Codec = N:M
connection.

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

diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
index 60b5e3fd1115f..dc663af784fd1 100644
--- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
+++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
@@ -19,6 +19,8 @@ definitions:
     properties:
       mclk-fs:
         $ref: simple-card.yaml#/definitions/mclk-fs
+      ch_maps:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
 
   endpoint-base:
     allOf:
-- 
2.25.1


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

* Re: [PATCH 1/4] ASoC: makes CPU/Codec channel connection map more generic
  2023-10-10  1:21 ` [PATCH 1/4] " Kuninori Morimoto
@ 2023-10-10 13:40   ` Pierre-Louis Bossart
  2023-10-10 23:26     ` Kuninori Morimoto
  0 siblings, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2023-10-10 13:40 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown, Bard Liao, bard.liao; +Cc: Linux-ALSA



On 10/9/23 21:21, Kuninori Morimoto wrote:
> Current ASoC CPU:Codec = N:M connection is using connection mapping idea,
> but it is used for CPU < Codec 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.
> Because it will use default mapping, no conversion patch is needed
> to exising CPU/Codec drivers.
> 
> More over, CPU:Codec = M:N (M > N) also supported in the same time.
> 
> Link: https://lore.kernel.org/r/87fs6wuszr.wl-kuninori.morimoto.gx@renesas.com
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  include/sound/soc.h              | 48 ++++++++++++++++--
>  sound/soc/intel/boards/sof_sdw.c | 14 +++---
>  sound/soc/soc-core.c             | 83 ++++++++++++++++++++++++++++++++
>  sound/soc/soc-dapm.c             | 47 +++++++-----------
>  sound/soc/soc-pcm.c              | 73 +++++++++++++++-------------
>  5 files changed, 191 insertions(+), 74 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 63b57f58cc569..13f1158df2b1e 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -655,8 +655,50 @@ 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_maps is from CPU
> + *
> + *	.num_cpus   = 1;
> + *	.num_codecs = 1;
> + *	.connected_node = [0];
> + *
> + * CPU0 <---> Codec_x
> + * CPU1 <---> Codec_y
> + * CPU2 <---> Codec_z
> + *
> + *	.ch_maps is from CPU
> + *
> + *	.num_cpus   = 3;
> + *	.num_codecs = 3;
> + *	.connected_node = [x, y, z];
> + *
> + * CPU0 <---> Codec_x
> + * CPU1 <-+-> Codec_y
> + * CPU2 <-/
> + *
> + *	.ch_maps is from CPU
> + *
> + *	.num_cpus   = 3;
> + *	.num_codecs = 2;
> + *	.connected_node = [x, y, y];
> + *
> + *
> + * CPU_x <---> Codec0
> + * CPU_y <-+-> Codec1
> + *	   \-> Codec2
> + *
> + *	.ch_maps is from Codec

how would we know what the convention is? Is this based on the largest
number of dais, so here num_codecs > num_cpus so we use a codec-centric
convention? That would be worth explaining in clear text

> + *
> + *	.num_cpus   = 2;
> + *	.num_codecs = 3;
> + *	.connected_node = [x, y, y];
> + */
> +struct snd_soc_dai_link_ch_map {
> +	unsigned int connected_node;

connected_node is a scalar here and an array above. maybe split this
patch between a rename and a functionality change?

>  	unsigned int ch_mask;
>  };
>  
> @@ -688,7 +730,7 @@ 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;
> +	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
> diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
> index 226a74a4c340f..7927b729866d9 100644
> --- a/sound/soc/intel/boards/sof_sdw.c
> +++ b/sound/soc/intel/boards/sof_sdw.c
> @@ -579,7 +579,7 @@ int sdw_hw_params(struct snd_pcm_substream *substream,
>  	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 */
> @@ -607,9 +607,9 @@ int sdw_hw_params(struct snd_pcm_substream *substream,
>  	 */
>  	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)
> +			if (rtd->dai_link->ch_maps[j].connected_node != i)


>  				continue;
> -			rtd->dai_link->codec_ch_maps[j].ch_mask = ch_mask << (j * step);
> +			rtd->dai_link->ch_maps[j].ch_mask = ch_mask << (j * step);
>  		}
>  	}
>  	return 0;
> @@ -1350,7 +1350,7 @@ 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;
> @@ -1358,7 +1358,7 @@ static void set_dailink_map(struct snd_soc_dai_link_codec_ch_map *sdw_codec_ch_m
>  
>  	step = codec_num / cpu_num;
>  	for (i = 0; i < codec_num; i++)
> -		sdw_codec_ch_maps[i].connected_cpu_id = i / step;
> +		sdw_codec_ch_maps[i].connected_node = i / step;
>  }
>  
>  static const char * const type_strings[] = {"SimpleJack", "SmartAmp", "SmartMic"};
> @@ -1453,7 +1453,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 +1530,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 c305e94762c39..a4bb4c29331cf 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1824,6 +1824,84 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const char *flavour)
>  EXPORT_SYMBOL_GPL(snd_soc_set_dmi_name);
>  #endif /* CONFIG_DMI */
>  
> +#define MAX_DEFAULT_CONNECTION_MAP_SIZE 7

why 7?

> +static struct snd_soc_dai_link_ch_map default_connction_map1[MAX_DEFAULT_CONNECTION_MAP_SIZE] = {

typo: connection

this is repeated multiple times in comments below

> +	{ .connected_node = 0 },
> +	{ .connected_node = 1 },
> +	{ .connected_node = 2 },
> +	{ .connected_node = 3 },
> +	{ .connected_node = 4 },
> +	{ .connected_node = 5 },
> +	{ .connected_node = 6 },
> +};
> +static struct snd_soc_dai_link_ch_map default_connction_map2[MAX_DEFAULT_CONNECTION_MAP_SIZE] = {
> +	{ .connected_node = 0 },
> +	{ .connected_node = 0 },
> +	{ .connected_node = 0 },
> +	{ .connected_node = 0 },
> +	{ .connected_node = 0 },
> +	{ .connected_node = 0 },
> +	{ .connected_node = 0 },
> +};
> +
> +static int snd_soc_compensate_connection_map(struct snd_soc_card *card)
> +{
> +	struct snd_soc_dai_link *dai_link;
> +	int i, j, n, 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]
> +	 */
> +	for_each_card_prelinks(card, i, dai_link) {
> +
> +		/* 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_CONNECTION_MAP_SIZE ||
> +		    dai_link->num_codecs > MAX_DEFAULT_CONNECTION_MAP_SIZE) {
> +			dev_err(card->dev, "soc-core.c needs update default_connction_maps");
> +			return -EINVAL;
> +		}
> +
> +		/* Compensate missing map for ... */
> +		if (dai_link->num_cpus == dai_link->num_codecs)
> +			dai_link->ch_maps = default_connction_map1; /* for 1:1 or N:N */
> +		else
> +			dai_link->ch_maps = default_connction_map2; /* for 1:N or N:1 */
> +
> +sanity_check:
> +		if (dai_link->num_cpus >= dai_link->num_codecs) {
> +			n   = dai_link->num_cpus;
> +			max = dai_link->num_codecs;
> +		} else {
> +			n   = dai_link->num_codecs;
> +			max = dai_link->num_cpus;
> +		}
> +
> +		for (j = 0; j < n; j++)
> +			if (dai_link->ch_maps[j].connected_node >= max) {
> +				dev_err(card->dev, "strange connected_node (%d) was added to ch_maps",

maybe elaborate on what "strange" might mean so that average users can
figure this out?

> +					dai_link->ch_maps[j].connected_node);
> +				return -EINVAL;
> +			}
> +	}
> +
> +	return 0;
> +}
> +


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

* Re: [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic
  2023-10-10  1:20 [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2023-10-10  1:21 ` [PATCH 4/4] dt-bindings: audio-graph-port: ch_maps property Kuninori Morimoto
@ 2023-10-10 13:43 ` Pierre-Louis Bossart
  4 siblings, 0 replies; 9+ messages in thread
From: Pierre-Louis Bossart @ 2023-10-10 13:43 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown, Bard Liao, bard.liao; +Cc: Linux-ALSA



On 10/9/23 21:20, Kuninori Morimoto wrote:
> 
> Hi Mark
> Cc Bard, Pierre-Louis
> 
> Current ASoC is supporting CPU/Codec = N:M (N < M) connection by using
> ch_map idea. This patch expand that all connection uses this idea,
> and no limit of N < M [1].
> 
> This patch is tested on Audio-Graph-Card2 with sample dtsi,
> but needs Tested-by, at least from Intel.

We can test the next version (comments in separate mail) but we don't
have a configuration with more cpu dais than codec dais I am afraid, so
the best we can contribute is a non-regression for the N < M case.

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

* Re: [PATCH 1/4] ASoC: makes CPU/Codec channel connection map more generic
  2023-10-10 13:40   ` Pierre-Louis Bossart
@ 2023-10-10 23:26     ` Kuninori Morimoto
  0 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2023-10-10 23:26 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Mark Brown, Bard Liao, bard.liao, Linux-ALSA


Hi Pierre-Louis

Thank you for your review !

> We can test the next version (comments in separate mail) but we don't
> have a configuration with more cpu dais than codec dais I am afraid, so
> the best we can contribute is a non-regression for the N < M case.

Yes, it is enough. Thanks a lot !

> > + * CPU_x <---> Codec0
> > + * CPU_y <-+-> Codec1
> > + *	   \-> Codec2
> > + *
> > + *	.ch_maps is from Codec
> 
> how would we know what the convention is? Is this based on the largest
> number of dais, so here num_codecs > num_cpus so we use a codec-centric
> convention? That would be worth explaining in clear text

These samles is good enough I thought,
but yes, adding clear text is better. Will do in v2.

> > + *	.num_cpus   = 2;
> > + *	.num_codecs = 3;
> > + *	.connected_node = [x, y, y];
> > + */
> > +struct snd_soc_dai_link_ch_map {
> > +	unsigned int connected_node;
> 
> connected_node is a scalar here and an array above. maybe split this
> patch between a rename and a functionality change?

The sample image is simplified to avoid complex image.
But let's use more actual image if there is no misunderstanding.

	CPU0 <---> Codec_x
	CPU1 <-+-> Codec_y
	CPU2 <-/

	.ch_maps is from CPU

	.num_cpus   = 3;
	.num_codecs = 2;
=>	.ch_map[] = {{connected_node = x,},
	             {connected_node = y,},
	             {connected_node = y,}};


> > +#define MAX_DEFAULT_CONNECTION_MAP_SIZE 7
> 
> why 7?

No big reason, but I thought 7 is big enough as default.
We need to increase it if 7 was not enough for default.

	/* check default map size */
	if (dai_link->num_cpus   > MAX_DEFAULT_CONNECTION_MAP_SIZE ||
	    dai_link->num_codecs > MAX_DEFAULT_CONNECTION_MAP_SIZE) {
		dev_err(card->dev, "soc-core.c needs update default_connction_maps");
		return -EINVAL;
	}

> > +static struct snd_soc_dai_link_ch_map default_connction_map1[MAX_DEFAULT_CONNECTION_MAP_SIZE] = {
> 
> typo: connection

Oops, thank you for pointing it.

> > +		for (j = 0; j < n; j++)
> > +			if (dai_link->ch_maps[j].connected_node >= max) {
> > +				dev_err(card->dev, "strange connected_node (%d) was added to ch_maps",
> 
> maybe elaborate on what "strange" might mean so that average users can
> figure this out?

Thanks. Will fix in v2

Thank you for your help !!

Best regards
---
Kuninori Morimoto

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

* Re: [PATCH 4/4] dt-bindings: audio-graph-port: ch_maps property
  2023-10-10  1:21 ` [PATCH 4/4] dt-bindings: audio-graph-port: ch_maps property Kuninori Morimoto
@ 2023-10-11 14:20   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-11 14:20 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown, Bard Liao, Pierre-Louis Bossart,
	bard.liao
  Cc: Linux-ALSA

On 10/10/2023 03:21, 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>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

You got this feedback multiple times. Multiple. NAK.

> ---
>  Documentation/devicetree/bindings/sound/audio-graph-port.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> index 60b5e3fd1115f..dc663af784fd1 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> @@ -19,6 +19,8 @@ definitions:
>      properties:
>        mclk-fs:
>          $ref: simple-card.yaml#/definitions/mclk-fs
> +      ch_maps:

No underscores in node names.

Best regards,
Krzysztof


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

end of thread, other threads:[~2023-10-11 15:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-10  1:20 [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Kuninori Morimoto
2023-10-10  1:21 ` [PATCH 1/4] " Kuninori Morimoto
2023-10-10 13:40   ` Pierre-Louis Bossart
2023-10-10 23:26     ` Kuninori Morimoto
2023-10-10  1:21 ` [PATCH 2/4] ASoC: audio-graph-card2: add CPU:Codec = N:M support Kuninori Morimoto
2023-10-10  1:21 ` [PATCH 3/4] ASoC: audio-graph-card2-custom-sample: add CPU/Codec = N:M sample Kuninori Morimoto
2023-10-10  1:21 ` [PATCH 4/4] dt-bindings: audio-graph-port: ch_maps property Kuninori Morimoto
2023-10-11 14:20   ` Krzysztof Kozlowski
2023-10-10 13:43 ` [PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic Pierre-Louis Bossart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox