Ubuntu

Mute LEDs turn off when on battery mode (HP / Realtek)

Reported by David Henningsson on 2013-11-06
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Undecided
David Henningsson

Bug Description

Some HP machines with Realtek codecs have mute LEDs connected to VREF pins.
However when these go into runtime suspend, the pin powers down and its
pin control is disabled, thus disabling the LED too.

Some HP machines with Realtek codecs have mute LEDs connected to VREF pins.
However when these go into runtime suspend, the pin powers down and its
pin control is disabled, thus disabling the LED too.

This patch fixes that issue by making sure that the pin stays in D0 with
correct pin control.

Cc: <email address hidden>
BugLink: https://bugs.launchpad.net/bugs/1248465
Tested-by: Franz Hsieh <email address hidden>
Signed-off-by: David Henningsson <email address hidden>
---
 sound/pci/hda/patch_realtek.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index a51f48c..0acbe82 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -2955,6 +2955,23 @@ static void alc269_fixup_mic_mute_hook(void *private_data, int enabled)
   snd_hda_set_pin_ctl_cache(codec, spec->mute_led_nid, pinval);
 }

+/* Make sure the led works even in runtime suspend */
+static unsigned int led_power_filter(struct hda_codec *codec,
+ hda_nid_t nid,
+ unsigned int power_state)
+{
+ struct alc_spec *spec = codec->spec;
+
+ if (power_state != AC_PWRST_D3 || nid != spec->mute_led_nid)
+ return power_state;
+
+ /* Set pin ctl again, it might have just been set to 0 */
+ snd_hda_set_pin_ctl(codec, nid,
+ snd_hda_codec_get_pin_target(codec, nid));
+
+ return AC_PWRST_D0;
+}
+
 static void alc269_fixup_hp_mute_led(struct hda_codec *codec,
          const struct hda_fixup *fix, int action)
 {
@@ -2974,6 +2991,7 @@ static void alc269_fixup_hp_mute_led(struct hda_codec *codec,
   spec->mute_led_nid = pin - 0x0a + 0x18;
   spec->gen.vmaster_mute.hook = alc269_fixup_mic_mute_hook;
   spec->gen.vmaster_mute_enum = 1;
+ codec->power_filter = led_power_filter;
   snd_printd("Detected mute LED for %x:%d\n", spec->mute_led_nid,
       spec->mute_led_polarity);
   break;
@@ -2989,6 +3007,7 @@ static void alc269_fixup_hp_mute_led_mic1(struct hda_codec *codec,
   spec->mute_led_nid = 0x18;
   spec->gen.vmaster_mute.hook = alc269_fixup_mic_mute_hook;
   spec->gen.vmaster_mute_enum = 1;
+ codec->power_filter = led_power_filter;
  }
 }

@@ -3001,6 +3020,7 @@ static void alc269_fixup_hp_mute_led_mic2(struct hda_codec *codec,
   spec->mute_led_nid = 0x19;
   spec->gen.vmaster_mute.hook = alc269_fixup_mic_mute_hook;
   spec->gen.vmaster_mute_enum = 1;
+ codec->power_filter = led_power_filter;
  }
 }

--
1.7.9.5

Changed in linux (Ubuntu):
status: New → In Progress
assignee: nobody → David Henningsson (diwic)
Takashi Iwai (tiwai) wrote :

At Wed, 6 Nov 2013 10:50:44 +0100,
David Henningsson wrote:
>
> Some HP machines with Realtek codecs have mute LEDs connected to VREF pins.
> However when these go into runtime suspend, the pin powers down and its
> pin control is disabled, thus disabling the LED too.
>
> This patch fixes that issue by making sure that the pin stays in D0 with
> correct pin control.
>
> Cc: <email address hidden>
> BugLink: https://bugs.launchpad.net/bugs/1248465
> Tested-by: Franz Hsieh <email address hidden>
> Signed-off-by: David Henningsson <email address hidden>

Thanks, applied.

I do wonder, though, whether the LED survives even after turning AFG
to D3. With IDT codecs, we had to keep AFG D0 just for keeping this
VREF stuff. Maybe Realtek codecs behave differently.

Takashi

> ---
> sound/pci/hda/patch_realtek.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index a51f48c..0acbe82 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -2955,6 +2955,23 @@ static void alc269_fixup_mic_mute_hook(void *private_data, int enabled)
> snd_hda_set_pin_ctl_cache(codec, spec->mute_led_nid, pinval);
> }
>
> +/* Make sure the led works even in runtime suspend */
> +static unsigned int led_power_filter(struct hda_codec *codec,
> + hda_nid_t nid,
> + unsigned int power_state)
> +{
> + struct alc_spec *spec = codec->spec;
> +
> + if (power_state != AC_PWRST_D3 || nid != spec->mute_led_nid)
> + return power_state;
> +
> + /* Set pin ctl again, it might have just been set to 0 */
> + snd_hda_set_pin_ctl(codec, nid,
> + snd_hda_codec_get_pin_target(codec, nid));
> +
> + return AC_PWRST_D0;
> +}
> +
> static void alc269_fixup_hp_mute_led(struct hda_codec *codec,
> const struct hda_fixup *fix, int action)
> {
> @@ -2974,6 +2991,7 @@ static void alc269_fixup_hp_mute_led(struct hda_codec *codec,
> spec->mute_led_nid = pin - 0x0a + 0x18;
> spec->gen.vmaster_mute.hook = alc269_fixup_mic_mute_hook;
> spec->gen.vmaster_mute_enum = 1;
> + codec->power_filter = led_power_filter;
> snd_printd("Detected mute LED for %x:%d\n", spec->mute_led_nid,
> spec->mute_led_polarity);
> break;
> @@ -2989,6 +3007,7 @@ static void alc269_fixup_hp_mute_led_mic1(struct hda_codec *codec,
> spec->mute_led_nid = 0x18;
> spec->gen.vmaster_mute.hook = alc269_fixup_mic_mute_hook;
> spec->gen.vmaster_mute_enum = 1;
> + codec->power_filter = led_power_filter;
> }
> }
>
> @@ -3001,6 +3020,7 @@ static void alc269_fixup_hp_mute_led_mic2(struct hda_codec *codec,
> spec->mute_led_nid = 0x19;
> spec->gen.vmaster_mute.hook = alc269_fixup_mic_mute_hook;
> spec->gen.vmaster_mute_enum = 1;
> + codec->power_filter = led_power_filter;
> }
> }
>
> --
> 1.7.9.5
>

David Henningsson (diwic) wrote :

On 11/06/2013 11:11 AM, Takashi Iwai wrote:
> At Wed, 6 Nov 2013 10:50:44 +0100,
> David Henningsson wrote:
>>
>> Some HP machines with Realtek codecs have mute LEDs connected to VREF pins.
>> However when these go into runtime suspend, the pin powers down and its
>> pin control is disabled, thus disabling the LED too.
>>
>> This patch fixes that issue by making sure that the pin stays in D0 with
>> correct pin control.
>>
>> Cc: <email address hidden>
>> BugLink: https://bugs.launchpad.net/bugs/1248465
>> Tested-by: Franz Hsieh <email address hidden>
>> Signed-off-by: David Henningsson <email address hidden>
>
> Thanks, applied.
>
> I do wonder, though, whether the LED survives even after turning AFG
> to D3. With IDT codecs, we had to keep AFG D0 just for keeping this
> VREF stuff. Maybe Realtek codecs behave differently.

Well, the patch was tested by Franz and found working, so it seems
indeed that AFG can be in D3 and the LED still works.

(Actually it could even be the other way around - maybe even the pin can
remain in D3 and still have vref output, as long as the pin ctl is set.
I never tested that explicitly.)

--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Changed in linux (Ubuntu):
status: In Progress → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux - 3.12.0-5.13

---------------
linux (3.12.0-5.13) trusty; urgency=low

  [ Tim Gardner ]

  * Rebase to v3.12.2
  * Release tracker
    - LP: #1257003

  [ Upstream Kernel Changes ]

  * rebase to v3.12.2
    - LP: #1253636
    - LP: #1250377
    - LP: #1248465
 -- Tim Gardner <email address hidden> Mon, 02 Dec 2013 09:26:35 -0700

Changed in linux (Ubuntu):
status: Fix Committed → Fix Released
Brad Figg (brad-figg) wrote :

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-saucy' to 'verification-done-saucy'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-saucy
Brad Figg (brad-figg) on 2014-02-11
tags: added: verification-done-saucy
removed: verification-needed-saucy
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers