Re: Support for X1 tablet keyboard (was Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY 0x4012, 0x4013 events)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

finally I got to investigate that patch. Thanks for your draft and explanations!

On 2/12/21 12:42 AM, Hans de Goede wrote:
> Hi,
> 
> On 2/9/21 4:16 PM, Alexander Kobel wrote:
>> Hi,
>>
>> On 2/8/21 11:17 AM, Hans de Goede wrote:
>>> On 2/7/21 6:55 PM, Alexander Kobel wrote:
>>>> <snip>
>>>> I'll go off and try to improve.
>>>
>>> So Nitin has been kind enough to provide us with some docs for this,
>>> please see me reply to Nitin's email and lets continue this part of this mail
>>> thread there.
>>
>> Right. I have the other patch ready, thanks to your great help. I'm
>> waiting for Nitin's okay whether / how much info I can copy from the
>> reference sheet to source code comments. Once I have that confirmation,
>> I will post the revised patch.
>>
>>> <snip>
>>>
>>>> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
>>>> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
>>>> very question is among them.
>>>> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
>>>> and Nitin can already hint about the keyboard shortcuts, it'd be highly
>>>> appreciated.
>>>
>>> I think I might be able to help there, a couple of months ago I bought
>>> a second-hand thinkpad-10 tablet which also has a USB attached keyboard.
>>>
>>> In hindsight I guess I could have asked Mark and Nitin for some more info,
>>> but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
>>> to see which events all the keys send.
>>>
>>> And I fired up an usb-sniffer under Windows to figure out the audio-leds,
>>> since I'm used to just figure these things out without help from the vendor :)
>>
>> Yeah, why take the boring route if you know how to do all the work on
>> your own... ;-)
>>
>>> See the recent commits here for my work on this:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c
>>
>> Thanks, very helpful.
>>
>>> So on the ibm-acpi list message you said that the kbd sends the following:
>>>
>>> type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
>>> type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1
>>>
>>> For the Fn-keys, does it send the same MSC_SCAN code for *all* the
>>> non-working Fn-keys ?
>>
>> Correct. And I can confirm that /dev/hidraw1 lets me distinguish between
>> the keys.
>>
>>> If so then it seems that this is very much like the thinkpad 10 kbd dock
>>> which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
>>> function in drivers/hid/hid-lenovo.c .
>>>
>>> If I have that right, then I think we should be able to get the
>>> Fn keys to work without too much trouble. You could try hacking up
>>> drivers/hid/hid-lenovo.c a bit:
>>
>> (Not yet there, but will investigate.)
>>
>>> 1. Add an entry to the lenovo_devices array like this:
>>>
>>> 	/*
>>> 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
>>> 	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
>>> 	 */
>>>         { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>>>                      USB_VENDOR_ID_LENOVO,
>>>                      USB_DEVICE_ID_LENOVO_X1_TAB),
>>>
>>> 2. Add the following entry to the switch-case in lenovo_input_mapping() :
>>>
>>>         case USB_DEVICE_ID_LENOVO_X1_TAB:
>>>                 return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
>>>                                                                usage, bit, max);
>>>
>>> And then build hid-lenovo.c and modprobe it.
>>>
>>> After the modprobe to:
>>>
>>> ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver
>>>
>>> This should show 2 devices (I guess) with one being bound to hid-lenovo
>>> and 1 being bound to hid-multitouch.
>>
>> So far (without patching hid-lenovo), 2 bound to hid-generic and 1 to
>> hid-multitouch.
> 
> Ok, so it seems that they kept the thinkpad 10 kbd bits (mostly) with
> 1 keyboard interface using the usb boot kbd interface (so that it will
> also work inside the BIOS) and a second interface for multimedia-keys +
> the mouse emulation of the thinkpad 10 touchpad, those are interfaces
> 1 and 2, except that they removed the mouse emulation as they added a
> new proper multi-touch capable touchpad as interface 3; and that one
> also handles the pointing stick I believe.
> 
> So yes 2 bound to hid-generic, 1 bound to hid-multitouch seems to be
> correct.

Right, that's what I observe.

>>> If this works some of your Fn + F# keys will now hopefully start doing
>>> something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
>>> to make it do the right thing for your kbd.
>>> z
>>> ###
>>>
>>> About LED support, just enabling the LED support bits for the
>>> USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
>>> but there is a tiny chance that sending the wrong command somehow puts
>>> the kbd in firmware update mode, I had that happen once with a Logitech
>>> kbd which did not seem to have any kind of handshake / passcode to avoid
>>> accidental fw updates (*).
>>>
>>> If you can give me a dump of the hid-descriptors for your keyboard,
>>> then I can check if that the LEDs might work the same way too (or not).
>>>
>>> The easiest way to get a dump is to run the following command as root:
>>>
>>> cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc
>>>
>>> And then attach rdesc to your next email.
>>
>> Please find this one attached already now.
>> In case it helps, the * expands to 0057 0058 0059 on my system.
> 
> Ok, so there still is an output-report number 9 on the second interface,
> which probably still controls the LEDS but its descriptors are subtly
> different. Although different in a good way I guess because the thinkpad
> 10 dock descriptor describes the 2 bytes in the output report as being
> in the range of 0-1 which is not how they are actually used.
> 
> So I think that the code for the Thinkpad 10 ultrabook keyboard as
> Lenovo calls it, should also work on the X1 tablet thin keyboard.

Mostly, modulo some key mappings, as expected.

The good:

LEDs are working exactly as expected with your patch, with the appropriate triggers automatically active. Perfect!


The bad:

I could adjust some of the key mappings for the X1 Tablet 2nd keyboard. What I couldn't do is to get Fn+F10, Fn+F11, Fn+F12 and Fn+PrtSc to work.
Following the logic of /dev/hidraw1 capture (attached), those should be on usage_index 16 to 19. But apparently those are on a different usage page or something like that? Unfortunately, my RTFM skills didn't really help with figuring out how that's supposed to work.
(Is looking at the bit indices in /dev/hidraw traces how you figure out those mappings? If there's a better way, I'm eager to be told...)

Similarly - I assume - Fn+S should emit SysRq according to https://download.lenovo.com/pccbbs/mobiles_pdf/x1_tablet_gen_2_ug_en.pdf, page 51. This is not on the "consumer control" device, but the usual keyboard, so /dev/hidraw0. Again, couldn't get much further than producing a capture. But I cannot make sense of this one, because way more bits are set, so I cannot extrapolate from your code.


The ugly:

Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.

Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.
Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.


Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?

Also, I'd make sure that this is about the "ThinkPad X1 Tablet Thin Keyboard **Gen 2**". The consumer functions are different for the **Gen 1** keyboard, so I would also adjust the function names. I do have an old Gen 1 keyboard lying around, but unfortunately it's either broken (it lights up shortly after attaching in Windows, but doesn't report keypresses at all, and pretends to be completely dead in Linux), or it's incompatible with my X1 Tablet 2nd Gen. So I cannot test how your patch might impact the Gen1, too... :-/


> I've prepared a set of patches which enable the tp10ubkbd code on
> the X1 tablet thin keyboard. But beware as mentioned before there is a
> tiny chance that sending the wrong command somehow puts the kbd in
> firmware update mode. I believe that trying the tp10ubkbd code is safe,
> esp. since this is using a 2 byte large output report and using that
> for fw-updating would be a bit weird. Still there is a small risk
> (there always is when poking hw) so I will leave it up to you if
> you are willing to try this.

No issue at all, and everything below works just as expected.

> Here is how I test this (note you will need to adjust the paths a bit) :
> 
> Toggle the 2 mute LEDs:
> 
> [root@localhost ~]# echo 1 > /sys/class/leds/0003:17EF:6062.000E:amber:micmute/brightness
> [root@localhost ~]# echo 0 > /sys/class/leds/0003:17EF:6062.000E:amber:micmute/brightness
> [root@localhost ~]# echo 1 > /sys/class/leds/0003:17EF:6062.000E:amber:mute/brightness
> [root@localhost ~]# echo 0 > /sys/class/leds/0003:17EF:6062.000E:amber:mute/brightness
> 
> Check Fnlock LED state (toggle on kbd by pressing Fn + Esc) :
> 
> [root@localhost ~]# cat /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> 1
> [root@localhost ~]# cat /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> 0
> 
> Change Fnlock state from within Linux:
> 
> [root@localhost ~]# echo 1 > /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> [root@localhost ~]# echo 0 > /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> 
> (The Led on the kbd should update; and the F## key behavior should change)
> 
> Regards,
> 
> Hans


Cheers,
Alex
# echo 'Fn + [ s, 4 ]'; sleep 1; sudo xxd -b -c4 /dev/hidraw0
Fn + [ s, 4 ]

# Fn+S should be SysRq
Fn+S press:   00000000: 00000000 00000000 10011010 00000000  ....
              00000004: 00000000 00000000 00000000 00000000  ....
Fn+S release: 00000008: 00000000 00000000 00000000 00000000  ....
              0000000c: 00000000 00000000 00000000 00000000  ....

# Fn+4 should be "sleep"
Fn+4 press:   00000000: 00000000 00000000 01110010 00000000  ..r.
              00000004: 00000000 00000000 00000000 00000000  ....
Fn+4 release: 00000008: 00000000 00000000 00000000 00000000  ....
              0000000c: 00000000 00000000 00000000 00000000  ....
# subsequent presses of Fn+4 do not report anything
# until hid_lenovo is removed and re-inserted
# echo 'Fn + [ FnLock, F1, ..., F12, PrtSc, Space, Space, Space, Space ]'; xxd -b -c4 /dev/hidraw1
Fn + [ FnLock, F1, ..., F12, PrtSc, Space, Space, Space, Space ]

FnLock Press:   00000000: 00000011 00000000 00000001 00000000  ....
FnLock Release: 00000004: 00000011 00000000 00000000 00000000  ....

F1 Press:       00000008: 00000011 00100000 00000000 00000000  . ..
F1 Release:     0000000c: 00000011 00000000 00000000 00000000  ....
F2 Press:       00000010: 00000011 00000000 00000000 00010000  ....
F2 Release:     00000014: 00000011 00000000 00000000 00000000  ....
F3 Press:       00000018: 00000011 00000000 00000000 00100000  ...
F3 Release:     0000001c: 00000011 00000000 00000000 00000000  ....
F4 Press:       00000020: 00000011 00000000 00000010 00000000  ....
F4 Release:     00000024: 00000011 00000000 00000000 00000000  ....
F5 Press:       00000028: 00000011 00000000 00000000 01000000  ...@
F5 Release:     0000002c: 00000011 00000000 00000000 00000000  ....
F6 Press:       00000030: 00000011 00000000 00000000 00001000  ....
F6 Release:     00000034: 00000011 00000000 00000000 00000000  ....
F7 Press:       00000038: 00000011 00000000 00100000 00000000  .. .
F7 Release:     0000003c: 00000011 00000000 00000000 00000000  ....
F8 Press:       00000040: 00000011 00000000 01000000 00000000  ..@.
F8 Release:     00000044: 00000011 00000000 00000000 00000000  ....
F9 Press:       00000048: 00000011 00000000 00000100 00000000  ....
F9 Release:     0000004c: 00000011 00000000 00000000 00000000  ....
F10 Press:      00000050: 00000011 00000001 00000000 00000000  ....
F10 Release:    00000054: 00000011 00000000 00000000 00000000  ....
F11 Press:      00000058: 00000011 00000010 00000000 00000000  ....
F11 Release:    0000005c: 00000011 00000000 00000000 00000000  ....
F12 Press:      00000060: 00000011 00000100 00000000 00000000  ....
F12 Release:    00000064: 00000011 00000000 00000000 00000000  ....

PrtSc Press:    00000068: 00000011 00001000 00000000 00000000  ....
PrtSc Release:  0000006c: 00000011 00000000 00000000 00000000  ....

Space Press:    00000070: 00000011 00000000 10010000 00000001  ....
Space Press:    00000074: 00000011 00000000 10010000 00000001  ....
Space Press:    00000078: 00000011 00000000 10010000 00000001  ....
Space Press:    0000007c: 00000011 00000000 10010000 00000001  ....
static int lenovo_input_mapping_x1_tab_kbd(struct hid_device *hdev,
		struct hid_input *hi, struct hid_field *field,
		struct hid_usage *usage, unsigned long **bit, int *max)
{
	/*
	 * The ThinkPad X1 Tablet Thin Keyboard uses 0x000c0001 usage for
	 * a bunch of keys which have no standard consumer page code.
	 */
	// FIXME the below mappings are taken for the tp10_ultrabook_kbd mapping function
	// these may very well need to be changed for the X1 Tablet Thin Keyboard
	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: Mute/unmute microphone */
			map_key_clear(KEY_MICMUTE);
			return 1;
		case 10: /* Fn-F9: Settings */
			map_key_clear(KEY_CONFIG);
			return 1;
		case 13: /* Fn-F7: Manage external displays */
			map_key_clear(KEY_SWITCHVIDEOMODE);
			return 1;
		case 14: /* Fn-F8: Enable/disable wifi */
			map_key_clear(KEY_WLAN);
			return 1;

		/* FIXME: cases 16-19 don't work */
		case 16: /* Fn-F10: Enable/disable bluetooth */
			map_key_clear(KEY_BLUETOOTH);
			return 1;
		case 17: /* Fn-F11: Keyboard settings */
			map_key_clear(KEY_KEYBOARD);
			return 1;
		case 18: /* Fn-F12: User function / Cortana */
			map_key_clear(KEY_MACRO1);
			return 1;
		case 19: /* Fn-PrtSc: Snipping tool */
			map_key_clear(KEY_SELECTIVE_SCREENSHOT);
			return 1;
		}
	}

	return 0;
}

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux