]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
ASoC: soc-pcm: Indicate warning if dpcm_playback/capture were used for availability...
authorKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Wed, 21 Aug 2024 02:13:56 +0000 (02:13 +0000)
committerMark Brown <broonie@kernel.org>
Thu, 22 Aug 2024 12:36:45 +0000 (13:36 +0100)
I have been wondering why DPCM needs special flag (= dpcm_playback/capture)
to use it. Below is the history why it was added to ASoC.

(A) In beginning, there was no dpcm_xxx flag on ASoC.
    It checks channels_min for DPCM, same as current non-DPCM.
    Let's name it as "validation check" here.

if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
if (cpu_dai->driver->playback.channels_min)
playback = 1;
if (cpu_dai->driver->capture.channels_min)
capture = 1;

(B) commit 1e9de42f4324 ("ASoC: dpcm: Explicitly set BE DAI link supported
    stream directions") force to use dpcm_xxx flag on DPCM. According to
    this commit log, this is because "Some BE dummy DAI doesn't set
    channels_min for playback/capture". But we don't know which DAI is it,
    and not know why it can't/don't have channels_min. Let's name it as
    "no_chan_DAI" here. According to the code and git-log, it is used as
    DCPM-BE and is CPU DAI. I think the correct solution was set
    channels_min on "no_chan_DAI" side, not update ASoC framework side. But
    everything is under smoke today.

if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
playback = rtd->dai_link->dpcm_playback;
capture  = rtd->dai_link->dpcm_capture;

(C) commit 9b5db059366a ("ASoC: soc-pcm: dpcm: Only allow playback/capture
    if supported") checks channels_min (= validation check) again. Because
    DPCM availability was handled by dpcm_xxx flag at that time, but some
    Sound Card set it even though it wasn't available. Clearly there's
    a contradiction here. I think correct solution was update Sound Card
    side instead of ASoC framework. Sound Card side will be updated to
    handle this issue later (commit 25612477d20b ("ASoC: soc-dai: set
    dai_link dpcm_ flags with a helper"))

if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
...
playback = rtd->dai_link->dpcm_playback &&
   snd_soc_dai_stream_valid(cpu_dai, ...);
capture = rtd->dai_link->dpcm_capture &&
   snd_soc_dai_stream_valid(cpu_dai, ...);

This (C) patch should have broken "no_chan_DAI" which doesn't have
channels_min, but there was no such report during this 4 years.
Possibilities case are as follows
- No one is using "no_chan_DAI"
- "no_chan_DAI" is no longer exist : was removed ?
- "no_chan_DAI" is no longer exist : has channels_min ?

Because of these history, this dpcm_xxx is unneeded flag today. But because
we have been used it for 10 years since (B), it may have been used
differently. For example some DAI available both playback/capture, but it
set dpcm_playback flag only, in this case dpcm_xxx flag is used as
availability limitation. We can use playback_only flag instead in this
case, but it is very difficult to find such DAI today.

Let's add grace time to remove dpcm_playback/capture flag.

This patch don't use dpcm_xxx flag anymore, and indicates warning to use
xxx_only flag if both playback/capture were available but using only
one of dpcm_xxx flag, and not using xxx_only flag.

Link: https://lore.kernel.org/r/87edaym2cg.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://patch.msgid.link/87seuyaahn.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
include/sound/soc.h
sound/soc/soc-pcm.c

index e844f6afc5b550d84b3f6d0b4900d6c9c42a5047..81ef380b2e061bedb0fd156dddf14df1953effce 100644 (file)
@@ -815,6 +815,7 @@ struct snd_soc_dai_link {
        /* This DAI link can route to other DAI links at runtime (Frontend)*/
        unsigned int dynamic:1;
 
+       /* REMOVE ME */
        /* DPCM capture and Playback support */
        unsigned int dpcm_capture:1;
        unsigned int dpcm_playback:1;
index baf8b81e71103dfbcfbbc83ba2fe09d4477a2742..c4834c741fa4ba444723fb71f96a4a54f1343996 100644 (file)
@@ -2739,6 +2739,7 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
 {
        struct snd_soc_dai_link *dai_link = rtd->dai_link;
        struct snd_soc_dai *cpu_dai;
+       struct snd_soc_dai_link_ch_map *ch_maps;
        int has_playback = 0;
        int has_capture  = 0;
        int i;
@@ -2749,43 +2750,51 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
        }
 
        if (dai_link->dynamic || dai_link->no_pcm) {
-               int stream;
 
-               if (dai_link->dpcm_playback) {
-                       stream = SNDRV_PCM_STREAM_PLAYBACK;
+               for_each_rtd_ch_maps(rtd, i, ch_maps) {
+                       cpu_dai   = snd_soc_rtd_to_cpu(rtd,   ch_maps->cpu);
 
-                       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 (snd_soc_dai_stream_valid(cpu_dai, SNDRV_PCM_STREAM_PLAYBACK))
+                               has_playback = 1;
+
+                       if (snd_soc_dai_stream_valid(cpu_dai, SNDRV_PCM_STREAM_CAPTURE))
+                               has_capture = 1;
                }
-               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;
-                               }
+               /*
+                * REMOVE ME
+                *
+                * dpcm_xxx flag will be removed soon, Indicates warning if dpcm_xxx flag was used
+                * as availability limitation
+                */
+               if (has_playback && has_capture) {
+                       if ( dai_link->dpcm_playback &&
+                           !dai_link->dpcm_capture  &&
+                           !dai_link->playback_only) {
+                               dev_warn(rtd->card->dev,
+                                        "both playback/capture are available,"
+                                        " but not using playback_only flag (%s)\n",
+                                        dai_link->stream_name);
+                               dev_warn(rtd->card->dev,
+                                        "dpcm_playback/capture are no longer needed,"
+                                        " please use playback/capture_only instead\n");
+                               has_capture = 0;
                        }
 
-                       if (!has_capture) {
-                               dev_err(rtd->card->dev,
-                                       "No CPU DAIs support capture for stream %s\n",
-                                       dai_link->stream_name);
-                               return -EINVAL;
+                       if (!dai_link->dpcm_playback &&
+                            dai_link->dpcm_capture  &&
+                           !dai_link->capture_only) {
+                               dev_warn(rtd->card->dev,
+                                        "both playback/capture are available,"
+                                        " but not using capture_only flag (%s)\n",
+                                        dai_link->stream_name);
+                               dev_warn(rtd->card->dev,
+                                        "dpcm_playback/capture are no longer needed,"
+                                        " please use playback/capture_only instead\n");
+                               has_playback = 0;
                        }
                }
        } else {
-               struct snd_soc_dai_link_ch_map *ch_maps;
                struct snd_soc_dai *codec_dai;
 
                /* Adapt stream for codec2codec links */