Message-ID: <Z2Rmj6V0FFbWUhCd@google.com>
Date: Thu, 19 Dec 2024 10:31:43 -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

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.

-- 
Dmitry

Powered by blists - more mailing lists