linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda/realtek: Enable headphone amp on HP Folio 9480m
@ 2015-07-14 16:40 Keith Packard
  2015-07-14 17:44 ` [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED " Keith Packard
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Packard @ 2015-07-14 16:40 UTC (permalink / raw)
  To: Jaroslav Kysela
  Cc: Takashi Iwai, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel, Keith Packard

This laptop needs GPIO4 pulled high to enable the headphone
amplifier. I modelled the patch on the existing GPIO4 code which pulls
the line low for the same purpose.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 sound/pci/hda/patch_realtek.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 6d01045..9060f1f 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -3436,6 +3436,23 @@ static void alc280_fixup_hp_gpio4(struct hda_codec *codec,
 	}
 }
 
+static void alc280_fixup_hp_gpio4_hp_amp(struct hda_codec *codec,
+					 const struct hda_fixup *fix,
+					 int action)
+{
+	/* Pull GPIO4 high to enable headphone amp */
+	static const struct hda_verb gpio_init[] = {
+		{ 0x01, AC_VERB_SET_GPIO_MASK, 0x10 },
+		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x10 },
+		{ 0x01, AC_VERB_SET_GPIO_DATA, 0x10 },
+		{}
+	};
+
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		snd_hda_add_verbs(codec, gpio_init);
+	}
+}
+
 static void gpio2_mic_hotkey_event(struct hda_codec *codec,
 				   struct hda_jack_callback *event)
 {
@@ -4512,6 +4529,7 @@ enum {
 	ALC286_FIXUP_HP_GPIO_LED,
 	ALC280_FIXUP_HP_GPIO2_MIC_HOTKEY,
 	ALC280_FIXUP_HP_DOCK_PINS,
+	ALC280_FIXUP_HP_GPIO4_HP_AMP,
 	ALC288_FIXUP_DELL_HEADSET_MODE,
 	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC288_FIXUP_DELL_XPS_13_GPIO6,
@@ -5012,6 +5030,10 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC280_FIXUP_HP_GPIO4
 	},
+	[ALC280_FIXUP_HP_GPIO4_HP_AMP] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc280_fixup_hp_gpio4_hp_amp,
+	},
 	[ALC288_FIXUP_DELL_HEADSET_MODE] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_headset_mode_dell_alc288,
@@ -5103,6 +5125,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x22b7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22bf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22cf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x22db, "HP", ALC280_FIXUP_HP_GPIO4_HP_AMP),
 	SND_PCI_QUIRK(0x103c, 0x22dc, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	SND_PCI_QUIRK(0x103c, 0x22fb, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	/* ALC290 */
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-14 16:40 [PATCH] ALSA: hda/realtek: Enable headphone amp on HP Folio 9480m Keith Packard
@ 2015-07-14 17:44 ` Keith Packard
  2015-07-14 19:29   ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Packard @ 2015-07-14 17:44 UTC (permalink / raw)
  To: Jaroslav Kysela
  Cc: Takashi Iwai, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel, Keith Packard

This laptop needs GPIO4 pulled high to enable the headphone amplifier,
and has a mute LED on GPIO3. I modelled the patch on the existing
GPIO4 code which pulls the line low for the same purpose; this time,
the HP amp line is pulled high.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 sound/pci/hda/patch_realtek.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 6d01045..de031f7 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -3436,6 +3436,29 @@ static void alc280_fixup_hp_gpio4(struct hda_codec *codec,
 	}
 }
 
+static void alc280_fixup_hp_gpio4_hp_amp(struct hda_codec *codec,
+					 const struct hda_fixup *fix,
+					 int action)
+{
+	/* Pull GPIO4 high to enable headphone amp */
+	struct alc_spec *spec = codec->spec;
+	static const struct hda_verb gpio_init[] = {
+		{ 0x01, AC_VERB_SET_GPIO_MASK, 0x18 },
+		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x18 },
+		{ 0x01, AC_VERB_SET_GPIO_DATA, 0x10 },
+		{}
+	};
+
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook;
+		spec->gpio_led = 0x10;
+		spec->mute_led_polarity = 0;
+		spec->gpio_mute_led_mask = 0x08;
+		snd_hda_add_verbs(codec, gpio_init);
+		codec->power_filter = led_power_filter;
+	}
+}
+
 static void gpio2_mic_hotkey_event(struct hda_codec *codec,
 				   struct hda_jack_callback *event)
 {
@@ -4512,6 +4535,7 @@ enum {
 	ALC286_FIXUP_HP_GPIO_LED,
 	ALC280_FIXUP_HP_GPIO2_MIC_HOTKEY,
 	ALC280_FIXUP_HP_DOCK_PINS,
+	ALC280_FIXUP_HP_GPIO4_HP_AMP,
 	ALC288_FIXUP_DELL_HEADSET_MODE,
 	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC288_FIXUP_DELL_XPS_13_GPIO6,
@@ -5012,6 +5036,10 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC280_FIXUP_HP_GPIO4
 	},
+	[ALC280_FIXUP_HP_GPIO4_HP_AMP] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc280_fixup_hp_gpio4_hp_amp,
+	},
 	[ALC288_FIXUP_DELL_HEADSET_MODE] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_headset_mode_dell_alc288,
@@ -5103,6 +5131,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x22b7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22bf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22cf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x22db, "HP", ALC280_FIXUP_HP_GPIO4_HP_AMP),
 	SND_PCI_QUIRK(0x103c, 0x22dc, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	SND_PCI_QUIRK(0x103c, 0x22fb, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	/* ALC290 */
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-14 17:44 ` [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED " Keith Packard
@ 2015-07-14 19:29   ` Takashi Iwai
  2015-07-15  2:37     ` Keith Packard
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2015-07-14 19:29 UTC (permalink / raw)
  To: Keith Packard
  Cc: Jaroslav Kysela, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel

On Tue, 14 Jul 2015 19:44:31 +0200,
Keith Packard wrote:
> 
> This laptop needs GPIO4 pulled high to enable the headphone amplifier,
> and has a mute LED on GPIO3. I modelled the patch on the existing
> GPIO4 code which pulls the line low for the same purpose; this time,
> the HP amp line is pulled high.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  sound/pci/hda/patch_realtek.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 6d01045..de031f7 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -3436,6 +3436,29 @@ static void alc280_fixup_hp_gpio4(struct hda_codec *codec,
>  	}
>  }
>  
> +static void alc280_fixup_hp_gpio4_hp_amp(struct hda_codec *codec,
> +					 const struct hda_fixup *fix,
> +					 int action)
> +{
> +	/* Pull GPIO4 high to enable headphone amp */
> +	struct alc_spec *spec = codec->spec;
> +	static const struct hda_verb gpio_init[] = {
> +		{ 0x01, AC_VERB_SET_GPIO_MASK, 0x18 },
> +		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x18 },
> +		{ 0x01, AC_VERB_SET_GPIO_DATA, 0x10 },
> +		{}
> +	};
> +
> +	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> +		spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook;
> +		spec->gpio_led = 0x10;
> +		spec->mute_led_polarity = 0;
> +		spec->gpio_mute_led_mask = 0x08;
> +		snd_hda_add_verbs(codec, gpio_init);
> +		codec->power_filter = led_power_filter;
> +	}
> +}
> +
>  static void gpio2_mic_hotkey_event(struct hda_codec *codec,
>  				   struct hda_jack_callback *event)
>  {
> @@ -4512,6 +4535,7 @@ enum {
>  	ALC286_FIXUP_HP_GPIO_LED,
>  	ALC280_FIXUP_HP_GPIO2_MIC_HOTKEY,
>  	ALC280_FIXUP_HP_DOCK_PINS,
> +	ALC280_FIXUP_HP_GPIO4_HP_AMP,
>  	ALC288_FIXUP_DELL_HEADSET_MODE,
>  	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
>  	ALC288_FIXUP_DELL_XPS_13_GPIO6,
> @@ -5012,6 +5036,10 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC280_FIXUP_HP_GPIO4
>  	},
> +	[ALC280_FIXUP_HP_GPIO4_HP_AMP] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = alc280_fixup_hp_gpio4_hp_amp,
> +	},
>  	[ALC288_FIXUP_DELL_HEADSET_MODE] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = alc_fixup_headset_mode_dell_alc288,
> @@ -5103,6 +5131,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x103c, 0x22b7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
>  	SND_PCI_QUIRK(0x103c, 0x22bf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
>  	SND_PCI_QUIRK(0x103c, 0x22cf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
> +	SND_PCI_QUIRK(0x103c, 0x22db, "HP", ALC280_FIXUP_HP_GPIO4_HP_AMP),
>  	SND_PCI_QUIRK(0x103c, 0x22dc, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
>  	SND_PCI_QUIRK(0x103c, 0x22fb, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
>  	/* ALC290 */

Thanks for the patch.  But this looks suboptimal, unfortunately, since
it keeps the amp always on, and more badly, it would block the power
save of the widget root node.

Can just using gpio_mute_led_mask=0x18 and gpio_led=0 (also drop
AC_VERB_SET_GPIO_DATA in gpio_init[]) work instead?  If GPIO4 is the
the amp, we can associate it with the master mute control together
with the mute LED.  The only concern would be the possible click
noise, but it doesn't happen on most machines.


Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-14 19:29   ` Takashi Iwai
@ 2015-07-15  2:37     ` Keith Packard
  2015-07-15  8:14       ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Packard @ 2015-07-15  2:37 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1098 bytes --]

Takashi Iwai <tiwai@suse.de> writes:

> Thanks for the patch.  But this looks suboptimal, unfortunately, since
> it keeps the amp always on, and more badly, it would block the power
> save of the widget root node.

Thanks very much for your feedback; I wasn't sure precisely how this
code worked and tried to make a change that was as close as I could
manage to existing examples.

> Can just using gpio_mute_led_mask=0x18 and gpio_led=0 (also drop
> AC_VERB_SET_GPIO_DATA in gpio_init[]) work instead?  If GPIO4 is the
> the amp, we can associate it with the master mute control together
> with the mute LED.  The only concern would be the possible click
> noise, but it doesn't happen on most machines.

It's not quite that simple; the GPIO4 value is inverted from the mute
LED value (the amp is powered up when GPIO4 is set).

What I've done is to make the amp powered only when a headphone is
plugged in, and then removed the code which was disabling power saving,
which lets everything (including the amp) get turned back off when the
device goes idle.

Here's a second version of the patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ALSA-hda-realtek-Enable-HP-amp-and-mute-LED-on-HP-Fo.patch --]
[-- Type: text/x-diff, Size: 5528 bytes --]

From 60e2c02d651b0ca6e4b72aa1cab21660400fe2eb Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Tue, 14 Jul 2015 09:30:33 -0700
Subject: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio
 9480m [v2]

This laptop needs GPIO4 pulled high to enable the headphone amplifier,
and has a mute LED on GPIO3. I modelled the patch on the existing
GPIO4 code which pulls the line low for the same purpose; this time,
the HP amp line is pulled high.

v2: Disable the headphone amplifier when no headphone is connected.
    Don't disable power savings to preserve the LED state.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 sound/pci/hda/patch_realtek.c | 112 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 6d01045..621c195 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -99,6 +99,7 @@ struct alc_spec {
 	unsigned int gpio_led; /* used for alc269_fixup_hp_gpio_led() */
 	unsigned int gpio_mute_led_mask;
 	unsigned int gpio_mic_led_mask;
+	unsigned int gpio_hp_amp_mask;
 
 	hda_nid_t headset_mic_pin;
 	hda_nid_t headphone_mic_pin;
@@ -4435,6 +4436,111 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
 	}
 }
 
+/* Set headphone amp power via a GPIO depending on whether the
+ * headphones are plugged in or not
+ */
+static void alc280_set_hp_amp_power(struct hda_codec *codec)
+{
+	struct alc_spec *spec = codec->spec;
+	unsigned int oldval = spec->gpio_led;
+
+	/* Headphone amp enable when headphone present */
+	if (spec->gen.hp_jack_present)
+		spec->gpio_led |= spec->gpio_hp_amp_mask;
+	else
+		spec->gpio_led &= ~spec->gpio_hp_amp_mask;
+
+	codec_dbg(codec,
+		  "set_hp_amp_power present %d oldval %02x current %02x\n",
+		  spec->gen.hp_jack_present,
+		  oldval, spec->gpio_led);
+
+	if (spec->gpio_led != oldval)
+		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA,
+				    spec->gpio_led);
+}
+
+/* Detect headphone connection, then go update the headphone amp
+ * GPIO */
+static void alc280_update_headset_mode(struct hda_codec *codec)
+{
+	alc_update_headset_mode(codec);
+	alc280_set_hp_amp_power(codec);
+}
+
+/* Hook to update headphone amp GPIO on config changes */
+static void
+alc280_update_headset_mode_hook(struct hda_codec *codec,
+				struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	alc280_update_headset_mode(codec);
+}
+
+/* Hook to update amp GPIO for automute */
+static void alc280_update_headset_jack_cb(struct hda_codec *codec,
+					  struct hda_jack_callback *jack)
+{
+	alc_update_headset_jack_cb(codec, jack);
+	alc280_set_hp_amp_power(codec);
+}
+
+/* Manage GPIOs for HP EliteBook Folio 9480m.
+ *
+ * GPIO4 is the headphone amplifier power control
+ * GPIO3 is the audio output mute indicator LED
+ */
+
+static void alc280_fixup_hp_9480m(struct hda_codec *codec,
+				  const struct hda_fixup *fix,
+				  int action)
+{
+	struct alc_spec *spec = codec->spec;
+	static const struct hda_verb gpio_init[] = {
+		{ 0x01, AC_VERB_SET_GPIO_MASK, 0x18 },
+		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x18 },
+		{}
+	};
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+
+		/* Set the hooks to turn the headphone amp on/off
+		 * as needed
+		 */
+		spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook;
+		spec->gen.cap_sync_hook = alc280_update_headset_mode_hook;
+		spec->gen.automute_hook = alc280_update_headset_mode;
+		spec->gen.hp_automute_hook = alc280_update_headset_jack_cb;
+
+		/* The GPIOs are currently off */
+		spec->gpio_led = 0;
+
+		/* GPIO4 controls the headphone amp,
+		 * high is on, low is off
+		 */
+		spec->gpio_hp_amp_mask = 0x10;
+
+		/* GPIO3 is connected to the output mute LED,
+		 * high is on, low is off
+		 */
+		spec->mute_led_polarity = 0;
+		spec->gpio_mute_led_mask = 0x08;
+
+		/* Initialize GPIO configuration */
+		snd_hda_add_verbs(codec, gpio_init);
+		break;
+	case HDA_FIXUP_ACT_INIT:
+
+		/* Force configuration of headphone amp
+		 * GPIO
+		 */
+		spec->current_headset_mode = 0;
+		alc280_update_headset_mode(codec);
+		break;
+	}
+}
+
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
@@ -4512,6 +4618,7 @@ enum {
 	ALC286_FIXUP_HP_GPIO_LED,
 	ALC280_FIXUP_HP_GPIO2_MIC_HOTKEY,
 	ALC280_FIXUP_HP_DOCK_PINS,
+	ALC280_FIXUP_HP_9480M,
 	ALC288_FIXUP_DELL_HEADSET_MODE,
 	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC288_FIXUP_DELL_XPS_13_GPIO6,
@@ -5012,6 +5119,10 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC280_FIXUP_HP_GPIO4
 	},
+	[ALC280_FIXUP_HP_9480M] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc280_fixup_hp_9480m,
+	},
 	[ALC288_FIXUP_DELL_HEADSET_MODE] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_headset_mode_dell_alc288,
@@ -5103,6 +5214,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x22b7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22bf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22cf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x22db, "HP", ALC280_FIXUP_HP_9480M),
 	SND_PCI_QUIRK(0x103c, 0x22dc, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	SND_PCI_QUIRK(0x103c, 0x22fb, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	/* ALC290 */
-- 
2.1.4


[-- Attachment #1.3: Type: text/plain, Size: 15 bytes --]


-- 
-keith

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-15  2:37     ` Keith Packard
@ 2015-07-15  8:14       ` Takashi Iwai
  2015-07-15 19:14         ` Keith Packard
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2015-07-15  8:14 UTC (permalink / raw)
  To: Keith Packard
  Cc: Jaroslav Kysela, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel

On Wed, 15 Jul 2015 04:37:44 +0200,
Keith Packard wrote:
> 
> Takashi Iwai <tiwai@suse.de> writes:
> 
> > Thanks for the patch.  But this looks suboptimal, unfortunately, since
> > it keeps the amp always on, and more badly, it would block the power
> > save of the widget root node.
> 
> Thanks very much for your feedback; I wasn't sure precisely how this
> code worked and tried to make a change that was as close as I could
> manage to existing examples.
> 
> > Can just using gpio_mute_led_mask=0x18 and gpio_led=0 (also drop
> > AC_VERB_SET_GPIO_DATA in gpio_init[]) work instead?  If GPIO4 is the
> > the amp, we can associate it with the master mute control together
> > with the mute LED.  The only concern would be the possible click
> > noise, but it doesn't happen on most machines.
> 
> It's not quite that simple; the GPIO4 value is inverted from the mute
> LED value (the amp is powered up when GPIO4 is set).

Ah, right, there was already a fixup for GPIO4 low, and yours is for
GPIO4 high.

> What I've done is to make the amp powered only when a headphone is
> plugged in, and then removed the code which was disabling power saving,
> which lets everything (including the amp) get turned back off when the
> device goes idle.
> 
> Here's a second version of the patch.

Thanks!  The new patch looks good, but I think we don't have to use
the headset code, as your case looks more like a headphone, not a
combo headset that needs the special handling.  If so, the change can
be reduced something like below.  Could you check whether this is
enough?


Takashi

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -3430,6 +3430,37 @@ static void alc280_fixup_hp_gpio4(struct hda_codec *codec,
 	}
 }
 
+static void alc280_hp_gpio4_hp_automue(struct hda_codec *codec,
+				       struct hda_jack_callback *jack)
+{
+	snd_hda_gen_hp_automute(codec, jack);
+	/* mute_led_polarity is set to 0, so we pass inverted value here */
+	alc_update_gpio_led(codec, 0x10, !spec->gen.hp_jack_present);
+}
+
+/* GPIO3 = mute LED, GPIO4 high = enable headphone amp */
+static void alc280_fixup_hp_gpio4_hp_amp(struct hda_codec *codec,
+					 const struct hda_fixup *fix,
+					 int action)
+{
+	struct alc_spec *spec = codec->spec;
+	static const struct hda_verb gpio_init[] = {
+		{ 0x01, AC_VERB_SET_GPIO_MASK, 0x18 },
+		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x18 },
+		{}
+	};
+
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook;
+		spec->gen.hp_automute_hook = alc280_hp_gpio4_hp_automue;
+		spec->gpio_led = 0;
+		spec->mute_led_polarity = 0;
+		spec->gpio_mute_led_mask = 0x08;
+		snd_hda_add_verbs(codec, gpio_init);
+		codec->power_filter = led_power_filter;
+	}
+}
+

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-15  8:14       ` Takashi Iwai
@ 2015-07-15 19:14         ` Keith Packard
  2015-07-16 10:26           ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Packard @ 2015-07-15 19:14 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 724 bytes --]

Takashi Iwai <tiwai@suse.de> writes:

> Thanks!  The new patch looks good, but I think we don't have to use
> the headset code, as your case looks more like a headphone, not a
> combo headset that needs the special handling.  If so, the change can
> be reduced something like below.  Could you check whether this is
> enough?

Yes, that seems a lot simpler and works fine. I did remove the
led_power_filter setting; that doesn't seem relevant for GPIO-based
leds, and isn't necessary on my device in any case.

This patch is now otherwise essentially the same as yours, with the
addition of comments.

Thanks again for your help; it's always interesting to dive into some
different area of the kernel and see how it works.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ALSA-hda-realtek-Enable-HP-amp-and-mute-LED-on-HP-Fo.patch --]
[-- Type: text/x-diff, Size: 3802 bytes --]

From ca694a8c6d481fd327822dd2adec35e82affb2be Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Tue, 14 Jul 2015 09:30:33 -0700
Subject: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio
 9480m [v3]

This laptop needs GPIO4 pulled high to enable the headphone amplifier,
and has a mute LED on GPIO3. I modelled the patch on the existing
GPIO4 code which pulls the line low for the same purpose; this time,
the HP amp line is pulled high.

v2: Disable the headphone amplifier when no headphone is connected.
    Don't disable power savings to preserve the LED state.

v3: Remove headset-specific hooks and code; this is just a headphone.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 sound/pci/hda/patch_realtek.c | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 6d01045..0e1f8c8 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4435,6 +4435,55 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
 	}
 }
 
+/* Hook to update amp GPIO4 for automute */
+static void alc280_hp_gpio4_automute_hook(struct hda_codec *codec,
+					  struct hda_jack_callback *jack)
+{
+	struct alc_spec *spec = codec->spec;
+
+	snd_hda_gen_hp_automute(codec, jack);
+	/* mute_led_polarity is set to 0, so we pass inverted value here */
+	alc_update_gpio_led(codec, 0x10, !spec->gen.hp_jack_present);
+}
+
+/* Manage GPIOs for HP EliteBook Folio 9480m.
+ *
+ * GPIO4 is the headphone amplifier power control
+ * GPIO3 is the audio output mute indicator LED
+ */
+
+static void alc280_fixup_hp_9480m(struct hda_codec *codec,
+				  const struct hda_fixup *fix,
+				  int action)
+{
+	struct alc_spec *spec = codec->spec;
+	static const struct hda_verb gpio_init[] = {
+		{ 0x01, AC_VERB_SET_GPIO_MASK, 0x18 },
+		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x18 },
+		{}
+	};
+
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		/* Set the hooks to turn the headphone amp on/off
+		 * as needed
+		 */
+		spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook;
+		spec->gen.hp_automute_hook = alc280_hp_gpio4_automute_hook;
+
+		/* The GPIOs are currently off */
+		spec->gpio_led = 0;
+
+		/* GPIO3 is connected to the output mute LED,
+		 * high is on, low is off
+		 */
+		spec->mute_led_polarity = 0;
+		spec->gpio_mute_led_mask = 0x08;
+
+		/* Initialize GPIO configuration */
+		snd_hda_add_verbs(codec, gpio_init);
+	}
+}
+
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
@@ -4512,6 +4561,7 @@ enum {
 	ALC286_FIXUP_HP_GPIO_LED,
 	ALC280_FIXUP_HP_GPIO2_MIC_HOTKEY,
 	ALC280_FIXUP_HP_DOCK_PINS,
+	ALC280_FIXUP_HP_9480M,
 	ALC288_FIXUP_DELL_HEADSET_MODE,
 	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC288_FIXUP_DELL_XPS_13_GPIO6,
@@ -5012,6 +5062,10 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC280_FIXUP_HP_GPIO4
 	},
+	[ALC280_FIXUP_HP_9480M] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc280_fixup_hp_9480m,
+	},
 	[ALC288_FIXUP_DELL_HEADSET_MODE] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_headset_mode_dell_alc288,
@@ -5103,6 +5157,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x22b7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22bf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22cf, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x22db, "HP", ALC280_FIXUP_HP_9480M),
 	SND_PCI_QUIRK(0x103c, 0x22dc, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	SND_PCI_QUIRK(0x103c, 0x22fb, "HP", ALC269_FIXUP_HP_GPIO_MIC1_LED),
 	/* ALC290 */
-- 
2.1.4


[-- Attachment #1.3: Type: text/plain, Size: 15 bytes --]


-- 
-keith

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-15 19:14         ` Keith Packard
@ 2015-07-16 10:26           ` Takashi Iwai
  2015-07-16 14:48             ` Keith Packard
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2015-07-16 10:26 UTC (permalink / raw)
  To: Keith Packard
  Cc: Jaroslav Kysela, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel

On Wed, 15 Jul 2015 21:14:39 +0200,
Keith Packard wrote:
> 
> Takashi Iwai <tiwai@suse.de> writes:
> 
> > Thanks!  The new patch looks good, but I think we don't have to use
> > the headset code, as your case looks more like a headphone, not a
> > combo headset that needs the special handling.  If so, the change can
> > be reduced something like below.  Could you check whether this is
> > enough?
> 
> Yes, that seems a lot simpler and works fine. I did remove the
> led_power_filter setting; that doesn't seem relevant for GPIO-based
> leds, and isn't necessary on my device in any case.
> 
> This patch is now otherwise essentially the same as yours, with the
> addition of comments.
> 
> Thanks again for your help; it's always interesting to dive into some
> different area of the kernel and see how it works.

Great, I queued the patch now.  It'll be included in the next pull
request for 4.2-rc3.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m
  2015-07-16 10:26           ` Takashi Iwai
@ 2015-07-16 14:48             ` Keith Packard
  0 siblings, 0 replies; 8+ messages in thread
From: Keith Packard @ 2015-07-16 14:48 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, Kailang Yang, Hui Wang, David Henningsson,
	alsa-devel, linux-kernel

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

Takashi Iwai <tiwai@suse.de> writes:

> Great, I queued the patch now.  It'll be included in the next pull
> request for 4.2-rc3.

Thanks -- you're an awesome maintainer.

-- 
-keith

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-07-16 14:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-14 16:40 [PATCH] ALSA: hda/realtek: Enable headphone amp on HP Folio 9480m Keith Packard
2015-07-14 17:44 ` [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED " Keith Packard
2015-07-14 19:29   ` Takashi Iwai
2015-07-15  2:37     ` Keith Packard
2015-07-15  8:14       ` Takashi Iwai
2015-07-15 19:14         ` Keith Packard
2015-07-16 10:26           ` Takashi Iwai
2015-07-16 14:48             ` Keith Packard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).