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

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

[Brent Lu]

A new feature to compose quirk value according to the result of
amplifier and SoC type detection at runtime.

Brent Lu (2):
  ASoC: Intel: add helpers to detect SoCs
  ASoC: Intel: sof_rt5682: quirk auto detection

 include/linux/platform_data/x86/soc.h         |  24 +++
 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 +-
 5 files changed, 101 insertions(+), 120 deletions(-)

-- 
2.25.1

[PATCH 1/2] ASoC: Intel: add helpers to detect SoCs

[Brent Lu]

Add helpers to detect JSL, TGL, ADL, and ADL-N SoCs.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 include/linux/platform_data/x86/soc.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/linux/platform_data/x86/soc.h b/include/linux/platform_data/x86/soc.h
index da05f425587a..08d08c91d57b 100644
--- a/include/linux/platform_data/x86/soc.h
+++ b/include/linux/platform_data/x86/soc.h
@@ -33,6 +33,10 @@ SOC_INTEL_IS_CPU(cht, ATOM_AIRMONT);
 SOC_INTEL_IS_CPU(apl, ATOM_GOLDMONT);
 SOC_INTEL_IS_CPU(glk, ATOM_GOLDMONT_PLUS);
 SOC_INTEL_IS_CPU(cml, KABYLAKE_L);
+SOC_INTEL_IS_CPU(jsl, ATOM_TREMONT_L);
+SOC_INTEL_IS_CPU(tgl, TIGERLAKE_L);
+SOC_INTEL_IS_CPU(adl, ALDERLAKE_L);
+SOC_INTEL_IS_CPU(adl_n, ALDERLAKE_N);
 
 #else /* IS_ENABLED(CONFIG_X86) */
 
@@ -60,6 +64,26 @@ static inline bool soc_intel_is_cml(void)
 {
 	return false;
 }
+
+static inline bool soc_intel_is_jsl(void)
+{
+	return false;
+}
+
+static inline bool soc_intel_is_tgl(void)
+{
+	return false;
+}
+
+static inline bool soc_intel_is_adl(void)
+{
+	return false;
+}
+
+static inline bool soc_intel_is_adl_n(void)
+{
+	return false;
+}
 #endif /* IS_ENABLED(CONFIG_X86) */
 
 #endif /* __PLATFORM_DATA_X86_SOC_H */
-- 
2.25.1

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

[Brent Lu]

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);
+	} else if (soc_intel_is_adl() || soc_intel_is_adl_n()) {
+		quirk |= SOF_RT5682_MCLK_EN |
+			 SOF_RT5682_NUM_HDMIDEV(4) |
+			 SOF_RT5682_SSP_CODEC(0) |
+			 SOF_BT_OFFLOAD_SSP(2) |
+			 SOF_SSP_BT_OFFLOAD_PRESENT;
+
+		if (quirk & SOF_SPEAKER_AMP_PRESENT)
+			quirk |= SOF_RT5682_SSP_AMP(1);
+	} else
+		dev_err(dev, "Auto-detect failed due to unknown platform\n");
+
+	return quirk;
+}
+
 static int sof_audio_probe(struct platform_device *pdev)
 {
 	struct snd_soc_dai_link *dai_links;
@@ -855,6 +915,9 @@ static int sof_audio_probe(struct platform_device *pdev)
 
 	dmi_check_system(sof_rt5682_quirk_table);
 
+	if (sof_rt5682_quirk & SOF_RT5682_AUTO_DETECT)
+		sof_rt5682_quirk |= sof_audio_auto_detect(&pdev->dev);
+
 	mach = pdev->dev.platform_data;
 
 	/* A speaker amp might not be present when the quirk claims one is.
@@ -975,43 +1038,8 @@ static const struct platform_device_id board_ids[] = {
 		.name = "sof_rt5682",
 	},
 	{
-		.name = "tgl_mx98357_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
-	{
-		.name = "jsl_rt5682_rt1015",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_MCLK_24MHZ |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_RT1015_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1)),
-	},
-	{
-		.name = "tgl_mx98373_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_MAX98373_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
-	{
-		.name = "jsl_rt5682_mx98360",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_MCLK_24MHZ |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_MAX98360A_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1)),
+		.name = "sof_rt5682_auto",
+		.driver_data = (kernel_ulong_t)(SOF_RT5682_AUTO_DETECT),
 	},
 	{
 		.name = "cml_rt1015_rt5682",
@@ -1022,37 +1050,6 @@ static const struct platform_device_id board_ids[] = {
 					SOF_RT1015_SPEAKER_AMP_PRESENT |
 					SOF_RT5682_SSP_AMP(1)),
 	},
-	{
-		.name = "tgl_rt1011_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_RT1011_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
-	{
-		.name = "jsl_rt5682_rt1015p",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_MCLK_24MHZ |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_RT1015P_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1)),
-	},
-	{
-		.name = "adl_mx98373_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_MAX98373_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
 	{
 		.name = "adl_mx98357_rt5682",
 		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
@@ -1061,46 +1058,6 @@ static const struct platform_device_id board_ids[] = {
 					SOF_RT5682_SSP_AMP(2) |
 					SOF_RT5682_NUM_HDMIDEV(4)),
 	},
-	{
-		.name = "adl_max98390_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_MAX98390_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
-	{
-		.name = "adl_mx98360_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_MAX98360A_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
-	{
-		.name = "adl_rt5682",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_RT5682_NUM_HDMIDEV(4) |
-					SOF_BT_OFFLOAD_SSP(2) |
-					SOF_SSP_BT_OFFLOAD_PRESENT),
-	},
-	{
-		.name = "adl_rt1019_rt5682s",
-		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
-					SOF_RT5682_SSP_CODEC(0) |
-					SOF_RT5682S_HEADPHONE_CODEC_PRESENT |
-					SOF_SPEAKER_AMP_PRESENT |
-					SOF_RT1019_SPEAKER_AMP_PRESENT |
-					SOF_RT5682_SSP_AMP(1) |
-					SOF_RT5682_NUM_HDMIDEV(4)),
-	},
 	{
 		.name = "mtl_mx98357_rt5682",
 		.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
diff --git a/sound/soc/intel/common/soc-acpi-intel-adl-match.c b/sound/soc/intel/common/soc-acpi-intel-adl-match.c
index 9990d5502d26..598e3d493369 100644
--- a/sound/soc/intel/common/soc-acpi-intel-adl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-adl-match.c
@@ -448,7 +448,7 @@ static const struct snd_soc_acpi_codecs adl_lt6911_hdmi = {
 struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = {
 	{
 		.comp_ids = &adl_rt5682_rt5682s_hp,
-		.drv_name = "adl_mx98373_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &adl_max98373_amp,
 		.sof_tplg_filename = "sof-adl-max98373-rt5682.tplg",
@@ -462,7 +462,7 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = {
 	},
 	{
 		.comp_ids = &adl_rt5682_rt5682s_hp,
-		.drv_name = "adl_mx98360_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &adl_max98360a_amp,
 		.sof_tplg_filename = "sof-adl-max98360a-rt5682.tplg",
@@ -489,8 +489,8 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = {
 		.sof_tplg_filename = "sof-adl-max98360a-nau8825.tplg",
 	},
 	{
-		.id = "RTL5682",
-		.drv_name = "adl_rt1019_rt5682s",
+		.comp_ids = &adl_rt5682_rt5682s_hp,
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &adl_rt1019p_amp,
 		.sof_tplg_filename = "sof-adl-rt1019-rt5682.tplg",
@@ -502,14 +502,14 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = {
 	},
 	{
 		.comp_ids = &adl_rt5682_rt5682s_hp,
-		.drv_name = "adl_max98390_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &adl_max98390_amp,
 		.sof_tplg_filename = "sof-adl-max98390-rt5682.tplg",
 	},
 	{
 		.comp_ids = &adl_rt5682_rt5682s_hp,
-		.drv_name = "adl_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.sof_tplg_filename = "sof-adl-rt5682.tplg",
 	},
 	{
diff --git a/sound/soc/intel/common/soc-acpi-intel-jsl-match.c b/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
index b95c4b2cda94..1b7e56fa9211 100644
--- a/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
@@ -59,21 +59,21 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_jsl_machines[] = {
 	},
 	{
 		.comp_ids = &rt5682_rt5682s_hp,
-		.drv_name = "jsl_rt5682_rt1015",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &rt1015_spk,
 		.sof_tplg_filename = "sof-jsl-rt5682-rt1015.tplg",
 	},
 	{
 		.comp_ids = &rt5682_rt5682s_hp,
-		.drv_name = "jsl_rt5682_rt1015p",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &rt1015p_spk,
 		.sof_tplg_filename = "sof-jsl-rt5682-rt1015.tplg",
 	},
 	{
 		.comp_ids = &rt5682_rt5682s_hp,
-		.drv_name = "jsl_rt5682_mx98360",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &mx98360a_spk,
 		.sof_tplg_filename = "sof-jsl-rt5682-mx98360a.tplg",
diff --git a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
index ef19150e7b2e..b882deedfab4 100644
--- a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
@@ -376,21 +376,21 @@ static const struct snd_soc_acpi_codecs tgl_lt6911_hdmi = {
 struct snd_soc_acpi_mach snd_soc_acpi_intel_tgl_machines[] = {
 	{
 		.comp_ids = &tgl_rt5682_rt5682s_hp,
-		.drv_name = "tgl_mx98357_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &tgl_codecs,
 		.sof_tplg_filename = "sof-tgl-max98357a-rt5682.tplg",
 	},
 	{
 		.comp_ids = &tgl_rt5682_rt5682s_hp,
-		.drv_name = "tgl_mx98373_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &tgl_max98373_amp,
 		.sof_tplg_filename = "sof-tgl-max98373-rt5682.tplg",
 	},
 	{
 		.comp_ids = &tgl_rt5682_rt5682s_hp,
-		.drv_name = "tgl_rt1011_rt5682",
+		.drv_name = "sof_rt5682_auto",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &tgl_rt1011_amp,
 		.sof_tplg_filename = "sof-tgl-rt1011-rt5682.tplg",
-- 
2.25.1

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

[Pierre-Louis Bossart]

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.

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

[Lu, Brent]

> 
> 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)),
> 	},

I checked Volteer reference kit, it should use SSP1 for amplifier. It seems to me 
this quirk is for some customer variants which implements MAX98373 on SSP2.

> 
> Same for Brya and all usages of SSP_AMP(2)
> 
> 

It's a compromise that Google implements amplifiers on SSP2 on Brya so they can 
connect SDW codec to SSP1 pins, but we asked customers to implement amplifiers 
on SSP1 to reserve BT offload capability.

> > -	{
> > -		.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.
> 

Headphone type will be detected later in the sof_audio_probe().

> > -					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.

I'm thinking about using kernel module parameters for those boards which do not
use default SSP port allocation. Not sure it's doable for machine driver module.

Regards,
Brent

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

[Pierre-Louis Bossart]

On 10/26/22 19:13, Lu, Brent wrote:
>>
>> 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)),
>> 	},
> 
> I checked Volteer reference kit, it should use SSP1 for amplifier. It seems to me 
> this quirk is for some customer variants which implements MAX98373 on SSP2.
> 
>>
>> Same for Brya and all usages of SSP_AMP(2)
>>
>>
> 
> It's a compromise that Google implements amplifiers on SSP2 on Brya so they can 
> connect SDW codec to SSP1 pins, but we asked customers to implement amplifiers 
> on SSP1 to reserve BT offload capability.

I appreciate what you are trying to suggest, and I am all for trying to
put this proliferation of quirks under control, but the autodetection
based on rules seems out-of-reach if we don't first cleanup the existing
quirks.

If "volteer" and "brya" mean different things to different people, and
reference and derivatives are not well identified, the odds of
regressions seem very high to me.

> 
>>> -	{
>>> -		.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.
>>
> 
> Headphone type will be detected later in the sof_audio_probe().
> 
>>> -					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.
> 
> I'm thinking about using kernel module parameters for those boards which do not
> use default SSP port allocation. Not sure it's doable for machine driver module.

That's not a working solution IMHO, the kernel parameters should be used
by expert developers only for specific and short-term debug. It's not
possible to add a dependency on kernel parameters, that would prevent a
kernel update.

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

[Lu, Brent]

> 
> I appreciate what you are trying to suggest, and I am all for trying to put this
> proliferation of quirks under control, but the autodetection based on rules
> seems out-of-reach if we don't first cleanup the existing quirks.
> 
> If "volteer" and "brya" mean different things to different people, and reference
> and derivatives are not well identified, the odds of regressions seem very high to
> me.
> 

The reason that we need so many DMI quirks is that we are not able to know
which SSP port is used by headphone and amplifier. However, it seems to me
that platform and machine driver does not care about the DAI name.

I've tested to rename the backend DAIs as something like 'SSP-HP', 'SSP-SPK',
and 'SSP-BT' so we don't need to create new board info just because the SSP
port is different. However, we need to modify topology since DAI name is
changed.

Regards,
Brent

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Wed, Oct 26, 2022 at 08:30:00PM -0500, Pierre-Louis Bossart wrote:
> On 10/26/22 19:13, Lu, Brent wrote:

> > I'm thinking about using kernel module parameters for those boards which do not
> > use default SSP port allocation. Not sure it's doable for machine driver module.

> That's not a working solution IMHO, the kernel parameters should be used
> by expert developers only for specific and short-term debug. It's not
> possible to add a dependency on kernel parameters, that would prevent a
> kernel update.

Right, and it really doesn't work for distributions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]