2021-10-30 10:06:17

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

On Fri, 29 Oct 2021 17:43:13 +0200,
Jonathan Clarke wrote:
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210633
> Signed-off-by: Jonathan Clarke <[email protected]>

Could you give more descriptions? The patch isn't trivial at all, and
it needs more explanations.

> ---
> sound/pci/hda/patch_realtek.c | 46 +++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 22d27b12c..e3c6d17ea 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -4409,6 +4409,13 @@ static void alc245_fixup_hp_x360_amp(struct hda_codec *codec,
> case HDA_FIXUP_ACT_PRE_PROBE:
> spec->gpio_mask |= 0x01;
> spec->gpio_dir |= 0x01;
> +
> + /* use only amp at 0x02 for bottom(front) speaker,
> + * otherwise it is set to use 0x02,0x03,0x06 and when used in conjunction
> + * with top(rear) speaker 0x14, gets locked at full volume */
> + static const hda_nid_t conn1[] = { 0x02 };
> + snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn1), conn1);
> +
> break;
> case HDA_FIXUP_ACT_INIT:
> /* need to toggle GPIO to enable the amp */
> @@ -4503,6 +4510,26 @@ static void alc236_fixup_hp_mute_led_coefbit(struct hda_codec *codec,
> }
> }
>
> +static void alc245_fixup_hp_x360_mute_leds(struct hda_codec *codec,
> + const struct hda_fixup *fix, int action)
> +{
> + struct alc_spec *spec = codec->spec;
> + if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> + /* mic mute is set via gpio 0x04 */
> + spec->micmute_led_polarity = 1;
> + codec->power_filter = led_power_filter;
> + alc_fixup_hp_gpio_led(codec, action, 0x00, 0x04);
> +
> + /* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
> + spec->mute_led_polarity = 0;
> + spec->mute_led_coef.idx = 0x0b;
> + spec->mute_led_coef.mask = 0xffff;
> + spec->mute_led_coef.on = 0xa02f;
> + spec->mute_led_coef.off = 0x7774;
> + snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);

I guess this COEF isn't only about mute-LED but actually does mute the
output? IIRC, the bit 0x08 corresponds to the LED. If so, it's
better to split. Basically this snd_hda_gen_add_mute_led_cdev() and
mute_led_coef stuff are only for the mute LED control. e.g. you can
change the mute LED independently via sysfs.


thanks,

Takashi


2021-11-01 10:34:44

by Jonathan Clarke

[permalink] [raw]
Subject: Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

Thanks for taking a look at this patch so quickly, Takashi.

On Sat, Oct 30, 2021 at 12:01:03PM +0200, Takashi Iwai wrote:
> Could you give more descriptions? The patch isn't trivial at all, and
> it needs more explanations.

Yes, will do.

> > + /* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
> > + spec->mute_led_polarity = 0;
> > + spec->mute_led_coef.idx = 0x0b;
> > + spec->mute_led_coef.mask = 0xffff;
> > + spec->mute_led_coef.on = 0xa02f;
> > + spec->mute_led_coef.off = 0x7774;
> > + snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);
>
> I guess this COEF isn't only about mute-LED but actually does mute the
> output? IIRC, the bit 0x08 corresponds to the LED. If so, it's
> better to split. Basically this snd_hda_gen_add_mute_led_cdev() and
> mute_led_coef stuff are only for the mute LED control. e.g. you can
> change the mute LED independently via sysfs.

Thanks for suggesting this.

Having tested, I can confirm that setting this coef only affects
the output mute LED, and does not affect output.

I will therefore assume that current implementation in my patch is OK,
but let me know if it still needs changing (maybe I've misunderstood).

For reference to other users, the commands to test are:
# output LED on
hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0xa02f

# output LED off
hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0x7774

Many thanks, Jonathan