Bug 199073

Summary: [Regression 4.14->4.15] Mute LED no longer working on HP laptop (snd_hda_intel)
Product: Drivers Reporter: John Lindgren (john)
Component: Sound(ALSA)Assignee: Jaroslav Kysela (perex)
Status: RESOLVED CODE_FIX    
Severity: normal CC: tiwai
Priority: P1    
Hardware: x86-64   
OS: Linux   
Kernel Version: 4.15.8 Tree: Mainline
Regression: Yes
Attachments: Output from alsa-info.sh
alsa-info with 4.14, unmuted
alsa-info with 4.14, muted
alsa-info with 4.15, unmuted
alsa-info with 4.15, muted
Fix patch
Fix patch (revised)
Partial revert of power_save option behavior
Fix mute-LED during power-saving
Another fix patch
Corrected fix patch
Corrected fix patch

Description John Lindgren 2018-03-10 22:05:07 UTC
Created attachment 274645 [details]
Output from alsa-info.sh

With Linux 4.15.x, the mute LED on this HP laptop (Pavilion 15 ab253cl) stopped working.  When the driver loads, the LED gets stuck either ON or OFF, and muting/unmuting the Master channel does not change the LED.  In 4.14.x the LED worked fine.

Output of alsa-info.sh is attached.  Please let me know if there is any other info I can provide.
Comment 1 John Lindgren 2018-03-10 22:08:43 UTC
In alsamixer, the card shows up as HDA Intel PCH - Realtek ALC3241.  There is a "Mute-LED Mode" switch which can be set to "On", "Off", or "Follow Master", but with no effect (the LED is still stuck ON or OFF).
Comment 2 Takashi Iwai 2018-03-11 08:00:53 UTC
Could you get alsa-info.sh output also from the working kernel, at both mute-LED on and off?  Then we can compare in more details.
Comment 3 John Lindgren 2018-03-11 09:01:08 UTC
Created attachment 274653 [details]
alsa-info with 4.14, unmuted
Comment 4 John Lindgren 2018-03-11 09:01:47 UTC
Created attachment 274655 [details]
alsa-info with 4.14, muted
Comment 5 Takashi Iwai 2018-03-11 09:24:12 UTC
Thanks.  It shows that NID 0x18 VREF is the mute LED.
Could you check the same for non-working 4.15 kernel, with mute LED on and off?
Comment 6 Takashi Iwai 2018-03-11 09:29:54 UTC
Also, try to adjust NID 0x18 VREF dynamically via hda-verb program,

  hda-verb /dev/snd/hwC0D0 0x18 SET_PIN_WID 0x04

and

  hda-verb /dev/snd/hwC0D0 0x18 SET_PIN_WID 0x00

to see whether the mute LED is turned on/off or vice-versa.
Comment 7 John Lindgren 2018-03-11 09:35:56 UTC
Okay, with 4.15, the weirdest thing (which I didn't notice until now) is that the mere act of running alsa-info.sh causes the LED to update to its correct state.

So:
fresh boot, unmuted, LED stuck ON -> run alsa-info.sh, LED turns off
press button to mute, LED stays OFF -> run alsa-info.sh, LED turns on

The also-info output shows the same "Pin-ctls" line as with 4.14, but I will attach it anyway.
Comment 8 John Lindgren 2018-03-11 09:36:37 UTC
Created attachment 274657 [details]
alsa-info with 4.15, unmuted
Comment 9 John Lindgren 2018-03-11 09:37:01 UTC
Created attachment 274659 [details]
alsa-info with 4.15, muted
Comment 10 John Lindgren 2018-03-11 09:39:47 UTC
I can confirm that the LED is turned on by this command:

  sudo hda-verb /dev/snd/hwC0D0 0x18 SET_PIN_WID 0x04

And off by this one:

  sudo hda-verb /dev/snd/hwC0D0 0x18 SET_PIN_WID 0x00

So it seems that ALSA knows how to control the LED all right, it is just missing some kind of trigger when the audio is muted/unmuted.
Comment 11 Takashi Iwai 2018-03-11 11:06:55 UTC
By pressing the mute button, is the mixer mute status really changed?
Check the output of "amixer -c0 contents" before and after the mute button toggle.

If the lack of the sync is the cause, it's possibly something about the runtime PM.  But I don't remember of any crucial changes between 4.14 and 4.15.
A git bisection would be required to spot it, I'm afraid.
Comment 12 John Lindgren 2018-03-11 15:49:14 UTC
The mixer mute status is changed immediately (amixer -c0 contents shows that the value of 'Master Playback Switch' changes between 'on' and 'off').

If audio is playing, the mute LED does change right away.  It's only when no audio is playing that the problem appears.
Comment 13 Takashi Iwai 2018-03-11 17:27:14 UTC
If so, it's likely about the runtime PM.
And now I'm seeing the difference in power_save option of snd-hda-intel module.  It's zero in 4.14 while it's -1 in 4.15.

The value -1 was introduced recently as the "default" value considering the blacklist.  But it might be that this confuses the user-space.

Try to set the value 1 to /sys/module/snd_hda_intel/parameters/power_save on 4.14 kernel:
  echo 1 > /sys/module/snd_hda_intel/parameters/power_save

And try to play and stop.  After one second, this will bring to the similar behavior as 4.15 kernel, I suppose.

If so, the lack of mute LED during power-save is basically a long-standing problem, although the "regression" was brought by power_save parameter change...
Comment 14 Takashi Iwai 2018-03-11 17:44:11 UTC
The patch below should fix the mute LED issue.
But it means that the machine can't reach to the lowest power saving.
Comment 15 Takashi Iwai 2018-03-11 17:44:39 UTC
Created attachment 274663 [details]
Fix patch
Comment 16 John Lindgren 2018-03-11 18:02:22 UTC
Unfortunately the patch does not fix the problem for me.

However, adding "options snd_hda_intel power_save=0" to /etc/modprobe.d does fix the problem.
Comment 17 Takashi Iwai 2018-03-11 19:54:31 UTC
OK, the previous patch was incomplete.  Try the patch below instead.
Comment 18 Takashi Iwai 2018-03-11 19:54:53 UTC
Created attachment 274665 [details]
Fix patch (revised)
Comment 19 Takashi Iwai 2018-03-11 22:03:09 UTC
... or this might not work, either.
And it also sucks to prevent the runtime PM, so it wouldn't be a good fix, in anyway.

Below is another attempt: it partially reverts the power_save option behavior, and no longer set to -1 as default.  Instead a new option is added for controlling the blacklist.  This should make 4.15 back to 4.14-compatible.
Comment 20 Takashi Iwai 2018-03-11 22:03:44 UTC
Created attachment 274671 [details]
Partial revert of power_save option behavior
Comment 21 Takashi Iwai 2018-03-12 15:02:13 UTC
Just to be sure: the patch in comment 20 should make 4.15 behaving compatible as 4.14.  That is, user-space will set power_save to 0 properly again, so you won't see the mute LED issue after that.

This is indeed a regression, and the patch in comment 20 was now merged into my tree, and will be included in the next Linus / stable tree.  So far, so good.

However, the real problem is that the mute LED is gone after the power-saving (aka runtime PM) is enabled.  This has to be addressed.

The patch in comment 18 was considered to cover, but it won't, I guess.
The patch below should work better, hopefully.  Please test it with power_save=1.
Comment 22 Takashi Iwai 2018-03-12 15:02:52 UTC
Created attachment 274687 [details]
Fix mute-LED during power-saving
Comment 23 John Lindgren 2018-03-15 01:18:09 UTC
Unfortunately, the patch in comment 22 still doesn't fix the problem for me, with power_save=1.

> However, the real problem is that the mute LED is gone after the power-saving
> (aka runtime PM) is enabled.  This has to be addressed.

I'm not sure, but there might be some misunderstanding here between us.  Just to be clear, the mute LED doesn't necessarily turn off when power-saving kicks in.  It can get stuck in the "on" state also.  That is, I can turn it on or off at will while audio is playing, and then it will remain stuck in that same state after the audio stops and power-save takes effect.

The patch in comment 22 looks like it's trying to keep some part of the hardware powered when mute is active, to keep the LED on.  This may not be necessary, since apparently the LED is already able to remain on during power-save.  Rather it seems to me that the trigger that ought to be changing the state of the LED, when muted/unmuted, is somehow missed or inhibited during power-save.

I don't really know much about the HW or the driver, so please pardon me if you've already considered this; I just don't want you to be spending time trying to fix something other than the real problem due to my poor communication.
Comment 24 Takashi Iwai 2018-03-15 08:57:33 UTC
Aha, indeed it's a big misunderstanding.  So the LED gets just stuck without changing the actual value during the power save.  Then the fix should be easy.

Try the patch below instead.  It's simpler, just forcibly power up/down at changing the LED status.
Comment 25 Takashi Iwai 2018-03-15 08:58:09 UTC
Created attachment 274747 [details]
Another fix patch
Comment 26 John Lindgren 2018-03-17 17:57:05 UTC
The patch in comment #25 apparently causes the driver to hang.  Any program trying to access the audio device also hangs in an "uninterruptible" state.
Comment 27 Takashi Iwai 2018-03-17 19:41:31 UTC
Sorry, mea culpa, it must be snd_hda_power_up_pm() instead of snd_hda_power_up().
Below is the corrected patch.
Comment 28 Takashi Iwai 2018-03-17 19:42:18 UTC
Created attachment 274793 [details]
Corrected fix patch
Comment 29 Takashi Iwai 2018-03-17 19:44:02 UTC
Created attachment 274795 [details]
Corrected fix patch

The previous one was an incomplete one, sorry, scratch that.
Comment 30 John Lindgren 2018-03-17 21:32:57 UTC
This one (comment 29) works, thank you!
Comment 31 Takashi Iwai 2018-03-20 17:27:44 UTC
The patch was queued to sound.git tree and will be included in the next pull request to Linus.