[PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Brent Lu]

This patch adds jsl_rt5682_rt1015p which supports the
RT5682 headset codec and RT1015P speaker amplifier combination
on JasperLake platform.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/codecs/rt1015p.c                    | 10 ++
 sound/soc/intel/boards/Kconfig                |  1 +
 sound/soc/intel/boards/sof_realtek_common.c   | 94 +++++++++++++++++++
 sound/soc/intel/boards/sof_realtek_common.h   |  8 ++
 sound/soc/intel/boards/sof_rt5682.c           | 31 +++++-
 .../intel/common/soc-acpi-intel-jsl-match.c   | 13 +++
 6 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
index 671f2a2130fe..3a564272156b 100644
--- a/sound/soc/codecs/rt1015p.c
+++ b/sound/soc/codecs/rt1015p.c
@@ -4,6 +4,7 @@
 //
 // Copyright 2020 The Linux Foundation. All rights reserved.
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -130,10 +131,19 @@ static const struct of_device_id rt1015p_device_id[] = {
 MODULE_DEVICE_TABLE(of, rt1015p_device_id);
 #endif
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt1015p_acpi_match[] = {
+	{ "RTL1015", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, rt1015p_acpi_match);
+#endif
+
 static struct platform_driver rt1015p_platform_driver = {
 	.driver = {
 		.name = "rt1015p",
 		.of_match_table = of_match_ptr(rt1015p_device_id),
+		.acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
 	},
 	.probe = rt1015p_platform_probe,
 };
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index d1d28129a32b..58379393b8e4 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -457,6 +457,7 @@ config SND_SOC_INTEL_SOF_RT5682_MACH
 	select SND_SOC_MAX98373_I2C
 	select SND_SOC_RT1011
 	select SND_SOC_RT1015
+	select SND_SOC_RT1015P
 	select SND_SOC_RT5682_I2C
 	select SND_SOC_DMIC
 	select SND_SOC_HDAC_HDMI
diff --git a/sound/soc/intel/boards/sof_realtek_common.c b/sound/soc/intel/boards/sof_realtek_common.c
index f3cf73c620ba..4b95aa2cbff8 100644
--- a/sound/soc/intel/boards/sof_realtek_common.c
+++ b/sound/soc/intel/boards/sof_realtek_common.c
@@ -136,3 +136,97 @@ void sof_rt1011_codec_conf(struct snd_soc_card *card)
 	card->codec_conf = rt1011_codec_confs;
 	card->num_configs = ARRAY_SIZE(rt1011_codec_confs);
 }
+
+#define RT1015P_SHARE_EN_SPK	BIT(0)
+
+static unsigned long rt1015p_quirk;
+
+static const struct snd_soc_dapm_route rt1015p_dapm_routes[] = {
+	/* speaker */
+	{ "Left Spk", NULL, "Left Speaker" },
+	{ "Right Spk", NULL, "Right Speaker" },
+};
+
+static const struct snd_soc_dapm_route rt1015p_share_en_spk_dapm_routes[] = {
+	/* speaker */
+	{ "Left Spk", NULL, "Speaker" },
+	{ "Right Spk", NULL, "Speaker" },
+};
+
+static struct snd_soc_codec_conf rt1015p_codec_confs[] = {
+	{
+		.dlc = COMP_CODEC_CONF(RT1015P_DEV0_NAME),
+		.name_prefix = "Left",
+	},
+	{
+		.dlc = COMP_CODEC_CONF(RT1015P_DEV1_NAME),
+		.name_prefix = "Right",
+	},
+};
+
+static struct snd_soc_dai_link_component rt1015p_dai_link_components[] = {
+	{
+		.name = RT1015P_DEV0_NAME,
+		.dai_name = RT1015P_CODEC_DAI,
+	},
+	{
+		.name = RT1015P_DEV1_NAME,
+		.dai_name = RT1015P_CODEC_DAI,
+	},
+};
+
+void sof_rt1015p_set_share_en_spk(void)
+{
+	/* Two amps share one en pin so there is only one device in acpi
+	 * table
+	 */
+	rt1015p_quirk |= RT1015P_SHARE_EN_SPK;
+}
+
+static int rt1015p_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params)
+{
+	/* reserved for debugging purpose */
+
+	return 0;
+}
+
+static const struct snd_soc_ops rt1015p_ops = {
+	.hw_params = rt1015p_hw_params,
+};
+
+static int rt1015p_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	int ret;
+
+	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
+		ret = snd_soc_dapm_add_routes(&card->dapm, rt1015p_share_en_spk_dapm_routes,
+					      ARRAY_SIZE(rt1015p_share_en_spk_dapm_routes));
+	else
+		ret = snd_soc_dapm_add_routes(&card->dapm, rt1015p_dapm_routes,
+					      ARRAY_SIZE(rt1015p_dapm_routes));
+	if (ret)
+		dev_err(rtd->dev, "Speaker map addition failed: %d\n", ret);
+	return ret;
+}
+
+void sof_rt1015p_dai_link(struct snd_soc_dai_link *link)
+{
+	link->codecs = rt1015p_dai_link_components;
+	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
+		link->num_codecs = 1;
+	else
+		link->num_codecs = ARRAY_SIZE(rt1015p_dai_link_components);
+	link->init = rt1015p_init;
+	link->ops = &rt1015p_ops;
+}
+
+void sof_rt1015p_codec_conf(struct snd_soc_card *card)
+{
+	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
+		return;
+
+	card->codec_conf = rt1015p_codec_confs;
+	card->num_configs = ARRAY_SIZE(rt1015p_codec_confs);
+}
diff --git a/sound/soc/intel/boards/sof_realtek_common.h b/sound/soc/intel/boards/sof_realtek_common.h
index 87cb3812b926..d7865681f2bd 100644
--- a/sound/soc/intel/boards/sof_realtek_common.h
+++ b/sound/soc/intel/boards/sof_realtek_common.h
@@ -21,4 +21,12 @@
 void sof_rt1011_dai_link(struct snd_soc_dai_link *link);
 void sof_rt1011_codec_conf(struct snd_soc_card *card);
 
+#define RT1015P_CODEC_DAI	"HiFi"
+#define RT1015P_DEV0_NAME	"RTL1015:00"
+#define RT1015P_DEV1_NAME	"RTL1015:01"
+
+void sof_rt1015p_set_share_en_spk(void);
+void sof_rt1015p_dai_link(struct snd_soc_dai_link *link);
+void sof_rt1015p_codec_conf(struct snd_soc_card *card);
+
 #endif /* __SOF_REALTEK_COMMON_H */
diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c
index 55505e207bc0..154f2ee9f406 100644
--- a/sound/soc/intel/boards/sof_rt5682.c
+++ b/sound/soc/intel/boards/sof_rt5682.c
@@ -45,8 +45,9 @@
 #define SOF_RT1011_SPEAKER_AMP_PRESENT		BIT(13)
 #define SOF_RT1015_SPEAKER_AMP_PRESENT		BIT(14)
 #define SOF_RT1015_SPEAKER_AMP_100FS		BIT(15)
-#define SOF_MAX98373_SPEAKER_AMP_PRESENT	BIT(16)
-#define SOF_MAX98360A_SPEAKER_AMP_PRESENT	BIT(17)
+#define SOF_RT1015P_SPEAKER_AMP_PRESENT		BIT(16)
+#define SOF_MAX98373_SPEAKER_AMP_PRESENT	BIT(17)
+#define SOF_MAX98360A_SPEAKER_AMP_PRESENT	BIT(18)
 
 /* Default: MCLK on, MCLK 19.2M, SSP0  */
 static unsigned long sof_rt5682_quirk = SOF_RT5682_MCLK_EN |
@@ -723,6 +724,8 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev,
 			links[id].num_codecs = ARRAY_SIZE(rt1015_components);
 			links[id].init = speaker_codec_init_lr;
 			links[id].ops = &sof_rt1015_ops;
+		} else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
+			sof_rt1015p_dai_link(&links[id]);
 		} else if (sof_rt5682_quirk &
 				SOF_MAX98373_SPEAKER_AMP_PRESENT) {
 			links[id].codecs = max_98373_components;
@@ -847,10 +850,24 @@ static int sof_audio_probe(struct platform_device *pdev)
 	if (sof_rt5682_quirk & SOF_SPEAKER_AMP_PRESENT)
 		sof_audio_card_rt5682.num_links++;
 
+	/* setup amp quirk if any */
+	if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
+		/* There may be only one device instance if two amps
+		 * sharing one en pin
+		 */
+		if (!acpi_dev_present("RTL1015", "1", -1)) {
+			dev_dbg(&pdev->dev, "Device %s not exist\n",
+				RT1015P_DEV1_NAME);
+			sof_rt1015p_set_share_en_spk();
+		}
+	}
+
 	if (sof_rt5682_quirk & SOF_MAX98373_SPEAKER_AMP_PRESENT)
 		sof_max98373_codec_conf(&sof_audio_card_rt5682);
 	else if (sof_rt5682_quirk & SOF_RT1011_SPEAKER_AMP_PRESENT)
 		sof_rt1011_codec_conf(&sof_audio_card_rt5682);
+	else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT)
+		sof_rt1015p_codec_conf(&sof_audio_card_rt5682);
 
 	dai_links = sof_card_dai_links_create(&pdev->dev, ssp_codec, ssp_amp,
 					      dmic_be_num, hdmi_num);
@@ -940,6 +957,15 @@ static const struct platform_device_id board_ids[] = {
 					SOF_RT5682_SSP_AMP(1) |
 					SOF_RT5682_NUM_HDMIDEV(4)),
 	},
+	{
+		.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)),
+	},
 	{ }
 };
 
@@ -966,3 +992,4 @@ MODULE_ALIAS("platform:tgl_max98373_rt5682");
 MODULE_ALIAS("platform:jsl_rt5682_max98360a");
 MODULE_ALIAS("platform:cml_rt1015_rt5682");
 MODULE_ALIAS("platform:tgl_rt1011_rt5682");
+MODULE_ALIAS("platform:jsl_rt5682_rt1015p");
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 52238db0bcb5..73fe4f89a82d 100644
--- a/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
@@ -19,6 +19,11 @@ static struct snd_soc_acpi_codecs rt1015_spk = {
 	.codecs = {"10EC1015"}
 };
 
+static struct snd_soc_acpi_codecs rt1015p_spk = {
+	.num_codecs = 1,
+	.codecs = {"RTL1015"}
+};
+
 static struct snd_soc_acpi_codecs mx98360a_spk = {
 	.num_codecs = 1,
 	.codecs = {"MX98360A"}
@@ -52,6 +57,14 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_jsl_machines[] = {
 		.quirk_data = &rt1015_spk,
 		.sof_tplg_filename = "sof-jsl-rt5682-rt1015.tplg",
 	},
+	{
+		.id = "10EC5682",
+		.drv_name = "jsl_rt5682_rt1015p",
+		.sof_fw_filename = "sof-jsl.ri",
+		.machine_quirk = snd_soc_acpi_codec_list,
+		.quirk_data = &rt1015p_spk,
+		.sof_tplg_filename = "sof-jsl-rt5682-rt1015.tplg",
+	},
 	{
 		.id = "10EC5682",
 		.drv_name = "jsl_rt5682_max98360a",
-- 
2.17.1

[PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Brent Lu]

This patch adds jsl_rt5682_rt1015p which supports the
RT5682 headset codec and RT1015P speaker amplifier combination
on JasperLake platform.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/codecs/rt1015p.c                    | 10 ++
 sound/soc/intel/boards/Kconfig                |  1 +
 sound/soc/intel/boards/sof_realtek_common.c   | 94 +++++++++++++++++++
 sound/soc/intel/boards/sof_realtek_common.h   |  8 ++
 sound/soc/intel/boards/sof_rt5682.c           | 31 +++++-
 .../intel/common/soc-acpi-intel-jsl-match.c   | 13 +++
 6 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
index 671f2a2130fe..3a564272156b 100644
--- a/sound/soc/codecs/rt1015p.c
+++ b/sound/soc/codecs/rt1015p.c
@@ -4,6 +4,7 @@
 //
 // Copyright 2020 The Linux Foundation. All rights reserved.
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -130,10 +131,19 @@ static const struct of_device_id rt1015p_device_id[] = {
 MODULE_DEVICE_TABLE(of, rt1015p_device_id);
 #endif
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt1015p_acpi_match[] = {
+	{ "RTL1015", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, rt1015p_acpi_match);
+#endif
+
 static struct platform_driver rt1015p_platform_driver = {
 	.driver = {
 		.name = "rt1015p",
 		.of_match_table = of_match_ptr(rt1015p_device_id),
+		.acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
 	},
 	.probe = rt1015p_platform_probe,
 };
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index d1d28129a32b..58379393b8e4 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -457,6 +457,7 @@ config SND_SOC_INTEL_SOF_RT5682_MACH
 	select SND_SOC_MAX98373_I2C
 	select SND_SOC_RT1011
 	select SND_SOC_RT1015
+	select SND_SOC_RT1015P
 	select SND_SOC_RT5682_I2C
 	select SND_SOC_DMIC
 	select SND_SOC_HDAC_HDMI
diff --git a/sound/soc/intel/boards/sof_realtek_common.c b/sound/soc/intel/boards/sof_realtek_common.c
index f3cf73c620ba..4b95aa2cbff8 100644
--- a/sound/soc/intel/boards/sof_realtek_common.c
+++ b/sound/soc/intel/boards/sof_realtek_common.c
@@ -136,3 +136,97 @@ void sof_rt1011_codec_conf(struct snd_soc_card *card)
 	card->codec_conf = rt1011_codec_confs;
 	card->num_configs = ARRAY_SIZE(rt1011_codec_confs);
 }
+
+#define RT1015P_SHARE_EN_SPK	BIT(0)
+
+static unsigned long rt1015p_quirk;
+
+static const struct snd_soc_dapm_route rt1015p_dapm_routes[] = {
+	/* speaker */
+	{ "Left Spk", NULL, "Left Speaker" },
+	{ "Right Spk", NULL, "Right Speaker" },
+};
+
+static const struct snd_soc_dapm_route rt1015p_share_en_spk_dapm_routes[] = {
+	/* speaker */
+	{ "Left Spk", NULL, "Speaker" },
+	{ "Right Spk", NULL, "Speaker" },
+};
+
+static struct snd_soc_codec_conf rt1015p_codec_confs[] = {
+	{
+		.dlc = COMP_CODEC_CONF(RT1015P_DEV0_NAME),
+		.name_prefix = "Left",
+	},
+	{
+		.dlc = COMP_CODEC_CONF(RT1015P_DEV1_NAME),
+		.name_prefix = "Right",
+	},
+};
+
+static struct snd_soc_dai_link_component rt1015p_dai_link_components[] = {
+	{
+		.name = RT1015P_DEV0_NAME,
+		.dai_name = RT1015P_CODEC_DAI,
+	},
+	{
+		.name = RT1015P_DEV1_NAME,
+		.dai_name = RT1015P_CODEC_DAI,
+	},
+};
+
+void sof_rt1015p_set_share_en_spk(void)
+{
+	/* Two amps share one en pin so there is only one device in acpi
+	 * table
+	 */
+	rt1015p_quirk |= RT1015P_SHARE_EN_SPK;
+}
+
+static int rt1015p_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params)
+{
+	/* reserved for debugging purpose */
+
+	return 0;
+}
+
+static const struct snd_soc_ops rt1015p_ops = {
+	.hw_params = rt1015p_hw_params,
+};
+
+static int rt1015p_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	int ret;
+
+	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
+		ret = snd_soc_dapm_add_routes(&card->dapm, rt1015p_share_en_spk_dapm_routes,
+					      ARRAY_SIZE(rt1015p_share_en_spk_dapm_routes));
+	else
+		ret = snd_soc_dapm_add_routes(&card->dapm, rt1015p_dapm_routes,
+					      ARRAY_SIZE(rt1015p_dapm_routes));
+	if (ret)
+		dev_err(rtd->dev, "Speaker map addition failed: %d\n", ret);
+	return ret;
+}
+
+void sof_rt1015p_dai_link(struct snd_soc_dai_link *link)
+{
+	link->codecs = rt1015p_dai_link_components;
+	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
+		link->num_codecs = 1;
+	else
+		link->num_codecs = ARRAY_SIZE(rt1015p_dai_link_components);
+	link->init = rt1015p_init;
+	link->ops = &rt1015p_ops;
+}
+
+void sof_rt1015p_codec_conf(struct snd_soc_card *card)
+{
+	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
+		return;
+
+	card->codec_conf = rt1015p_codec_confs;
+	card->num_configs = ARRAY_SIZE(rt1015p_codec_confs);
+}
diff --git a/sound/soc/intel/boards/sof_realtek_common.h b/sound/soc/intel/boards/sof_realtek_common.h
index 87cb3812b926..d7865681f2bd 100644
--- a/sound/soc/intel/boards/sof_realtek_common.h
+++ b/sound/soc/intel/boards/sof_realtek_common.h
@@ -21,4 +21,12 @@
 void sof_rt1011_dai_link(struct snd_soc_dai_link *link);
 void sof_rt1011_codec_conf(struct snd_soc_card *card);
 
+#define RT1015P_CODEC_DAI	"HiFi"
+#define RT1015P_DEV0_NAME	"RTL1015:00"
+#define RT1015P_DEV1_NAME	"RTL1015:01"
+
+void sof_rt1015p_set_share_en_spk(void);
+void sof_rt1015p_dai_link(struct snd_soc_dai_link *link);
+void sof_rt1015p_codec_conf(struct snd_soc_card *card);
+
 #endif /* __SOF_REALTEK_COMMON_H */
diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c
index 55505e207bc0..154f2ee9f406 100644
--- a/sound/soc/intel/boards/sof_rt5682.c
+++ b/sound/soc/intel/boards/sof_rt5682.c
@@ -45,8 +45,9 @@
 #define SOF_RT1011_SPEAKER_AMP_PRESENT		BIT(13)
 #define SOF_RT1015_SPEAKER_AMP_PRESENT		BIT(14)
 #define SOF_RT1015_SPEAKER_AMP_100FS		BIT(15)
-#define SOF_MAX98373_SPEAKER_AMP_PRESENT	BIT(16)
-#define SOF_MAX98360A_SPEAKER_AMP_PRESENT	BIT(17)
+#define SOF_RT1015P_SPEAKER_AMP_PRESENT		BIT(16)
+#define SOF_MAX98373_SPEAKER_AMP_PRESENT	BIT(17)
+#define SOF_MAX98360A_SPEAKER_AMP_PRESENT	BIT(18)
 
 /* Default: MCLK on, MCLK 19.2M, SSP0  */
 static unsigned long sof_rt5682_quirk = SOF_RT5682_MCLK_EN |
@@ -723,6 +724,8 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev,
 			links[id].num_codecs = ARRAY_SIZE(rt1015_components);
 			links[id].init = speaker_codec_init_lr;
 			links[id].ops = &sof_rt1015_ops;
+		} else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
+			sof_rt1015p_dai_link(&links[id]);
 		} else if (sof_rt5682_quirk &
 				SOF_MAX98373_SPEAKER_AMP_PRESENT) {
 			links[id].codecs = max_98373_components;
@@ -847,10 +850,24 @@ static int sof_audio_probe(struct platform_device *pdev)
 	if (sof_rt5682_quirk & SOF_SPEAKER_AMP_PRESENT)
 		sof_audio_card_rt5682.num_links++;
 
+	/* setup amp quirk if any */
+	if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
+		/* There may be only one device instance if two amps
+		 * sharing one en pin
+		 */
+		if (!acpi_dev_present("RTL1015", "1", -1)) {
+			dev_dbg(&pdev->dev, "Device %s not exist\n",
+				RT1015P_DEV1_NAME);
+			sof_rt1015p_set_share_en_spk();
+		}
+	}
+
 	if (sof_rt5682_quirk & SOF_MAX98373_SPEAKER_AMP_PRESENT)
 		sof_max98373_codec_conf(&sof_audio_card_rt5682);
 	else if (sof_rt5682_quirk & SOF_RT1011_SPEAKER_AMP_PRESENT)
 		sof_rt1011_codec_conf(&sof_audio_card_rt5682);
+	else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT)
+		sof_rt1015p_codec_conf(&sof_audio_card_rt5682);
 
 	dai_links = sof_card_dai_links_create(&pdev->dev, ssp_codec, ssp_amp,
 					      dmic_be_num, hdmi_num);
@@ -940,6 +957,15 @@ static const struct platform_device_id board_ids[] = {
 					SOF_RT5682_SSP_AMP(1) |
 					SOF_RT5682_NUM_HDMIDEV(4)),
 	},
+	{
+		.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)),
+	},
 	{ }
 };
 
@@ -966,3 +992,4 @@ MODULE_ALIAS("platform:tgl_max98373_rt5682");
 MODULE_ALIAS("platform:jsl_rt5682_max98360a");
 MODULE_ALIAS("platform:cml_rt1015_rt5682");
 MODULE_ALIAS("platform:tgl_rt1011_rt5682");
+MODULE_ALIAS("platform:jsl_rt5682_rt1015p");
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 52238db0bcb5..73fe4f89a82d 100644
--- a/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-jsl-match.c
@@ -19,6 +19,11 @@ static struct snd_soc_acpi_codecs rt1015_spk = {
 	.codecs = {"10EC1015"}
 };
 
+static struct snd_soc_acpi_codecs rt1015p_spk = {
+	.num_codecs = 1,
+	.codecs = {"RTL1015"}
+};
+
 static struct snd_soc_acpi_codecs mx98360a_spk = {
 	.num_codecs = 1,
 	.codecs = {"MX98360A"}
@@ -52,6 +57,14 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_jsl_machines[] = {
 		.quirk_data = &rt1015_spk,
 		.sof_tplg_filename = "sof-jsl-rt5682-rt1015.tplg",
 	},
+	{
+		.id = "10EC5682",
+		.drv_name = "jsl_rt5682_rt1015p",
+		.sof_fw_filename = "sof-jsl.ri",
+		.machine_quirk = snd_soc_acpi_codec_list,
+		.quirk_data = &rt1015p_spk,
+		.sof_tplg_filename = "sof-jsl-rt5682-rt1015.tplg",
+	},
 	{
 		.id = "10EC5682",
 		.drv_name = "jsl_rt5682_max98360a",
-- 
2.17.1

RE: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Lu, Brent]

> 
> This patch adds jsl_rt5682_rt1015p which supports the
> RT5682 headset codec and RT1015P speaker amplifier combination on
> JasperLake platform.
> 
> Signed-off-by: Brent Lu <brent.lu@intel.com>
> ---
>  sound/soc/codecs/rt1015p.c                    | 10 ++
>  sound/soc/intel/boards/Kconfig                |  1 +
>  sound/soc/intel/boards/sof_realtek_common.c   | 94
> +++++++++++++++++++
>  sound/soc/intel/boards/sof_realtek_common.h   |  8 ++
>  sound/soc/intel/boards/sof_rt5682.c           | 31 +++++-
>  .../intel/common/soc-acpi-intel-jsl-match.c   | 13 +++
>  6 files changed, 155 insertions(+), 2 deletions(-)
> 

Sorry it contains unnecessary patch for codec driver. I'll remove it and
upload a version 2.

I'm also wondering if it's a good idea to use acpi_dev_present() function
in driver probe to determine the number of amp device instance. Without
using it, we will need to use quirk for those devices using one EN pin for
two amps.


Regards,
Brent

RE: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Lu, Brent]

> 
> This patch adds jsl_rt5682_rt1015p which supports the
> RT5682 headset codec and RT1015P speaker amplifier combination on
> JasperLake platform.
> 
> Signed-off-by: Brent Lu <brent.lu@intel.com>
> ---
>  sound/soc/codecs/rt1015p.c                    | 10 ++
>  sound/soc/intel/boards/Kconfig                |  1 +
>  sound/soc/intel/boards/sof_realtek_common.c   | 94
> +++++++++++++++++++
>  sound/soc/intel/boards/sof_realtek_common.h   |  8 ++
>  sound/soc/intel/boards/sof_rt5682.c           | 31 +++++-
>  .../intel/common/soc-acpi-intel-jsl-match.c   | 13 +++
>  6 files changed, 155 insertions(+), 2 deletions(-)
> 

Sorry it contains unnecessary patch for codec driver. I'll remove it and
upload a version 2.

I'm also wondering if it's a good idea to use acpi_dev_present() function
in driver probe to determine the number of amp device instance. Without
using it, we will need to use quirk for those devices using one EN pin for
two amps.


Regards,
Brent

Re: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Pierre-Louis Bossart]

I am not a big fan of the code partition you've selected.

> +void sof_rt1015p_set_share_en_spk(void)
> +{
> +	/* Two amps share one en pin so there is only one device in acpi
> +	 * table
> +	 */
> +	rt1015p_quirk |= RT1015P_SHARE_EN_SPK;
> +}

This is a function now used in the machine driver, see below [1]
This adds a new interface between machine and realtek common code, which 
we are trying to move to a module with well-defined APIs.

> +void sof_rt1015p_dai_link(struct snd_soc_dai_link *link)
> +{
> +	link->codecs = rt1015p_dai_link_components;
> +	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
> +		link->num_codecs = 1;
> +	else
> +		link->num_codecs = ARRAY_SIZE(rt1015p_dai_link_components);
> +	link->init = rt1015p_init;
> +	link->ops = &rt1015p_ops;
> +}
> +
> +void sof_rt1015p_codec_conf(struct snd_soc_card *card)
> +{
> +	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
> +		return;
> +
> +	card->codec_conf = rt1015p_codec_confs;
> +	card->num_configs = ARRAY_SIZE(rt1015p_codec_confs);
> +}

could we not handle this quirk inside one of the two dai_link or 
codec_conf functions above?
The machine driver does not care about this RT1015P_SHARE_EN_SPK quirk, 
it's only used in those two functions so should be set in this scope. 
There's no need to make it visible to the machine driver, is there?

> +	/* setup amp quirk if any */
> +	if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
> +		/* There may be only one device instance if two amps
> +		 * sharing one en pin
> +		 */
> +		if (!acpi_dev_present("RTL1015", "1", -1)) {
> +			dev_dbg(&pdev->dev, "Device %s not exist\n",
> +				RT1015P_DEV1_NAME);
> +			sof_rt1015p_set_share_en_spk();
> +		}
> +	}
> +

[1]

I see no problem using the _UID (Unique ID) to check if a second 
amplifier is present or not. This seems to follow the ACPI spec Section 
6.1.12, as long as "the _UID must be unique across all devices with 
either a common _HID or _CID"


> +	else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT)
> +		sof_rt1015p_codec_conf(&sof_audio_card_rt5682);

Re: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Pierre-Louis Bossart]

I am not a big fan of the code partition you've selected.

> +void sof_rt1015p_set_share_en_spk(void)
> +{
> +	/* Two amps share one en pin so there is only one device in acpi
> +	 * table
> +	 */
> +	rt1015p_quirk |= RT1015P_SHARE_EN_SPK;
> +}

This is a function now used in the machine driver, see below [1]
This adds a new interface between machine and realtek common code, which 
we are trying to move to a module with well-defined APIs.

> +void sof_rt1015p_dai_link(struct snd_soc_dai_link *link)
> +{
> +	link->codecs = rt1015p_dai_link_components;
> +	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
> +		link->num_codecs = 1;
> +	else
> +		link->num_codecs = ARRAY_SIZE(rt1015p_dai_link_components);
> +	link->init = rt1015p_init;
> +	link->ops = &rt1015p_ops;
> +}
> +
> +void sof_rt1015p_codec_conf(struct snd_soc_card *card)
> +{
> +	if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
> +		return;
> +
> +	card->codec_conf = rt1015p_codec_confs;
> +	card->num_configs = ARRAY_SIZE(rt1015p_codec_confs);
> +}

could we not handle this quirk inside one of the two dai_link or 
codec_conf functions above?
The machine driver does not care about this RT1015P_SHARE_EN_SPK quirk, 
it's only used in those two functions so should be set in this scope. 
There's no need to make it visible to the machine driver, is there?

> +	/* setup amp quirk if any */
> +	if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
> +		/* There may be only one device instance if two amps
> +		 * sharing one en pin
> +		 */
> +		if (!acpi_dev_present("RTL1015", "1", -1)) {
> +			dev_dbg(&pdev->dev, "Device %s not exist\n",
> +				RT1015P_DEV1_NAME);
> +			sof_rt1015p_set_share_en_spk();
> +		}
> +	}
> +

[1]

I see no problem using the _UID (Unique ID) to check if a second 
amplifier is present or not. This seems to follow the ACPI spec Section 
6.1.12, as long as "the _UID must be unique across all devices with 
either a common _HID or _CID"


> +	else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT)
> +		sof_rt1015p_codec_conf(&sof_audio_card_rt5682);

RE: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Lu, Brent]

> 
> could we not handle this quirk inside one of the two dai_link or codec_conf
> functions above?
> The machine driver does not care about this RT1015P_SHARE_EN_SPK quirk,
> it's only used in those two functions so should be set in this scope.
> There's no need to make it visible to the machine driver, is there?
> 

Thank you for the comment; this patch looks terrible... I will improve the code
and upload a v2 patch.

Regards,
Brent

RE: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Lu, Brent]

> 
> could we not handle this quirk inside one of the two dai_link or codec_conf
> functions above?
> The machine driver does not care about this RT1015P_SHARE_EN_SPK quirk,
> it's only used in those two functions so should be set in this scope.
> There's no need to make it visible to the machine driver, is there?
> 

Thank you for the comment; this patch looks terrible... I will improve the code
and upload a v2 patch.

Regards,
Brent