| Message ID | 87h6n6g69d.wl-kuninori.morimoto.gx@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: CPU/Codec connection cleanup - step1 | expand |
Hi Morimoto-san, > Current soc_get_playback_capture() are checking validation of CPU/Codec > like below > > static int soc_get_playback_capture(...) > { > ... > ^ if (dai_link->dynamic || dai_link->no_pcm) { > (X) ... > v } else { > ^ ... > | for_each_rtd_codec_dais(rtd, i, codec_dai) { > | ... > | if (snd_soc_dai_stream_valid(codec_dai, ...) && > | snd_soc_dai_stream_valid(cpu_dai, ...)) > (Y)(a) has_playback = 1; > | if (snd_soc_dai_stream_valid(codec_dai, ...) && > | snd_soc_dai_stream_valid(cpu_dai, ..)) > | (b) has_capture = 1; > | } > v } > ... > } > > (X) is for DPCM connection, (Y) is for Normal connection. > In Normal connection (Y), it is handling CPU/Codec, and it will set > has_playback/capture = 1 at (a)(b), but it means today is "at least one > of CPU/Codec pair was valid" in multi CPU/Codec case. > > This is wrong, it should be handled when "all CPU/Codec are valid". > This patch fixup it. I knew this code looked familiar and sure enough we've been there before in 2020. This code was introduced by 4f8721542f7b ASoC: core: use less strict tests for dailink capabilities the initial stricter tests caused a number of regressions reported by Jerome Brunet so we lowered the bar from "all dais" to "at least one dai" to be backwards-compatible. I don't think we can revisit this without hitting the same sort of issues. Regards, -Pierre
Hi Jerome, Pierre-Louis Thank you for your feedback > Here is an example: > 1 CPU - 1 dai link - 2 codecs: > * 1 codec handles the playback and just that > * the other does same capture (snip) > Going with 'all must be valid for the direction' makes this use case > impossible. Each codec would disable the direction of the other one. Ah..., OK, I see... > Do you have an actual problem because/error because of this ? CPU/Codec N:M support is added on ASoC, but the code is hackish, so I want makes it more generic. In the same time, this DAI validation check which is related to it is too much complex for now. I will re-consider around there. Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 8c168dc553f6..a45c0cf0fa14 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2787,9 +2787,10 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, if (dai_link->dpcm_playback) { stream = SNDRV_PCM_STREAM_PLAYBACK; + has_playback = (dai_link->num_cpus > 0); for_each_rtd_cpu_dais(rtd, i, cpu_dai) { - if (snd_soc_dai_stream_valid(cpu_dai, stream)) { - has_playback = 1; + if (!snd_soc_dai_stream_valid(cpu_dai, stream)) { + has_playback = 0; break; } } @@ -2803,9 +2804,10 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, if (dai_link->dpcm_capture) { stream = SNDRV_PCM_STREAM_CAPTURE; + has_capture = (dai_link->num_cpus > 0); for_each_rtd_cpu_dais(rtd, i, cpu_dai) { - if (snd_soc_dai_stream_valid(cpu_dai, stream)) { - has_capture = 1; + if (!snd_soc_dai_stream_valid(cpu_dai, stream)) { + has_capture = 0; break; } } @@ -2824,6 +2826,8 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, int cpu_capture = snd_soc_get_stream_cpu(dai_link, SNDRV_PCM_STREAM_CAPTURE); int cpu_playback = snd_soc_get_stream_cpu(dai_link, SNDRV_PCM_STREAM_PLAYBACK); + has_playback = (dai_link->num_codecs > 0); + has_capture = (dai_link->num_codecs > 0); for_each_rtd_codec_dais(rtd, i, codec_dai) { if (dai_link->num_cpus == 1) { cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); @@ -2848,12 +2852,12 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, return -EINVAL; } - if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) && - snd_soc_dai_stream_valid(cpu_dai, cpu_playback)) - has_playback = 1; - if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) && - snd_soc_dai_stream_valid(cpu_dai, cpu_capture)) - has_capture = 1; + if (!(snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) && + snd_soc_dai_stream_valid(cpu_dai, cpu_playback))) + has_playback = 0; + if (!(snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) && + snd_soc_dai_stream_valid(cpu_dai, cpu_capture))) + has_capture = 0; } }
Current soc_get_playback_capture() are checking validation of CPU/Codec like below static int soc_get_playback_capture(...) { ... ^ if (dai_link->dynamic || dai_link->no_pcm) { (X) ... v } else { ^ ... | for_each_rtd_codec_dais(rtd, i, codec_dai) { | ... | if (snd_soc_dai_stream_valid(codec_dai, ...) && | snd_soc_dai_stream_valid(cpu_dai, ...)) (Y)(a) has_playback = 1; | if (snd_soc_dai_stream_valid(codec_dai, ...) && | snd_soc_dai_stream_valid(cpu_dai, ..)) | (b) has_capture = 1; | } v } ... } (X) is for DPCM connection, (Y) is for Normal connection. In Normal connection (Y), it is handling CPU/Codec, and it will set has_playback/capture = 1 at (a)(b), but it means today is "at least one of CPU/Codec pair was valid" in multi CPU/Codec case. This is wrong, it should be handled when "all CPU/Codec are valid". This patch fixup it. Link: https://lore.kernel.org/r/87mt1ihhm3.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- sound/soc/soc-pcm.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)