On 6/7/23 04:29, Richard Fitzgerald wrote:
> On 07/06/2023 04:12, Bard Liao wrote:
>> 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 series 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.
>>
>> And add the codec_ch_maps to SOF SoundWire machine driver.
>>
>
> I think there is a much simpler way to handle this. The existing ASoC
> code is trying to map CPU to CODEC to match the physical connection. But
> the physical connection is not important. The dailink represents the
> _logical_ link.
Humm, that's not really true. Each SoundWire link and the CPU DAI they
expose are handled by different auxiliary devices with their own
pm_runtime handling.
> You are declaring that all the CPU and CODEC in the dailink behave as a
> single logical link. So you can just connect all CPUs to all CODECS.
>
> That also fixes a problem with the existing N CPU to N CODEC mapping. It
> assumes that means CPU0 is connected to CODEC0, CPU1 is connected to
> CODEC1, ... But that isn't necessarily true. You could have an N:N
> mapping where multiple CPUs have been combined to create a multi-channel
> stream that is sent to all CODECs.
This is questionable when the CPUs are connected to different links.
SoundWire is not a giant switch matrix, there's a clear parent-child
dependency and limited scope.
Example topology for a 2 link/4 amplifier solution.
link1 CPU DAI1 - Codec1 DAI1 - Codec2 DAI1
link2 CPU DAI1 - Codec1 DAI1 - Codec2 DAI1
There is no physical or logical connection between link2 CPU DAI1 and
the two Codec1 DAI1 and Codec2 DAI1.
I don't think it's wise to make DAPM connections between devices that
are not on the same link. Each link is clock- and powered-managed
separately, I only see trouble ahead with such virtual connections.
But the existing N:N code won't hook
> that up correctly.
>
> I suggest that the simple fix is this:
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 5d9a671e50f1..f4f955a991d5 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -4423,8 +4423,8 @@ 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 *codec_dai;
> - int i;
> + struct snd_soc_dai *codec_dai, *cpu_dai;
> + int i, j;
>
> /* for each BE DAI link... */
> for_each_card_rtds(card, rtd) {
> @@ -4435,17 +4435,9 @@ 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,
> - asoc_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,
> - asoc_rtd_to_cpu(rtd, i));
> - } else {
> - dev_err(card->dev,
> - "N cpus to M codecs link is not supported yet\n");
> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
> + for_each_rtd_cpu_dais(rtd, j, cpu_dai)
> + dapm_connect_dai_pair(card, rtd, codec_dai, cpu_dai);
> }
> }
> }
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 7958c9defd49..43b1166eb333 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -2747,7 +2747,7 @@ static int soc_get_playback_capture(struct
> snd_soc_pcm_runtime *rtd,
> int *playback, int *capture)
> {
> struct snd_soc_dai *cpu_dai;
> - int i;
> + int i, j;
>
> if (rtd->dai_link->dynamic && rtd->dai_link->num_cpus > 1) {
> dev_err(rtd->dev,
> @@ -2801,22 +2801,14 @@ static int soc_get_playback_capture(struct
> snd_soc_pcm_runtime *rtd,
> SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
>
> for_each_rtd_codec_dais(rtd, i, codec_dai) {
> - if (rtd->dai_link->num_cpus == 1) {
> - cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> - } else if (rtd->dai_link->num_cpus ==
> rtd->dai_link->num_codecs) {
> - cpu_dai = asoc_rtd_to_cpu(rtd, i);
> - } else {
> - dev_err(rtd->card->dev,
> - "N cpus to M codecs link is not supported yet\n");
> - return -EINVAL;
> + for_each_rtd_cpu_dais(rtd, j, cpu_dai) {
> + if (snd_soc_dai_stream_valid(codec_dai,
> SNDRV_PCM_STREAM_PLAYBACK) &&
> + snd_soc_dai_stream_valid(cpu_dai, cpu_playback))
> + *playback = 1;
> + if (snd_soc_dai_stream_valid(codec_dai,
> SNDRV_PCM_STREAM_CAPTURE) &&
> + snd_soc_dai_stream_valid(cpu_dai, cpu_capture))
> + *capture = 1;
> }
> -
> - if (snd_soc_dai_stream_valid(codec_dai,
> SNDRV_PCM_STREAM_PLAYBACK) &&
> - snd_soc_dai_stream_valid(cpu_dai, cpu_playback))
> - *playback = 1;
> - if (snd_soc_dai_stream_valid(codec_dai,
> SNDRV_PCM_STREAM_CAPTURE) &&
> - snd_soc_dai_stream_valid(cpu_dai, cpu_capture))
> - *capture = 1;
> }
> }
>
