Re: [PATCH 2/2] ASoC: Intel: sof_rt5682: quirk auto detection

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 10/26/22 02:14, Brent Lu wrote:
> To get rid of increasing board_ids array with similar quirks for every
> new platform, we implement a special board config 'sof_rt5682_auto' to
> compose quirk value at runtime. It detects SoC and amplifier type to
> determine the quirk value.
> 
> Signed-off-by: Brent Lu <brent.lu@intel.com>
> ---
>  sound/soc/intel/boards/sof_rt5682.c           | 173 +++++++-----------
>  .../intel/common/soc-acpi-intel-adl-match.c   |  12 +-
>  .../intel/common/soc-acpi-intel-jsl-match.c   |   6 +-
>  .../intel/common/soc-acpi-intel-tgl-match.c   |   6 +-
>  4 files changed, 77 insertions(+), 120 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c
> index 2358be208c1f..2dfccbaae53c 100644
> --- a/sound/soc/intel/boards/sof_rt5682.c
> +++ b/sound/soc/intel/boards/sof_rt5682.c
> @@ -60,6 +60,7 @@
>  #define SOF_MAX98390_SPEAKER_AMP_PRESENT	BIT(24)
>  #define SOF_MAX98390_TWEETER_SPEAKER_PRESENT	BIT(25)
>  #define SOF_RT1019_SPEAKER_AMP_PRESENT	BIT(26)
> +#define SOF_RT5682_AUTO_DETECT	BIT(27)
>  
>  
>  /* Default: MCLK on, MCLK 19.2M, SSP0  */
> @@ -838,6 +839,65 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev,
>  	return NULL;
>  }
>  
> +static const struct {
> +	const u8 id[ACPI_ID_LEN];
> +	unsigned long quirk;
> +} spk_quirk_map[] = {
> +	{.id = "10EC1011", .quirk = SOF_RT1011_SPEAKER_AMP_PRESENT},
> +	{.id = "10EC1015", .quirk = SOF_RT1015_SPEAKER_AMP_PRESENT},
> +	{.id = "RTL1015", .quirk = SOF_RT1015P_SPEAKER_AMP_PRESENT},
> +	{.id = "RTL1019", .quirk = SOF_RT1019_SPEAKER_AMP_PRESENT},
> +	{.id = "MX98357A", .quirk = 0},
> +	{.id = "MX98360A", .quirk = SOF_MAX98360A_SPEAKER_AMP_PRESENT},
> +	{.id = "MX98373", .quirk = SOF_MAX98373_SPEAKER_AMP_PRESENT},
> +	{.id = "MX98390", .quirk = SOF_MAX98390_SPEAKER_AMP_PRESENT},
> +};
> +
> +static unsigned long sof_audio_auto_detect(struct device *dev)
> +{
> +	unsigned long quirk = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(spk_quirk_map); i++) {
> +		if (!acpi_dev_present(spk_quirk_map[i].id, NULL, -1))
> +			continue;
> +
> +		dev_dbg(dev, "%s detected\n", spk_quirk_map[i].id);
> +		quirk |= spk_quirk_map[i].quirk;
> +		quirk |= SOF_SPEAKER_AMP_PRESENT;
> +		break;
> +	}
> +
> +	if (soc_intel_is_jsl()) {
> +		quirk |= SOF_RT5682_MCLK_EN |
> +			 SOF_RT5682_MCLK_24MHZ |
> +			 SOF_RT5682_NUM_HDMIDEV(3) |
> +			 SOF_RT5682_SSP_CODEC(0);
> +
> +		if (quirk & SOF_SPEAKER_AMP_PRESENT)
> +			quirk |= SOF_RT5682_SSP_AMP(1);
> +	} else if (soc_intel_is_tgl()) {
> +		quirk |= SOF_RT5682_MCLK_EN |
> +			 SOF_RT5682_NUM_HDMIDEV(4) |
> +			 SOF_RT5682_SSP_CODEC(0);
> +
> +		if (quirk & SOF_SPEAKER_AMP_PRESENT)
> +			quirk |= SOF_RT5682_SSP_AMP(1);

This is a bit confusing: this quirk does not work for Volteer

	{
		.callback = sof_rt5682_quirk_cb,
		.matches = {
			DMI_MATCH(DMI_PRODUCT_FAMILY, "Google_Volteer"),
			DMI_MATCH(DMI_OEM_STRING, "AUDIO-MAX98373_ALC5682I_I2S_UP4"),
		},
		.driver_data = (void *)(SOF_RT5682_MCLK_EN |
					SOF_RT5682_SSP_CODEC(0) |
					SOF_SPEAKER_AMP_PRESENT |
					SOF_MAX98373_SPEAKER_AMP_PRESENT |
					SOF_RT5682_SSP_AMP(2) |
					SOF_RT5682_NUM_HDMIDEV(4)),
	},

Same for Brya and all usages of SSP_AMP(2)


> -	{
> -		.name = "adl_rt1019_rt5682s",
> -		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
> -					SOF_RT5682_SSP_CODEC(0) |
> -					SOF_RT5682S_HEADPHONE_CODEC_PRESENT |

and HEADPHONE_CODEC_PRESENT is not handled either.

> -					SOF_SPEAKER_AMP_PRESENT |
> -					SOF_RT1019_SPEAKER_AMP_PRESENT |
> -					SOF_RT5682_SSP_AMP(1) |
> -					SOF_RT5682_NUM_HDMIDEV(4)),
> -	},

Overall I doubt that the SOC alone can tell you what the quirk is.

Maybe it's a default to avoid repeats of the same baseline
configuration, but there's not much else that can be infer from an SOC
definition in light of the creativity of our hardware friends who
routinely swap interfaces.