Hi, On 2/7/21 6:10 PM, Hans de Goede wrote: > Hi, > > On 2/7/21 5:34 PM, Alexander Kobel wrote: >> Those events occur when a keyboard cover is attached to a ThinkPad >> Tablet device. Typically, they are used to switch from normal to tablet >> mode in userspace; e.g., to offer touch keyboard choices when focus goes >> to a text box and no keyboard is attached, or to enable autorotation of >> the display according to the builtin orientation sensor. > > Thank you for your patch. Thank you for your swift response. >> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is >> left to userspace. > > I don't understand this part, in order for userspace to respond to these > events the thinkpad_acpi driver needs to emit events for this; and emitting > SW_TABLET_MODE seems like it is the right thing to do. > > Why are you not doing this ? Quite frankly, because I did not know how to, reliably. (First ever patch attempt to the kernel here, so I'd rather err on the safe side.) There are a number of events (e.g., TP_HKEY_EV_HOTPLUG_DOCK) that do not propagate to userspace, but only report a specific message. I figured it's better to have a meaningful entry in the log rather than just the warning about an unknown event. But on second thought: pretending to react, but not actually doing something, isn't too valuable. I'll go off and try to improve. > Note that it is important to only advertise SW_TABLET_MODE functionality > on devices where it actually works. Which might be challenging I guess... Right. I only have a two devices to test, one of them a classic laptop, so any input is greatly appreciated. > But we have contacts inside Lenovo now, so perhaps they can help. > > Mark, Nitin, is there a way for the thinkpad_acpi code to figure out > if 0x4012 / 0x4013 events will be send by a device? > > Also is there a way to get the current state of the > keyboard-cover being attached at boot or not ? IIUC, tpacpi_input_send_tabletsw() will not work as-is on this device, as it should do for TP_HKEY_EV_TABLET_TABLET on X41t-X61t; mostly, because I'm not aware of a method to read the current state. Nevertheless, at least emitting events on change should be doable. 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: This [patch] is pretty much trivial and conservative, I assume. However, it's not 100% clear to me whether emitting a EV_SW event for SW_TABLET_MODE would be in order, as mentioned in the description. If so, I'd have a deeper look on how to do that, and perhaps require some hand-holding. Besides, the X1 Tablet has a few Fn+smth combos that are not working currently, and instead report (in evtest): Input driver version is 1.0.1 Input device ID: bus 0x3 vendor 0x17ef product 0x60a3 version 0x111 Input device name: "PRIMAX ThinkPad X1 Tablet Thin Keyboard Gen 2 Consumer Control" type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001 type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1 Keys affected are Fn+Esc (Fn lock) and Fn+F7 - Fn+F12 (various settings like radio/bluetooth on/off). Also, the mute and micmute leds are not working, and Fn lock led and function is unavailable. I'd love to give those minor issues a shot, too, but I'm not sure where to start. Would this go via thinkpad_acpi or hid_lenovo [or hid_primax]? Cheers, Alex > Regards, > > Hans > > > >> So this patch is mainly to avoid warnings about >> unknown and unhandled events, which are now reported as: >> >> * Event 0x4012: attached keyboard cover >> * Event 0x4013: detached keyboard cover >> >> Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as >> non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal >> clamshell, so it does not have a keyboard cover). >> >> Signed-off-by: Alexander Kobel <a-kobel@xxxxxxxxxx> >> --- >> drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c >> index c404706379d9..fd5322b5bbbd 100644 >> --- a/drivers/platform/x86/thinkpad_acpi.c >> +++ b/drivers/platform/x86/thinkpad_acpi.c >> @@ -174,6 +174,8 @@ enum tpacpi_hkey_event_t { >> or port replicator */ >> TP_HKEY_EV_HOTPLUG_UNDOCK = 0x4011, /* undocked from hotplug >> dock or port replicator */ >> + TP_HKEY_EV_KBD_COVER_ATTACH = 0x4012, /* attached keyboard cover */ >> + TP_HKEY_EV_KBD_COVER_DETACH = 0x4013, /* detached keyboard cover */ >> >> /* User-interface events */ >> TP_HKEY_EV_LID_CLOSE = 0x5001, /* laptop lid closed */ >> @@ -3989,6 +3991,12 @@ static bool hotkey_notify_dockevent(const u32 hkey, >> case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port replicator */ >> pr_info("undocked from hotplug port replicator\n"); >> return true; >> + case TP_HKEY_EV_KBD_COVER_ATTACH: >> + pr_info("attached keyboard cover\n"); >> + return true; >> + case TP_HKEY_EV_KBD_COVER_DETACH: >> + pr_info("detached keyboard cover\n"); >> + return true; >> >> default: >> return false; >> -- >> 2.30.0 >> >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
