Hello, >-----Original Message----- >From: Mark RH Pearson <markpearson@xxxxxxxxxx> >Sent: Friday, March 19, 2021 5:13 AM >To: Hans de Goede <hdegoede@xxxxxxxxxx>; Esteve Varela Colominas ><esteve.varela@xxxxxxxxx>; Henrique de Moraes Holschuh <ibm- >acpi@xxxxxxxxxx>; Nitin Joshi1 <njoshi1@xxxxxxxxxx> >Cc: ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx; platform-driver- >x86@xxxxxxxxxxxxxxx >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@xxxxxxxxx> >>> --- >>> 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
