Re: [PATCH] ASoC: SOF: ipc4-topology: Add support for NHLT with 16-bit only DMIC blob

[Péter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>]

Hi Mark,

On 16/05/2024 10:56, Peter Ujfalusi wrote:
> The ACPI NHLT table always had 32-bit DMIC blob even if 16-bit was also
> present and taken as a 'rule' which obviously got broken and there is at
> least one device on the market which ships with only 16-bit DMIC
> configuration blob.
> This corner case has never been supported and it is going to need topology
> updates for DMIC copier to support multiple formats.
> 
> As for the kernel side: if the copier supports multiple formats and the
> preferred 32-bit DMIC blob is not found then we will try to get a 16-bit
> DMIC configuration and look for a 16-bit copier config.

Please ignore this patch, I will send it again along with few more patch
building on this to close the gaps with the logic of selecting the NHLT
blob.

> Fixes: f9209644ae76 ("ASoC: SOF: ipc4-topology: Correct DAI copier config and NHLT blob request")
> Link: https://github.com/thesofproject/linux/issues/4973
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>
> Reviewed-by: Seppo Ingalsuo <seppo.ingalsuo@xxxxxxxxxxxxxxx>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
> ---
>  sound/soc/sof/ipc4-topology.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
> index beff10989324..521b4dcba601 100644
> --- a/sound/soc/sof/ipc4-topology.c
> +++ b/sound/soc/sof/ipc4-topology.c
> @@ -1483,6 +1483,8 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai
>  					   dir, dev_type);
>  
>  	if (!cfg) {
> +		bool get_new_blob = false;
> +
>  		if (format_change) {
>  			/*
>  			 * The 32-bit blob was not found in NHLT table, try to
> @@ -1490,7 +1492,20 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai
>  			 */
>  			bit_depth = params_width(params);
>  			format_change = false;
> +			get_new_blob = true;
> +		} else if (linktype == SOF_DAI_INTEL_DMIC && !single_format) {
> +			/*
> +			 * The requested 32-bit blob (no format change for the
> +			 * blob request) was not found in NHLT table, try to
> +			 * look for 16-bit blob if the copier supports multiple
> +			 * formats
> +			 */
> +			bit_depth = 16;
> +			format_change = true;
> +			get_new_blob = true;
> +		}
>  
> +		if (get_new_blob) {
>  			cfg = intel_nhlt_get_endpoint_blob(sdev->dev, ipc4_data->nhlt,
>  							   dai_index, nhlt_type,
>  							   bit_depth, bit_depth,
> @@ -1513,8 +1528,8 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai
>  
>  	if (format_change) {
>  		/*
> -		 * Update the params to reflect that we have loaded 32-bit blob
> -		 * instead of the 16-bit.
> +		 * Update the params to reflect that different blob was loaded
> +		 * instead of the requested bit depth (16 -> 32 or 32 -> 16).
>  		 * This information is going to be used by the caller to find
>  		 * matching copier format on the dai side.
>  		 */
> @@ -1522,7 +1537,11 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai
>  
>  		m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
>  		snd_mask_none(m);
> -		snd_mask_set_format(m, SNDRV_PCM_FORMAT_S32_LE);
> +		if (bit_depth == 16)
> +			snd_mask_set_format(m, SNDRV_PCM_FORMAT_S16_LE);
> +		else
> +			snd_mask_set_format(m, SNDRV_PCM_FORMAT_S32_LE);
> +
>  	}
>  
>  	return 0;

-- 
Péter