Hello Hans,
>-----Original Message-----
>From: Hans de Goede <hdegoede@xxxxxxxxxx>
>Sent: Monday, February 8, 2021 6:05 PM
>To: Nitin Joshi1 <njoshi1@xxxxxxxxxx>; Alexander Kobel <a-kobel@a-
>kobel.de>; platform-driver-x86@xxxxxxxxxxxxxxx; Mark Pearson
><mpearson@xxxxxxxxxx>
>Subject: Re: [External] Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY
>0x4012, 0x4013 events
>
>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 ?
Thanks for your comments.
We should check version by checking response equal or greater than 0x0103 and not
By checking upper byte and lower byte.
> 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
Thanks & Regards,
Nitin Joshi
>
>
>
>>>> 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
>>>>
>>
