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

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;

+}