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: <s5hio9m4jxf.wl-tiwai@suse.de>
Date:	Tue, 14 Jul 2015 21:29:32 +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 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@...thp.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
--
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/