[PATCH 0/4] ASoC: makes CPU/Codec channel connection map more generic
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(-)
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; + } } }
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);
} } return 0;rtd->dai_link->ch_maps[j].ch_mask = ch_mask << (j * step);
@@ -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;
char *name, *cpu_name; int playback, capture; static const char * const sdw_stream_name[] = {struct snd_soc_dai_link_ch_map *sdw_codec_ch_maps;
@@ -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;
ret = set_codec_init_func(card, adr_link, dai_links + (*link_index)++, playback, group_id, adr_index, dai_index); if (ret < 0) {dai_links[*link_index].ch_maps = sdw_codec_ch_maps;
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;
+}
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
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);
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>; }; }; }; }; };
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:
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
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.
participants (3)
-
Krzysztof Kozlowski
-
Kuninori Morimoto
-
Pierre-Louis Bossart