diff mbox series

[v2,15/16] ASoC: soc-pcm: indicate warning if DPCM BE Codec has no settings

Message ID 87edbpudm9.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series ASoC: Replace dpcm_playback/capture to playback/capture_only | expand

Commit Message

Kuninori Morimoto April 1, 2024, 12:32 a.m. UTC
Historically, ASoC doesn't have validation check for DPCM BE Codec,
but it should have. Current ASoC is ignoring it same as before,
but let's indicate the warning about that.

This warning and code should be removed and cleanuped if DPCM BE Codec
has necessary settings.
One of the big user which doesn't have it is Intel.

	--- sound/soc/codecs/hda.c ---

	static struct snd_soc_dai_driver card_binder_dai = {
		.id = -1,
		.name = "codec-probing-DAI",
+		.capture.channels_min = 1,
+		.playback.channels_min = 1,
	};

	--- sound/pci/hda/patch_hdmi.c ---

	static int generic_hdmi_build_pcms(...)
	{
		...
		for (...) {
			...
+			pstr->channels_min = 1;
		}

		return 0;
	}

Link: https://lore.kernel.org/r/ab3f0c0a-62fd-a468-b3cf-0e4b59bac6ae@linux.intel.com
Cc: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-pcm.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Comments

Pierre-Louis Bossart April 1, 2024, 4:28 p.m. UTC | #1
On 3/31/24 19:32, Kuninori Morimoto wrote:
> Historically, ASoC doesn't have validation check for DPCM BE Codec,
> but it should have. Current ASoC is ignoring it same as before,
> but let's indicate the warning about that.
> 
> This warning and code should be removed and cleanuped if DPCM BE Codec
> has necessary settings.
> One of the big user which doesn't have it is Intel.
> 
> 	--- sound/soc/codecs/hda.c ---
> 
> 	static struct snd_soc_dai_driver card_binder_dai = {
> 		.id = -1,
> 		.name = "codec-probing-DAI",
> +		.capture.channels_min = 1,
> +		.playback.channels_min = 1,
> 	};
> 
> 	--- sound/pci/hda/patch_hdmi.c ---
> 
> 	static int generic_hdmi_build_pcms(...)
> 	{
> 		...
> 		for (...) {
> 			...
> +			pstr->channels_min = 1;
> 		}
> 
> 		return 0;
> 	}
> 
> Link: https://lore.kernel.org/r/ab3f0c0a-62fd-a468-b3cf-0e4b59bac6ae@linux.intel.com
> Cc: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  sound/soc/soc-pcm.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index ac42c089815b..95a5e28dead3 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -2796,7 +2796,6 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
>  	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;
> @@ -2817,24 +2816,36 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
>  	 *	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);
> +		int cpu_play_t, cpu_capture_t;
> +		int codec_play_t, codec_capture_t;
> +
> +		cpu_dai		= snd_soc_rtd_to_cpu(rtd,   ch_maps->cpu);
> +		codec_dai	= snd_soc_rtd_to_codec(rtd, ch_maps->codec);
> +
> +		cpu_play_t	= snd_soc_dai_stream_valid(cpu_dai,   cpu_playback);
> +		codec_play_t	= snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK);
> +
> +		cpu_capture_t	= snd_soc_dai_stream_valid(cpu_dai,   cpu_capture);
> +		codec_capture_t	= snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE);
>  
>  		/*
> -		 * FIXME
> +		 * FIXME / CLEAN-UP-ME
>  		 *
>  		 * DPCM BE Codec has been no checked before.
>  		 * It should be checked, but it breaks compatibility.
>  		 * It ignores BE Codec here, so far.
>  		 */
> -		if (dai_link->no_pcm)
> -			codec_dai = dummy_dai;
> +		if ((dai_link->no_pcm) &&
> +		    (!codec_play_t && !codec_capture_t)) {
> +			dev_warn_once(rtd->dev, "DCPM BE Codec has no stream settings (%s)\n",
> +				      codec_dai->name);
> +			codec_play_t	= 1;
> +			codec_capture_t	= 1;
> +		}
>  
> -		if (snd_soc_dai_stream_valid(cpu_dai,   cpu_playback) &&
> -		    snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK))
> +		if (cpu_play_t && codec_play_t)
>  			has_playback = 1;
> -		if (snd_soc_dai_stream_valid(cpu_dai,   cpu_capture) &&
> -		    snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE))
> +		if (cpu_capture_t && codec_capture_t)
>  			has_capture = 1;
>  	}

All that code should be added earlier, and there's still the issue that
all this code is now overridden by the dai_link settings.
diff mbox series

Patch

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index ac42c089815b..95a5e28dead3 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2796,7 +2796,6 @@  static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
 	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;
@@ -2817,24 +2816,36 @@  static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
 	 *	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);
+		int cpu_play_t, cpu_capture_t;
+		int codec_play_t, codec_capture_t;
+
+		cpu_dai		= snd_soc_rtd_to_cpu(rtd,   ch_maps->cpu);
+		codec_dai	= snd_soc_rtd_to_codec(rtd, ch_maps->codec);
+
+		cpu_play_t	= snd_soc_dai_stream_valid(cpu_dai,   cpu_playback);
+		codec_play_t	= snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK);
+
+		cpu_capture_t	= snd_soc_dai_stream_valid(cpu_dai,   cpu_capture);
+		codec_capture_t	= snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE);
 
 		/*
-		 * FIXME
+		 * FIXME / CLEAN-UP-ME
 		 *
 		 * DPCM BE Codec has been no checked before.
 		 * It should be checked, but it breaks compatibility.
 		 * It ignores BE Codec here, so far.
 		 */
-		if (dai_link->no_pcm)
-			codec_dai = dummy_dai;
+		if ((dai_link->no_pcm) &&
+		    (!codec_play_t && !codec_capture_t)) {
+			dev_warn_once(rtd->dev, "DCPM BE Codec has no stream settings (%s)\n",
+				      codec_dai->name);
+			codec_play_t	= 1;
+			codec_capture_t	= 1;
+		}
 
-		if (snd_soc_dai_stream_valid(cpu_dai,   cpu_playback) &&
-		    snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK))
+		if (cpu_play_t && codec_play_t)
 			has_playback = 1;
-		if (snd_soc_dai_stream_valid(cpu_dai,   cpu_capture) &&
-		    snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE))
+		if (cpu_capture_t && codec_capture_t)
 			has_capture = 1;
 	}