[PATCH] ALSA: hda - restore the gpio led after resume

[PATCH] ALSA: hda - restore the gpio led after resume

[Hui Wang]

On some HP laptops, the mute led is controlled by codec gpio.

When some machine resume from s3/s4, the codec gpio data will be
cleared to 0 by BIOS:
Before suspend:
  IO[3]: enable=1, dir=1, wake=0, sticky=0, data=1, unsol=0
After resume:
  IO[3]: enable=1, dir=1, wake=0, sticky=0, data=0, unsol=0

To skip the AFG node to enter S3 can't fix this problem.

A workaround is to restore the gpio data when the system resume
back from s3/s4. It is safe even on the machines without this
problem.

BugLink: https://bugs.launchpad.net/bugs/1358116
Tested-by: Franz Hsieh <franz.hsieh@canonical.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/pci/hda/patch_realtek.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index b32ce08..9c49bf5 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -3432,6 +3432,12 @@ static unsigned int led_power_filter(struct hda_codec *codec,
 	    (nid != spec->mute_led_nid && nid != spec->cap_mute_led_nid))
 		return power_state;
 
+	if (nid == codec->afg && power_state == AC_PWRST_D0 && spec->gpio_led) {
+		snd_hda_codec_write(codec, codec->afg, 0, AC_VERB_SET_GPIO_DATA,
+			    spec->gpio_led);
+		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));
-- 
1.9.1

Re: [PATCH] ALSA: hda - restore the gpio led after resume

[Takashi Iwai]

At Mon, 18 Aug 2014 15:25:19 +0800,
Hui Wang wrote:
> 
> On some HP laptops, the mute led is controlled by codec gpio.
> 
> When some machine resume from s3/s4, the codec gpio data will be
> cleared to 0 by BIOS:
> Before suspend:
>   IO[3]: enable=1, dir=1, wake=0, sticky=0, data=1, unsol=0
> After resume:
>   IO[3]: enable=1, dir=1, wake=0, sticky=0, data=0, unsol=0
> 
> To skip the AFG node to enter S3 can't fix this problem.
> 
> A workaround is to restore the gpio data when the system resume
> back from s3/s4. It is safe even on the machines without this
> problem.
> 
> BugLink: https://bugs.launchpad.net/bugs/1358116
> Tested-by: Franz Hsieh <franz.hsieh@canonical.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  sound/pci/hda/patch_realtek.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index b32ce08..9c49bf5 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -3432,6 +3432,12 @@ static unsigned int led_power_filter(struct hda_codec *codec,
>  	    (nid != spec->mute_led_nid && nid != spec->cap_mute_led_nid))
>  		return power_state;
>  
> +	if (nid == codec->afg && power_state == AC_PWRST_D0 && spec->gpio_led) {
> +		snd_hda_codec_write(codec, codec->afg, 0, AC_VERB_SET_GPIO_DATA,
> +			    spec->gpio_led);
> +		return power_state;

In general, the power filter is a wrong place to put such a verb
write.  This is called *before* powering up the codec NID.

Yes, I see the pinctl is handled in the code below, and I don't like
this either.  The codec write verb should be at best in the resume
callback after power up.


Takashi

> +	}
> +
>  	/* 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));
> -- 
> 1.9.1
> 

Re: [alsa-devel] [PATCH] ALSA: hda - restore the gpio led after resume

[hwang4]

On 2014年08月18日 16:59, Takashi Iwai wrote:
> At Mon, 18 Aug 2014 15:25:19 +0800,
> Hui Wang wrote:
>> On some HP laptops, the mute led is controlled by codec gpio.
>>
>> When some machine resume from s3/s4, the codec gpio data will be
>> cleared to 0 by BIOS:
>> Before suspend:
>>    IO[3]: enable=1, dir=1, wake=0, sticky=0, data=1, unsol=0
>> After resume:
>>    IO[3]: enable=1, dir=1, wake=0, sticky=0, data=0, unsol=0
>>
>> To skip the AFG node to enter S3 can't fix this problem.
>>
>> A workaround is to restore the gpio data when the system resume
>> back from s3/s4. It is safe even on the machines without this
>> problem.
>>
>> BugLink: https://bugs.launchpad.net/bugs/1358116
>> Tested-by: Franz Hsieh <franz.hsieh@canonical.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>   sound/pci/hda/patch_realtek.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>> index b32ce08..9c49bf5 100644
>> --- a/sound/pci/hda/patch_realtek.c
>> +++ b/sound/pci/hda/patch_realtek.c
>> @@ -3432,6 +3432,12 @@ static unsigned int led_power_filter(struct hda_codec *codec,
>>   	    (nid != spec->mute_led_nid && nid != spec->cap_mute_led_nid))
>>   		return power_state;
>>   
>> +	if (nid == codec->afg && power_state == AC_PWRST_D0 && spec->gpio_led) {
>> +		snd_hda_codec_write(codec, codec->afg, 0, AC_VERB_SET_GPIO_DATA,
>> +			    spec->gpio_led);
>> +		return power_state;
> In general, the power filter is a wrong place to put such a verb
> write.  This is called *before* powering up the codec NID.
Understand now.
>
> Yes, I see the pinctl is handled in the code below, and I don't like
> this either.  The codec write verb should be at best in the resume
> callback after power up.
OK, got it.

Thanks,
Hui.
>
> Takashi
>
>> +	}
>> +
>>   	/* 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));
>> -- 
>> 1.9.1
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
>