| Message ID | 20241224083316.20222-1-xy-jackie@139.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] ALSA: hda: Support for Ideapad hotkey mute LEDs | expand |
On Tue, 24 Dec 2024 09:33:16 +0100, Jackie Dong wrote: > > --- 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); > +} So this unconditionally call alc_fixup_no_shutup(), and this introduces another behavior to the existing entry -- i.e. there is a chance of breakage. If we want to be very conservative, this call should be limited to Ideapad. thanks, Takashi
> On Tue, 24 Dec 2024 09:33:16 +0100, > Jackie Dong wrote: >> >> --- 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); >> +} > > So this unconditionally call alc_fixup_no_shutup(), and this > introduces another behavior to the existing entry -- i.e. there is a > chance of breakage. > > If we want to be very conservative, this call should be limited to > Ideapad. > For alc_fixup_no_shutup(codec, fix, action), I want to keep same behavior with alc_fixup_thinkpad_apci() and alc_fixup_idea_acpi() for one sound card. So, I add alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). ----------Related source code of patch_reatek.c are FYR as below. static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, const struct hda_fixup *fix, int action) { alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ 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); } Thanks, Jackie > thanks, > > Takashi >
On Mon, 30 Dec 2024 01:33:01 +0100, Jackie EG1 Dong wrote: > > > On Tue, 24 Dec 2024 09:33:16 +0100, > > Jackie Dong wrote: > >> > >> --- 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); > >> +} > > > > So this unconditionally call alc_fixup_no_shutup(), and this > introduces another behavior to the existing entry -- i.e. there is a > chance of breakage. > > > > If we want to be very conservative, this call should be limited to > Ideapad. > > For alc_fixup_no_shutup(codec, fix, action), > I want to keep same behavior with alc_fixup_thinkpad_apci() and alc_fixup_idea_acpi() for one sound card. So, I add alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). > ----------Related source code of patch_reatek.c are FYR as below. > static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, > const struct hda_fixup *fix, int > action) > { > alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ > 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); > } Oh yeah, but then it can be bad in other way round; the chain call of alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). That is, alc_fixup_no_shutup() will be called twice for Thinkpad. Instead, you can change like: --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6925,11 +6925,16 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec, /* for hda_fixup_thinkpad_acpi() */ #include "thinkpad_helper.c" -static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, - const struct hda_fixup *fix, int action) +/* for hda_fixup_ideapad_acpi() */ +#include "ideapad_hotkey_led_helper.c" + +/* generic fixup for both Lenovo Thinkpad and Ideapad */ +static void alc_fixup_xpad_acpi(struct hda_codec *codec, + const struct hda_fixup *fix, int action) { alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ hda_fixup_thinkpad_acpi(codec, fix, action); + hda_fixup_ideapad_acpi(codec, fix, action); } /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */ @@ -8321,7 +8326,7 @@ static const struct hda_fixup alc269_fixups[] = { }, [ALC269_FIXUP_THINKPAD_ACPI] = { .type = HDA_FIXUP_FUNC, - .v.func = alc_fixup_thinkpad_acpi, + .v.func = alc_fixup_xpad_acpi, .chained = true, .chain_id = ALC269_FIXUP_SKU_IGNORE, }, Since hda_fixup_ideapad_acpi() is NOP except for Ideapad, this shouldn't break other models, while it covers the Ideadpad now. thanks, Takashi
On 2025/1/3 23:17, Takashi Iwai wrote: > On Mon, 30 Dec 2024 01:33:01 +0100, > Jackie EG1 Dong wrote: >> >>> On Tue, 24 Dec 2024 09:33:16 +0100, >> > Jackie Dong wrote: >> >> >> >> --- 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); >> >> +} >> > >> > So this unconditionally call alc_fixup_no_shutup(), and this > introduces another behavior to the existing entry -- i.e. there is a > chance of breakage. >> > >> > If we want to be very conservative, this call should be limited to > Ideapad. >> > For alc_fixup_no_shutup(codec, fix, action), >> I want to keep same behavior with alc_fixup_thinkpad_apci() and alc_fixup_idea_acpi() for one sound card. So, I add alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). >> ----------Related source code of patch_reatek.c are FYR as below. >> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, >> const struct hda_fixup *fix, int >> action) >> { >> alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ >> 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); >> } > > Oh yeah, but then it can be bad in other way round; the chain call of > alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the > alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). > That is, alc_fixup_no_shutup() will be called twice for Thinkpad. > Many thanks to Takashi for your detail comments and sample code, I understand it now. I'll check the logic of the code and update the patch later. Best Regards, Jackie Dong > Instead, you can change like: > > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -6925,11 +6925,16 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec, > /* for hda_fixup_thinkpad_acpi() */ > #include "thinkpad_helper.c" > > -static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, > - const struct hda_fixup *fix, int action) > +/* for hda_fixup_ideapad_acpi() */ > +#include "ideapad_hotkey_led_helper.c" > + > +/* generic fixup for both Lenovo Thinkpad and Ideapad */ > +static void alc_fixup_xpad_acpi(struct hda_codec *codec, > + const struct hda_fixup *fix, int action) > { > alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ > hda_fixup_thinkpad_acpi(codec, fix, action); > + hda_fixup_ideapad_acpi(codec, fix, action); > } > > /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */ > @@ -8321,7 +8326,7 @@ static const struct hda_fixup alc269_fixups[] = { > }, > [ALC269_FIXUP_THINKPAD_ACPI] = { > .type = HDA_FIXUP_FUNC, > - .v.func = alc_fixup_thinkpad_acpi, > + .v.func = alc_fixup_xpad_acpi, > .chained = true, > .chain_id = ALC269_FIXUP_SKU_IGNORE, > }, > > Since hda_fixup_ideapad_acpi() is NOP except for Ideapad, this > shouldn't break other models, while it covers the Ideadpad now. > > > thanks, > > Takashi
On 1/6/25 20:49, Jackie Dong wrote: > On 2025/1/3 23:17, Takashi Iwai wrote: >> On Mon, 30 Dec 2024 01:33:01 +0100, >> Jackie EG1 Dong wrote: >>> >>>> On Tue, 24 Dec 2024 09:33:16 +0100, >>> > Jackie Dong wrote: >>> >> >>> >> --- 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); >>> >> +} >>> > >>> > So this unconditionally call alc_fixup_no_shutup(), and this > >>> introduces another behavior to the existing entry -- i.e. there is a >>> > chance of breakage. >>> > >>> > If we want to be very conservative, this call should be limited >>> to > Ideapad. >>> > For alc_fixup_no_shutup(codec, fix, action), >>> I want to keep same behavior with alc_fixup_thinkpad_apci() and >>> alc_fixup_idea_acpi() for one sound card. So, I add >>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). >>> ----------Related source code of patch_reatek.c are FYR as below. >>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, >>> const struct hda_fixup *fix, int >>> action) >>> { >>> alc_fixup_no_shutup(codec, fix, action); /* reduce click >>> noise */ >>> 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); >>> } >> >> Oh yeah, but then it can be bad in other way round; the chain call of >> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the >> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). >> That is, alc_fixup_no_shutup() will be called twice for Thinkpad. >> > Many thanks to Takashi for your detail comments and sample code, I > understand it now. > > I'll check the logic of the code and update the patch later. > > Best Regards, > > Jackie Dong Hi Takashi, For this function, I added three debug message in patch_realtek.c as below. I find alc_fixup_no_shutup() only run once, no matter it's in alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some kernel log for your reference. So, I think the patch is ok for this concern. If you have any other concern for the patch, let me know. Thanks for your comment and guide in past. ---------------------------------------------------------------------- ./sound/pci/hda/patch_realtek.c ...... 6122 static void alc_fixup_no_shutup(struct hda_codec *codec, 6123 const struct hda_fixup *fix, int action) 6124 { 6125 if (action == HDA_FIXUP_ACT_PRE_PROBE) { 6126 struct alc_spec *spec = codec->spec; 6127 spec->no_shutup_pins = 1; 6128 } 6129 printk("This is from alc_fixup_no_shutup++444444444444444444444444444444+-.\n");//Deg 6130 } ...... 6929 #include "thinkpad_helper.c" 6930 6931 static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, 6932 const struct hda_fixup *fix, int action) 6933 { 6934 alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ //Deg 6935 printk("This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.\n");//Deg 6936 hda_fixup_thinkpad_acpi(codec, fix, action); 6937 } 6938 6939 /* for hda_fixup_ideapad_acpi() */ 6940 #include "ideapad_hotkey_led_helper.c" 6941 6942 static void alc_fixup_ideapad_acpi(struct hda_codec *codec, 6943 const struct hda_fixup *fix, int action) 6944 { 6945 alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ //Deg 6946 printk("This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.\n");//Deg 6947 hda_fixup_ideapad_acpi(codec, fix, action); 6948 } ...... ---------------------------------------------------------------------- Some log from /var/log/kerlog of ThinkBook 16 G8 IRL. ...... 2025-01-13T16:24:34.109926+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: ALC257: picked fixup for PCI SSID 17aa:0000 2025-01-13T16:24:34.109927+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:24:34.109927+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:24:34.109928+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: autoconfig for ALC257: line_outs=1 (0x14/0x0/0x0/0x0/0x0) type:speaker 2025-01-13T16:24:34.109928+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: speaker_outs=0 (0x0/0x0/0x0/0x0/0x0) 2025-01-13T16:24:34.109929+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: hp_outs=1 (0x21/0x0/0x0/0x0/0x0) 2025-01-13T16:24:34.109929+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: mono: mono_out=0x0 2025-01-13T16:24:34.109930+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: inputs: 2025-01-13T16:24:34.109930+08:00 test-ThinkBook-16-G8-IRL kernel: snd_hda_codec_realtek ehdaudio0D0: Mic=0x19 2025-01-13T16:24:34.109931+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:24:34.109931+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:24:34.109932+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:24:34.109932+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:24:34.109933+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:24:34.109933+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:24:34.109934+08:00 test-ThinkBook-16-G8-IRL kernel: skl_hda_dsp_generic skl_hda_dsp_generic: hda_dsp_hdmi_build_controls: no PCM in topology for HDMI converter 3 2025-01-13T16:24:34.109934+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: base HW address: a8:59:5f:e8:58:dc 2025-01-13T16:24:34.109935+08:00 test-ThinkBook-16-G8-IRL kernel: input: sof-hda-dsp Mic as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input19 2025-01-13T16:24:34.109935+08:00 test-ThinkBook-16-G8-IRL kernel: input: sof-hda-dsp Headphone as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input20 2025-01-13T16:24:34.109936+08:00 test-ThinkBook-16-G8-IRL kernel: input: sof-hda-dsp HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input21 2025-01-13T16:24:34.109936+08:00 test-ThinkBook-16-G8-IRL kernel: input: sof-hda-dsp HDMI/DP,pcm=4 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input22 2025-01-13T16:24:34.109937+08:00 test-ThinkBook-16-G8-IRL kernel: input: sof-hda-dsp HDMI/DP,pcm=5 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input23 2025-01-13T16:24:34.109937+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3 wlp0s20f3: renamed from wlan0 2025-01-13T16:24:34.109938+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Waiting for firmware download to complete 2025-01-13T16:24:34.109938+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Firmware loaded in 1480525 usecs 2025-01-13T16:24:34.109939+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Waiting for device to boot 2025-01-13T16:24:34.109939+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Device booted in 15688 usecs 2025-01-13T16:24:34.109940+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-0040-0041.ddc 2025-01-13T16:24:34.109940+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Applying Intel DDC parameters completed 2025-01-13T16:24:34.109941+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Firmware timestamp 2023.48 buildtype 1 build 75324 2025-01-13T16:24:34.109941+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Firmware SHA1: 0x23bac558 2025-01-13T16:24:34.109942+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Fseq status: Success (0x00) 2025-01-13T16:24:34.109942+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Fseq executed: 00.00.02.41 2025-01-13T16:24:34.109942+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: hci0: Fseq BT Top: 00.00.02.41 2025-01-13T16:24:34.109943+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:24:34.109943+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:24:34.109944+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: BNEP (Ethernet Emulation) ver 1.3 2025-01-13T16:24:34.109944+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: BNEP filters: protocol multicast 2025-01-13T16:24:34.109945+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: BNEP socket layer initialized 2025-01-13T16:24:34.109945+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: MGMT ver 1.23 2025-01-13T16:24:34.109946+08:00 test-ThinkBook-16-G8-IRL kernel: NET: Registered PF_ALG protocol family 2025-01-13T16:24:34.109946+08:00 test-ThinkBook-16-G8-IRL kernel: nvme nvme0: using unchecked data buffer 2025-01-13T16:24:34.132755+08:00 test-ThinkBook-16-G8-IRL kernel: NET: Registered PF_QIPCRTR protocol family 2025-01-13T16:24:34.796796+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: WFPM_UMAC_PD_NOTIFICATION: 0x20 2025-01-13T16:24:34.796810+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: WFPM_LMAC2_PD_NOTIFICATION: 0x1f 2025-01-13T16:24:34.796811+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: WFPM_AUTH_KEY_0: 0x90 2025-01-13T16:24:34.796811+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: CNVI_SCU_SEQ_DATA_DW9: 0x0 2025-01-13T16:24:34.796812+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: RFIm is deactivated, reason = 4 2025-01-13T16:24:34.900941+08:00 test-ThinkBook-16-G8-IRL kernel: iwlwifi 0000:00:14.3: Registered PHC clock: iwlwifi-PTP, with index: 1 2025-01-13T16:24:35.092715+08:00 test-ThinkBook-16-G8-IRL kernel: loop13: detected capacity change from 0 to 8 2025-01-13T16:24:35.360758+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:24:35.360768+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:24:35.476784+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: RFCOMM TTY layer initialized 2025-01-13T16:24:35.476792+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: RFCOMM socket layer initialized 2025-01-13T16:24:35.476793+08:00 test-ThinkBook-16-G8-IRL kernel: Bluetooth: RFCOMM ver 1.11 2025-01-13T16:24:36.000717+08:00 test-ThinkBook-16-G8-IRL kernel: rfkill: input handler disabled 2025-01-13T16:24:37.928735+08:00 test-ThinkBook-16-G8-IRL kernel: wlp0s20f3: authenticate with c2:95:70:cf:7f:31 (local address=a8:59:5f:e8:58:dc) 2025-01-13T16:24:37.928746+08:00 test-ThinkBook-16-G8-IRL kernel: wlp0s20f3: send auth to c2:95:70:cf:7f:31 (try 1/3) 2025-01-13T16:24:37.960811+08:00 test-ThinkBook-16-G8-IRL kernel: wlp0s20f3: authenticated 2025-01-13T16:24:37.960839+08:00 test-ThinkBook-16-G8-IRL kernel: wlp0s20f3: associate with c2:95:70:cf:7f:31 (try 1/3) 2025-01-13T16:24:37.964860+08:00 test-ThinkBook-16-G8-IRL kernel: wlp0s20f3: RX AssocResp from c2:95:70:cf:7f:31 (capab=0x531 status=0 aid=24) 2025-01-13T16:24:37.972839+08:00 test-ThinkBook-16-G8-IRL kernel: wlp0s20f3: associated 2025-01-13T16:25:20.616738+08:00 test-ThinkBook-16-G8-IRL kernel: kauditd_printk_skb: 156 callbacks suppressed 2025-01-13T16:25:20.616752+08:00 test-ThinkBook-16-G8-IRL kernel: audit: type=1400 audit(1736756720.611:168): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2188 comm="snap-confine" capability=12 capname="net_admin" 2025-01-13T16:25:20.616755+08:00 test-ThinkBook-16-G8-IRL kernel: audit: type=1400 audit(1736756720.611:169): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2188 comm="snap-confine" capability=38 capname="perfmon" 2025-01-13T16:25:20.860725+08:00 test-ThinkBook-16-G8-IRL kernel: rfkill: input handler enabled 2025-01-13T16:25:20.984744+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:25:20.984754+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:25:21.588723+08:00 test-ThinkBook-16-G8-IRL kernel: rfkill: input handler disabled 2025-01-13T16:25:23.560752+08:00 test-ThinkBook-16-G8-IRL kernel: audit: type=1400 audit(1736756723.553:170): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=3165 comm="snap-confine" capability=12 capname="net_admin" 2025-01-13T16:25:23.560790+08:00 test-ThinkBook-16-G8-IRL kernel: audit: type=1400 audit(1736756723.553:171): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=3165 comm="snap-confine" capability=38 capname="perfmon" 2025-01-13T16:25:55.972879+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++444444444444444444444444444444+-. 2025-01-13T16:25:55.972922+08:00 test-ThinkBook-16-G8-IRL kernel: This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-. 2025-01-13T16:25:58.090951+08:00 test-ThinkBook-16-G8-IRL kernel: atkbd serio0: Unknown key pressed (translated set 2, code 0xac on isa0060/serio0). ...... ---------------------------------------------------------------------- Some log from /var/log/kerlog of ThinkPad X1 Nano Gen1. ...... Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727147] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727150] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727758] snd_hda_codec_realtek ehdaudio0D0: autoconfig for ALC287: line_outs=2 (0x14/0x17/0x0/0x0/0x0) type:speaker Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727764] snd_hda_codec_realtek ehdaudio0D0: speaker_outs=0 (0x0/0x0/0x0/0x0/0x0) Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727777] snd_hda_codec_realtek ehdaudio0D0: hp_outs=1 (0x21/0x0/0x0/0x0/0x0) Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727779] snd_hda_codec_realtek ehdaudio0D0: mono: mono_out=0x0 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727781] snd_hda_codec_realtek ehdaudio0D0: inputs: Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.727782] snd_hda_codec_realtek ehdaudio0D0: Mic=0x19 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.732045] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.732059] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.775739] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.775743] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.776998] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.777000] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.777636] skl_hda_dsp_generic skl_hda_dsp_generic: hda_dsp_hdmi_build_controls: no PCM in topology for HDMI converter 3 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.812497] input: sof-hda-dsp Mic as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input23 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.812568] input: sof-hda-dsp Headphone as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input24 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.812826] input: sof-hda-dsp HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input25 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.812884] input: sof-hda-dsp HDMI/DP,pcm=4 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input26 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.812930] input: sof-hda-dsp HDMI/DP,pcm=5 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input27 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.841455] Bluetooth: BNEP (Ethernet Emulation) ver 1.3 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.841460] Bluetooth: BNEP filters: protocol multicast Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 5.841464] Bluetooth: BNEP socket layer initialized Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 6.142838] iwlwifi 0000:00:14.3: Registered PHC clock: iwlwifi-PTP, with index: 0 Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 6.241587] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 6.241590] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.175456] loop14: detected capacity change from 0 to 8 Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.234454] Bluetooth: hci0: Waiting for firmware download to complete Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.234793] Bluetooth: hci0: Firmware loaded in 1803474 usecs Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.234871] Bluetooth: hci0: Waiting for device to boot Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.249852] Bluetooth: hci0: Device booted in 14679 usecs Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.250205] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-19-0-4.ddc Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.251900] Bluetooth: hci0: Applying Intel DDC parameters completed Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.252821] Bluetooth: hci0: Firmware revision 0.4 build 206 week 22 2023 Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.254845] Bluetooth: hci0: HCI LE Coded PHY feature bit is set, but its usage is not supported. Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.319888] Bluetooth: MGMT ver 1.23 Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.322558] NET: Registered PF_ALG protocol family Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 7.618988] rfkill: input handler disabled Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9.150754] wlp0s20f3: authenticate with c2:95:70:cf:7f:31 (local address=dc:41:a9:86:dc:a1) Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9.151962] wlp0s20f3: send auth to c2:95:70:cf:7f:31 (try 1/3) Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9.180543] wlp0s20f3: authenticated Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9.181813] wlp0s20f3: associate with c2:95:70:cf:7f:31 (try 1/3) Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9.183387] wlp0s20f3: RX AssocResp from c2:95:70:cf:7f:31 (capab=0x531 status=0 aid=25) Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9.196275] wlp0s20f3: associated Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.438595] kauditd_printk_skb: 52 callbacks suppressed Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.438598] audit: type=1400 audit(1736757039.724:64): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=1542 comm="snap-confine" capability=12 capname="net_admin" Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.438608] audit: type=1400 audit(1736757039.724:65): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=1542 comm="snap-confine" capability=38 capname="perfmon" Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.484757] Bluetooth: RFCOMM TTY layer initialized Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.484764] Bluetooth: RFCOMM socket layer initialized Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.484768] Bluetooth: RFCOMM ver 1.11 Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.660767] rfkill: input handler enabled Jan 13 16:30:40 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.861572] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:30:40 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 29.861577] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 16:30:41 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 30.808052] rfkill: input handler disabled Jan 13 16:30:42 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 32.255105] audit: type=1400 audit(1736757042.540:66): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2163 comm="snap-confine" capability=12 capname="net_admin" Jan 13 16:30:42 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 32.255113] audit: type=1400 audit(1736757042.540:67): apparmor="DENIED" operation="capable" class="cap" profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2163 comm="snap-confine" capability=38 capname="perfmon" Jan 13 16:31:35 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 85.593309] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 13 16:31:35 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 85.593316] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 13 19:06:02 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9352.480329] loop14: detected capacity change from 0 to 562880 Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.618422] audit: type=1400 audit(1736766365.698:68): apparmor="STATUS" operation="profile_replace" info="same as current profile, skipping" profile="unconfined" name="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=3116 comm="apparmor_parser" Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.618433] audit: type=1400 audit(1736766365.698:69): apparmor="STATUS" operation="profile_replace" info="same as current profile, skipping" profile="unconfined" name="/snap/snapd/23545/usr/lib/snapd/snap-confine//mount-namespace-capture-helper" pid=3116 comm="apparmor_parser" Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.730349] audit: type=1400 audit(1736766365.810:70): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.firefox.hook.configure" pid=3121 comm="apparmor_parser" Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.764308] audit: type=1400 audit(1736766365.846:71): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.firefox.hook.post-refresh" pid=3123 comm="apparmor_parser" Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.766762] audit: type=1400 audit(1736766365.846:72): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.firefox.hook.disconnect-plug-host-hunspell" pid=3122 comm="apparmor_parser" Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.049245] audit: type=1400 audit(1736766366.130:73): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.firefox.firefox" pid=3119 comm="apparmor_parser" Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.088026] audit: type=1400 audit(1736766366.170:74): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.firefox.geckodriver" pid=3120 comm="apparmor_parser" Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.104996] audit: type=1400 audit(1736766366.186:75): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap-update-ns.firefox" pid=3118 comm="apparmor_parser" Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.294691] audit: type=1400 audit(1736766366.374:76): apparmor="STATUS" operation="profile_replace" info="same as current profile, skipping" profile="unconfined" name="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=3204 comm="apparmor_parser" Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.294696] audit: type=1400 audit(1736766366.374:77): apparmor="STATUS" operation="profile_replace" info="same as current profile, skipping" profile="unconfined" name="/snap/snapd/23545/usr/lib/snapd/snap-confine//mount-namespace-capture-helper" pid=3204 comm="apparmor_parser" Jan 14 00:00:02 test-ThinkPad-X1-Nano-Gen-1 kernel: [26992.726977] kauditd_printk_skb: 7 callbacks suppressed Jan 14 00:00:02 test-ThinkPad-X1-Nano-Gen-1 kernel: [26992.726981] audit: type=1400 audit(1736784002.633:85): apparmor="DENIED" operation="capable" class="cap" profile="/usr/sbin/cupsd" pid=4432 comm="cupsd" capability=12 capname="net_admin" Jan 14 03:54:13 test-ThinkPad-X1-Nano-Gen-1 kernel: [41043.929183] nvme nvme0: using unchecked data buffer Jan 14 14:13:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [78189.824014] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 14 14:13:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [78189.824021] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. Jan 14 14:37:32 test-ThinkPad-X1-Nano-Gen-1 kernel: [79643.222000] This is from alc_fixup_no_shutup++444444444444444444444444444444+-. Jan 14 14:37:32 test-ThinkPad-X1-Nano-Gen-1 kernel: [79643.222006] This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-. ...... >> Instead, you can change like: >> >> --- a/sound/pci/hda/patch_realtek.c >> +++ b/sound/pci/hda/patch_realtek.c >> @@ -6925,11 +6925,16 @@ static void alc285_fixup_hp_envy_x360(struct >> hda_codec *codec, >> /* for hda_fixup_thinkpad_acpi() */ >> #include "thinkpad_helper.c" >> -static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, >> - const struct hda_fixup *fix, int action) >> +/* for hda_fixup_ideapad_acpi() */ >> +#include "ideapad_hotkey_led_helper.c" >> + >> +/* generic fixup for both Lenovo Thinkpad and Ideapad */ >> +static void alc_fixup_xpad_acpi(struct hda_codec *codec, >> + const struct hda_fixup *fix, int action) >> { >> alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ >> hda_fixup_thinkpad_acpi(codec, fix, action); >> + hda_fixup_ideapad_acpi(codec, fix, action); >> } >> /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset >> removal. */ >> @@ -8321,7 +8326,7 @@ static const struct hda_fixup alc269_fixups[] = { >> }, >> [ALC269_FIXUP_THINKPAD_ACPI] = { >> .type = HDA_FIXUP_FUNC, >> - .v.func = alc_fixup_thinkpad_acpi, >> + .v.func = alc_fixup_xpad_acpi, >> .chained = true, >> .chain_id = ALC269_FIXUP_SKU_IGNORE, >> }, >> >> Since hda_fixup_ideapad_acpi() is NOP except for Ideapad, this >> shouldn't break other models, while it covers the Ideadpad now. >> >> >> thanks, >> >> Takashi >
On Tue, 14 Jan 2025 07:54:01 +0100, Jackie Dong wrote: > > On 1/6/25 20:49, Jackie Dong wrote: > > On 2025/1/3 23:17, Takashi Iwai wrote: > >> On Mon, 30 Dec 2024 01:33:01 +0100, > >> Jackie EG1 Dong wrote: > >>> > >>>> On Tue, 24 Dec 2024 09:33:16 +0100, > >>> > Jackie Dong wrote: > >>> >> > >>> >> --- 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); > >>> >> +} > >>> > > >>> > So this unconditionally call alc_fixup_no_shutup(), and this > >>> > introduces another behavior to the existing entry -- i.e. there > >>> is a > chance of breakage. > >>> > > >>> > If we want to be very conservative, this call should be > >>> limited to > Ideapad. > >>> > For alc_fixup_no_shutup(codec, fix, action), > >>> I want to keep same behavior with alc_fixup_thinkpad_apci() and > >>> alc_fixup_idea_acpi() for one sound card. So, I add > >>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). > >>> ----------Related source code of patch_reatek.c are FYR as below. > >>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, > >>> const struct hda_fixup *fix, int > >>> action) > >>> { > >>> alc_fixup_no_shutup(codec, fix, action); /* reduce click > >>> noise */ > >>> 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); > >>> } > >> > >> Oh yeah, but then it can be bad in other way round; the chain call of > >> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the > >> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). > >> That is, alc_fixup_no_shutup() will be called twice for Thinkpad. > >> > > Many thanks to Takashi for your detail comments and sample code, I > > understand it now. > > > > I'll check the logic of the code and update the patch later. > > > > Best Regards, > > > > Jackie Dong > > Hi Takashi, > For this function, I added three debug message in patch_realtek.c as > below. I find alc_fixup_no_shutup() only run once, no matter it's in > alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some > kernel log for your reference. > So, I think the patch is ok for this concern. > If you have any other concern for the patch, let me know. > Thanks for your comment and guide in past. That's really weird. Are you testing your v2 patch, right? (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls alc_fixup_ideadpad_acpi() and is chained with ALC269_FIXUP_THINKPAD_ACPI. If this entry is really used, it *must* call the alc_fixup_thinkpad_acpi() as well. Please double-check. Takashi
On Tue, 14 Jan 2025 08:19:17 +0100, Takashi Iwai wrote: > > On Tue, 14 Jan 2025 07:54:01 +0100, > Jackie Dong wrote: > > > > On 1/6/25 20:49, Jackie Dong wrote: > > > On 2025/1/3 23:17, Takashi Iwai wrote: > > >> On Mon, 30 Dec 2024 01:33:01 +0100, > > >> Jackie EG1 Dong wrote: > > >>> > > >>>> On Tue, 24 Dec 2024 09:33:16 +0100, > > >>> > Jackie Dong wrote: > > >>> >> > > >>> >> --- 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); > > >>> >> +} > > >>> > > > >>> > So this unconditionally call alc_fixup_no_shutup(), and this > > >>> > introduces another behavior to the existing entry -- i.e. there > > >>> is a > chance of breakage. > > >>> > > > >>> > If we want to be very conservative, this call should be > > >>> limited to > Ideapad. > > >>> > For alc_fixup_no_shutup(codec, fix, action), > > >>> I want to keep same behavior with alc_fixup_thinkpad_apci() and > > >>> alc_fixup_idea_acpi() for one sound card. So, I add > > >>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). > > >>> ----------Related source code of patch_reatek.c are FYR as below. > > >>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, > > >>> const struct hda_fixup *fix, int > > >>> action) > > >>> { > > >>> alc_fixup_no_shutup(codec, fix, action); /* reduce click > > >>> noise */ > > >>> 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); > > >>> } > > >> > > >> Oh yeah, but then it can be bad in other way round; the chain call of > > >> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the > > >> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). > > >> That is, alc_fixup_no_shutup() will be called twice for Thinkpad. > > >> > > > Many thanks to Takashi for your detail comments and sample code, I > > > understand it now. > > > > > > I'll check the logic of the code and update the patch later. > > > > > > Best Regards, > > > > > > Jackie Dong > > > > Hi Takashi, > > For this function, I added three debug message in patch_realtek.c as > > below. I find alc_fixup_no_shutup() only run once, no matter it's in > > alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some > > kernel log for your reference. > > So, I think the patch is ok for this concern. > > If you have any other concern for the patch, let me know. > > Thanks for your comment and guide in past. > > That's really weird. Are you testing your v2 patch, right? > (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls > alc_fixup_ideadpad_acpi() and is chained with > ALC269_FIXUP_THINKPAD_ACPI. If this entry is really used, it *must* > call the alc_fixup_thinkpad_acpi() as well. > > Please double-check. On the second thought, alc_fixup_no_shutup() itself is mostly harmless even if it's called multiple times, as it sets only the flag. But, unifying the quirk function makes more sense as it results in smaller changes. In anyway, the check of the alc_fixup_no_shutup() should be done again; if a test is negative, it doesn't mean it's OK but it's something wrong. thanks, Takashi
On 1/14/25 16:28, Takashi Iwai wrote: > On Tue, 14 Jan 2025 08:19:17 +0100, > Takashi Iwai wrote: >> >> On Tue, 14 Jan 2025 07:54:01 +0100, >> Jackie Dong wrote: >>> >>> On 1/6/25 20:49, Jackie Dong wrote: >>>> On 2025/1/3 23:17, Takashi Iwai wrote: >>>>> On Mon, 30 Dec 2024 01:33:01 +0100, >>>>> Jackie EG1 Dong wrote: >>>>>> >>>>>>> On Tue, 24 Dec 2024 09:33:16 +0100, >>>>>> > Jackie Dong wrote: >>>>>> >> >>>>>> >> --- 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); >>>>>> >> +} >>>>>> > >>>>>> > So this unconditionally call alc_fixup_no_shutup(), and this >>>>>>> introduces another behavior to the existing entry -- i.e. there >>>>>> is a > chance of breakage. >>>>>> > >>>>>> > If we want to be very conservative, this call should be >>>>>> limited to > Ideapad. >>>>>> > For alc_fixup_no_shutup(codec, fix, action), >>>>>> I want to keep same behavior with alc_fixup_thinkpad_apci() and >>>>>> alc_fixup_idea_acpi() for one sound card. So, I add >>>>>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). >>>>>> ----------Related source code of patch_reatek.c are FYR as below. >>>>>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, >>>>>> const struct hda_fixup *fix, int >>>>>> action) >>>>>> { >>>>>> alc_fixup_no_shutup(codec, fix, action); /* reduce click >>>>>> noise */ >>>>>> 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); >>>>>> } >>>>> >>>>> Oh yeah, but then it can be bad in other way round; the chain call of >>>>> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the >>>>> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). >>>>> That is, alc_fixup_no_shutup() will be called twice for Thinkpad. >>>>> >>>> Many thanks to Takashi for your detail comments and sample code, I >>>> understand it now. >>>> >>>> I'll check the logic of the code and update the patch later. >>>> >>>> Best Regards, >>>> >>>> Jackie Dong >>> >>> Hi Takashi, >>> For this function, I added three debug message in patch_realtek.c as >>> below. I find alc_fixup_no_shutup() only run once, no matter it's in >>> alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some >>> kernel log for your reference. >>> So, I think the patch is ok for this concern. >>> If you have any other concern for the patch, let me know. >>> Thanks for your comment and guide in past. >> >> That's really weird. Are you testing your v2 patch, right? >> (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls >> alc_fixup_ideadpad_acpi() and is chained with >> ALC269_FIXUP_THINKPAD_ACPI. If this entry is really used, it *must* >> call the alc_fixup_thinkpad_acpi() as well. >> >> Please double-check. Hi Takashi, You're right. I commented two lines in [ALC269_FIXUP_LENOVO_XPAD_ACPI] and got the result of previous mail. I try to look for which funcion call alc_fixup_thinkpad_acpi() after add below patch. And I hope to impeleted the function with minimum changes. Many thanks, ----------------------------------------------------------------------- --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6126,6 +6126,7 @@ static void alc_fixup_no_shutup(struct hda_codec *codec, struct alc_spec *spec = codec->spec; spec->no_shutup_pins = 1; } + printk("This is from alc_fixup_no_shutup++444444444444444444444444444444+-.\n");//Deg } static void alc_fixup_disable_aamix(struct hda_codec *codec, @@ -6930,10 +6931,22 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec, static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, const struct hda_fixup *fix, int action) { - alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ + alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ //Deg + printk("This is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.\n");//Deg 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 */ //Deg + printk("This is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.\n");//Deg + 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 +7569,7 @@ enum { ALC290_FIXUP_SUBWOOFER, ALC290_FIXUP_SUBWOOFER_HSJACK, ALC269_FIXUP_THINKPAD_ACPI, + ALC269_FIXUP_LENOVO_XPAD_ACPI, ALC269_FIXUP_DMIC_THINKPAD_ACPI, ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13, ALC269VC_FIXUP_INFINIX_Y4_MAX, @@ -8327,6 +8341,12 @@ static const struct hda_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC269_FIXUP_SKU_IGNORE, }, + [ALC269_FIXUP_LENOVO_XPAD_ACPI] = { + .type = HDA_FIXUP_FUNC, + .v.func = alc_fixup_ideapad_acpi, +// .chained = true, +// .chain_id = ALC269_FIXUP_THINKPAD_ACPI, + }, [ALC269_FIXUP_DMIC_THINKPAD_ACPI] = { .type = HDA_FIXUP_FUNC, .v.func = alc_fixup_inv_dmic, @@ -11065,7 +11085,7 @@ static const struct hda_quirk alc269_fixup_vendor_tbl[] = { SND_PCI_QUIRK_VENDOR(0x1025, "Acer Aspire", ALC271_FIXUP_DMIC), SND_PCI_QUIRK_VENDOR(0x103c, "HP", ALC269_FIXUP_HP_MUTE_LED), SND_PCI_QUIRK_VENDOR(0x104d, "Sony VAIO", ALC269_FIXUP_SONY_VAIO), - SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", ALC269_FIXUP_THINKPAD_ACPI), + SND_PCI_QUIRK_VENDOR(0x17aa, "Lenovo XPAD", ALC269_FIXUP_LENOVO_XPAD_ACPI), SND_PCI_QUIRK_VENDOR(0x19e5, "Huawei Matebook", ALC255_FIXUP_MIC_MUTE_LED), {} }; @@ -11130,6 +11150,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {.id = ALC290_FIXUP_MONO_SPEAKERS_HSJACK, .name = "mono-speakers"}, {.id = ALC290_FIXUP_SUBWOOFER_HSJACK, .name = "alc290-subwoofer"}, {.id = ALC269_FIXUP_THINKPAD_ACPI, .name = "thinkpad"}, + {.id = ALC269_FIXUP_LENOVO_XPAD_ACPI, .name = "lenovo xpad led"}, {.id = ALC269_FIXUP_DMIC_THINKPAD_ACPI, .name = "dmic-thinkpad"}, {.id = ALC255_FIXUP_ACER_MIC_NO_PRESENCE, .name = "alc255-acer"}, {.id = ALC255_FIXUP_ASUS_MIC_NO_PRESENCE, .name = "alc255-asus"},
On 1/14/25 23:47, Jackie Dong wrote: > On 1/14/25 16:28, Takashi Iwai wrote: >> On Tue, 14 Jan 2025 08:19:17 +0100, >> Takashi Iwai wrote: >>> >>> On Tue, 14 Jan 2025 07:54:01 +0100, >>> Jackie Dong wrote: >>>> >>>> On 1/6/25 20:49, Jackie Dong wrote: >>>>> On 2025/1/3 23:17, Takashi Iwai wrote: >>>>>> On Mon, 30 Dec 2024 01:33:01 +0100, >>>>>> Jackie EG1 Dong wrote: >>>>>>> >>>>>>>> On Tue, 24 Dec 2024 09:33:16 +0100, >>>>>>> > Jackie Dong wrote: >>>>>>> >> >>>>>>> >> --- 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); >>>>>>> >> +} >>>>>>> > >>>>>>> > So this unconditionally call alc_fixup_no_shutup(), and this >>>>>>>> introduces another behavior to the existing entry -- i.e. there >>>>>>> is a > chance of breakage. >>>>>>> > >>>>>>> > If we want to be very conservative, this call should be >>>>>>> limited to > Ideapad. >>>>>>> > For alc_fixup_no_shutup(codec, fix, action), >>>>>>> I want to keep same behavior with alc_fixup_thinkpad_apci() and >>>>>>> alc_fixup_idea_acpi() for one sound card. So, I add >>>>>>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi(). >>>>>>> ----------Related source code of patch_reatek.c are FYR as below. >>>>>>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, >>>>>>> const struct hda_fixup >>>>>>> *fix, int >>>>>>> action) >>>>>>> { >>>>>>> alc_fixup_no_shutup(codec, fix, action); /* reduce click >>>>>>> noise */ >>>>>>> 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); >>>>>>> } >>>>>> >>>>>> Oh yeah, but then it can be bad in other way round; the chain call of >>>>>> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the >>>>>> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup(). >>>>>> That is, alc_fixup_no_shutup() will be called twice for Thinkpad. >>>>>> >>>>> Many thanks to Takashi for your detail comments and sample code, I >>>>> understand it now. >>>>> >>>>> I'll check the logic of the code and update the patch later. >>>>> >>>>> Best Regards, >>>>> >>>>> Jackie Dong >>>> >>>> Hi Takashi, >>>> For this function, I added three debug message in patch_realtek.c as >>>> below. I find alc_fixup_no_shutup() only run once, no matter it's in >>>> alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some >>>> kernel log for your reference. >>>> So, I think the patch is ok for this concern. >>>> If you have any other concern for the patch, let me know. >>>> Thanks for your comment and guide in past. >>> >>> That's really weird. Are you testing your v2 patch, right? >>> (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls >>> alc_fixup_ideadpad_acpi() and is chained with >>> ALC269_FIXUP_THINKPAD_ACPI. If this entry is really used, it *must* >>> call the alc_fixup_thinkpad_acpi() as well. >>> >>> Please double-check. > Hi Takashi, > You're right. > I commented two lines in [ALC269_FIXUP_LENOVO_XPAD_ACPI] and got the > result of previous mail. I try to look for which funcion call > alc_fixup_thinkpad_acpi() after add below patch. And I hope to impeleted > the function with minimum changes. > Many thanks, > ----------------------------------------------------------------------- > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -6126,6 +6126,7 @@ static void alc_fixup_no_shutup(struct hda_codec > *codec, > struct alc_spec *spec = codec->spec; > spec->no_shutup_pins = 1; > } > + printk("This is from alc_fixup_no_shutup+ > +444444444444444444444444444444+-.\n");//Deg > } > > static void alc_fixup_disable_aamix(struct hda_codec *codec, > @@ -6930,10 +6931,22 @@ static void alc285_fixup_hp_envy_x360(struct > hda_codec *codec, > static void alc_fixup_thinkpad_acpi(struct hda_codec *codec, > const struct hda_fixup *fix, int > action) > { > - alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */ > + alc_fixup_no_shutup(codec, fix, action); /* reduce click noise > */ //Deg > + printk("This is from alc_fixup_no_shutup+ > +TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.\n");//Deg > 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 > */ //Deg > + printk("This is from alc_fixup_no_shutup+ > +IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.\n");//Deg > + 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 +7569,7 @@ enum { > ALC290_FIXUP_SUBWOOFER, > ALC290_FIXUP_SUBWOOFER_HSJACK, > ALC269_FIXUP_THINKPAD_ACPI, > + ALC269_FIXUP_LENOVO_XPAD_ACPI, > ALC269_FIXUP_DMIC_THINKPAD_ACPI, > ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13, > ALC269VC_FIXUP_INFINIX_Y4_MAX, > @@ -8327,6 +8341,12 @@ static const struct hda_fixup alc269_fixups[] = { > .chained = true, > .chain_id = ALC269_FIXUP_SKU_IGNORE, > }, > + [ALC269_FIXUP_LENOVO_XPAD_ACPI] = { > + .type = HDA_FIXUP_FUNC, > + .v.func = alc_fixup_ideapad_acpi, > +// .chained = true, > +// .chain_id = ALC269_FIXUP_THINKPAD_ACPI, > + }, > [ALC269_FIXUP_DMIC_THINKPAD_ACPI] = { > .type = HDA_FIXUP_FUNC, > .v.func = alc_fixup_inv_dmic, > @@ -11065,7 +11085,7 @@ static const struct hda_quirk > alc269_fixup_vendor_tbl[] = { > SND_PCI_QUIRK_VENDOR(0x1025, "Acer Aspire", ALC271_FIXUP_DMIC), > SND_PCI_QUIRK_VENDOR(0x103c, "HP", ALC269_FIXUP_HP_MUTE_LED), > SND_PCI_QUIRK_VENDOR(0x104d, "Sony VAIO", ALC269_FIXUP_SONY_VAIO), > - SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", > ALC269_FIXUP_THINKPAD_ACPI), > + SND_PCI_QUIRK_VENDOR(0x17aa, "Lenovo XPAD", > ALC269_FIXUP_LENOVO_XPAD_ACPI), > SND_PCI_QUIRK_VENDOR(0x19e5, "Huawei Matebook", > ALC255_FIXUP_MIC_MUTE_LED), > {} > }; > @@ -11130,6 +11150,7 @@ static const struct hda_model_fixup > alc269_fixup_models[] = { > {.id = ALC290_FIXUP_MONO_SPEAKERS_HSJACK, .name = "mono- > speakers"}, > {.id = ALC290_FIXUP_SUBWOOFER_HSJACK, .name = "alc290-subwoofer"}, > {.id = ALC269_FIXUP_THINKPAD_ACPI, .name = "thinkpad"}, > + {.id = ALC269_FIXUP_LENOVO_XPAD_ACPI, .name = "lenovo xpad led"}, > {.id = ALC269_FIXUP_DMIC_THINKPAD_ACPI, .name = "dmic-thinkpad"}, > {.id = ALC255_FIXUP_ACER_MIC_NO_PRESENCE, .name = "alc255-acer"}, > {.id = ALC255_FIXUP_ASUS_MIC_NO_PRESENCE, .name = "alc255-asus"}, Hi Takashi, I study for the logic of ALC269_FIXUP_THINKPAD_ACPI call and test many times. From my view, only removed alc_fixup_no_shutup() from alc_fixup_ideapad_acpi() is the best way which has no impact for other functions call ALC269_FIXUP_THINKPAD_ACPI. I have submitted the v3 patch which only removed alc_fixup_no_shutup() from alc_fixup_ideapad_acpi() to make sure alc_fixup_no_shutup() only be called once for Thinkpad and Ideapad. Pls review it again and give your comments. Best Regards, Jackie Dong
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..c10d97964d49 --- /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..4985e72b9094 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_LENOVO_XPAD_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,12 @@ static const struct hda_fixup cxt_fixups[] = { .type = HDA_FIXUP_FUNC, .v.func = hda_fixup_thinkpad_acpi, }, + [CXT_FIXUP_LENOVO_XPAD_ACPI] = { + .type = HDA_FIXUP_FUNC, + .v.func = hda_fixup_ideapad_acpi, + .chained = true, + .chain_id = CXT_FIXUP_THINKPAD_ACPI, + }, [CXT_FIXUP_OLPC_XO] = { .type = HDA_FIXUP_FUNC, .v.func = cxt_fixup_olpc_xo, @@ -1119,7 +1129,7 @@ static const struct hda_quirk cxt5066_fixups[] = { SND_PCI_QUIRK(0x17aa, 0x3977, "Lenovo IdeaPad U310", CXT_FIXUP_STEREO_DMIC), 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, "Thinkpad/Ideapad", CXT_FIXUP_LENOVO_XPAD_ACPI), 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 +1143,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_LENOVO_XPAD_ACPI, .name = "thinkpad-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..5f9927401322 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_LENOVO_XPAD_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_LENOVO_XPAD_ACPI] = { + .type = HDA_FIXUP_FUNC, + .v.func = alc_fixup_ideapad_acpi, + .chained = true, + .chain_id = ALC269_FIXUP_THINKPAD_ACPI, + }, [ALC269_FIXUP_DMIC_THINKPAD_ACPI] = { .type = HDA_FIXUP_FUNC, .v.func = alc_fixup_inv_dmic, @@ -11065,7 +11082,7 @@ static const struct hda_quirk alc269_fixup_vendor_tbl[] = { SND_PCI_QUIRK_VENDOR(0x1025, "Acer Aspire", ALC271_FIXUP_DMIC), SND_PCI_QUIRK_VENDOR(0x103c, "HP", ALC269_FIXUP_HP_MUTE_LED), SND_PCI_QUIRK_VENDOR(0x104d, "Sony VAIO", ALC269_FIXUP_SONY_VAIO), - SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", ALC269_FIXUP_THINKPAD_ACPI), + SND_PCI_QUIRK_VENDOR(0x17aa, "Lenovo XPAD", ALC269_FIXUP_LENOVO_XPAD_ACPI), SND_PCI_QUIRK_VENDOR(0x19e5, "Huawei Matebook", ALC255_FIXUP_MIC_MUTE_LED), {} }; @@ -11130,6 +11147,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {.id = ALC290_FIXUP_MONO_SPEAKERS_HSJACK, .name = "mono-speakers"}, {.id = ALC290_FIXUP_SUBWOOFER_HSJACK, .name = "alc290-subwoofer"}, {.id = ALC269_FIXUP_THINKPAD_ACPI, .name = "thinkpad"}, + {.id = ALC269_FIXUP_LENOVO_XPAD_ACPI, .name = "lenovo xpad led"}, {.id = ALC269_FIXUP_DMIC_THINKPAD_ACPI, .name = "dmic-thinkpad"}, {.id = ALC255_FIXUP_ACER_MIC_NO_PRESENCE, .name = "alc255-acer"}, {.id = ALC255_FIXUP_ASUS_MIC_NO_PRESENCE, .name = "alc255-asus"},
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@squebb.ca> Signed-off-by: Jackie Dong <xy-jackie@139.com> --- Changes in v2: - Add a new enum CXT_FIXUP_LENOVO_XPAD_ACPI and define it as an entry in patch_conexant.c - Add a new enum ALC269_FIXUP_LENOVO_XPAD_ACPI and define it as an entry in patch_realtek.c sound/pci/hda/ideapad_hotkey_led_helper.c | 36 +++++++++++++++++++++++ sound/pci/hda/patch_conexant.c | 13 +++++++- sound/pci/hda/patch_realtek.c | 20 ++++++++++++- 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 sound/pci/hda/ideapad_hotkey_led_helper.c