Re: [PATCH] ALSA: hda/realtek: Add inverted LED quirk for HP ZBook 8 G2a

[Takashi Iwai <tiwai@suse.de>]

On Wed, 08 Jul 2026 04:47:57 +0200,
Chris Chiu wrote:
> 
> HP ZBook 8 G2a 14 and 16 (SSIDs 0x103c:0x8f94 and 0x103c:0x8f95)
> require special handling for the speaker mute LED with inverted
> behavior compared to the existing ALC245_FIXUP_HP_TAS2781_I2C_MUTE_LED.
> 
> Add a new dedicated quirk ALC245_FIXUP_HP_TAS2781_I2C_MUTE_LED_INVERTED
> to address the inverted speaker mute led issue. This ensures the LED
> behaves correctly on the new HP ZBook models.
> 
> Signed-off-by: Chris Chiu <chris.chiu@canonical.com>
> ---
>  sound/hda/codecs/realtek/alc269.c | 44 +++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/sound/hda/codecs/realtek/alc269.c b/sound/hda/codecs/realtek/alc269.c
> index b26ed9c1f09f..07f5adce075a 100644
> --- a/sound/hda/codecs/realtek/alc269.c
> +++ b/sound/hda/codecs/realtek/alc269.c
> @@ -3776,6 +3776,15 @@ static void alc245_hp_spk_mute_led_update(void *private_data, int enabled)
>  	alc_update_coef_idx(codec, 0x0b, 0x0c, val);
>  }
>  
> +static void alc245_hp_spk_mute_led_update_inverted(void *private_data, int enabled)
> +{
> +	struct hda_codec *codec = private_data;
> +	unsigned int val;
> +
> +	val = enabled ? 0x04 : 0x08; /* inverted: 0x04 led off, 0x08 led on */
> +	alc_update_coef_idx(codec, 0x0b, 0x0c, val);
> +}
> +
>  /* JD2: mute led GPIO3: micmute led */
>  static void alc245_tas2781_i2c_hp_fixup_muteled(struct hda_codec *codec,
>  					  const struct hda_fixup *fix, int action)
> @@ -3804,6 +3813,34 @@ static void alc245_tas2781_i2c_hp_fixup_muteled(struct hda_codec *codec,
>  		alc245_fixup_hp_mute_led_coefbit(codec, fix, action);
>  	alc285_fixup_hp_coef_micmute_led(codec, fix, action);
>  }
> +
> +static void alc245_tas2781_i2c_hp_fixup_muteled_inverted(struct hda_codec *codec,
> +const struct hda_fixup *fix, int action)
> +{
> +	struct alc_spec *spec = codec->spec;
> +	hda_nid_t hp_pin = alc_get_hp_pin(spec);
> +	static const hda_nid_t conn[] = { 0x02 };
> +
> +	switch (action) {
> +	case HDA_FIXUP_ACT_PRE_PROBE:
> +		if (!hp_pin) {
> +			spec->gen.vmaster_mute.hook = alc245_hp_spk_mute_led_update_inverted;
> +			spec->gen.vmaster_mute_led = 1;
> +		}

Is there a case without HP pin at all?  Those are laptops, right?

> +		spec->gen.auto_mute_via_amp = 1;
> +		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> +		break;
> +	case HDA_FIXUP_ACT_INIT:
> +		if (!hp_pin)
> +			alc245_hp_spk_mute_led_update_inverted(codec, !spec->gen.master_mute);
> +		break;
> +	}
> +
> +	tas2781_fixup_txnw_i2c(codec, fix, action);
> +	if (hp_pin)
> +		alc245_fixup_hp_mute_led_coefbit(codec, fix, action);
> +	alc285_fixup_hp_coef_micmute_led(codec, fix, action);

Hmm, the code flow depends on hp_pin, and it isn't explained why you
have to do it in that way.  Please give either more comment or
description to understand better your changes.


thanks,

Takashi