RE: [External] Re: [PATCH] thinkpad_acpi: Allow the FnLock LED to change state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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  




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux