Re: [PATCH v4 1/4] ASoC: makes CPU/Codec channel connection map more generic

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

Hi Morimoto-san,
we're facing an across-the-board regression with this patch, even in
regular 'nocodec' configurations with dummy dais and no codec.

> @@ -1055,22 +1054,28 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
>  		/* copy params for each cpu */
>  		tmp_params = *params;
>  
> -		if (!rtd->dai_link->codec_ch_maps)
> -			goto hw_params;

By removing this test, we now proceed and deal with both FE and BE...

>  		/*
>  		 * construct cpu channel mask by combining ch_mask of each
>  		 * codec which maps to the cpu.
> +		 * see
> +		 *	soc.h :: [dai_link->ch_maps Image sample]
>  		 */
> -		for_each_rtd_codec_dais(rtd, j, codec_dai) {
> -			if (rtd->dai_link->codec_ch_maps[j].connected_cpu_id == i)
> -				ch_mask |= rtd->dai_link->codec_ch_maps[j].ch_mask;
> +		if (rtd->dai_link->num_cpus >= rtd->dai_link->num_codecs) {
> +			/* .ch_map is from CPU */
> +			ch_mask = rtd->dai_link->ch_maps[i].ch_mask;

... and for a FE dailink there's no ch_maps so this results in a kernel
oops.

> +		} else {
> +			int j;
> +
> +			/* .ch_map is from Codec */
> +			for_each_rtd_codec_dais(rtd, j, codec_dai)
> +				if (rtd->dai_link->ch_maps[j].connected_node == i)
> +					ch_mask |= rtd->dai_link->ch_maps[j].ch_mask;
>  		}
>  
>  		/* fixup cpu channel number */
>  		if (ch_mask)
>  			soc_pcm_codec_params_fixup(&tmp_params, ch_mask);
>  
> -hw_params:
>  		ret = snd_soc_dai_hw_params(cpu_dai, substream, &tmp_params);
>  		if (ret < 0)
>  			goto out;

Bard suggested the following diff (being tested now), comments welcome.

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 0bfff2ea111d..ce84d9c1d8be 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1054,6 +1054,9 @@ static int __soc_pcm_hw_params(struct
snd_soc_pcm_runtime *rtd,
                /* copy params for each cpu */
                tmp_params = *params;

+               /* ch_map is only set in BE dai link */
+               if (rtd->dai_link->dynamic)
+                       goto run;
                /*
                 * construct cpu channel mask by combining ch_mask of each
                 * codec which maps to the cpu.
@@ -1075,7 +1078,7 @@ static int __soc_pcm_hw_params(struct
snd_soc_pcm_runtime *rtd,
                /* fixup cpu channel number */
                if (ch_mask)
                        soc_pcm_codec_params_fixup(&tmp_params, ch_mask);
-
+run:
                ret = snd_soc_dai_hw_params(cpu_dai, substream,
&tmp_params);
                if (ret < 0)
                        goto out;