diff mbox series

[v2,6/7] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE

Message ID 20210220122438.21857-7-hdegoede@redhat.com
State Superseded
Headers show
Series HID: lenovo: Mute LED handling fixes and improvements | expand

Commit Message

Hans de Goede Feb. 20, 2021, 12:24 p.m. UTC
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(-)

Comments

Marek Behún Feb. 21, 2021, 1:42 a.m. UTC | #1
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
Hans de Goede Feb. 21, 2021, 10:42 a.m. UTC | #2
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
Marek Behún Feb. 21, 2021, 11:37 a.m. UTC | #3
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.
Hans de Goede Feb. 21, 2021, 11:50 a.m. UTC | #4
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 mbox series

Patch

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