diff mbox series

Input: atkbd: Fix so copilot key generates F23 keycode

Message ID 20241219151910.14235-1-mpearson-lenovo@squebb.ca (mailing list archive)
State New
Headers show
Series Input: atkbd: Fix so copilot key generates F23 keycode | expand

Commit Message

Mark Pearson Dec. 19, 2024, 3:18 p.m. UTC
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.

Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
 drivers/input/keyboard/atkbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Hans de Goede Dec. 19, 2024, 3:28 p.m. UTC | #1
+Cc Peter Hutterer

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.

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?

Regards,

Hans



> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.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,
>
Mark Pearson Dec. 19, 2024, 3:48 p.m. UTC | #2
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!)

>
> 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.

> Regards,
>
> Hans
>

Thanks!
Mark

>
>
>> 
>> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.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,
>>
Hans de Goede Dec. 19, 2024, 4:01 p.m. UTC | #3
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@squebb.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,
>>>
>
Dmitry Torokhov Dec. 19, 2024, 6:15 p.m. UTC | #4
Hi,

On Thu, Dec 19, 2024 at 05:01:09PM +0100, Hans de Goede wrote:
> 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.

We have discussed this with Peter and came to the conclusion that
KEY_ASSISTANT should cover this use case.

Also, this tweak should go into udev rules (/lib/udev/hwdb.d/60-keyboard.hwdb)
instead of adding a vendor-specific tweak to the main atkbd table.

For the future releases you may want to add "linux,keymap" device
property to your ACPI/DSDT to control the scancode->keycode mapping when
Linux is running.

> > 
> > 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

No, you have to delay to see if it is real press or part of sequence.

> <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.

Yes, we have a form of this in drivers/tty/sysrq.c and it indeed is not
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.

So vendor yet again encoded a shortcut sequence into firmware,
beautiful. I guess you can try to install a 8042 filter
(via i8042_add_filter()) in drivers/platform/x86/lenovo-<something>.c
to monitor for this specific scancode sequence and replace it with
something else (through an auxiliary input device). 

Thanks.
Hans de Goede Dec. 19, 2024, 6:26 p.m. UTC | #5
Hi,

On 19-Dec-24 7:15 PM, Dmitry Torokhov wrote:
> Hi,
> 
> On Thu, Dec 19, 2024 at 05:01:09PM +0100, Hans de Goede wrote:
>> 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.
> 
> We have discussed this with Peter and came to the conclusion that
> KEY_ASSISTANT should cover this use case.
> 
> Also, this tweak should go into udev rules (/lib/udev/hwdb.d/60-keyboard.hwdb)
> instead of adding a vendor-specific tweak to the main atkbd table.
> 
> For the future releases you may want to add "linux,keymap" device
> property to your ACPI/DSDT to control the scancode->keycode mapping when
> Linux is running.
> 
>>>
>>> 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
> 
> No, you have to delay to see if it is real press or part of sequence.
> 
>> <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.
> 
> Yes, we have a form of this in drivers/tty/sysrq.c and it indeed is not
> 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.
> 
> So vendor yet again encoded a shortcut sequence into firmware,
> beautiful. I guess you can try to install a 8042 filter
> (via i8042_add_filter()) in drivers/platform/x86/lenovo-<something>.c
> to monitor for this specific scancode sequence and replace it with
> something else (through an auxiliary input device). 

If we want to filter out these in essence fake modifier
events then this needs to be done at some core level,
because AFAIK the shift + meta + F23 key-combo is what
microsoft is telling OEMs to use, so we are going to see this on
laptops from all vendors including whitelabel laptops.

Regards,

Hans
Dmitry Torokhov Dec. 19, 2024, 6:31 p.m. UTC | #6
On Thu, Dec 19, 2024 at 07:26:19PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 19-Dec-24 7:15 PM, Dmitry Torokhov wrote:
> > Hi,
> > 
> > On Thu, Dec 19, 2024 at 05:01:09PM +0100, Hans de Goede wrote:
> >> 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.
> > 
> > We have discussed this with Peter and came to the conclusion that
> > KEY_ASSISTANT should cover this use case.
> > 
> > Also, this tweak should go into udev rules (/lib/udev/hwdb.d/60-keyboard.hwdb)
> > instead of adding a vendor-specific tweak to the main atkbd table.
> > 
> > For the future releases you may want to add "linux,keymap" device
> > property to your ACPI/DSDT to control the scancode->keycode mapping when
> > Linux is running.
> > 
> >>>
> >>> 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
> > 
> > No, you have to delay to see if it is real press or part of sequence.
> > 
> >> <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.
> > 
> > Yes, we have a form of this in drivers/tty/sysrq.c and it indeed is not
> > 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.
> > 
> > So vendor yet again encoded a shortcut sequence into firmware,
> > beautiful. I guess you can try to install a 8042 filter
> > (via i8042_add_filter()) in drivers/platform/x86/lenovo-<something>.c
> > to monitor for this specific scancode sequence and replace it with
> > something else (through an auxiliary input device). 
> 
> If we want to filter out these in essence fake modifier
> events then this needs to be done at some core level,
> because AFAIK the shift + meta + F23 key-combo is what
> microsoft is telling OEMs to use, so we are going to see this on
> laptops from all vendors including whitelabel laptops.

Hm, then I'd rather leave it to the userspace shortcut handling to deal
with. It's probably gonna disappear the same way as others in a couple
of years ;) and be replaced with some thing else.

And mapping to F23 as I said should be done through udev. I doubt they
will get all OEMs settle on the same scancode.

Thanks.
Mark Pearson Dec. 19, 2024, 6:40 p.m. UTC | #7
On Thu, Dec 19, 2024, at 1:31 PM, Dmitry Torokhov wrote:
> On Thu, Dec 19, 2024 at 07:26:19PM +0100, Hans de Goede wrote:
>> Hi,
>> 
>> On 19-Dec-24 7:15 PM, Dmitry Torokhov wrote:
>> > Hi,
>> > 
>> > On Thu, Dec 19, 2024 at 05:01:09PM +0100, Hans de Goede wrote:
>> >> 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.
>> > 
>> > We have discussed this with Peter and came to the conclusion that
>> > KEY_ASSISTANT should cover this use case.
>> > 
>> > Also, this tweak should go into udev rules (/lib/udev/hwdb.d/60-keyboard.hwdb)
>> > instead of adding a vendor-specific tweak to the main atkbd table.
>> > 
>> > For the future releases you may want to add "linux,keymap" device
>> > property to your ACPI/DSDT to control the scancode->keycode mapping when
>> > Linux is running.

I can look into this, but gut feeling is it's a bad solution for the Linux ecosystem as it will limit it to only platforms in the Lenovo Linux program. Be nicer to have a more widespread solution.

>> > 
>> >>>
>> >>> 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
>> > 
>> > No, you have to delay to see if it is real press or part of sequence.
>> > 
>> >> <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.
>> > 
>> > Yes, we have a form of this in drivers/tty/sysrq.c and it indeed is not
>> > 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.
>> > 
>> > So vendor yet again encoded a shortcut sequence into firmware,
>> > beautiful. I guess you can try to install a 8042 filter
>> > (via i8042_add_filter()) in drivers/platform/x86/lenovo-<something>.c
>> > to monitor for this specific scancode sequence and replace it with
>> > something else (through an auxiliary input device). 
>> 
>> If we want to filter out these in essence fake modifier
>> events then this needs to be done at some core level,
>> because AFAIK the shift + meta + F23 key-combo is what
>> microsoft is telling OEMs to use, so we are going to see this on
>> laptops from all vendors including whitelabel laptops.
>
> Hm, then I'd rather leave it to the userspace shortcut handling to deal
> with. It's probably gonna disappear the same way as others in a couple
> of years ;) and be replaced with some thing else.
>
> And mapping to F23 as I said should be done through udev. I doubt they
> will get all OEMs settle on the same scancode.
>

I'll see if we can find a way to check on other vendor platforms what scancode is used.
If it is a common scancode, across multiple vendors, would the patch be acceptable?

If it isn't then I agree it's not suitable. I assumed it would be common and hadn't really considered that it wouldn't be - my bad.

Mark
Dmitry Torokhov Dec. 19, 2024, 7:17 p.m. UTC | #8
On Thu, Dec 19, 2024 at 01:40:24PM -0500, Mark Pearson wrote:
> On Thu, Dec 19, 2024, at 1:31 PM, Dmitry Torokhov wrote:
> > On Thu, Dec 19, 2024 at 07:26:19PM +0100, Hans de Goede wrote:
> >> Hi,
> >> 
> >> On 19-Dec-24 7:15 PM, Dmitry Torokhov wrote:
> >> > Hi,
> >> > 
> >> > On Thu, Dec 19, 2024 at 05:01:09PM +0100, Hans de Goede wrote:
> >> >> 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.
> >> > 
> >> > We have discussed this with Peter and came to the conclusion that
> >> > KEY_ASSISTANT should cover this use case.
> >> > 
> >> > Also, this tweak should go into udev rules (/lib/udev/hwdb.d/60-keyboard.hwdb)
> >> > instead of adding a vendor-specific tweak to the main atkbd table.
> >> > 
> >> > For the future releases you may want to add "linux,keymap" device
> >> > property to your ACPI/DSDT to control the scancode->keycode mapping when
> >> > Linux is running.
> 
> I can look into this, but gut feeling is it's a bad solution for the Linux ecosystem as it will limit it to only platforms in the Lenovo Linux program. Be nicer to have a more widespread solution.
> 
> >> > 
> >> >>>
> >> >>> 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
> >> > 
> >> > No, you have to delay to see if it is real press or part of sequence.
> >> > 
> >> >> <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.
> >> > 
> >> > Yes, we have a form of this in drivers/tty/sysrq.c and it indeed is not
> >> > 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.
> >> > 
> >> > So vendor yet again encoded a shortcut sequence into firmware,
> >> > beautiful. I guess you can try to install a 8042 filter
> >> > (via i8042_add_filter()) in drivers/platform/x86/lenovo-<something>.c
> >> > to monitor for this specific scancode sequence and replace it with
> >> > something else (through an auxiliary input device). 
> >> 
> >> If we want to filter out these in essence fake modifier
> >> events then this needs to be done at some core level,
> >> because AFAIK the shift + meta + F23 key-combo is what
> >> microsoft is telling OEMs to use, so we are going to see this on
> >> laptops from all vendors including whitelabel laptops.
> >
> > Hm, then I'd rather leave it to the userspace shortcut handling to deal
> > with. It's probably gonna disappear the same way as others in a couple
> > of years ;) and be replaced with some thing else.
> >
> > And mapping to F23 as I said should be done through udev. I doubt they
> > will get all OEMs settle on the same scancode.
> >
> 
> I'll see if we can find a way to check on other vendor platforms what scancode is used.
> If it is a common scancode, across multiple vendors, would the patch be acceptable?

It is currently unmapped by default, so maybe.

FWIW:

dtor@dtor-ws:~/kernel/work $ grep KEY_6e /lib/udev/hwdb.d/60-keyboard.hwdb
KEYBOARD_KEY_6e=wlan
KEYBOARD_KEY_6e=left                                   # left on d-pad
KEYBOARD_KEY_6e=search

That 2nd entry is actually from one of Thinkpad models ;)

Thanks.
diff mbox series

Patch

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,