| Message ID | 20210220122438.21857-7-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HID: lenovo: Mute LED handling fixes and improvements | expand |
On Sat, 20 Feb 2021 13:24:37 +0100 Hans de Goede <hdegoede@redhat.com> wrote: > Mapping the mic-mute button to KEY_MICMUTE is technically correct but > KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X, > which does not fit in 8 bits, so it does not work. Why does it need to fit 8 bits? Where is the problem? Marek
Hi, On 2/21/21 2:42 AM, Marek Behun wrote: > On Sat, 20 Feb 2021 13:24:37 +0100 > Hans de Goede <hdegoede@redhat.com> wrote: > >> Mapping the mic-mute button to KEY_MICMUTE is technically correct but >> KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X, >> which does not fit in 8 bits, so it does not work. > > Why does it need to fit 8 bits? Where is the problem? As the commit message says, "under X" aka X11 / Xorg. This is a well known limitation of the X11 input stack / of XKB *as implemented in X11* the Wayland input stack does not have this limitations and does allow using raw key-codes >= 248. If you look at e.g. : https://github.com/systemd/systemd/blob/main/hwdb.d/60-keyboard.hwdb Which (mostly) maps custom PS/2 scancodes used for some "media" keys on laptops to linux evdev KEY_FOO codes, then you will see that there are no lines there which end with "=micmute" instead there are quite a few lines like this: KEYBOARD_KEY_8a=f20 # Microphone mute button; should be micmute Arguably it would be more correct to have the kernel still send KEY_MICMUTE and do the remapping to KEY_F20 in userspace in e.g. hwdb. But that will not work here, the remapping is done based on mapping the HID usage-code to a new evdev KEY_FOO code, basically overriding lenovo_input_mapping_tp10_ultrabook_kbd() mapping. But the "Lenovo ThinkPad 10 Ultrabook Keyboard" uses the same 0x000c0001 usage code for all of its custom Fn+F# media keys, so instead of doing the mapping purely on usage-code it is done on a combination of usage-code + the index of the key in the input-report (since the usage-code is not unique for a single key): /* * The ThinkPad 10 Ultrabook Keyboard uses 0x000c0001 usage for * a bunch of keys which have no standard consumer page code. */ if (usage->hid == 0x000c0001) { switch (usage->usage_index) { case 8: /* Fn-Esc: Fn-lock toggle */ map_key_clear(KEY_FN_ESC); return 1; case 9: /* Fn-F4: Mic mute */ map_key_clear(LENOVO_KEY_MICMUTE); return 1; ... So in this case we cannot fixup the mapping from userspace, as userspace remapping is purely done based on the "scancode" which in case of HID devices is the HID usage-code. I don't even know what will happen if we were to try. I guess that either the first key with a matching usage-code is remapped, or all of them are remapped, both of which are wrong. Regards, Hans
On Sun, 21 Feb 2021 11:42:16 +0100 Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 2/21/21 2:42 AM, Marek Behun wrote: > > On Sat, 20 Feb 2021 13:24:37 +0100 > > Hans de Goede <hdegoede@redhat.com> wrote: > > > >> Mapping the mic-mute button to KEY_MICMUTE is technically correct but > >> KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X, > >> which does not fit in 8 bits, so it does not work. > > > > Why does it need to fit 8 bits? Where is the problem? > > As the commit message says, "under X" aka X11 / Xorg. This is a well known > limitation of the X11 input stack / of XKB *as implemented in X11* > the Wayland input stack does not have this limitations and does allow > using raw key-codes >= 248. > > If you look at e.g. : > https://github.com/systemd/systemd/blob/main/hwdb.d/60-keyboard.hwdb > > Which (mostly) maps custom PS/2 scancodes used for some "media" keys > on laptops to linux evdev KEY_FOO codes, then you will see that there > are no lines there which end with "=micmute" instead there are quite > a few lines like this: > > KEYBOARD_KEY_8a=f20 # Microphone mute button; should be micmute > > Arguably it would be more correct to have the kernel still send > KEY_MICMUTE and do the remapping to KEY_F20 in userspace in e.g. hwdb. > > But that will not work here, the remapping is done based on mapping > the HID usage-code to a new evdev KEY_FOO code, basically overriding > lenovo_input_mapping_tp10_ultrabook_kbd() mapping. > > But the "Lenovo ThinkPad 10 Ultrabook Keyboard" uses the same 0x000c0001 > usage code for all of its custom Fn+F# media keys, so instead of doing > the mapping purely on usage-code it is done on a combination of usage-code + > the index of the key in the input-report (since the usage-code is not unique > for a single key): > > /* > * The ThinkPad 10 Ultrabook Keyboard uses 0x000c0001 usage for > * a bunch of keys which have no standard consumer page code. > */ > if (usage->hid == 0x000c0001) { > switch (usage->usage_index) { > case 8: /* Fn-Esc: Fn-lock toggle */ > map_key_clear(KEY_FN_ESC); > return 1; > case 9: /* Fn-F4: Mic mute */ > map_key_clear(LENOVO_KEY_MICMUTE); > return 1; > ... > > > So in this case we cannot fixup the mapping from userspace, as userspace > remapping is purely done based on the "scancode" which in case of HID devices > is the HID usage-code. > > I don't even know what will happen if we were to try. I guess that either the > first key with a matching usage-code is remapped, or all of them are remapped, > both of which are wrong. > > Regards, > > Hans > And no one ever solved this for X? OMFG :( Very well then.
Hi, On 2/21/21 12:37 PM, Marek Behún wrote: > On Sun, 21 Feb 2021 11:42:16 +0100 > Hans de Goede <hdegoede@redhat.com> wrote: > >> Hi, >> >> On 2/21/21 2:42 AM, Marek Behun wrote: >>> On Sat, 20 Feb 2021 13:24:37 +0100 >>> Hans de Goede <hdegoede@redhat.com> wrote: >>> >>>> Mapping the mic-mute button to KEY_MICMUTE is technically correct but >>>> KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X, >>>> which does not fit in 8 bits, so it does not work. >>> >>> Why does it need to fit 8 bits? Where is the problem? >> >> As the commit message says, "under X" aka X11 / Xorg. This is a well known >> limitation of the X11 input stack / of XKB *as implemented in X11* >> the Wayland input stack does not have this limitations and does allow >> using raw key-codes >= 248. >> >> If you look at e.g. : >> https://github.com/systemd/systemd/blob/main/hwdb.d/60-keyboard.hwdb >> >> Which (mostly) maps custom PS/2 scancodes used for some "media" keys >> on laptops to linux evdev KEY_FOO codes, then you will see that there >> are no lines there which end with "=micmute" instead there are quite >> a few lines like this: >> >> KEYBOARD_KEY_8a=f20 # Microphone mute button; should be micmute >> >> Arguably it would be more correct to have the kernel still send >> KEY_MICMUTE and do the remapping to KEY_F20 in userspace in e.g. hwdb. >> >> But that will not work here, the remapping is done based on mapping >> the HID usage-code to a new evdev KEY_FOO code, basically overriding >> lenovo_input_mapping_tp10_ultrabook_kbd() mapping. >> >> But the "Lenovo ThinkPad 10 Ultrabook Keyboard" uses the same 0x000c0001 >> usage code for all of its custom Fn+F# media keys, so instead of doing >> the mapping purely on usage-code it is done on a combination of usage-code + >> the index of the key in the input-report (since the usage-code is not unique >> for a single key): >> >> /* >> * The ThinkPad 10 Ultrabook Keyboard uses 0x000c0001 usage for >> * a bunch of keys which have no standard consumer page code. >> */ >> if (usage->hid == 0x000c0001) { >> switch (usage->usage_index) { >> case 8: /* Fn-Esc: Fn-lock toggle */ >> map_key_clear(KEY_FN_ESC); >> return 1; >> case 9: /* Fn-F4: Mic mute */ >> map_key_clear(LENOVO_KEY_MICMUTE); >> return 1; >> ... >> >> >> So in this case we cannot fixup the mapping from userspace, as userspace >> remapping is purely done based on the "scancode" which in case of HID devices >> is the HID usage-code. >> >> I don't even know what will happen if we were to try. I guess that either the >> first key with a matching usage-code is remapped, or all of them are remapped, >> both of which are wrong. >> >> Regards, >> >> Hans >> > > And no one ever solved this for X? OMFG :( Many people have looked into fixing this, but X11 is a network protocol, and the other side could be a many many years old X-terminal (one of those devices which don't have their own OS but connect over xdmcp to show a desktop running on some big machine somewhere else on the LAN). XKB in X is layered on top of the original X input protocol, and the data gets passed around multiple times in multiple different structs and it is limited to a 8 bit wide int everywhere. Note it is not just micmute. The F13 - F24 F-key range has been used to work around this for a while now. I'm aware of the following "mappings" being used for this: evdev -> interpreted by userspace as KEY_F20 -> mic-mute toggle KEY_F21 -> touchpad on/off toggle KEY_F22 -> touchpad on KEY_F23 -> touchpad off This is not pretty, I know. Regards, Hans
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c index d936edb88f07..041bfa1937a8 100644 --- a/drivers/hid/hid-lenovo.c +++ b/drivers/hid/hid-lenovo.c @@ -33,6 +33,9 @@ #include "hid-ids.h" +/* Userspace expects F20 for mic-mute KEY_MICMUTE does not work */ +#define LENOVO_KEY_MICMUTE KEY_F20 + struct lenovo_drvdata { u8 led_report[3]; /* Must be first for proper alignment */ int led_state; @@ -128,7 +131,7 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev, if (usage->hid == (HID_UP_BUTTON | 0x0010)) { /* This sub-device contains trackpoint, mark it */ hid_set_drvdata(hdev, (void *)1); - map_key_clear(KEY_MICMUTE); + map_key_clear(LENOVO_KEY_MICMUTE); return 1; } return 0; @@ -143,7 +146,7 @@ static int lenovo_input_mapping_cptkbd(struct hid_device *hdev, (usage->hid & HID_USAGE_PAGE) == HID_UP_LNVENDOR) { switch (usage->hid & HID_USAGE) { case 0x00f1: /* Fn-F4: Mic mute */ - map_key_clear(KEY_MICMUTE); + map_key_clear(LENOVO_KEY_MICMUTE); return 1; case 0x00f2: /* Fn-F5: Brightness down */ map_key_clear(KEY_BRIGHTNESSDOWN); @@ -233,7 +236,7 @@ static int lenovo_input_mapping_tp10_ultrabook_kbd(struct hid_device *hdev, map_key_clear(KEY_FN_ESC); return 1; case 9: /* Fn-F4: Mic mute */ - map_key_clear(KEY_MICMUTE); + map_key_clear(LENOVO_KEY_MICMUTE); return 1; case 10: /* Fn-F7: Control panel */ map_key_clear(KEY_CONFIG);
Mapping the mic-mute button to KEY_MICMUTE is technically correct but KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X, which does not fit in 8 bits, so it does not work. Because of this userspace is expecting KEY_F20 instead, theoretically KEY_MICMUTE should work under Wayland but even there it does not work, because the desktop-environment is listening only for KEY_F20 and not for KEY_MICMUTE. Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support") Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/hid/hid-lenovo.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)