diff mbox series

ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

Message ID 20210313134038.5577-1-brent.lu@intel.com
State New
Headers show
Series ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support | expand

Commit Message

Brent Lu March 13, 2021, 1:40 p.m. UTC
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(-)

Comments

Pierre-Louis Bossart March 15, 2021, 2:26 p.m. UTC | #1
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);
Brent Lu March 16, 2021, 10 a.m. UTC | #2
> 

> 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
diff mbox series

Patch

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",