> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: Tuesday, June 13, 2023 8:05 AM
> To: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
> Cc: broonie@xxxxxxxxxx; tiwai@xxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; pierre-
> louis.bossart@xxxxxxxxxxxxxxx; Liao, Bard <bard.liao@xxxxxxxxx>
> Subject: Re: [PATCH 1/2] ASoC: add N cpus to M codecs dai link support
>
>
> Hi Bard
>
> > Currently, ASoC supports dailinks with the following mappings:
> > 1 cpu DAI to N codec DAIs
> > N cpu DAIs to N codec DAIs
> > But the mapping between N cpu DAIs and M codec DAIs is not supported.
> > The reason is that we didn't have a mechanism to map cpu and codec DAIs
> >
> > This patch suggests a new snd_soc_dai_link_codec_ch_map struct in
> > struct snd_soc_dai_link{} which provides codec DAI to cpu DAI mapping
> > information used to implement N cpu DAIs to M codec DAIs
> > support.
> >
> > When a dailink contains two or more cpu DAIs, we should set channel
> > number of cpus based on its channel mask. The new struct also provides
> > channel mask information for each codec and we can construct the cpu
> > channel mask by combining all codec channel masks which map to the cpu.
> >
> > The N:M mapping is however restricted to the N <= M case due to physical
> > restrictions on a time-multiplexed bus such as I2S/TDM, AC97, SoundWire
> > and HDaudio.
>
> I like CPU:Codec = N:M support !
> OTOH, I have interesting to ASoC code cleanup too.
> So this is meddlesome from me.
>
> > +struct snd_soc_dai_link_codec_ch_map {
> > + unsigned int connected_cpu_id;
> > + unsigned int ch_mask;
> > +};
>
> If my understanding was correct, this map is used only for N:M mapping,
> but we want to use it for all cases.
Thanks Morimoto, I hope so, too.
> In such case, the code can be more flexible and more clean.
> see below.
>
> > @@ -1041,13 +1045,32 @@ static int __soc_pcm_hw_params(struct
> snd_soc_pcm_runtime *rtd,
> > if (!snd_soc_dai_stream_valid(cpu_dai, substream->stream))
> > continue;
> >
> > - ret = snd_soc_dai_hw_params(cpu_dai, substream, params);
> > + /* copy params for each cpu */
> > + cpu_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.
> > + */
> > + 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;
> > + }
>
> Can we re-use snd_soc_dai_tdm_mask_get() for this purpose instead
> of .ch_mask ?
> May be we want to rename it and/or want to have new xxx_mask on dai-
> >stream[].
The reason that we didn't use tdm_mask is because the N:M cases is not
a notion of tdm. But, it should work for the N:M cases if we rename it and
add a new map as you said.
> I'm asking because it is natural to reuse existing data and/or have variable on
> similar place instead of on new structure.
>
>
> Maybe I'm not 100% well understanding about CPU:Codec = N:M connection,
> but if we can assume like below, we can use it on all cases not only for N:M
> case.
>
> We can have connection map on dai_link which is for from M side (here N <=
> M).
> The image is like this.
>
> CPU0 <---> Codec2
> CPU1 <-+-> Codec0
> \-> Codec1
>
> .num_cpus = 2;
> .num_codecs = 3;
> .map = [1, 1, 0]; // From Codec point of view.
> // sizeof(map) = num_codecs, because 3 > 2;
>
> In this rule, we can also handle CPU > Codec case, too.
>
> CPU0 <---> Codec1
> CPU1 <-+-> Codec0
> CPU2 <-/
>
> .num_cpus = 3;
> .num_codecs = 2;
> .map = [1, 0, 0]; // From CPU point of view.
>
> We can use this idea for existing connection, too.
>
> 1:1 case
> CPU0 <-> Codec0
>
> N:N case
> CPU0 <---> Codec0
> CPU1 <---> Codec1
>
> 1:N case
> CPU0 <-+-> Codec0
> \-> Codec1
>
> default_map1 = [0, 1, 2, 3,...];
> default_map2 = [0, 0, 0, 0,...];
>
> if (!dai_link->map) {
> if (dai_link->num_cpus == dai_link->num_codecs)
> dai_link->map = default_map1; /* for 1:1, N:N */
> else
> dai_link->map = default_map2; /* for 1:N */
> }
>
> We can handle more flexible N:N connection as Richard said
>
> fixup N:N case
> CPU0 <---> Codec2
> CPU1 <---> Codec1
> CPU2 <---> Codec0
>
> .num_cpus = 3;
> .num_codecs = 3;
> .map = [2, 1, 0]; // From CPU point of view.
>
> with is new .map, we can handle existing 1:1, N:N, 1:N, and new N:M (and
> M:N)
> connection with same code.
> This is just meddlesome from me, but I hope you can think about this.
Thanks for the suggestion. I will think about it.
>
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto
