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.
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[].
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.
Thank you for your help !!
Best regards
---
Kuninori Morimoto
