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@xxxxxxxxxxx > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> > --- > 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; > +} > +
