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

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx
> Cc: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> ---
>  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.