On 3/31/24 19:30, Kuninori Morimoto wrote:
Current soc_get_playback_capture() (A) is checking playback/capture availability for DPCM (X) / Normal (Y) / Codec2Codec (Z) connections.
(A) static int soc_get_playback_capture(...) { ... ^ if (dai_link->dynamic || dai_link->no_pcm) { | ... |(a) if (dai_link->dpcm_playback) { | ... | ^ for_each_rtd_cpu_dais(rtd, i, cpu_dai) { |(*) ... | v } | ... (X) } |(b) if (dai_link->dpcm_capture) { | ... | ^ for_each_rtd_cpu_dais(rtd, i, cpu_dai) { |(*) ... | v } | ... v } } else { ^ ^ /* Adapt stream for codec2codec links */ |(Z) int cpu_capture = ... | v int cpu_playback = ... (Y) | ^ for_each_rtd_ch_maps(rtd, i, ch_maps) { |(*) ... v v } } ... }
(*) part is checking each DAI's availability.
At first, (X) part is for DPCM, and it checks playback/capture availability if dai_link has dpcm_playback/capture flag (a)(b). But we are already using playback/capture_only flag for Normal (Y) and Codec2Codec (Z). We can use this flags for DPCM too.
Before After dpcm_playback = 1; => /* no flags */ dpcm_capture = 1;
dpcm_playback = 1; => playback_only = 1;
dpcm_capture = 1; => capture_only = 1;
dpcm_playback = 0; => error dpcm_capture = 0;
This patch convert dpcm_ flags to _only flag, and dpcm_ flag will be removed if all driver switched to _only flags.
Here, CPU <-> Codec relationship is like this
DPCM [CPU/dummy]-[dummy/Codec] ^^^^ ^^^^^ Normal [CPU/Codec] ^^^^^^^^^^^
DPCM part (X) is checking only CPU DAI, and Normal part (Y) is checking both CPU/Codec DAI
Here, validation check on dummy DAI is always true, Therefor we want to expand validation check to all cases.
One note here is that unfortunately DPCM BE Codec had been no validation check before, but all cases validation check breaks compatibility on some vender's devices. Thus this patch ignore it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-pcm.c | 90 +++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 52 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 77ee103b7cd1..8761ae8fc05f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2793,7 +2793,12 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, int *playback, int *capture) { struct snd_soc_dai_link *dai_link = rtd->dai_link;
- struct snd_soc_dai_link_ch_map *ch_maps; struct snd_soc_dai *cpu_dai;
- struct snd_soc_dai *codec_dai;
- struct snd_soc_dai *dummy_dai = snd_soc_find_dai(&snd_soc_dummy_dlc);
- int cpu_playback;
- int cpu_capture; int has_playback = 0; int has_capture = 0; int i;
@@ -2803,65 +2808,46 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, return -EINVAL; }
- /* REMOVE ME */ if (dai_link->dynamic || dai_link->no_pcm) {
int stream;if (dai_link->dpcm_playback) {stream = SNDRV_PCM_STREAM_PLAYBACK;for_each_rtd_cpu_dais(rtd, i, cpu_dai) {if (snd_soc_dai_stream_valid(cpu_dai, stream)) {has_playback = 1;break;}}if (!has_playback) {dev_err(rtd->card->dev,"No CPU DAIs support playback for stream %s\n",dai_link->stream_name);return -EINVAL;}
if (dai_link->dpcm_playback && !dai_link->dpcm_capture)dai_link->playback_only = 1;if (!dai_link->dpcm_playback && dai_link->dpcm_capture)dai_link->capture_only = 1;if (!dai_link->dpcm_playback && !dai_link->dpcm_capture) {dev_err(rtd->dev, "no dpcm_playback/capture are selected\n");return -EINVAL;
if (dai_link->dpcm_capture) {stream = SNDRV_PCM_STREAM_CAPTURE;
- }
for_each_rtd_cpu_dais(rtd, i, cpu_dai) {if (snd_soc_dai_stream_valid(cpu_dai, stream)) {has_capture = 1;break;}}
- /* Adapt stream for codec2codec links */
- cpu_playback = snd_soc_get_stream_cpu(dai_link, SNDRV_PCM_STREAM_PLAYBACK);
- cpu_capture = snd_soc_get_stream_cpu(dai_link, SNDRV_PCM_STREAM_CAPTURE);
if (!has_capture) {dev_err(rtd->card->dev,"No CPU DAIs support capture for stream %s\n",dai_link->stream_name);return -EINVAL;}}- } else {
struct snd_soc_dai_link_ch_map *ch_maps;struct snd_soc_dai *codec_dai;/* Adapt stream for codec2codec links */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);
/*
* see* soc.h :: [dai_link->ch_maps Image sample]*/for_each_rtd_ch_maps(rtd, i, ch_maps) {
cpu_dai = snd_soc_rtd_to_cpu(rtd, ch_maps->cpu);codec_dai = snd_soc_rtd_to_codec(rtd, ch_maps->codec);/*
* see* soc.h :: [dai_link->ch_maps Image sample]
* FIXME** DPCM BE Codec has been no checked before.* It should be checked, but it breaks compatibility.* It ignores BE Codec here, so far.
for_each_rtd_ch_maps(rtd, i, ch_maps) {cpu_dai = snd_soc_rtd_to_cpu(rtd, ch_maps->cpu);codec_dai = snd_soc_rtd_to_codec(rtd, ch_maps->codec);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 (dai_link->no_pcm)codec_dai = dummy_dai;if (snd_soc_dai_stream_valid(cpu_dai, cpu_playback) &&snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK))has_playback = 1;if (snd_soc_dai_stream_valid(cpu_dai, cpu_capture) &&snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE))has_capture = 1;
The problem I have is with the following code (not shown with diff)
if (dai_link->playback_only) has_capture = 0;
if (dai_link->capture_only) has_playback = 0;
So with this grand unification, all the loops above may make a decision that could be overridden by these two branches.
This was not the case before for DPCM, all the 'has_capture' and 'has_playback' variables were used as a verification of the dai_link settings with an error thrown e.g. if the dpcm_playback was set without any DAIs supporting playback.
Now the dailink settings are used unconditionally. There is one warning added if there are no settings for a dailink, but we've lost the detection of a mismatch between dailink and the set of cpu/codec dais that are part of this dailink.