* [PATCH] Allow the FnLock LED to change state @ 2021-03-15 19:58 Esteve Varela Colominas 2021-03-18 16:49 ` [PATCH] thinkpad_acpi: " Hans de Goede 2021-03-21 16:38 ` [PATCH] " Hans de Goede 0 siblings, 2 replies; 12+ messages in thread From: Esteve Varela Colominas @ 2021-03-15 19:58 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Esteve Varela Colominas, ibm-acpi-devel, platform-driver-x86 On many recent ThinkPad laptops, there's a new LED next to the ESC key, that indicates the FnLock status. When the Fn+ESC combo is pressed, FnLock is toggled, which causes the Media Key functionality to change, making it so that the media keys either perform their media key function, or function as an F-key by default. The Fn key can be used the access the alternate function at any time. With the current linux kernel, the LED doens't change state if you press the Fn+ESC key combo. However, the media key functionality *does* change. This is annoying, since the LED will stay on if it was on during bootup, and it makes it hard to keep track what the current state of the FnLock is. This patch calls an ACPI function, that gets the current media key state, when the Fn+ESC key combo is pressed. Through testing it was discovered that this function causes the LED to update correctly to reflect the current state when this function is called. The relevant ACPI calls are the following: \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if the FnLock mode is enabled, and 0x602 if it's disabled. \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will enable FnLock mode, and a 0 will disable it. Relevant discussion: https://bugzilla.kernel.org/show_bug.cgi?id=207841 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> --- drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index c40470637..09362dd74 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -4079,13 +4079,19 @@ static bool hotkey_notify_6xxx(const u32 hkey, case TP_HKEY_EV_KEY_NUMLOCK: case TP_HKEY_EV_KEY_FN: - case TP_HKEY_EV_KEY_FN_ESC: /* key press events, we just ignore them as long as the EC * is still reporting them in the normal keyboard stream */ *send_acpi_ev = false; *ignore_acpi_ev = true; return true; + case TP_HKEY_EV_KEY_FN_ESC: + /* Get the media key status to foce the status LED to update */ + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); + *send_acpi_ev = false; + *ignore_acpi_ev = true; + return true; + case TP_HKEY_EV_TABLET_CHANGED: tpacpi_input_send_tabletsw(); hotkey_tablet_mode_notify_change(); -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state 2021-03-15 19:58 [PATCH] Allow the FnLock LED to change state Esteve Varela Colominas @ 2021-03-18 16:49 ` Hans de Goede 2021-03-18 20:13 ` [External] " Mark Pearson 2021-03-21 16:38 ` [PATCH] " Hans de Goede 1 sibling, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-03-18 16:49 UTC (permalink / raw) To: Esteve Varela Colominas, Henrique de Moraes Holschuh, Mark Pearson Cc: ibm-acpi-devel, platform-driver-x86 Hi, On 3/15/21 8:58 PM, Esteve Varela Colominas wrote: > On many recent ThinkPad laptops, there's a new LED next to the ESC key, > that indicates the FnLock status. > When the Fn+ESC combo is pressed, FnLock is toggled, which causes the > Media Key functionality to change, making it so that the media keys > either perform their media key function, or function as an F-key by > default. The Fn key can be used the access the alternate function at any > time. > > With the current linux kernel, the LED doens't change state if you press > the Fn+ESC key combo. However, the media key functionality *does* > change. This is annoying, since the LED will stay on if it was on during > bootup, and it makes it hard to keep track what the current state of the > FnLock is. > > This patch calls an ACPI function, that gets the current media key > state, when the Fn+ESC key combo is pressed. Through testing it was > discovered that this function causes the LED to update correctly to > reflect the current state when this function is called. > > The relevant ACPI calls are the following: > \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if the FnLock mode is enabled, and 0x602 if it's disabled. > \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will enable FnLock mode, and a 0 will disable it. > > Relevant discussion: > https://bugzilla.kernel.org/show_bug.cgi?id=207841 > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 > > Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> > --- > drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index c40470637..09362dd74 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -4079,13 +4079,19 @@ static bool hotkey_notify_6xxx(const u32 hkey, > > case TP_HKEY_EV_KEY_NUMLOCK: > case TP_HKEY_EV_KEY_FN: > - case TP_HKEY_EV_KEY_FN_ESC: > /* key press events, we just ignore them as long as the EC > * is still reporting them in the normal keyboard stream */ > *send_acpi_ev = false; > *ignore_acpi_ev = true; > return true; > > + case TP_HKEY_EV_KEY_FN_ESC: > + /* Get the media key status to foce the status LED to update */ > + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); Sicne this is a getter function I guess that calling it is mostly harmless and if it works around what seems to be a firmware bug on some of the E?95 ThinkPad models then I guess that this is fine by me. Mark, do you have any comments on this ? Regards, Hans > + *send_acpi_ev = false; > + *ignore_acpi_ev = true; > + return true; > + > case TP_HKEY_EV_TABLET_CHANGED: > tpacpi_input_send_tabletsw(); > hotkey_tablet_mode_notify_change(); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state 2021-03-18 16:49 ` [PATCH] thinkpad_acpi: " Hans de Goede @ 2021-03-18 20:13 ` Mark Pearson 2021-03-19 10:39 ` Nitin Joshi1 0 siblings, 1 reply; 12+ messages in thread From: Mark Pearson @ 2021-03-18 20:13 UTC (permalink / raw) To: Hans de Goede, Esteve Varela Colominas, Henrique de Moraes Holschuh, Nitin Joshi1 Cc: ibm-acpi-devel, platform-driver-x86 Thanks Hans On 18/03/2021 12:49, Hans de Goede wrote: > Hi, > > On 3/15/21 8:58 PM, Esteve Varela Colominas wrote: >> On many recent ThinkPad laptops, there's a new LED next to the ESC key, >> that indicates the FnLock status. >> When the Fn+ESC combo is pressed, FnLock is toggled, which causes the >> Media Key functionality to change, making it so that the media keys >> either perform their media key function, or function as an F-key by >> default. The Fn key can be used the access the alternate function at any >> time. >> >> With the current linux kernel, the LED doens't change state if you press >> the Fn+ESC key combo. However, the media key functionality *does* >> change. This is annoying, since the LED will stay on if it was on during >> bootup, and it makes it hard to keep track what the current state of the >> FnLock is. >> >> This patch calls an ACPI function, that gets the current media key >> state, when the Fn+ESC key combo is pressed. Through testing it was >> discovered that this function causes the LED to update correctly to >> reflect the current state when this function is called. >> >> The relevant ACPI calls are the following: >> \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if the FnLock mode is enabled, and 0x602 if it's disabled. >> \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will enable FnLock mode, and a 0 will disable it. >> >> Relevant discussion: >> https://bugzilla.kernel.org/show_bug.cgi?id=207841 >> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 >> >> Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> >> --- >> drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c >> index c40470637..09362dd74 100644 >> --- a/drivers/platform/x86/thinkpad_acpi.c >> +++ b/drivers/platform/x86/thinkpad_acpi.c >> @@ -4079,13 +4079,19 @@ static bool hotkey_notify_6xxx(const u32 hkey, >> >> case TP_HKEY_EV_KEY_NUMLOCK: >> case TP_HKEY_EV_KEY_FN: >> - case TP_HKEY_EV_KEY_FN_ESC: >> /* key press events, we just ignore them as long as the EC >> * is still reporting them in the normal keyboard stream */ >> *send_acpi_ev = false; >> *ignore_acpi_ev = true; >> return true; >> >> + case TP_HKEY_EV_KEY_FN_ESC: >> + /* Get the media key status to foce the status LED to update */ >> + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); > > Sicne this is a getter function I guess that calling it is mostly harmless > and if it works around what seems to be a firmware bug on some of the E?95 > ThinkPad models then I guess that this is fine by me. > > Mark, do you have any comments on this ? I'd like to follow up with the firmware team to make sure we've got the correct details and implementation (kudos on the reverse engineering though). Nitin - you've worked with the firmware team on hotkeys, would you mind digging into this one please to confirm. In particular if there's been a change how do we make sure we don't impact older platforms etc. Mark ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state 2021-03-18 20:13 ` [External] " Mark Pearson @ 2021-03-19 10:39 ` Nitin Joshi1 2021-03-19 11:35 ` Enrico Weigelt, metux IT consult 2021-03-21 16:37 ` Hans de Goede 0 siblings, 2 replies; 12+ messages in thread From: Nitin Joshi1 @ 2021-03-19 10:39 UTC (permalink / raw) To: Mark RH Pearson, Hans de Goede, Esteve Varela Colominas, Henrique de Moraes Holschuh Cc: ibm-acpi-devel, platform-driver-x86 Hello, >-----Original Message----- >From: Mark RH Pearson <markpearson@lenovo.com> >Sent: Friday, March 19, 2021 5:13 AM >To: Hans de Goede <hdegoede@redhat.com>; Esteve Varela Colominas ><esteve.varela@gmail.com>; Henrique de Moraes Holschuh <ibm- >acpi@hmh.eng.br>; Nitin Joshi1 <njoshi1@lenovo.com> >Cc: ibm-acpi-devel@lists.sourceforge.net; platform-driver- >x86@vger.kernel.org >Subject: Re: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to >change state > >Thanks Hans > >On 18/03/2021 12:49, Hans de Goede wrote: >> Hi, >> >> On 3/15/21 8:58 PM, Esteve Varela Colominas wrote: >>> On many recent ThinkPad laptops, there's a new LED next to the ESC >>> key, that indicates the FnLock status. >>> When the Fn+ESC combo is pressed, FnLock is toggled, which causes the >>> Media Key functionality to change, making it so that the media keys >>> either perform their media key function, or function as an F-key by >>> default. The Fn key can be used the access the alternate function at >>> any time. >>> >>> With the current linux kernel, the LED doens't change state if you >>> press the Fn+ESC key combo. However, the media key functionality >>> *does* change. This is annoying, since the LED will stay on if it was >>> on during bootup, and it makes it hard to keep track what the current >>> state of the FnLock is. >>> >>> This patch calls an ACPI function, that gets the current media key >>> state, when the Fn+ESC key combo is pressed. Through testing it was >>> discovered that this function causes the LED to update correctly to >>> reflect the current state when this function is called. >>> >>> The relevant ACPI calls are the following: >>> \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if >the FnLock mode is enabled, and 0x602 if it's disabled. >>> \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will >enable FnLock mode, and a 0 will disable it. >>> >>> Relevant discussion: >>> https://bugzilla.kernel.org/show_bug.cgi?id=207841 >>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 >>> >>> Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> >>> --- >>> drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/platform/x86/thinkpad_acpi.c >>> b/drivers/platform/x86/thinkpad_acpi.c >>> index c40470637..09362dd74 100644 >>> --- a/drivers/platform/x86/thinkpad_acpi.c >>> +++ b/drivers/platform/x86/thinkpad_acpi.c >>> @@ -4079,13 +4079,19 @@ static bool hotkey_notify_6xxx(const u32 >>> hkey, >>> >>> case TP_HKEY_EV_KEY_NUMLOCK: >>> case TP_HKEY_EV_KEY_FN: >>> - case TP_HKEY_EV_KEY_FN_ESC: >>> /* key press events, we just ignore them as long as the EC >>> * is still reporting them in the normal keyboard stream */ >>> *send_acpi_ev = false; >>> *ignore_acpi_ev = true; >>> return true; >>> >>> + case TP_HKEY_EV_KEY_FN_ESC: >>> + /* Get the media key status to foce the status LED to update */ >>> + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); >> >> Sicne this is a getter function I guess that calling it is mostly >> harmless and if it works around what seems to be a firmware bug on >> some of the E?95 ThinkPad models then I guess that this is fine by me. >> >> Mark, do you have any comments on this ? >I'd like to follow up with the firmware team to make sure we've got the correct >details and implementation (kudos on the reverse engineering though). > >Nitin - you've worked with the firmware team on hotkeys, would you mind >digging into this one please to confirm. In particular if there's been a change >how do we make sure we don't impact older platforms etc. Regarding "GMKS" method, it does not have "version" related information. So , its unlikely to impact any older platforms. However, I got it confirmed that definition of GMKS method itself doesn't include any workaround feature. But, since its getter function , I also think its harmless and if it workaround some issue then I don’t see any concern. > >Mark Thanks & Regards, Nitin Joshi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state 2021-03-19 10:39 ` Nitin Joshi1 @ 2021-03-19 11:35 ` Enrico Weigelt, metux IT consult 2021-03-22 0:53 ` Nitin Joshi1 2021-03-21 16:37 ` Hans de Goede 1 sibling, 1 reply; 12+ messages in thread From: Enrico Weigelt, metux IT consult @ 2021-03-19 11:35 UTC (permalink / raw) To: Nitin Joshi1, Mark RH Pearson, Hans de Goede, Esteve Varela Colominas, Henrique de Moraes Holschuh Cc: ibm-acpi-devel, platform-driver-x86 On 19.03.21 11:39, Nitin Joshi1 wrote: Hi, > Regarding "GMKS" method, it does not have "version" related information. So , its unlikely to impact any older platforms. > However, I got it confirmed that definition of GMKS method itself doesn't include any workaround feature. > > But, since its getter function , I also think its harmless and if it workaround some issue then I don’t see any concern. How about publishing schematics / specs, so we can confirm what it's really doing ? In recent decades I've learn to mistrust vendor-provided firmware (especially when it comes to acpi). The reason why I'm currently bisecting AMD's AGESA blob, in order to completely replace it. It's a tedious job, had to write my own analysis tool, but step by step making progress. (and already learned they've been using a pretty inefficient compiler that can't even inline trivial memcpy(). Probably some older msvc ... :p) --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state 2021-03-19 11:35 ` Enrico Weigelt, metux IT consult @ 2021-03-22 0:53 ` Nitin Joshi1 0 siblings, 0 replies; 12+ messages in thread From: Nitin Joshi1 @ 2021-03-22 0:53 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult, Mark RH Pearson, Hans de Goede, Esteve Varela Colominas, Henrique de Moraes Holschuh Cc: ibm-acpi-devel, platform-driver-x86 Hello Enrico, >-----Original Message----- >From: Enrico Weigelt, metux IT consult <info@metux.net> >Sent: Friday, March 19, 2021 8:36 PM >To: Nitin Joshi1 <njoshi1@lenovo.com>; Mark RH Pearson ><markpearson@lenovo.com>; Hans de Goede <hdegoede@redhat.com>; >Esteve Varela Colominas <esteve.varela@gmail.com>; Henrique de Moraes >Holschuh <ibm-acpi@hmh.eng.br> >Cc: ibm-acpi-devel@lists.sourceforge.net; platform-driver- >x86@vger.kernel.org >Subject: Re: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to >change state > >On 19.03.21 11:39, Nitin Joshi1 wrote: > >Hi, > >> Regarding "GMKS" method, it does not have "version" related information. >So , its unlikely to impact any older platforms. >> However, I got it confirmed that definition of GMKS method itself doesn't >include any workaround feature. >> >> But, since its getter function , I also think its harmless and if it workaround >some issue then I don’t see any concern. > >How about publishing schematics / specs, so we can confirm what it's really >doing ? Thank you for your comment. I will check regarding this and feedback you later. > >In recent decades I've learn to mistrust vendor-provided firmware (especially >when it comes to acpi). The reason why I'm currently bisecting AMD's AGESA >blob, in order to completely replace it. > >It's a tedious job, had to write my own analysis tool, but step by step making >progress. (and already learned they've been using a pretty inefficient compiler >that can't even inline trivial memcpy(). >Probably some older msvc ... :p) > >--mtx Thanks & Regards, Nitin Joshi > >-- >--- >Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert >werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren >GPG/PGP-Schlüssel zu. >--- >Enrico Weigelt, metux IT consult >Free software and Linux embedded engineering info@metux.net -- +49-151- >27565287 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state 2021-03-19 10:39 ` Nitin Joshi1 2021-03-19 11:35 ` Enrico Weigelt, metux IT consult @ 2021-03-21 16:37 ` Hans de Goede 1 sibling, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-03-21 16:37 UTC (permalink / raw) To: Nitin Joshi1, Mark RH Pearson, Esteve Varela Colominas, Henrique de Moraes Holschuh Cc: ibm-acpi-devel, platform-driver-x86 Hi, On 3/19/21 11:39 AM, Nitin Joshi1 wrote: > Hello, > >> -----Original Message----- >> From: Mark RH Pearson <markpearson@lenovo.com> >> Sent: Friday, March 19, 2021 5:13 AM >> To: Hans de Goede <hdegoede@redhat.com>; Esteve Varela Colominas >> <esteve.varela@gmail.com>; Henrique de Moraes Holschuh <ibm- >> acpi@hmh.eng.br>; Nitin Joshi1 <njoshi1@lenovo.com> >> Cc: ibm-acpi-devel@lists.sourceforge.net; platform-driver- >> x86@vger.kernel.org >> Subject: Re: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to >> change state >> >> Thanks Hans >> >> On 18/03/2021 12:49, Hans de Goede wrote: >>> Hi, >>> >>> On 3/15/21 8:58 PM, Esteve Varela Colominas wrote: >>>> On many recent ThinkPad laptops, there's a new LED next to the ESC >>>> key, that indicates the FnLock status. >>>> When the Fn+ESC combo is pressed, FnLock is toggled, which causes the >>>> Media Key functionality to change, making it so that the media keys >>>> either perform their media key function, or function as an F-key by >>>> default. The Fn key can be used the access the alternate function at >>>> any time. >>>> >>>> With the current linux kernel, the LED doens't change state if you >>>> press the Fn+ESC key combo. However, the media key functionality >>>> *does* change. This is annoying, since the LED will stay on if it was >>>> on during bootup, and it makes it hard to keep track what the current >>>> state of the FnLock is. >>>> >>>> This patch calls an ACPI function, that gets the current media key >>>> state, when the Fn+ESC key combo is pressed. Through testing it was >>>> discovered that this function causes the LED to update correctly to >>>> reflect the current state when this function is called. >>>> >>>> The relevant ACPI calls are the following: >>>> \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if >> the FnLock mode is enabled, and 0x602 if it's disabled. >>>> \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will >> enable FnLock mode, and a 0 will disable it. >>>> >>>> Relevant discussion: >>>> https://bugzilla.kernel.org/show_bug.cgi?id=207841 >>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 >>>> >>>> Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> >>>> --- >>>> drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c >>>> b/drivers/platform/x86/thinkpad_acpi.c >>>> index c40470637..09362dd74 100644 >>>> --- a/drivers/platform/x86/thinkpad_acpi.c >>>> +++ b/drivers/platform/x86/thinkpad_acpi.c >>>> @@ -4079,13 +4079,19 @@ static bool hotkey_notify_6xxx(const u32 >>>> hkey, >>>> >>>> case TP_HKEY_EV_KEY_NUMLOCK: >>>> case TP_HKEY_EV_KEY_FN: >>>> - case TP_HKEY_EV_KEY_FN_ESC: >>>> /* key press events, we just ignore them as long as the EC >>>> * is still reporting them in the normal keyboard stream */ >>>> *send_acpi_ev = false; >>>> *ignore_acpi_ev = true; >>>> return true; >>>> >>>> + case TP_HKEY_EV_KEY_FN_ESC: >>>> + /* Get the media key status to foce the status LED to update */ >>>> + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); >>> >>> Sicne this is a getter function I guess that calling it is mostly >>> harmless and if it works around what seems to be a firmware bug on >>> some of the E?95 ThinkPad models then I guess that this is fine by me. >>> >>> Mark, do you have any comments on this ? >> I'd like to follow up with the firmware team to make sure we've got the correct >> details and implementation (kudos on the reverse engineering though). >> >> Nitin - you've worked with the firmware team on hotkeys, would you mind >> digging into this one please to confirm. In particular if there's been a change >> how do we make sure we don't impact older platforms etc. > > Regarding "GMKS" method, it does not have "version" related information. So , its unlikely to impact any older platforms. > However, I got it confirmed that definition of GMKS method itself doesn't include any workaround feature. > > But, since its getter function , I also think its harmless and if it workaround some issue then I don’t see any concern. Ok, I'll merge this patch then. Regards, Hans ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Allow the FnLock LED to change state 2021-03-15 19:58 [PATCH] Allow the FnLock LED to change state Esteve Varela Colominas 2021-03-18 16:49 ` [PATCH] thinkpad_acpi: " Hans de Goede @ 2021-03-21 16:38 ` Hans de Goede 2021-03-21 17:15 ` Esteve Varela 1 sibling, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-03-21 16:38 UTC (permalink / raw) To: Esteve Varela Colominas, Henrique de Moraes Holschuh Cc: ibm-acpi-devel, platform-driver-x86 Hi, On 3/15/21 8:58 PM, Esteve Varela Colominas wrote: > On many recent ThinkPad laptops, there's a new LED next to the ESC key, > that indicates the FnLock status. > When the Fn+ESC combo is pressed, FnLock is toggled, which causes the > Media Key functionality to change, making it so that the media keys > either perform their media key function, or function as an F-key by > default. The Fn key can be used the access the alternate function at any > time. > > With the current linux kernel, the LED doens't change state if you press > the Fn+ESC key combo. However, the media key functionality *does* > change. This is annoying, since the LED will stay on if it was on during > bootup, and it makes it hard to keep track what the current state of the > FnLock is. > > This patch calls an ACPI function, that gets the current media key > state, when the Fn+ESC key combo is pressed. Through testing it was > discovered that this function causes the LED to update correctly to > reflect the current state when this function is called. > > The relevant ACPI calls are the following: > \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if the FnLock mode is enabled, and 0x602 if it's disabled. > \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will enable FnLock mode, and a 0 will disable it. > > Relevant discussion: > https://bugzilla.kernel.org/show_bug.cgi?id=207841 > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 > > Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans And I'll also add it to the fixes branch so that it gets included in one of the future 5.12-rc releases. Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index c40470637..09362dd74 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -4079,13 +4079,19 @@ static bool hotkey_notify_6xxx(const u32 hkey, > > case TP_HKEY_EV_KEY_NUMLOCK: > case TP_HKEY_EV_KEY_FN: > - case TP_HKEY_EV_KEY_FN_ESC: > /* key press events, we just ignore them as long as the EC > * is still reporting them in the normal keyboard stream */ > *send_acpi_ev = false; > *ignore_acpi_ev = true; > return true; > > + case TP_HKEY_EV_KEY_FN_ESC: > + /* Get the media key status to foce the status LED to update */ > + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); > + *send_acpi_ev = false; > + *ignore_acpi_ev = true; > + return true; > + > case TP_HKEY_EV_TABLET_CHANGED: > tpacpi_input_send_tabletsw(); > hotkey_tablet_mode_notify_change(); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Allow the FnLock LED to change state 2021-03-21 16:38 ` [PATCH] " Hans de Goede @ 2021-03-21 17:15 ` Esteve Varela 2021-03-21 17:23 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Esteve Varela @ 2021-03-21 17:15 UTC (permalink / raw) To: Hans de Goede Cc: Henrique de Moraes Holschuh, ibm-acpi-devel, platform-driver-x86 2021-03-21 16:38 GMT, Hans de Goede <hdegoede@redhat.com>: > Hi, > > Thank you for your patch, I've applied this patch to my review-hans > branch: > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans > > And I'll also add it to the fixes branch so that it gets included in > one of the future 5.12-rc releases. > > Note it will show up in my review-hans branch once I've pushed my > local branch there, which might take a while. > > Once I've run some tests on this branch the patches there will be > added to the platform-drivers-x86/for-next branch and eventually > will be included in the pdx86 pull-request to Linus for the next > merge-window. > > Regards, > > Hans Thanks a ton! Especially for making sure there's no hardware compatibility issues, since I was afraid that might've been an issue. I just noticed that there's a typo in a comment, "foce" that should be "force". Maybe that can be amended before the branch is merged elsewhere. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Allow the FnLock LED to change state 2021-03-21 17:15 ` Esteve Varela @ 2021-03-21 17:23 ` Hans de Goede 2021-03-21 18:35 ` [PATCH] platform/x86: thinkpad_acpi: Correct minor typo Esteve Varela Colominas 0 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-03-21 17:23 UTC (permalink / raw) To: Esteve Varela Cc: Henrique de Moraes Holschuh, ibm-acpi-devel, platform-driver-x86 Hi, On 3/21/21 6:15 PM, Esteve Varela wrote: > 2021-03-21 16:38 GMT, Hans de Goede <hdegoede@redhat.com>: >> Hi, >> >> Thank you for your patch, I've applied this patch to my review-hans >> branch: >> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans >> >> And I'll also add it to the fixes branch so that it gets included in >> one of the future 5.12-rc releases. >> >> Note it will show up in my review-hans branch once I've pushed my >> local branch there, which might take a while. >> >> Once I've run some tests on this branch the patches there will be >> added to the platform-drivers-x86/for-next branch and eventually >> will be included in the pdx86 pull-request to Linus for the next >> merge-window. >> >> Regards, >> >> Hans > > Thanks a ton! Especially for making sure there's no hardware > compatibility issues, since I was afraid that might've been an issue. > I just noticed that there's a typo in a comment, "foce" that should be > "force". Maybe that can be amended before the branch is merged > elsewhere. I've already pushed this to the public pdx86 git repo for-next and fixes branches, so too late to amend. If you can send a followup-patch fixing the type, then I'll add that to for-next. Regards, Hans ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] platform/x86: thinkpad_acpi: Correct minor typo 2021-03-21 17:23 ` Hans de Goede @ 2021-03-21 18:35 ` Esteve Varela Colominas 2021-03-23 20:10 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Esteve Varela Colominas @ 2021-03-21 18:35 UTC (permalink / raw) To: Hans de Goede Cc: Esteve Varela Colominas, ibm-acpi-devel, platform-driver-x86 Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> --- drivers/platform/x86/thinkpad_acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 09362dd74..f3f7ae6f6 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -4086,7 +4086,7 @@ static bool hotkey_notify_6xxx(const u32 hkey, return true; case TP_HKEY_EV_KEY_FN_ESC: - /* Get the media key status to foce the status LED to update */ + /* Get the media key status to force the status LED to update */ acpi_evalf(hkey_handle, NULL, "GMKS", "v"); *send_acpi_ev = false; *ignore_acpi_ev = true; -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] platform/x86: thinkpad_acpi: Correct minor typo 2021-03-21 18:35 ` [PATCH] platform/x86: thinkpad_acpi: Correct minor typo Esteve Varela Colominas @ 2021-03-23 20:10 ` Hans de Goede 0 siblings, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-03-23 20:10 UTC (permalink / raw) To: Esteve Varela Colominas; +Cc: ibm-acpi-devel, platform-driver-x86 Hi, On 3/21/21 7:35 PM, Esteve Varela Colominas wrote: > Signed-off-by: Esteve Varela Colominas <esteve.varela@gmail.com> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/thinkpad_acpi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 09362dd74..f3f7ae6f6 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -4086,7 +4086,7 @@ static bool hotkey_notify_6xxx(const u32 hkey, > return true; > > case TP_HKEY_EV_KEY_FN_ESC: > - /* Get the media key status to foce the status LED to update */ > + /* Get the media key status to force the status LED to update */ > acpi_evalf(hkey_handle, NULL, "GMKS", "v"); > *send_acpi_ev = false; > *ignore_acpi_ev = true; > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-03-23 20:11 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-15 19:58 [PATCH] Allow the FnLock LED to change state Esteve Varela Colominas 2021-03-18 16:49 ` [PATCH] thinkpad_acpi: " Hans de Goede 2021-03-18 20:13 ` [External] " Mark Pearson 2021-03-19 10:39 ` Nitin Joshi1 2021-03-19 11:35 ` Enrico Weigelt, metux IT consult 2021-03-22 0:53 ` Nitin Joshi1 2021-03-21 16:37 ` Hans de Goede 2021-03-21 16:38 ` [PATCH] " Hans de Goede 2021-03-21 17:15 ` Esteve Varela 2021-03-21 17:23 ` Hans de Goede 2021-03-21 18:35 ` [PATCH] platform/x86: thinkpad_acpi: Correct minor typo Esteve Varela Colominas 2021-03-23 20:10 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.