On Fri, 16 Aug 2019 01:08:34 +0200, Jerónimo Borque wrote: > > El jue., 15 de ago. de 2019 a la(s) 14:06, Takashi Iwai (tiwai@xxxxxxx) > escribió: > > On Thu, 15 Aug 2019 18:33:50 +0200, > Jerónimo Borque wrote: > > > > Hi Takashi, > > Modifying Mic Mute-LED Mode does indeed alter the behavior. The thing is > that > > this ends being confusing as in all machines I've been testing this > setting > > Mic Mute-LED Mode to "Follow Capture" actually makes it follow mute, as > > setting it to "On" turns the LED off. > > There is other setting called "mute_led_polarity" but this does not > work, as > > currently mic mute LED and mute LED do not follow the same logic. > > What I think may be causing confusion is "cxt_update_gpio_led" "enabled" > > parameter. Setting "enabled" to "true" sets the GPIO pin to 0 causing > the led > > to be turned off. I think "enabled" used to refer to the input capture > or > > output status and not to the LED being lit or not. Output or input not > enabled > > (enabled==false) caused the LED to be turned on. > > This logic in the function negates it on the GPIO output. > > > > if (enabled) > > spec->gpio_led &= ~mask; > > else > > spec->gpio_led |= mask; > > > > May be I can do a more comprehensive fix, reversing the behavior of > > "cxt_update_gpio_led" "enabled" parameter to refer the GPIO output value > ( > > enabled==true => GPIO pin output high ) > > Then also modify the call to "cxt_update_gpio_led" in > > "cxt_fixup_gpio_mute_hook" to make it work consistently. > > OK, if the "On" turns the LED off, it's indeed inverted. > Then we'd need to consider both fixing the inverted behavior and the > default mic-mute mode. > > Could you confirm the following? > > - Which models and codecs are checked? > > I've tested on HP ZBook 15U G3 (Conexant CX20724) and HP Probook 440 G4 > (Conexant CX8200) > > - GPIO pin high = mic LED on or off? > > GPIO pin high = mic LED on > > - How is the expected behavior on Windows? > Mute is on when mic is muted, or mute-on when mic is ready? > > Mute led is on when mic is muted. Thanks, it's clearer now. Then I'd rather like to correct cxt_update_gpio_led(), i.e. taking the argument led_on instead of enabled, and correct the caller in cxt_fixup_gpio_mute_hook() to invert instead. Something like below. Could you retest and resubmit with that change? thanks, Takashi --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -611,18 +611,18 @@ static void cxt_fixup_hp_gate_mic_jack(struct hda_codec *codec, /* update LED status via GPIO */ static void cxt_update_gpio_led(struct hda_codec *codec, unsigned int mask, - bool enabled) + bool led_on) { struct conexant_spec *spec = codec->spec; unsigned int oldval = spec->gpio_led; if (spec->mute_led_polarity) - enabled = !enabled; + led_on = !led_on; - if (enabled) - spec->gpio_led &= ~mask; - else + if (led_on) spec->gpio_led |= mask; + else + spec->gpio_led &= ~mask; if (spec->gpio_led != oldval) snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, spec->gpio_led); @@ -634,7 +634,8 @@ static void cxt_fixup_gpio_mute_hook(void *private_data, int enabled) struct hda_codec *codec = private_data; struct conexant_spec *spec = codec->spec; - cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled); + /* muted -> LED on */ + cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, !enabled); } /* turn on/off mic-mute LED via GPIO per capture hook */ _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel