linux-kernel - Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hd1zt4z3b.wl-tiwai@suse.de>
Date:	Wed, 15 Jul 2015 10:14:16 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Keith Packard <keithp@...thp.com>
Cc:	Jaroslav Kysela <perex@...ex.cz>,
	Kailang Yang <kailang@...ltek.com>,
	Hui Wang <hui.wang@...onical.com>,
	David Henningsson <david.henningsson@...onical.com>,
	alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ALSA: hda/realtek: Enable HP amp and mute LED on HP Folio 9480m

On Wed, 15 Jul 2015 04:37:44 +0200,
Keith Packard wrote:
> 
> Takashi Iwai <tiwai@...e.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;
+	}
+}
+
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/