summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKuninori Morimoto <kuninori.morimoto.gx@renesas.com>2024-08-21 04:13:56 +0200
committerMark Brown <broonie@kernel.org>2024-08-22 14:36:45 +0200
commitfd69dfe6789f4ed46d1fdb52e223cff83946d997 (patch)
tree170c946832d32ac6e0d8045b8e05b56caffe6a7c
parentASoC: SOF: amd: remove unused variable from sof_amd_acp_desc structure (diff)
downloadlinux-fd69dfe6789f4ed46d1fdb52e223cff83946d997.tar.xz
linux-fd69dfe6789f4ed46d1fdb52e223cff83946d997.zip
ASoC: soc-pcm: Indicate warning if dpcm_playback/capture were used for availability limition
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>
-rw-r--r--include/sound/soc.h1
-rw-r--r--sound/soc/soc-pcm.c65
2 files changed, 38 insertions, 28 deletions
diff --git a/include/sound/soc.h b/include/sound/soc.h
index e844f6afc5b5..81ef380b2e06 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -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;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index baf8b81e7110..c4834c741fa4 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -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 */