Message-ID: <877c1672-744a-473f-a4dd-45fd10cfee11@redhat.com>
Date: Thu, 19 Dec 2024 17:01:09 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Mark Pearson <mpearson-lenovo@...ebb.ca>,
 Peter Hutterer <peter.hutterer@...hat.com>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>, linux-input@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: atkbd: Fix so copilot key generates F23 keycode

Hi,

Really +Cc Peter Hutterer this time.

On 19-Dec-24 4:48 PM, Mark Pearson wrote:
> Hi Hans
> 
> On Thu, Dec 19, 2024, at 10:28 AM, Hans de Goede wrote:
>> +Cc Peter Hutterer
> 
> My bad - I've been discussing this with Peter and should have added him. Thanks for including (sorry Peter!)

Except I forgot to actually add Peter...

>> Hi Mark,
>>
>> Thank you for your patch.
>>
>> On 19-Dec-24 4:18 PM, Mark Pearson wrote:
>>> The copilot key on Lenovo laptops doesn't work as scancode 0x6e, which it
>>> generates is not mapped.
>>> This change lets scancode 0x6e generate keycode 193 (F23 key) which is
>>> the expected value for copilot.
>>>
>>> Tested on T14s G6 AMD.
>>> I've had reports from other users that their ThinkBooks are using the same
>>> scancode.
>>
>> Hmm, I'm not sure mapping this to KEY_F23 is the right thing to do,
>> there are 2 issues with this approach:
>>
>> 1. /usr/share/X11/xkb/symbols/inet currently maps this to
>>    XF86TouchpadOff as F20 - F23 where repurposed to
>>    TouchPad on/off/toggle / micmute to work around X11
>>    not allowing key-codes > 247.
>>
>>    We are actually working on removing this X11 workaround
>>    to make F20-F23 available as normal key-codes again
>>    for keyboards which actually have such keys.
>>
>> 2. There are some keyboards which have an actual F23 key
>>    and mapping the co-pilot key to that and then having
>>    desktop environments grow default keybindings on top
>>    of that will basically mean clobbering the F23 key or
>>    at least making it harder to use.
>>
>> I think was is necessary instead is to add a new
>> KEY_COPILOT to include/uapi/linux/input-event-codes.h
>> and use that instead.
> 
> Sorry, should have been clearer in the commit message.
> I'm doing this just on the Microsoft spec. The co-pilot key is left-shift, Windows/Meta key, F23. Weird combo I know....
> 
> Somewhere I had a MS page...but this Tom's HW page mentions it:
> https://www.tomshardware.com/software/windows/windows-copilot-key-is-secretly-from-the-ibm-era-but-you-can-remap-it-with-the-right-tools
> 
> I'll see if I can find something more formal.
> 
>>
>> Peter, I thought I read somewhere that you were looking
>> into mapping the copilot key to a new  KEY_COPILOT evdev
>> key for some other keyboards?
>>
> 
> Wouldn't this require the kernel catching all three key events and doing the interpretation? I have no idea how this would be done or if it makes sense.

So I guess I got caught off guard by your commit message
which suggests that only scancode 0x6e is generated.

If indeed a left-shift + Windows/Meta key + 0x6e combination
is send them this is a different story, since indeed we
cannot filter on that in the kernel. Although sometimes
I wonder if we should because we are seeing similar things
where left-shift + Windows/Meta key + xxxx is send for
e.g. touchpad on/off toggle.

To workaround this atm GNOME listens for XF86TouchpadToggle
as well as shift + meta + XF86TouchpadToggle, theoretically it
would be nice if we can recognize these special key-combos at
a lower level. But thinking about this that is nasty, because
then we would get an event sequence like this:

Report shift pressed
Report meta pressed
<oops special key, release them>
Report meta released
Report shift released
Report KEY_TOUCHPAD_TOGGLE
<and what do we do with the modifiers now?
 for correctness I guess we report them
 as pressed again until the hw reports them released>
Report shift pressed
Report meta pressed
<hw releases the fake modifiers>
Report meta released
Report shift released

So yeah handling this in the kernel is not going to be pretty.

So I think your right and just mapping this to F23 is probably
best, but I would like to hear what Peter thinks first.

Regards,

Hans




>>> Signed-off-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
>>> ---
>>>  drivers/input/keyboard/atkbd.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
>>> index 5855d4fc6e6a..f7b08b359c9c 100644
>>> --- a/drivers/input/keyboard/atkbd.c
>>> +++ b/drivers/input/keyboard/atkbd.c
>>> @@ -89,7 +89,7 @@ static const unsigned short atkbd_set2_keycode[ATKBD_KEYMAP_SIZE] = {
>>>  	  0, 46, 45, 32, 18,  5,  4, 95,  0, 57, 47, 33, 20, 19,  6,183,
>>>  	  0, 49, 48, 35, 34, 21,  7,184,  0,  0, 50, 36, 22,  8,  9,185,
>>>  	  0, 51, 37, 23, 24, 11, 10,  0,  0, 52, 53, 38, 39, 25, 12,  0,
>>> -	  0, 89, 40,  0, 26, 13,  0,  0, 58, 54, 28, 27,  0, 43,  0, 85,
>>> +	  0, 89, 40,  0, 26, 13,  0,193, 58, 54, 28, 27,  0, 43,  0, 85,
>>>  	  0, 86, 91, 90, 92,  0, 14, 94,  0, 79,124, 75, 71,121,  0,  0,
>>>  	 82, 83, 80, 76, 77, 72,  1, 69, 87, 78, 81, 74, 55, 73, 70, 99,
>>>
> 

Powered by blists - more mailing lists