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 |
+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, >
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, >>
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, >>> >
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.
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
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.
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
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.
Hi Dmitry, On Thu, Dec 19, 2024, at 2:17 PM, Dmitry Torokhov wrote: > 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: >> > 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 ;) > I got confirmation from the keyboard team that 0x6e is the scancode from F23, and is common for all PC vendors for Windows. They pointed me at this page as confirmation: https://learn.microsoft.com/en-us/windows/win32/inputdev/about-keyboard-input#scan-codes (F23 is in the table of scan codes) Does that make this patch valid again for consideration, in your opinion? Thanks Mark
On Sun, Dec 22, 2024, at 3:16 PM, Mark Pearson wrote: > Hi Dmitry, > > On Thu, Dec 19, 2024, at 2:17 PM, Dmitry Torokhov wrote: >> 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: >>> > 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 ;) >> > I got confirmation from the keyboard team that 0x6e is the scancode > from F23, and is common for all PC vendors for Windows. > > They pointed me at this page as confirmation: > https://learn.microsoft.com/en-us/windows/win32/inputdev/about-keyboard-input#scan-codes > (F23 is in the table of scan codes) > > Does that make this patch valid again for consideration, in your opinion? > Apologies for the multiple emails, but just wanted to add a note that Canonical kindly tested for me on a Dell and HP platform, and confirmed they are using the same scan code (as expected at this point). This change should benefit all vendors. Thanks Mark
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,
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(-)