All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
@ 2020-08-10  2:16 Hui Wang
  2020-08-10  6:30 ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Hui Wang @ 2020-08-10  2:16 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: stable

After installing the Ubuntu Linux, the micmute led status is not
correct. Users expect that the led is on if the capture is disabled,
but with the current kernel, the led is off with the capture disabled.

We tried the old linux kernel like linux-4.15, there is no this issue.
It looks like we introduced this issue when switching to the led_cdev.

Cc: <stable@vger.kernel.org>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/pci/hda/patch_realtek.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index daedcc0adc21..09d93dd88713 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4414,6 +4414,7 @@ static void alc233_fixup_lenovo_line2_mic_hotkey(struct hda_codec *codec,
 {
 	struct alc_spec *spec = codec->spec;
 
+	spec->micmute_led_polarity = 1;
 	alc_fixup_hp_gpio_led(codec, action, 0, 0x04);
 	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
 		spec->init_amp = ALC_INIT_DEFAULT;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-10  2:16 [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO Hui Wang
@ 2020-08-10  6:30 ` Takashi Iwai
  2020-08-10  6:34   ` Hui Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2020-08-10  6:30 UTC (permalink / raw)
  To: Hui Wang; +Cc: alsa-devel, stable

On Mon, 10 Aug 2020 04:16:59 +0200,
Hui Wang wrote:
> 
> After installing the Ubuntu Linux, the micmute led status is not
> correct. Users expect that the led is on if the capture is disabled,
> but with the current kernel, the led is off with the capture disabled.
> 
> We tried the old linux kernel like linux-4.15, there is no this issue.
> It looks like we introduced this issue when switching to the led_cdev.

The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
Which value does it have now?  If it's "Follow Capture", that's the
correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
indeed wrong.


thanks,

Takashi


> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  sound/pci/hda/patch_realtek.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index daedcc0adc21..09d93dd88713 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -4414,6 +4414,7 @@ static void alc233_fixup_lenovo_line2_mic_hotkey(struct hda_codec *codec,
>  {
>  	struct alc_spec *spec = codec->spec;
>  
> +	spec->micmute_led_polarity = 1;
>  	alc_fixup_hp_gpio_led(codec, action, 0, 0x04);
>  	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>  		spec->init_amp = ALC_INIT_DEFAULT;
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-10  6:30 ` Takashi Iwai
@ 2020-08-10  6:34   ` Hui Wang
  2020-08-10  6:51     ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Hui Wang @ 2020-08-10  6:34 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, stable


On 2020/8/10 下午2:30, Takashi Iwai wrote:
> On Mon, 10 Aug 2020 04:16:59 +0200,
> Hui Wang wrote:
>> After installing the Ubuntu Linux, the micmute led status is not
>> correct. Users expect that the led is on if the capture is disabled,
>> but with the current kernel, the led is off with the capture disabled.
>>
>> We tried the old linux kernel like linux-4.15, there is no this issue.
>> It looks like we introduced this issue when switching to the led_cdev.
> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
> Which value does it have now?  If it's "Follow Capture", that's the
> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
> indeed wrong.

It is "Follow Mute",  if I change it to "Follow Capture" manually, the 
led status becomes correct.

Thanks.

>
>
> thanks,
>
> Takashi
>
>
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>   sound/pci/hda/patch_realtek.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>> index daedcc0adc21..09d93dd88713 100644
>> --- a/sound/pci/hda/patch_realtek.c
>> +++ b/sound/pci/hda/patch_realtek.c
>> @@ -4414,6 +4414,7 @@ static void alc233_fixup_lenovo_line2_mic_hotkey(struct hda_codec *codec,
>>   {
>>   	struct alc_spec *spec = codec->spec;
>>   
>> +	spec->micmute_led_polarity = 1;
>>   	alc_fixup_hp_gpio_led(codec, action, 0, 0x04);
>>   	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>>   		spec->init_amp = ALC_INIT_DEFAULT;
>> -- 
>> 2.17.1
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-10  6:34   ` Hui Wang
@ 2020-08-10  6:51     ` Takashi Iwai
  2020-08-11  8:56       ` Kai-Heng Feng
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2020-08-10  6:51 UTC (permalink / raw)
  To: Hui Wang; +Cc: alsa-devel, stable

On Mon, 10 Aug 2020 08:34:36 +0200,
Hui Wang wrote:
> 
> 
> On 2020/8/10 下午2:30, Takashi Iwai wrote:
> > On Mon, 10 Aug 2020 04:16:59 +0200,
> > Hui Wang wrote:
> >> After installing the Ubuntu Linux, the micmute led status is not
> >> correct. Users expect that the led is on if the capture is disabled,
> >> but with the current kernel, the led is off with the capture disabled.
> >>
> >> We tried the old linux kernel like linux-4.15, there is no this issue.
> >> It looks like we introduced this issue when switching to the led_cdev.
> > The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
> > Which value does it have now?  If it's "Follow Capture", that's the
> > correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
> > indeed wrong.
> 
> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
> led status becomes correct.

OK, thanks for confirmation.  Applied now.


Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-10  6:51     ` Takashi Iwai
@ 2020-08-11  8:56       ` Kai-Heng Feng
  2020-08-11  9:03         ` Hui Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Kai-Heng Feng @ 2020-08-11  8:56 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Hui Wang, moderated list:SOUND, stable

Hi,

> On Aug 10, 2020, at 14:51, Takashi Iwai <tiwai@suse.de> wrote:
> 
> On Mon, 10 Aug 2020 08:34:36 +0200,
> Hui Wang wrote:
>> 
>> 
>> On 2020/8/10 下午2:30, Takashi Iwai wrote:
>>> On Mon, 10 Aug 2020 04:16:59 +0200,
>>> Hui Wang wrote:
>>>> After installing the Ubuntu Linux, the micmute led status is not
>>>> correct. Users expect that the led is on if the capture is disabled,
>>>> but with the current kernel, the led is off with the capture disabled.
>>>> 
>>>> We tried the old linux kernel like linux-4.15, there is no this issue.
>>>> It looks like we introduced this issue when switching to the led_cdev.
>>> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
>>> Which value does it have now?  If it's "Follow Capture", that's the
>>> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
>>> indeed wrong.
>> 
>> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
>> led status becomes correct.
> 
> OK, thanks for confirmation.  Applied now.

I wonder if it's because how brightness value passed to gpio_mute_led_set() and micmute_led_set():

static int gpio_mute_led_set(struct led_classdev *led_cdev,
                             enum led_brightness brightness)
{
        struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
        struct alc_spec *spec = codec->spec;

        alc_update_gpio_led(codec, spec->gpio_mute_led_mask,
                            spec->mute_led_polarity, !brightness);
        return 0;
}

static int micmute_led_set(struct led_classdev *led_cdev,
                           enum led_brightness brightness)
{
        struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
        struct alc_spec *spec = codec->spec;

        alc_update_gpio_led(codec, spec->gpio_mic_led_mask,
                            spec->micmute_led_polarity, !!brightness);      
        return 0;
}

Maybe we should let micmute_led_set() match gpio_mute_led_set() so the micmute polarity can be removed?

Kai-Heng

> 
> 
> Takashi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-11  8:56       ` Kai-Heng Feng
@ 2020-08-11  9:03         ` Hui Wang
  2020-08-11  9:22             ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Hui Wang @ 2020-08-11  9:03 UTC (permalink / raw)
  To: Kai-Heng Feng, Takashi Iwai; +Cc: moderated list:SOUND, stable


On 2020/8/11 下午4:56, Kai-Heng Feng wrote:
> Hi,
>
>> On Aug 10, 2020, at 14:51, Takashi Iwai <tiwai@suse.de> wrote:
>>
>> On Mon, 10 Aug 2020 08:34:36 +0200,
>> Hui Wang wrote:
>>>
>>> On 2020/8/10 下午2:30, Takashi Iwai wrote:
>>>> On Mon, 10 Aug 2020 04:16:59 +0200,
>>>> Hui Wang wrote:
>>>>> After installing the Ubuntu Linux, the micmute led status is not
>>>>> correct. Users expect that the led is on if the capture is disabled,
>>>>> but with the current kernel, the led is off with the capture disabled.
>>>>>
>>>>> We tried the old linux kernel like linux-4.15, there is no this issue.
>>>>> It looks like we introduced this issue when switching to the led_cdev.
>>>> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
>>>> Which value does it have now?  If it's "Follow Capture", that's the
>>>> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
>>>> indeed wrong.
>>> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
>>> led status becomes correct.
>> OK, thanks for confirmation.  Applied now.
> I wonder if it's because how brightness value passed to gpio_mute_led_set() and micmute_led_set():
>
> static int gpio_mute_led_set(struct led_classdev *led_cdev,
>                               enum led_brightness brightness)
> {
>          struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
>          struct alc_spec *spec = codec->spec;
>
>          alc_update_gpio_led(codec, spec->gpio_mute_led_mask,
>                              spec->mute_led_polarity, !brightness);
>          return 0;
> }
>
> static int micmute_led_set(struct led_classdev *led_cdev,
>                             enum led_brightness brightness)
> {
>          struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
>          struct alc_spec *spec = codec->spec;
>
>          alc_update_gpio_led(codec, spec->gpio_mic_led_mask,
>                              spec->micmute_led_polarity, !!brightness);
>          return 0;
> }
>
> Maybe we should let micmute_led_set() match gpio_mute_led_set() so the micmute polarity can be removed?

This will impact many many machines, I don't know if the current code 
could work correctly or not on other machines.

Thanks,

Hui.

>
> Kai-Heng
>
>>
>> Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-11  9:03         ` Hui Wang
@ 2020-08-11  9:22             ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2020-08-11  9:22 UTC (permalink / raw)
  To: Hui Wang; +Cc: Kai-Heng Feng, moderated list:SOUND, stable

On Tue, 11 Aug 2020 11:03:55 +0200,
Hui Wang wrote:
> 
> 
> On 2020/8/11 下午4:56, Kai-Heng Feng wrote:
> > Hi,
> >
> >> On Aug 10, 2020, at 14:51, Takashi Iwai <tiwai@suse.de> wrote:
> >>
> >> On Mon, 10 Aug 2020 08:34:36 +0200,
> >> Hui Wang wrote:
> >>>
> >>> On 2020/8/10 下午2:30, Takashi Iwai wrote:
> >>>> On Mon, 10 Aug 2020 04:16:59 +0200,
> >>>> Hui Wang wrote:
> >>>>> After installing the Ubuntu Linux, the micmute led status is not
> >>>>> correct. Users expect that the led is on if the capture is disabled,
> >>>>> but with the current kernel, the led is off with the capture disabled.
> >>>>>
> >>>>> We tried the old linux kernel like linux-4.15, there is no this issue.
> >>>>> It looks like we introduced this issue when switching to the led_cdev.
> >>>> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
> >>>> Which value does it have now?  If it's "Follow Capture", that's the
> >>>> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
> >>>> indeed wrong.
> >>> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
> >>> led status becomes correct.
> >> OK, thanks for confirmation.  Applied now.
> > I wonder if it's because how brightness value passed to gpio_mute_led_set() and micmute_led_set():
> >
> > static int gpio_mute_led_set(struct led_classdev *led_cdev,
> >                               enum led_brightness brightness)
> > {
> >          struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
> >          struct alc_spec *spec = codec->spec;
> >
> >          alc_update_gpio_led(codec, spec->gpio_mute_led_mask,
> >                              spec->mute_led_polarity, !brightness);
> >          return 0;
> > }
> >
> > static int micmute_led_set(struct led_classdev *led_cdev,
> >                             enum led_brightness brightness)
> > {
> >          struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
> >          struct alc_spec *spec = codec->spec;
> >
> >          alc_update_gpio_led(codec, spec->gpio_mic_led_mask,
> >                              spec->micmute_led_polarity, !!brightness);
> >          return 0;
> > }
> >
> > Maybe we should let micmute_led_set() match gpio_mute_led_set() so the micmute polarity can be removed?
> 
> This will impact many many machines, I don't know if the current code
> could work correctly or not on other machines.

True.  But we should rather fix this, the current flag is illogical.
I forgot about this problem while I also noticed during working on the
led cdev conversion.

I guess most of configurations can be verified with hda-emu or such.
Let's see...


Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
@ 2020-08-11  9:22             ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2020-08-11  9:22 UTC (permalink / raw)
  To: Hui Wang; +Cc: moderated list:SOUND, Kai-Heng Feng, stable

On Tue, 11 Aug 2020 11:03:55 +0200,
Hui Wang wrote:
> 
> 
> On 2020/8/11 下午4:56, Kai-Heng Feng wrote:
> > Hi,
> >
> >> On Aug 10, 2020, at 14:51, Takashi Iwai <tiwai@suse.de> wrote:
> >>
> >> On Mon, 10 Aug 2020 08:34:36 +0200,
> >> Hui Wang wrote:
> >>>
> >>> On 2020/8/10 下午2:30, Takashi Iwai wrote:
> >>>> On Mon, 10 Aug 2020 04:16:59 +0200,
> >>>> Hui Wang wrote:
> >>>>> After installing the Ubuntu Linux, the micmute led status is not
> >>>>> correct. Users expect that the led is on if the capture is disabled,
> >>>>> but with the current kernel, the led is off with the capture disabled.
> >>>>>
> >>>>> We tried the old linux kernel like linux-4.15, there is no this issue.
> >>>>> It looks like we introduced this issue when switching to the led_cdev.
> >>>> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
> >>>> Which value does it have now?  If it's "Follow Capture", that's the
> >>>> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
> >>>> indeed wrong.
> >>> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
> >>> led status becomes correct.
> >> OK, thanks for confirmation.  Applied now.
> > I wonder if it's because how brightness value passed to gpio_mute_led_set() and micmute_led_set():
> >
> > static int gpio_mute_led_set(struct led_classdev *led_cdev,
> >                               enum led_brightness brightness)
> > {
> >          struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
> >          struct alc_spec *spec = codec->spec;
> >
> >          alc_update_gpio_led(codec, spec->gpio_mute_led_mask,
> >                              spec->mute_led_polarity, !brightness);
> >          return 0;
> > }
> >
> > static int micmute_led_set(struct led_classdev *led_cdev,
> >                             enum led_brightness brightness)
> > {
> >          struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
> >          struct alc_spec *spec = codec->spec;
> >
> >          alc_update_gpio_led(codec, spec->gpio_mic_led_mask,
> >                              spec->micmute_led_polarity, !!brightness);
> >          return 0;
> > }
> >
> > Maybe we should let micmute_led_set() match gpio_mute_led_set() so the micmute polarity can be removed?
> 
> This will impact many many machines, I don't know if the current code
> could work correctly or not on other machines.

True.  But we should rather fix this, the current flag is illogical.
I forgot about this problem while I also noticed during working on the
led cdev conversion.

I guess most of configurations can be verified with hda-emu or such.
Let's see...


Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-11  9:22             ` Takashi Iwai
  (?)
@ 2020-08-11 11:55             ` Hui Wang
  2020-08-11 12:18               ` Takashi Iwai
  -1 siblings, 1 reply; 10+ messages in thread
From: Hui Wang @ 2020-08-11 11:55 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: moderated list:SOUND, Kai-Heng Feng, stable


On 2020/8/11 下午5:22, Takashi Iwai wrote:
> On Tue, 11 Aug 2020 11:03:55 +0200,
> Hui Wang wrote:
>>
>> On 2020/8/11 下午4:56, Kai-Heng Feng wrote:
>>> Hi,
>>>
>>>> On Aug 10, 2020, at 14:51, Takashi Iwai <tiwai@suse.de> wrote:
>>>>
>>>> On Mon, 10 Aug 2020 08:34:36 +0200,
>>>> Hui Wang wrote:
>>>>> On 2020/8/10 下午2:30, Takashi Iwai wrote:
>>>>>> On Mon, 10 Aug 2020 04:16:59 +0200,
>>>>>> Hui Wang wrote:
>>>>>>> After installing the Ubuntu Linux, the micmute led status is not
>>>>>>> correct. Users expect that the led is on if the capture is disabled,
>>>>>>> but with the current kernel, the led is off with the capture disabled.
>>>>>>>
>>>>>>> We tried the old linux kernel like linux-4.15, there is no this issue.
>>>>>>> It looks like we introduced this issue when switching to the led_cdev.
>>>>>> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
>>>>>> Which value does it have now?  If it's "Follow Capture", that's the
>>>>>> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
>>>>>> indeed wrong.
>>>>> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
>>>>> led status becomes correct.
>>>> OK, thanks for confirmation.  Applied now.
>>> I wonder if it's because how brightness value passed to gpio_mute_led_set() and micmute_led_set():
>>>
>>> static int gpio_mute_led_set(struct led_classdev *led_cdev,
>>>                                enum led_brightness brightness)
>>> {
>>>           struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
>>>           struct alc_spec *spec = codec->spec;
>>>
>>>           alc_update_gpio_led(codec, spec->gpio_mute_led_mask,
>>>                               spec->mute_led_polarity, !brightness);
>>>           return 0;
>>> }
>>>
>>> static int micmute_led_set(struct led_classdev *led_cdev,
>>>                              enum led_brightness brightness)
>>> {
>>>           struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
>>>           struct alc_spec *spec = codec->spec;
>>>
>>>           alc_update_gpio_led(codec, spec->gpio_mic_led_mask,
>>>                               spec->micmute_led_polarity, !!brightness);
>>>           return 0;
>>> }
>>>
>>> Maybe we should let micmute_led_set() match gpio_mute_led_set() so the micmute polarity can be removed?
>> This will impact many many machines, I don't know if the current code
>> could work correctly or not on other machines.
> True.  But we should rather fix this, the current flag is illogical.
> I forgot about this problem while I also noticed during working on the
> led cdev conversion.
>
> I guess most of configurations can be verified with hda-emu or such.
> Let's see...

I just checked, the commit 87dc36482cab (ALSA: hda/realtek - Add LED 
class support for micmute LED) introduced this issue. In the 
micmute_led_set(), it set the led with the !!mic_mute.led_value, while 
before this commit, the driver sets the led with !mic_mute.led_value.

Add with this commit 7cdf8c49b1df (ALSA: hda: generic: Add a helper for 
mic-mute LED with LED classdev),  all micmute led updating will call 
micmute_led_set(). It impacts alc269_fixup_hp_gpio_led(), 
alc285_fixup_hp_gpio_led(), alc286_fixup_hp_gpio_led(), 
alc280_fixup_hp_gpio2_mic_hotkey() and 
alc233_fixup_lenovo_line2_mic_hotkey()

I think it is safe to change the micmute_led_set() and remove all 
spec->micmute_led_polarity = 1.

Let me write a patch.

Thanks,

Hui.

> .


>
> Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO
  2020-08-11 11:55             ` Hui Wang
@ 2020-08-11 12:18               ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2020-08-11 12:18 UTC (permalink / raw)
  To: Hui Wang; +Cc: moderated list:SOUND, Kai-Heng Feng, stable

On Tue, 11 Aug 2020 13:55:25 +0200,
Hui Wang wrote:
> 
> 
> On 2020/8/11 下午5:22, Takashi Iwai wrote:
> > On Tue, 11 Aug 2020 11:03:55 +0200,
> > Hui Wang wrote:
> >>
> >> On 2020/8/11 下午4:56, Kai-Heng Feng wrote:
> >>> Hi,
> >>>
> >>>> On Aug 10, 2020, at 14:51, Takashi Iwai <tiwai@suse.de> wrote:
> >>>>
> >>>> On Mon, 10 Aug 2020 08:34:36 +0200,
> >>>> Hui Wang wrote:
> >>>>> On 2020/8/10 下午2:30, Takashi Iwai wrote:
> >>>>>> On Mon, 10 Aug 2020 04:16:59 +0200,
> >>>>>> Hui Wang wrote:
> >>>>>>> After installing the Ubuntu Linux, the micmute led status is not
> >>>>>>> correct. Users expect that the led is on if the capture is disabled,
> >>>>>>> but with the current kernel, the led is off with the capture disabled.
> >>>>>>>
> >>>>>>> We tried the old linux kernel like linux-4.15, there is no this issue.
> >>>>>>> It looks like we introduced this issue when switching to the led_cdev.
> >>>>>> The behavior can be controlled via "Mic Mute-LED Mode" enum kcontrol.
> >>>>>> Which value does it have now?  If it's "Follow Capture", that's the
> >>>>>> correct behavior.  OTOH, if it's "Follow Mute", the LED polarity is
> >>>>>> indeed wrong.
> >>>>> It is "Follow Mute",  if I change it to "Follow Capture" manually, the
> >>>>> led status becomes correct.
> >>>> OK, thanks for confirmation.  Applied now.
> >>> I wonder if it's because how brightness value passed to gpio_mute_led_set() and micmute_led_set():
> >>>
> >>> static int gpio_mute_led_set(struct led_classdev *led_cdev,
> >>>                                enum led_brightness brightness)
> >>> {
> >>>           struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
> >>>           struct alc_spec *spec = codec->spec;
> >>>
> >>>           alc_update_gpio_led(codec, spec->gpio_mute_led_mask,
> >>>                               spec->mute_led_polarity, !brightness);
> >>>           return 0;
> >>> }
> >>>
> >>> static int micmute_led_set(struct led_classdev *led_cdev,
> >>>                              enum led_brightness brightness)
> >>> {
> >>>           struct hda_codec *codec = dev_to_hda_codec(led_cdev->dev->parent);
> >>>           struct alc_spec *spec = codec->spec;
> >>>
> >>>           alc_update_gpio_led(codec, spec->gpio_mic_led_mask,
> >>>                               spec->micmute_led_polarity, !!brightness);
> >>>           return 0;
> >>> }
> >>>
> >>> Maybe we should let micmute_led_set() match gpio_mute_led_set() so the micmute polarity can be removed?
> >> This will impact many many machines, I don't know if the current code
> >> could work correctly or not on other machines.
> > True.  But we should rather fix this, the current flag is illogical.
> > I forgot about this problem while I also noticed during working on the
> > led cdev conversion.
> >
> > I guess most of configurations can be verified with hda-emu or such.
> > Let's see...
> 
> I just checked, the commit 87dc36482cab (ALSA: hda/realtek - Add LED
> class support for micmute LED) introduced this issue. In the
> micmute_led_set(), it set the led with the !!mic_mute.led_value, while
> before this commit, the driver sets the led with !mic_mute.led_value.
> 
> Add with this commit 7cdf8c49b1df (ALSA: hda: generic: Add a helper
> for mic-mute LED with LED classdev),  all micmute led updating will
> call micmute_led_set(). It impacts alc269_fixup_hp_gpio_led(),
> alc285_fixup_hp_gpio_led(), alc286_fixup_hp_gpio_led(),
> alc280_fixup_hp_gpio2_mic_hotkey() and
> alc233_fixup_lenovo_line2_mic_hotkey()
> 
> I think it is safe to change the micmute_led_set() and remove all
> spec->micmute_led_polarity = 1.
> 
> Let me write a patch.

Great, thanks for spotting out.


Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-08-11 12:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-10  2:16 [PATCH] ALSA: hda - fix the micmute led status for Lenovo ThinkCentre AIO Hui Wang
2020-08-10  6:30 ` Takashi Iwai
2020-08-10  6:34   ` Hui Wang
2020-08-10  6:51     ` Takashi Iwai
2020-08-11  8:56       ` Kai-Heng Feng
2020-08-11  9:03         ` Hui Wang
2020-08-11  9:22           ` Takashi Iwai
2020-08-11  9:22             ` Takashi Iwai
2020-08-11 11:55             ` Hui Wang
2020-08-11 12:18               ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.