Message-ID: <Z2Ri3cuiRYDap2f6@google.com>
Date: Thu, 19 Dec 2024 10:15:57 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Mark Pearson <mpearson-lenovo@...ebb.ca>,
	Peter Hutterer <peter.hutterer@...hat.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: atkbd: Fix so copilot key generates F23 keycode

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.

-- 
Dmitry

Powered by blists - more mailing lists