[PATCH 0/6] ASoC: Intel: bytcr_rt5640: Fix HP ElitePad 1000 G2 audio routing

[PATCH 0/6] ASoC: Intel: bytcr_rt5640: Fix HP ElitePad 1000 G2 audio routing

[Hans de Goede]

The HP Elitepad 1000 G2 has 2 headset jacks:

1. on the dock which uses the output of the codecs built-in HP-amp +
the standard IN2 input which is always used with the headset-jack.

2. on the tablet itself, this uses the line-out of the codec, combined
with an external HP-amp + IN1 for the headset-mic.

This series adds support for this, resolving:
https://bugzilla.kernel.org/show_bug.cgi?id=213415

Note this series does not add jack-detect support. I plan to add that
with a follow-up series when I can make some time to implement that.

Regards,

Hans


Hans de Goede (6):
  ASoC: Intel: bytcr_rt5640: Move "Platform Clock" routes to the maps
    for the matching in-/output
  ASoC: Intel: bytcr_rt5640: Add line-out support
  ASoC: Intel: bytcr_rt5640: Add a byt_rt5640_get_codec_dai() helper
  ASoC: Intel: bytcr_rt5640: Add support for a second headphones output
  ASoC: Intel: bytcr_rt5640: Add support for a second headset mic input
  ASoC: Intel: bytcr_rt5640: Fix HP ElitePad 1000 G2 quirk

 sound/soc/intel/boards/bytcr_rt5640.c | 116 ++++++++++++++++++++++----
 1 file changed, 100 insertions(+), 16 deletions(-)

-- 
2.31.1

[PATCH 1/6] ASoC: Intel: bytcr_rt5640: Move "Platform Clock" routes to the maps for the matching in-/output

[Hans de Goede]

Move the "Platform Clock" routes for the "Internal Mic" and "Speaker"
routes to the intmic_*_map[] / *_spk_map[] arrays.

This ensures that these "Platform Clock" routes do not get added when the
BYT_RT5640_NO_INTERNAL_MIC_MAP / BYT_RT5640_NO_SPEAKERS quirks are used.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 91a6d712eb58..c403fb672594 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -290,9 +290,6 @@ static const struct snd_soc_dapm_widget byt_rt5640_widgets[] = {
 static const struct snd_soc_dapm_route byt_rt5640_audio_map[] = {
 	{"Headphone", NULL, "Platform Clock"},
 	{"Headset Mic", NULL, "Platform Clock"},
-	{"Internal Mic", NULL, "Platform Clock"},
-	{"Speaker", NULL, "Platform Clock"},
-
 	{"Headset Mic", NULL, "MICBIAS1"},
 	{"IN2P", NULL, "Headset Mic"},
 	{"Headphone", NULL, "HPOL"},
@@ -300,19 +297,23 @@ static const struct snd_soc_dapm_route byt_rt5640_audio_map[] = {
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_intmic_dmic1_map[] = {
+	{"Internal Mic", NULL, "Platform Clock"},
 	{"DMIC1", NULL, "Internal Mic"},
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_intmic_dmic2_map[] = {
+	{"Internal Mic", NULL, "Platform Clock"},
 	{"DMIC2", NULL, "Internal Mic"},
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_intmic_in1_map[] = {
+	{"Internal Mic", NULL, "Platform Clock"},
 	{"Internal Mic", NULL, "MICBIAS1"},
 	{"IN1P", NULL, "Internal Mic"},
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_intmic_in3_map[] = {
+	{"Internal Mic", NULL, "Platform Clock"},
 	{"Internal Mic", NULL, "MICBIAS1"},
 	{"IN3P", NULL, "Internal Mic"},
 };
@@ -354,6 +355,7 @@ static const struct snd_soc_dapm_route byt_rt5640_ssp0_aif2_map[] = {
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_stereo_spk_map[] = {
+	{"Speaker", NULL, "Platform Clock"},
 	{"Speaker", NULL, "SPOLP"},
 	{"Speaker", NULL, "SPOLN"},
 	{"Speaker", NULL, "SPORP"},
@@ -361,6 +363,7 @@ static const struct snd_soc_dapm_route byt_rt5640_stereo_spk_map[] = {
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_mono_spk_map[] = {
+	{"Speaker", NULL, "Platform Clock"},
 	{"Speaker", NULL, "SPOLP"},
 	{"Speaker", NULL, "SPOLN"},
 };
-- 
2.31.1

[PATCH 2/6] ASoC: Intel: bytcr_rt5640: Add line-out support

[Hans de Goede]

Add support for boards which use the codecs Line Out output, this can
be enabled by using the newly added BYT_RT5640_LINEOUT quirk.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 31 +++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index c403fb672594..e8a8f6b5ef96 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -73,6 +73,7 @@ enum {
 #define BYT_RT5640_MCLK_EN		BIT(22)
 #define BYT_RT5640_MCLK_25MHZ		BIT(23)
 #define BYT_RT5640_NO_SPEAKERS		BIT(24)
+#define BYT_RT5640_LINEOUT		BIT(25)
 
 #define BYTCR_INPUT_DEFAULTS				\
 	(BYT_RT5640_IN3_MAP |				\
@@ -139,6 +140,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk MONO_SPEAKER enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_NO_SPEAKERS)
 		dev_info(dev, "quirk NO_SPEAKERS enabled\n");
+	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
+		dev_info(dev, "quirk LINEOUT enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_DIFF_MIC)
 		dev_info(dev, "quirk DIFF_MIC enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF1) {
@@ -281,10 +284,10 @@ static const struct snd_soc_dapm_widget byt_rt5640_widgets[] = {
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
 	SND_SOC_DAPM_MIC("Internal Mic", NULL),
 	SND_SOC_DAPM_SPK("Speaker", NULL),
+	SND_SOC_DAPM_LINE("Line Out", NULL),
 	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
 			    platform_clock_control, SND_SOC_DAPM_PRE_PMU |
 			    SND_SOC_DAPM_POST_PMD),
-
 };
 
 static const struct snd_soc_dapm_route byt_rt5640_audio_map[] = {
@@ -368,11 +371,18 @@ static const struct snd_soc_dapm_route byt_rt5640_mono_spk_map[] = {
 	{"Speaker", NULL, "SPOLN"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5640_lineout_map[] = {
+	{"Line Out", NULL, "Platform Clock"},
+	{"Line Out", NULL, "LOUTR"},
+	{"Line Out", NULL, "LOUTL"},
+};
+
 static const struct snd_kcontrol_new byt_rt5640_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
 	SOC_DAPM_PIN_SWITCH("Internal Mic"),
 	SOC_DAPM_PIN_SWITCH("Speaker"),
+	SOC_DAPM_PIN_SWITCH("Line Out"),
 };
 
 static struct snd_soc_jack_pin rt5640_pins[] = {
@@ -1056,6 +1066,14 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 	if (ret)
 		return ret;
 
+	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5640_lineout_map,
+					ARRAY_SIZE(byt_rt5640_lineout_map));
+		if (ret)
+			return ret;
+	}
+
 	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 		/*
 		 * The firmware might enable the clock at
@@ -1221,7 +1239,7 @@ static char byt_rt5640_codec_name[SND_ACPI_I2C_ID_LEN];
 #if !IS_ENABLED(CONFIG_SND_SOC_INTEL_USER_FRIENDLY_LONG_NAMES)
 static char byt_rt5640_long_name[40]; /* = "bytcr-rt5640-*-spk-*-mic" */
 #endif
-static char byt_rt5640_components[32]; /* = "cfg-spk:* cfg-mic:*" */
+static char byt_rt5640_components[64]; /* = "cfg-spk:* cfg-mic:* ..." */
 
 static int byt_rt5640_suspend(struct snd_soc_card *card)
 {
@@ -1291,6 +1309,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	static const char * const map_name[] = { "dmic1", "dmic2", "in1", "in3", "none" };
 	__maybe_unused const char *spk_type;
 	const struct dmi_system_id *dmi_id;
+	const char *lineout_string = "";
 	struct byt_rt5640_private *priv;
 	struct snd_soc_acpi_mach *mach;
 	const char *platform_name;
@@ -1453,9 +1472,13 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		spk_type = "stereo";
 	}
 
+	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
+		lineout_string = " cfg-lineout:1";
+
 	snprintf(byt_rt5640_components, sizeof(byt_rt5640_components),
-		 "cfg-spk:%d cfg-mic:%s aif:%d", cfg_spk,
-		 map_name[BYT_RT5640_MAP(byt_rt5640_quirk)], aif);
+		 "cfg-spk:%d cfg-mic:%s aif:%d%s", cfg_spk,
+		 map_name[BYT_RT5640_MAP(byt_rt5640_quirk)], aif,
+		 lineout_string);
 	byt_rt5640_card.components = byt_rt5640_components;
 #if !IS_ENABLED(CONFIG_SND_SOC_INTEL_USER_FRIENDLY_LONG_NAMES)
 	snprintf(byt_rt5640_long_name, sizeof(byt_rt5640_long_name),
-- 
2.31.1

[PATCH 3/6] ASoC: Intel: bytcr_rt5640: Add a byt_rt5640_get_codec_dai() helper

[Hans de Goede]

Add a byt_rt5640_get_codec_dai() helper, which gets the codec_dai
from a dapm_context.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index e8a8f6b5ef96..70faba13450c 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -227,6 +227,20 @@ static int byt_rt5640_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
 #define BYT_CODEC_DAI1	"rt5640-aif1"
 #define BYT_CODEC_DAI2	"rt5640-aif2"
 
+static struct snd_soc_dai *byt_rt5640_get_codec_dai(struct snd_soc_dapm_context *dapm)
+{
+	struct snd_soc_card *card = dapm->card;
+	struct snd_soc_dai *codec_dai;
+
+	codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI1);
+	if (!codec_dai)
+		codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI2);
+	if (!codec_dai)
+		dev_err(card->dev, "Error codec dai not found\n");
+
+	return codec_dai;
+}
+
 static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				  struct snd_kcontrol *k, int  event)
 {
@@ -236,15 +250,9 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
 	int ret;
 
-	codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI1);
+	codec_dai = byt_rt5640_get_codec_dai(dapm);
 	if (!codec_dai)
-		codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI2);
-
-	if (!codec_dai) {
-		dev_err(card->dev,
-			"Codec dai not found; Unable to set platform clock\n");
 		return -EIO;
-	}
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
 		if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
-- 
2.31.1

[PATCH 4/6] ASoC: Intel: bytcr_rt5640: Add support for a second headphones output

[Hans de Goede]

Some devices (HP Elitepad 1000 G2) have a second headphones output
(1 on the dock, 2nd on the tablet itself) which is implemented through
the line-out output of the codec combined with an external hp-amp
which gets enabled through the codec's GPIO1 pin.

Add support for this through a new BYT_RT5640_LINEOUT_AS_HP2 quirk.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 36 +++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 70faba13450c..51fb44ad9b4b 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -74,6 +74,7 @@ enum {
 #define BYT_RT5640_MCLK_25MHZ		BIT(23)
 #define BYT_RT5640_NO_SPEAKERS		BIT(24)
 #define BYT_RT5640_LINEOUT		BIT(25)
+#define BYT_RT5640_LINEOUT_AS_HP2	BIT(26)
 
 #define BYTCR_INPUT_DEFAULTS				\
 	(BYT_RT5640_IN3_MAP |				\
@@ -142,6 +143,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk NO_SPEAKERS enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
 		dev_info(dev, "quirk LINEOUT enabled\n");
+	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2)
+		dev_info(dev, "quirk LINEOUT_AS_HP2 enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_DIFF_MIC)
 		dev_info(dev, "quirk DIFF_MIC enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF1) {
@@ -287,12 +290,39 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static int byt_rt5640_event_lineout(struct snd_soc_dapm_widget *w,
+			struct snd_kcontrol *k, int event)
+{
+	unsigned int gpio_ctrl3_val = RT5640_GP1_PF_OUT;
+	struct snd_soc_dai *codec_dai;
+
+	if (!(byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2))
+		return 0;
+
+	/*
+	 * On devices which use line-out as a second headphones output,
+	 * the codec's GPIO1 pin is used to enable an external HP-amp.
+	 */
+
+	codec_dai = byt_rt5640_get_codec_dai(w->dapm);
+	if (!codec_dai)
+		return -EIO;
+
+	if (SND_SOC_DAPM_EVENT_ON(event))
+		gpio_ctrl3_val |= RT5640_GP1_OUT_HI;
+
+	snd_soc_component_update_bits(codec_dai->component, RT5640_GPIO_CTRL3,
+		RT5640_GP1_PF_MASK | RT5640_GP1_OUT_MASK, gpio_ctrl3_val);
+
+	return 0;
+}
+
 static const struct snd_soc_dapm_widget byt_rt5640_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
 	SND_SOC_DAPM_MIC("Internal Mic", NULL),
 	SND_SOC_DAPM_SPK("Speaker", NULL),
-	SND_SOC_DAPM_LINE("Line Out", NULL),
+	SND_SOC_DAPM_LINE("Line Out", byt_rt5640_event_lineout),
 	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
 			    platform_clock_control, SND_SOC_DAPM_PRE_PMU |
 			    SND_SOC_DAPM_POST_PMD),
@@ -1480,7 +1510,9 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		spk_type = "stereo";
 	}
 
-	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
+	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2)
+		lineout_string = " cfg-hp2:lineout";
+	else if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
 		lineout_string = " cfg-lineout:1";
 
 	snprintf(byt_rt5640_components, sizeof(byt_rt5640_components),
-- 
2.31.1

[PATCH 5/6] ASoC: Intel: bytcr_rt5640: Add support for a second headset mic input

[Hans de Goede]

Some devices (HP Elitepad 1000 G2) have 2 headset jacks (1 on the dock,
2nd on the tablet itself). The 2nd headset mic input on these is
connected to in1 (the internal mics on the HP Elitepad 1000 G2 use DMIC2).

Add support for this through a new BYT_RT5640_HSMIC2_ON_IN1 quirk.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 51fb44ad9b4b..7bf848a07553 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -75,6 +75,7 @@ enum {
 #define BYT_RT5640_NO_SPEAKERS		BIT(24)
 #define BYT_RT5640_LINEOUT		BIT(25)
 #define BYT_RT5640_LINEOUT_AS_HP2	BIT(26)
+#define BYT_RT5640_HSMIC2_ON_IN1	BIT(27)
 
 #define BYTCR_INPUT_DEFAULTS				\
 	(BYT_RT5640_IN3_MAP |				\
@@ -127,6 +128,8 @@ static void log_quirks(struct device *dev)
 		dev_err(dev, "quirk map 0x%x is not supported, microphone input will not work\n", map);
 		break;
 	}
+	if (byt_rt5640_quirk & BYT_RT5640_HSMIC2_ON_IN1)
+		dev_info(dev, "quirk HSMIC2_ON_IN1 enabled\n");
 	if (BYT_RT5640_JDSRC(byt_rt5640_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5640_JDSRC(byt_rt5640_quirk));
@@ -1072,6 +1075,14 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 	if (ret)
 		return ret;
 
+	if (byt_rt5640_quirk & BYT_RT5640_HSMIC2_ON_IN1) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5640_intmic_in1_map,
+					ARRAY_SIZE(byt_rt5640_intmic_in1_map));
+		if (ret)
+			return ret;
+	}
+
 	if (byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) {
 		ret = snd_soc_dapm_add_routes(&card->dapm,
 					byt_rt5640_ssp2_aif2_map,
@@ -1347,6 +1358,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	static const char * const map_name[] = { "dmic1", "dmic2", "in1", "in3", "none" };
 	__maybe_unused const char *spk_type;
 	const struct dmi_system_id *dmi_id;
+	const char *headset2_string = "";
 	const char *lineout_string = "";
 	struct byt_rt5640_private *priv;
 	struct snd_soc_acpi_mach *mach;
@@ -1515,10 +1527,13 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	else if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
 		lineout_string = " cfg-lineout:1";
 
+	if (byt_rt5640_quirk & BYT_RT5640_HSMIC2_ON_IN1)
+		headset2_string = " cfg-hs2:in1";
+
 	snprintf(byt_rt5640_components, sizeof(byt_rt5640_components),
-		 "cfg-spk:%d cfg-mic:%s aif:%d%s", cfg_spk,
+		 "cfg-spk:%d cfg-mic:%s aif:%d%s%s", cfg_spk,
 		 map_name[BYT_RT5640_MAP(byt_rt5640_quirk)], aif,
-		 lineout_string);
+		 lineout_string, headset2_string);
 	byt_rt5640_card.components = byt_rt5640_components;
 #if !IS_ENABLED(CONFIG_SND_SOC_INTEL_USER_FRIENDLY_LONG_NAMES)
 	snprintf(byt_rt5640_long_name, sizeof(byt_rt5640_long_name),
-- 
2.31.1

[PATCH 6/6] ASoC: Intel: bytcr_rt5640: Fix HP ElitePad 1000 G2 quirk

[Hans de Goede]

The HP Elitepad 1000 G2 has 2 headset jacks:

1. on the dock which uses the output of the codecs built-in HP-amp +
the standard IN2 input which is always used with the headset-jack.

2. on the tablet itself, this uses the line-out of the codec, combined
with an external HP-amp + IN1 for the headset-mic.

Fix the HP ElitePad 1000 G2 to properly reflect this now that the
machine-driver supports this setup.

Note this also changes the mapping for the internal mic. from
IN1 (which was pointing to the 2nd headset-jack mic) to DMIC2
which is the actual input for the internal mics.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213415
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 7bf848a07553..32afeaf6056b 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -644,8 +644,11 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "HP ElitePad 1000 G2"),
 		},
-		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
-					BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_DMIC2_MAP |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_LINEOUT |
+					BYT_RT5640_LINEOUT_AS_HP2 |
+					BYT_RT5640_HSMIC2_ON_IN1),
 	},
 	{	/* HP Pavilion x2 10-k0XX, 10-n0XX */
 		.matches = {
-- 
2.31.1

Re: [PATCH 4/6] ASoC: Intel: bytcr_rt5640: Add support for a second headphones output

[Pierre-Louis Bossart]

On 8/1/21 4:04 PM, Hans de Goede wrote:
> Some devices (HP Elitepad 1000 G2) have a second headphones output
> (1 on the dock, 2nd on the tablet itself) which is implemented through
> the line-out output of the codec combined with an external hp-amp
> which gets enabled through the codec's GPIO1 pin.
> 
> Add support for this through a new BYT_RT5640_LINEOUT_AS_HP2 quirk.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  sound/soc/intel/boards/bytcr_rt5640.c | 36 +++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> index 70faba13450c..51fb44ad9b4b 100644
> --- a/sound/soc/intel/boards/bytcr_rt5640.c
> +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> @@ -74,6 +74,7 @@ enum {
>  #define BYT_RT5640_MCLK_25MHZ		BIT(23)
>  #define BYT_RT5640_NO_SPEAKERS		BIT(24)
>  #define BYT_RT5640_LINEOUT		BIT(25)
> +#define BYT_RT5640_LINEOUT_AS_HP2	BIT(26)

The definitions aren't fully clear to me. It seems that the two quirks
above are mutually exclusive if I read the code below

+	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2)
+		lineout_string = " cfg-hp2:lineout";
+	else if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
 		lineout_string = " cfg-lineout:1";

But in the following patch the two are mixed:

+		.driver_data = (void *)(BYT_RT5640_DMIC2_MAP |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_LINEOUT |
+					BYT_RT5640_LINEOUT_AS_HP2 |
+					BYT_RT5640_HSMIC2_ON_IN1),

so maybe the test above should be

if (byt_rt5640_quirk & BYT_RT5640_LINEOUT) {
	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2)
		lineout_string = " cfg-hp2:lineout";
	else
 		lineout_string = " cfg-lineout:1";
}


I am also not very clear on the hp2 support, is there any sort of jack
detection or will this require always an explicit user choice?

Re: [PATCH 4/6] ASoC: Intel: bytcr_rt5640: Add support for a second headphones output

[Hans de Goede]

Hi Pierre-Louis,

On 8/2/21 3:45 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 8/1/21 4:04 PM, Hans de Goede wrote:
>> Some devices (HP Elitepad 1000 G2) have a second headphones output
>> (1 on the dock, 2nd on the tablet itself) which is implemented through
>> the line-out output of the codec combined with an external hp-amp
>> which gets enabled through the codec's GPIO1 pin.
>>
>> Add support for this through a new BYT_RT5640_LINEOUT_AS_HP2 quirk.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  sound/soc/intel/boards/bytcr_rt5640.c | 36 +++++++++++++++++++++++++--
>>  1 file changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
>> index 70faba13450c..51fb44ad9b4b 100644
>> --- a/sound/soc/intel/boards/bytcr_rt5640.c
>> +++ b/sound/soc/intel/boards/bytcr_rt5640.c
>> @@ -74,6 +74,7 @@ enum {
>>  #define BYT_RT5640_MCLK_25MHZ		BIT(23)
>>  #define BYT_RT5640_NO_SPEAKERS		BIT(24)
>>  #define BYT_RT5640_LINEOUT		BIT(25)
>> +#define BYT_RT5640_LINEOUT_AS_HP2	BIT(26)
> 
> The definitions aren't fully clear to me. It seems that the two quirks
> above are mutually exclusive if I read the code below
> 
> +	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2)
> +		lineout_string = " cfg-hp2:lineout";
> +	else if (byt_rt5640_quirk & BYT_RT5640_LINEOUT)
>  		lineout_string = " cfg-lineout:1";
> 
> But in the following patch the two are mixed:
> 
> +		.driver_data = (void *)(BYT_RT5640_DMIC2_MAP |
> +					BYT_RT5640_MCLK_EN |
> +					BYT_RT5640_LINEOUT |
> +					BYT_RT5640_LINEOUT_AS_HP2 |
> +					BYT_RT5640_HSMIC2_ON_IN1),
> 
> so maybe the test above should be
> 
> if (byt_rt5640_quirk & BYT_RT5640_LINEOUT) {
> 	if (byt_rt5640_quirk & BYT_RT5640_LINEOUT_AS_HP2)
> 		lineout_string = " cfg-hp2:lineout";
> 	else
>  		lineout_string = " cfg-lineout:1";
> }

Right, I decided to first add plain line-out support (which may be
useful elsewhere) and then to build the hp2-out using external-amp connected
to lineout on top of that. So your suggestion to change the test to set
the lineout_string makes sense. I'll fix that for v2 of the patch-series.

> I am also not very clear on the hp2 support, is there any sort of jack
> detection or will this require always an explicit user choice?

Quoting from:

"""
I've also figured out how jack-detect works, since the codec's GPIO1 is used for the external-hp-amp enable, the jack-detect signals are directly connected to the Bay Trail SoC's GPIOs:

  -gpioget 'INT33FC:02' 14 && gpioget 'INT33FC:00' 0 && gpioget 'INT33FC:00' 3
   Nothing inserted: 1 1 0
   Headset  in dock: 0 1 0
   Headphon in dock: 0 1 1
   Headset  in tabl: 1 0 0
   Headphon in tabl: 1 0 0
   Conclusion:
   GPO2 pin 14: !jack in dock
   GPO0 pin  0: !jack in tablet
   GPO0 pin  3: 1 when jack in dock with no mic

And I believe that the mic on the tablet jack can be detected using the normal micBIAS over current detection which is normally also used.

This will require some special jack-detect handling inside the machine driver for this model. I plan to also add support for this when I have some time to work on this.
"""

So ATM this requires an explicit user-choice, but I plan to add support for
jack-detect on both jacks when I've some more time to work on this.

Regards,

Hans

Re: [PATCH 4/6] ASoC: Intel: bytcr_rt5640: Add support for a second headphones output

[Pierre-Louis Bossart]

> I've also figured out how jack-detect works, since the codec's GPIO1 is used for the external-hp-amp enable, the jack-detect signals are directly connected to the Bay Trail SoC's GPIOs:
> 
>   -gpioget 'INT33FC:02' 14 && gpioget 'INT33FC:00' 0 && gpioget 'INT33FC:00' 3
>    Nothing inserted: 1 1 0
>    Headset  in dock: 0 1 0
>    Headphon in dock: 0 1 1
>    Headset  in tabl: 1 0 0
>    Headphon in tabl: 1 0 0
>    Conclusion:
>    GPO2 pin 14: !jack in dock
>    GPO0 pin  0: !jack in tablet
>    GPO0 pin  3: 1 when jack in dock with no mic

I am a bit confused about the logic. Could you have a case with 1 1 1
(separate headphones in tablet and dock jacks)?

> 
> And I believe that the mic on the tablet jack can be detected using the normal micBIAS over current detection which is normally also used.
> 
> This will require some special jack-detect handling inside the machine driver for this model. I plan to also add support for this when I have some time to work on this.
> """
> 
> So ATM this requires an explicit user-choice, but I plan to add support for
> jack-detect on both jacks when I've some more time to work on this.

Thanks!

Re: [PATCH 4/6] ASoC: Intel: bytcr_rt5640: Add support for a second headphones output

[Hans de Goede]

Hi,

On 8/2/21 4:35 PM, Pierre-Louis Bossart wrote:
> 
> 
>> I've also figured out how jack-detect works, since the codec's GPIO1 is used for the external-hp-amp enable, the jack-detect signals are directly connected to the Bay Trail SoC's GPIOs:
>>
>>   -gpioget 'INT33FC:02' 14 && gpioget 'INT33FC:00' 0 && gpioget 'INT33FC:00' 3
>>    Nothing inserted: 1 1 0
>>    Headset  in dock: 0 1 0
>>    Headphon in dock: 0 1 1
>>    Headset  in tabl: 1 0 0
>>    Headphon in tabl: 1 0 0
>>    Conclusion:
>>    GPO2 pin 14: !jack in dock
>>    GPO0 pin  0: !jack in tablet
>>    GPO0 pin  3: 1 when jack in dock with no mic
> 
> I am a bit confused about the logic. Could you have a case with 1 1 1
> (separate headphones in tablet and dock jacks)?

1 1 1 is not possible since 1 in column 0 + 1 means nothing inserted, iow
the jack-detect is active-low; and when nothing is inserted then the last
column is always 0.

With that said yes headphones in the dock + something in the tablet-jack should
be detectable as a new combination not in the table, this should give us
"0 0 x" (I did not test that yet, but the 2 jacks should be fully independent).
IOW we will end up with 2 separate snd_soc_jack-s which together register
4 pins. E.g. the declaration for the pins will look like this:

static struct snd_soc_jack_pin rt5640_pins[] = {
        {
                .pin    = "Headphone",
                .mask   = SND_JACK_HEADPHONE,
        },
        {
                .pin    = "Headset Mic",
                .mask   = SND_JACK_MICROPHONE,
        },
};

static struct snd_soc_jack_pin rt5640_pins2[] = {
        {
                .pin    = "Headphone 2",
                .mask   = SND_JACK_HEADPHONE,
        },
        {
                .pin    = "Headset Mic 2",
                .mask   = SND_JACK_MICROPHONE,
        },
};

And there will be 2 snd_soc_jack_add_gpio calls each using
1 of the set of pins (assuming I can use bias over-current
detect to differentiate between headphones/headset on the
jack on the tablet).

As you said in your other email "this is an interesting hardware setup"
I hope I won't hit any userspace issues when I have the kernel code
ready to register 2 jacks.

Regards,

Hans