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@xxxxxxx> 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