Re: [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs

[Takashi Iwai <tiwai@xxxxxxx>]

On Thu, 19 Dec 2024 11:15:31 +0100,
Jackie Dong wrote:
> 
> New ideapad helper file with support for handling FN key mute LEDS.
> Update conexant and realtec codec to add LED support.
> 
> Suggested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
> Signed-off-by: Jackie Dong  <xy-jackie@xxxxxxx>
> Signed-off-by: Jackie Dong  <dongeg1@xxxxxxxxxx>
> ---
>  sound/pci/hda/ideapad_hotkey_led_helper.c | 36 +++++++++++++++++++++++
>  sound/pci/hda/patch_conexant.c            | 10 +++++++
>  sound/pci/hda/patch_realtek.c             | 20 +++++++++++++
>  3 files changed, 66 insertions(+)
>  create mode 100644 sound/pci/hda/ideapad_hotkey_led_helper.c
> 
> diff --git a/sound/pci/hda/ideapad_hotkey_led_helper.c b/sound/pci/hda/ideapad_hotkey_led_helper.c
> new file mode 100644
> index 000000000000..e49765304cc0
> --- /dev/null
> +++ b/sound/pci/hda/ideapad_hotkey_led_helper.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Ideapad helper functions for Lenovo Ideapad LED control,
> + * It should be included from codec driver.
> + */
> +
> +#if IS_ENABLED(CONFIG_IDEAPAD_LAPTOP)
> +
> +#include <linux/acpi.h>
> +#include <linux/leds.h>
> +
> +static bool is_ideapad(struct hda_codec *codec)
> +{
> +	return (codec->core.subsystem_id >> 16 == 0x17aa) &&
> +	       (acpi_dev_found("LHK2019") || acpi_dev_found("VPC2004"));
> +}
> +
> +static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> +		if (!is_ideapad(codec))
> +			return;
> +		snd_hda_gen_add_mute_led_cdev(codec, NULL);
> +		snd_hda_gen_add_micmute_led_cdev(codec, NULL);
> +	}
> +}
> +
> +#else /* CONFIG_IDEAPAD_LAPTOP */
> +
> +static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +}
> +
> +#endif /* CONFIG_IDEAPAD_LAPTOP */
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index 538c37a78a56..127f9a9565c9 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -291,6 +291,7 @@ enum {
>  	CXT_FIXUP_GPIO1,
>  	CXT_FIXUP_ASPIRE_DMIC,
>  	CXT_FIXUP_THINKPAD_ACPI,
> +	CXT_FIXUP_IDEAPAD_ACPI,
>  	CXT_FIXUP_OLPC_XO,
>  	CXT_FIXUP_CAP_MIX_AMP,
>  	CXT_FIXUP_TOSHIBA_P105,
> @@ -313,6 +314,9 @@ enum {
>  /* for hda_fixup_thinkpad_acpi() */
>  #include "thinkpad_helper.c"
>  
> +/* for hda_fixup_ideapad_acpi() */
> +#include "ideapad_hotkey_led_helper.c"
> +
>  static void cxt_fixup_stereo_dmic(struct hda_codec *codec,
>  				  const struct hda_fixup *fix, int action)
>  {
> @@ -928,6 +932,10 @@ static const struct hda_fixup cxt_fixups[] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = hda_fixup_thinkpad_acpi,
>  	},
> +	[CXT_FIXUP_IDEAPAD_ACPI] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = hda_fixup_ideapad_acpi,
> +	},
>  	[CXT_FIXUP_OLPC_XO] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = cxt_fixup_olpc_xo,
> @@ -1120,6 +1128,7 @@ static const struct hda_quirk cxt5066_fixups[] = {
>  	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo G50-70", CXT_FIXUP_STEREO_DMIC),
>  	SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC),
>  	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI),
> +	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", CXT_FIXUP_IDEAPAD_ACPI),

I'm afraid that this doesn't work.  The former entry with the vendor
17aa already hits and it's used as the matched quirk.

You'd need to create a chained quirk entry instead.

>  	SND_PCI_QUIRK(0x1c06, 0x2011, "Lemote A1004", CXT_PINCFG_LEMOTE_A1004),
>  	SND_PCI_QUIRK(0x1c06, 0x2012, "Lemote A1205", CXT_PINCFG_LEMOTE_A1205),
>  	HDA_CODEC_QUIRK(0x2782, 0x12c3, "Sirius Gen1", CXT_PINCFG_TOP_SPEAKER),
> @@ -1133,6 +1142,7 @@ static const struct hda_model_fixup cxt5066_fixup_models[] = {
>  	{ .id = CXT_FIXUP_HEADPHONE_MIC_PIN, .name = "headphone-mic-pin" },
>  	{ .id = CXT_PINCFG_LENOVO_TP410, .name = "tp410" },
>  	{ .id = CXT_FIXUP_THINKPAD_ACPI, .name = "thinkpad" },
> +	{ .id = CXT_FIXUP_IDEAPAD_ACPI, .name = "ideapad" },
>  	{ .id = CXT_PINCFG_LEMOTE_A1004, .name = "lemote-a1004" },
>  	{ .id = CXT_PINCFG_LEMOTE_A1205, .name = "lemote-a1205" },
>  	{ .id = CXT_FIXUP_OLPC_XO, .name = "olpc-xo" },
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 61ba5dc35b8b..3b82be7469a8 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6934,6 +6934,16 @@ static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>  	hda_fixup_thinkpad_acpi(codec, fix, action);
>  }
>  
> +/* for hda_fixup_ideapad_acpi() */
> +#include "ideapad_hotkey_led_helper.c"
> +
> +static void alc_fixup_ideapad_acpi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +	alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
> +	hda_fixup_ideapad_acpi(codec, fix, action);
> +}
> +
>  /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */
>  static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec,
>  						  const struct hda_fixup *fix,
> @@ -7556,6 +7566,7 @@ enum {
>  	ALC290_FIXUP_SUBWOOFER,
>  	ALC290_FIXUP_SUBWOOFER_HSJACK,
>  	ALC269_FIXUP_THINKPAD_ACPI,
> +	ALC269_FIXUP_IDEAPAD_ACPI,
>  	ALC269_FIXUP_DMIC_THINKPAD_ACPI,
>  	ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13,
>  	ALC269VC_FIXUP_INFINIX_Y4_MAX,
> @@ -8327,6 +8338,12 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC269_FIXUP_SKU_IGNORE,
>  	},
> +	[ALC269_FIXUP_IDEAPAD_ACPI] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = alc_fixup_ideapad_acpi,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_SKU_IGNORE,
> +	},
>  	[ALC269_FIXUP_DMIC_THINKPAD_ACPI] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = alc_fixup_inv_dmic,
> @@ -10897,6 +10914,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x17aa, 0x3880, "Yoga S780-16 pro dual YC", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3881, "YB9 dual power mode2 YC", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3882, "Lenovo Yoga Pro 7 14APH8", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
> +	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", ALC269_FIXUP_IDEAPAD_ACPI),
>  	SND_PCI_QUIRK(0x17aa, 0x3884, "Y780 YG DUAL", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3886, "Y780 VECO DUAL", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3891, "Lenovo Yoga Pro 7 14AHP9", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),

Also, this doesn't look right.  The vendor match should be put after
all Lenovo entries.  I guess the similar chained quirk is needed like
for Conexant.


thanks,

Takashi