Hi Nitin, Alexander,
On 2/8/21 8:17 AM, Nitin Joshi1 wrote:
> Hello Hans,
>
>> -----Original Message-----
>> From: Hans de Goede <hdegoede@xxxxxxxxxx>
>> Sent: Monday, February 8, 2021 2:10 AM
>> To: Alexander Kobel <a-kobel@xxxxxxxxxx>; platform-driver-
>> x86@xxxxxxxxxxxxxxx; Mark Pearson <mpearson@xxxxxxxxxx>; Nitin Joshi1
>> <njoshi1@xxxxxxxxxx>
>> Subject: [External] Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY
>> 0x4012, 0x4013 events
>>
>> 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.
>>
>>> 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 ?
>>
>> Note that it is important to only advertise SW_TABLET_MODE functionality on
>> devices where it actually works. Which might be challenging I guess...
>>
>> 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?
>
> It seems , these events are used for not only keyboard cover, but also other tablet options.
> In attached document, Interface type 4 (Graft type) is of ThinkPad X1 Tablet Series.
Great, Nitin thank you for the docs!
Nitin, one question below about version checks, it would be great if you can help with
this.
>> Also is there a way to get the current state of the keyboard-cover being
>> attached at boot or not ?
> It seems "GTOP" ASL method can be used to get current state.
Ack.
Alexander, so it looks like we need to do the following to support this properly:
1. At a new TP_HOTKEY_TABLET_USES_GTOP to the hotkey_tablet enum
2. At probe time in hotkey_init_tablet_mode add a new if / case with I guess
the highest prio (so try before GMMS) which does:
1. Call GTOP with parameter 0x0000, check return equals 0x0103 (or newer?)
Nitin, how should the version check look like here, check that the
upper byte == 0x01 and the lower byte >= 0x03 ?
2. Call GTOP with parameter 0x0100, check return value equals 4
3. Call GTOP with parameter 0x0200, set in_tablet_mode based on bit 0
and bit 16. I think we should report SW_TABLET_MODE=1 when the thin-kbd
is attached, but folded to the back
3. Make hotkey_get_tablet_mode support the new TP_HOTKEY_TABLET_USES_GTOP case and
make it call GTOP with parameter 0x0200 and check bit 0 + bit 16
4. On the events which you identified call tpacpi_input_send_tabletsw()
And can you also check if there are events when folding the kbd behind the tablet?
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
>>>
>
