[PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[Alexander Kobel]

[-- Attachment #1: Type: text/plain, Size: 2727 bytes --]

Those events occur when a keyboard cover is attached to a ThinkPad
X1 Tablet series 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.

intel-vtbn already recognizes those events.  To avoid sending duplicate
events to userspace, they are simply ignored.  Thus, this patch only
avoids warnings about unknown and unhandled HKEYs 0x4012 and 0x4013.

For more information about the background and potential improvements for
different types of attachment options, such as the Pico cartridge dock
module, see
https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/

Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
---
 drivers/platform/x86/thinkpad_acpi.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index c404706379d9..e16d4b804c92 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -174,6 +174,12 @@ enum tpacpi_hkey_event_t {
 						     or port replicator */
 	TP_HKEY_EV_HOTPLUG_UNDOCK	= 0x4011, /* undocked from hotplug
 						     dock or port replicator */
+	/*
+	 * Thinkpad X1 Tablet series devices emit 0x4012 and 0x4013
+	 * when keyboard cover is attached, detached or folded onto the back
+	 */
+	TP_HKEY_EV_KBD_COVER_ATTACH	= 0x4012, /* keyboard cover attached */
+	TP_HKEY_EV_KBD_COVER_DETACH	= 0x4013, /* keyboard cover detached or folded back */
 
 	/* User-interface events */
 	TP_HKEY_EV_LID_CLOSE		= 0x5001, /* laptop lid closed */
@@ -3990,6 +3996,23 @@ static bool hotkey_notify_dockevent(const u32 hkey,
 		pr_info("undocked from hotplug port replicator\n");
 		return true;
 
+	/*
+	 * Deliberately ignore attaching and detaching the keybord cover to avoid
+	 * duplicates from intel-vbtn, which already emits SW_TABLET_MODE events
+	 * to userspace.
+	 *
+	 * Please refer to the following thread for more information and a preliminary
+	 * implementation using the GTOP ("Get Tablet OPtions") interface that could be
+	 * extended to other attachment options of the ThinkPad X1 Tablet series, such as
+	 * the Pico cartridge dock module:
+	 * https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/
+	 */
+	case TP_HKEY_EV_KBD_COVER_ATTACH:
+	case TP_HKEY_EV_KBD_COVER_DETACH:
+		*send_acpi_ev = false;
+		*ignore_acpi_ev = true;
+		return true;
+
 	default:
 		return false;
 	}
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5916 bytes --]

Re: [PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[Hans de Goede]

Hi,

On 2/13/21 4:13 PM, Alexander Kobel wrote:
> Those events occur when a keyboard cover is attached to a ThinkPad
> X1 Tablet series 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.
> 
> intel-vtbn already recognizes those events.  To avoid sending duplicate
> events to userspace, they are simply ignored.  Thus, this patch only
> avoids warnings about unknown and unhandled HKEYs 0x4012 and 0x4013.
> 
> For more information about the background and potential improvements for
> different types of attachment options, such as the Pico cartridge dock
> module, see
> https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/
> 
> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Note I will merge this into my review-hans branch and eventually pdx86/for-next
once 5.12-rc1 is released.

Regards,

Hans



> ---
>  drivers/platform/x86/thinkpad_acpi.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index c404706379d9..e16d4b804c92 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -174,6 +174,12 @@ enum tpacpi_hkey_event_t {
>  						     or port replicator */
>  	TP_HKEY_EV_HOTPLUG_UNDOCK	= 0x4011, /* undocked from hotplug
>  						     dock or port replicator */
> +	/*
> +	 * Thinkpad X1 Tablet series devices emit 0x4012 and 0x4013
> +	 * when keyboard cover is attached, detached or folded onto the back
> +	 */
> +	TP_HKEY_EV_KBD_COVER_ATTACH	= 0x4012, /* keyboard cover attached */
> +	TP_HKEY_EV_KBD_COVER_DETACH	= 0x4013, /* keyboard cover detached or folded back */
>  
>  	/* User-interface events */
>  	TP_HKEY_EV_LID_CLOSE		= 0x5001, /* laptop lid closed */
> @@ -3990,6 +3996,23 @@ static bool hotkey_notify_dockevent(const u32 hkey,
>  		pr_info("undocked from hotplug port replicator\n");
>  		return true;
>  
> +	/*
> +	 * Deliberately ignore attaching and detaching the keybord cover to avoid
> +	 * duplicates from intel-vbtn, which already emits SW_TABLET_MODE events
> +	 * to userspace.
> +	 *
> +	 * Please refer to the following thread for more information and a preliminary
> +	 * implementation using the GTOP ("Get Tablet OPtions") interface that could be
> +	 * extended to other attachment options of the ThinkPad X1 Tablet series, such as
> +	 * the Pico cartridge dock module:
> +	 * https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/
> +	 */
> +	case TP_HKEY_EV_KBD_COVER_ATTACH:
> +	case TP_HKEY_EV_KBD_COVER_DETACH:
> +		*send_acpi_ev = false;
> +		*ignore_acpi_ev = true;
> +		return true;
> +
>  	default:
>  		return false;
>  	}
> 

Re: [PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[Hans de Goede]

Hi,

On 2/13/21 4:13 PM, Alexander Kobel wrote:
> Those events occur when a keyboard cover is attached to a ThinkPad
> X1 Tablet series 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.
> 
> intel-vtbn already recognizes those events.  To avoid sending duplicate
> events to userspace, they are simply ignored.  Thus, this patch only
> avoids warnings about unknown and unhandled HKEYs 0x4012 and 0x4013.
> 
> For more information about the background and potential improvements for
> different types of attachment options, such as the Pico cartridge dock
> module, see
> https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/
> 
> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index c404706379d9..e16d4b804c92 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -174,6 +174,12 @@ enum tpacpi_hkey_event_t {
>  						     or port replicator */
>  	TP_HKEY_EV_HOTPLUG_UNDOCK	= 0x4011, /* undocked from hotplug
>  						     dock or port replicator */
> +	/*
> +	 * Thinkpad X1 Tablet series devices emit 0x4012 and 0x4013
> +	 * when keyboard cover is attached, detached or folded onto the back
> +	 */
> +	TP_HKEY_EV_KBD_COVER_ATTACH	= 0x4012, /* keyboard cover attached */
> +	TP_HKEY_EV_KBD_COVER_DETACH	= 0x4013, /* keyboard cover detached or folded back */
>  
>  	/* User-interface events */
>  	TP_HKEY_EV_LID_CLOSE		= 0x5001, /* laptop lid closed */
> @@ -3990,6 +3996,23 @@ static bool hotkey_notify_dockevent(const u32 hkey,
>  		pr_info("undocked from hotplug port replicator\n");
>  		return true;
>  
> +	/*
> +	 * Deliberately ignore attaching and detaching the keybord cover to avoid
> +	 * duplicates from intel-vbtn, which already emits SW_TABLET_MODE events
> +	 * to userspace.
> +	 *
> +	 * Please refer to the following thread for more information and a preliminary
> +	 * implementation using the GTOP ("Get Tablet OPtions") interface that could be
> +	 * extended to other attachment options of the ThinkPad X1 Tablet series, such as
> +	 * the Pico cartridge dock module:
> +	 * https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/
> +	 */
> +	case TP_HKEY_EV_KBD_COVER_ATTACH:
> +	case TP_HKEY_EV_KBD_COVER_DETACH:
> +		*send_acpi_ev = false;
> +		*ignore_acpi_ev = true;
> +		return true;
> +
>  	default:
>  		return false;
>  	}
> 

Re: [PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[Alexander Kobel]

[-- Attachment #1: Type: text/plain, Size: 3297 bytes --]

Hi,

thanks for reviewing.

On 2/10/21 10:48 PM, Barnabás Pőcze wrote:
> Hi
> 
> 2021. február 10., szerda 18:54 keltezéssel, Alexander Kobel írta:
> 
>> @@ -174,6 +174,11 @@ enum tpacpi_hkey_event_t {
>>  						     or port replicator */
>>  	TP_HKEY_EV_HOTPLUG_UNDOCK	= 0x4011, /* undocked from hotplug
>>  						     dock or port replicator */
>> +	/* Thinkpad X1 Tablet series (and probably other GTOP type 4) emit 0x4012 and 0x4013
> 
> The preferred style of multi-line comments is
> 
> /* <note the empty line here>
>  * Lorem ipsum ...
>  */

Corrected.

>> @@ -2166,11 +2173,32 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode)
>>  	return !!(mode & TP_ACPI_MULTI_MODE_TABLET_LIKE);
>>  }
>>
>> +static int hotkey_gtop_any_type_get_tablet_mode(int s)
>> +{
>> +	return !(s & 0x1);
>> +}
> 
> I believe it would be preferable to do something like
> 
>   #define TP_GTOP_TYPE_ANY_ATTACH_STATE BIT(0)
>   /* or possibly use an enum */
>   ...
>   return !(s & TP_GTOP_TYPE_ANY_ATTACH_STATE);
> 
> or let `s` be `unsigned long`, and then
> 
>   #define TP_GTOP_TYPE_ANY_ATTACH_STATE_BIT 0
>   ...
>   return !test_bit(TP_GTOP_TYPE_ANY_ATTACH_STATE_BIT, &s);
> 
> or something along these lines, and I also think the return type could be `bool`.

Indeed, thanks for the suggestion. I used the first variant.

>> +
>> +static int hotkey_gtop_x1_tablet_type_get_tablet_mode(int s)
>> +{
>> +	return (!(s & 0x1) /* keyboard NOT attached */
>> +		|| ((s >> 16) & 0x1) /* or folded onto the back */);
>> +}
> 
> Same here, I suggest using the `BIT()` macro or `test_bit()` and preferably
> name the constants.

Ditto.

>> @@ -3213,7 +3241,62 @@ static int hotkey_init_tablet_mode(void)
>>  	int in_tablet_mode = 0, res;
>>  	char *type = NULL;
>>
>> -	if (acpi_evalf(hkey_handle, &res, "GMMS", "qdd", 0)) {
>> +	if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x0000)
>> +	    && res >= 0x0103
>> +	    && acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x0100)) {
> 
> Minor thing, but checkpatch prefers `&&` at the end of the line. And the rest of
> the code places them at the end, too.

Corrected, thanks.

>> +		/*
>> +		 * GTOP ("Get Tablet OPtion") state ASL method definition:
>> +		 * - Input: 0x0000: Query version
>> +		 *   Output: 0x0103 (version 1.03)
>> +		 * - Input: 0x0100: Query interface type
>> +		 *   Output: DWORD But 31-0 Interface type
>                                     ^
> Shouldn't that be "Bit"?

Of course. I effectively turned the entire comment into descriptive constants, at the same time correcting the other issues.

>> +		switch (res) {
>> +		case 1:
>> +			tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE;
>> +			if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x200))
> 
> Please name that `0x200` something like TP_GTOP_CMD_GET_ATTACH_STATE or something
> you like. (Same for the rest of the GTOP calls.) I know it's just above in the comment,
> but I still think it'd be better to have concrete, more or less self-explanatory names
> in the actual command

Indeed. All magic constants are now collected at a single place before the HKEY constants.


Thanks again,
Alex


> Regards,
> Barnabás Pőcze
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5916 bytes --]

Re: [PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[Barnabás Pőcze]

Hi


2021. február 10., szerda 18:54 keltezéssel, Alexander Kobel írta:

> ThinkPad X1 Tablets emit HKEY 0x4012 and 0x4013 events when a keyboard
> cover is attached/detached or folded to the back of the device. 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
> built-in orientation sensor.
>
> This patch handles the two events by issuing corresponding
> SW_TABLET_MODE notifications to the ACPI userspace.
>
> 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@a-kobel.de>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 91 +++++++++++++++++++++++++++-
>  1 file changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index c404706379d9..8c1ff555f10b 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -174,6 +174,11 @@ enum tpacpi_hkey_event_t {
>  						     or port replicator */
>  	TP_HKEY_EV_HOTPLUG_UNDOCK	= 0x4011, /* undocked from hotplug
>  						     dock or port replicator */
> +	/* Thinkpad X1 Tablet series (and probably other GTOP type 4) emit 0x4012 and 0x4013

The preferred style of multi-line comments is

/* <note the empty line here>
 * Lorem ipsum ...
 */


> +	 * when keyboard cover is attached, detached or folded onto the back
> +	 */
> +	TP_HKEY_EV_KBD_COVER_ATTACH	= 0x4012, /* keyboard cover attached */
> +	TP_HKEY_EV_KBD_COVER_DETACH	= 0x4013, /* keyboard cover detached or folded back */
>
>  	/* User-interface events */
>  	TP_HKEY_EV_LID_CLOSE		= 0x5001, /* laptop lid closed */
> @@ -308,6 +313,8 @@ static struct {
>  		TP_HOTKEY_TABLET_NONE = 0,
>  		TP_HOTKEY_TABLET_USES_MHKG,
>  		TP_HOTKEY_TABLET_USES_GMMS,
> +		TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE,
> +		TP_HOTKEY_TABLET_USES_GTOP_X1_TABLET_TYPE,
>  	} hotkey_tablet;
>  	u32 kbdlight:1;
>  	u32 light:1;
> @@ -2166,11 +2173,32 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode)
>  	return !!(mode & TP_ACPI_MULTI_MODE_TABLET_LIKE);
>  }
>
> +static int hotkey_gtop_any_type_get_tablet_mode(int s)
> +{
> +	return !(s & 0x1);
> +}

I believe it would be preferable to do something like

  #define TP_GTOP_TYPE_ANY_ATTACH_STATE BIT(0)
  /* or possibly use an enum */
  ...
  return !(s & TP_GTOP_TYPE_ANY_ATTACH_STATE);

or let `s` be `unsigned long`, and then

  #define TP_GTOP_TYPE_ANY_ATTACH_STATE_BIT 0
  ...
  return !test_bit(TP_GTOP_TYPE_ANY_ATTACH_STATE_BIT, &s);

or something along these lines, and I also think the return type could be `bool`.


> +
> +static int hotkey_gtop_x1_tablet_type_get_tablet_mode(int s)
> +{
> +	return (!(s & 0x1) /* keyboard NOT attached */
> +		|| ((s >> 16) & 0x1) /* or folded onto the back */);
> +}

Same here, I suggest using the `BIT()` macro or `test_bit()` and preferably
name the constants.


> +
>  static int hotkey_get_tablet_mode(int *status)
>  {
>  	int s;
>
>  	switch (tp_features.hotkey_tablet) {
> +	case TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE:
> +		if (!acpi_evalf(hkey_handle, &s, "GTOP", "dd", 0x0200))
> +			return -EIO;
> +		*status = hotkey_gtop_any_type_get_tablet_mode(s);
> +		break;
> +	case TP_HOTKEY_TABLET_USES_GTOP_X1_TABLET_TYPE:
> +		if (!acpi_evalf(hkey_handle, &s, "GTOP", "dd", 0x0200))
> +			return -EIO;
> +		*status = hotkey_gtop_x1_tablet_type_get_tablet_mode(s);
> +		break;
>  	case TP_HOTKEY_TABLET_USES_MHKG:
>  		if (!acpi_evalf(hkey_handle, &s, "MHKG", "d"))
>  			return -EIO;
> @@ -3213,7 +3241,62 @@ static int hotkey_init_tablet_mode(void)
>  	int in_tablet_mode = 0, res;
>  	char *type = NULL;
>
> -	if (acpi_evalf(hkey_handle, &res, "GMMS", "qdd", 0)) {
> +	if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x0000)
> +	    && res >= 0x0103
> +	    && acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x0100)) {

Minor thing, but checkpatch prefers `&&` at the end of the line. And the rest of
the code places them at the end, too.


> +		/*
> +		 * GTOP ("Get Tablet OPtion") state ASL method definition:
> +		 * - Input: 0x0000: Query version
> +		 *   Output: 0x0103 (version 1.03)
> +		 * - Input: 0x0100: Query interface type
> +		 *   Output: DWORD But 31-0 Interface type
                                    ^
Shouldn't that be "Bit"?


> +		 *     0: Reserved
> +		 *     1: Any type
> +		 *     2: ThinkPad Helix series
> +		 *     3: ThinkPad 10 series
> +		 *     4: ThinkPad X1 Tablet series
> +		 * - Input: 0x0200: Get attach option
> +		 *   Output: Option attach state
> +		 *     (0: detached, 1: attached)
> +		 *     version >= 1.03 and interface type 1:
> +		 *       Bit 0: Any option attach state
> +		 *       Bit 31-1: Reserved(0)
> +		 *     version >= 1.03 and interface type 4:
> +		 *       Bit 0: Thin-KBD attach state
> +		 *       Bit 1: Pro-Cartridge attach state
> +		 *       Bit 3-2: Pico-Cartridge attach state
> +		 *         00: detached
> +		 *         01: attached
> +		 *         10: attached with battery error
> +		 *         11: Reserved
> +		 *       Bit 4: 3D Cartridge attach state
> +		 *       Bit 5: Reserve 1 attach state
> +		 *       Bit 6: Reserve 2 attach state
> +		 *       Bit 15-7: Reserved(0)
> +		 *       Bit 16: Folio keyboard location
> +		 *         (valid if folio attached)
> +		 *         0: keyboard is NOT folded onto the back
> +		 *         1: keyboard is folded onto the back of the system
> +		 *       Bit 31-17: Reserved(0)
> +		 */
> +		switch (res) {
> +		case 1:
> +			tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE;
> +			if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x200))

Please name that `0x200` something like TP_GTOP_CMD_GET_ATTACH_STATE or something
you like. (Same for the rest of the GTOP calls.) I know it's just above in the comment,
but I still think it'd be better to have concrete, more or less self-explanatory names
in the actual command.


> +				in_tablet_mode = hotkey_gtop_any_type_get_tablet_mode(res);
> +			type = "GTOP";
> +			break;
> +		case 4:
> +			tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_GTOP_X1_TABLET_TYPE;
> +			if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x200))
> +				in_tablet_mode = hotkey_gtop_x1_tablet_type_get_tablet_mode(res);
> +			type = "GTOP";
> +			break;
> +		default:
> +			pr_err("unsupported GTOP type, please report this to %s\n", TPACPI_MAIL);
> +			break;
> +		}
> +	} else if (acpi_evalf(hkey_handle, &res, "GMMS", "qdd", 0)) {
>  		int has_tablet_mode;
> [...]


Regards,
Barnabás Pőcze

[PATCH v2] platform/x86: thinkpad_acpi: Handle keyboard cover attach/detach events

[Alexander Kobel]

[-- Attachment #1: Type: text/plain, Size: 5942 bytes --]

ThinkPad X1 Tablets emit HKEY 0x4012 and 0x4013 events when a keyboard
cover is attached/detached or folded to the back of the device. 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
built-in orientation sensor.

This patch handles the two events by issuing corresponding
SW_TABLET_MODE notifications to the ACPI userspace.

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@a-kobel.de>
---
 drivers/platform/x86/thinkpad_acpi.c | 91 +++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index c404706379d9..8c1ff555f10b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -174,6 +174,11 @@ enum tpacpi_hkey_event_t {
 						     or port replicator */
 	TP_HKEY_EV_HOTPLUG_UNDOCK	= 0x4011, /* undocked from hotplug
 						     dock or port replicator */
+	/* Thinkpad X1 Tablet series (and probably other GTOP type 4) emit 0x4012 and 0x4013
+	 * when keyboard cover is attached, detached or folded onto the back
+	 */
+	TP_HKEY_EV_KBD_COVER_ATTACH	= 0x4012, /* keyboard cover attached */
+	TP_HKEY_EV_KBD_COVER_DETACH	= 0x4013, /* keyboard cover detached or folded back */
 
 	/* User-interface events */
 	TP_HKEY_EV_LID_CLOSE		= 0x5001, /* laptop lid closed */
@@ -308,6 +313,8 @@ static struct {
 		TP_HOTKEY_TABLET_NONE = 0,
 		TP_HOTKEY_TABLET_USES_MHKG,
 		TP_HOTKEY_TABLET_USES_GMMS,
+		TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE,
+		TP_HOTKEY_TABLET_USES_GTOP_X1_TABLET_TYPE,
 	} hotkey_tablet;
 	u32 kbdlight:1;
 	u32 light:1;
@@ -2166,11 +2173,32 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode)
 	return !!(mode & TP_ACPI_MULTI_MODE_TABLET_LIKE);
 }
 
+static int hotkey_gtop_any_type_get_tablet_mode(int s)
+{
+	return !(s & 0x1);
+}
+
+static int hotkey_gtop_x1_tablet_type_get_tablet_mode(int s)
+{
+	return (!(s & 0x1) /* keyboard NOT attached */
+		|| ((s >> 16) & 0x1) /* or folded onto the back */);
+}
+
 static int hotkey_get_tablet_mode(int *status)
 {
 	int s;
 
 	switch (tp_features.hotkey_tablet) {
+	case TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE:
+		if (!acpi_evalf(hkey_handle, &s, "GTOP", "dd", 0x0200))
+			return -EIO;
+		*status = hotkey_gtop_any_type_get_tablet_mode(s);
+		break;
+	case TP_HOTKEY_TABLET_USES_GTOP_X1_TABLET_TYPE:
+		if (!acpi_evalf(hkey_handle, &s, "GTOP", "dd", 0x0200))
+			return -EIO;
+		*status = hotkey_gtop_x1_tablet_type_get_tablet_mode(s);
+		break;
 	case TP_HOTKEY_TABLET_USES_MHKG:
 		if (!acpi_evalf(hkey_handle, &s, "MHKG", "d"))
 			return -EIO;
@@ -3213,7 +3241,62 @@ static int hotkey_init_tablet_mode(void)
 	int in_tablet_mode = 0, res;
 	char *type = NULL;
 
-	if (acpi_evalf(hkey_handle, &res, "GMMS", "qdd", 0)) {
+	if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x0000)
+	    && res >= 0x0103
+	    && acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x0100)) {
+		/*
+		 * GTOP ("Get Tablet OPtion") state ASL method definition:
+		 * - Input: 0x0000: Query version
+		 *   Output: 0x0103 (version 1.03)
+		 * - Input: 0x0100: Query interface type
+		 *   Output: DWORD But 31-0 Interface type
+		 *     0: Reserved
+		 *     1: Any type
+		 *     2: ThinkPad Helix series
+		 *     3: ThinkPad 10 series
+		 *     4: ThinkPad X1 Tablet series
+		 * - Input: 0x0200: Get attach option
+		 *   Output: Option attach state
+		 *     (0: detached, 1: attached)
+		 *     version >= 1.03 and interface type 1:
+		 *       Bit 0: Any option attach state
+		 *       Bit 31-1: Reserved(0)
+		 *     version >= 1.03 and interface type 4:
+		 *       Bit 0: Thin-KBD attach state
+		 *       Bit 1: Pro-Cartridge attach state
+		 *       Bit 3-2: Pico-Cartridge attach state
+		 *         00: detached
+		 *         01: attached
+		 *         10: attached with battery error
+		 *         11: Reserved
+		 *       Bit 4: 3D Cartridge attach state
+		 *       Bit 5: Reserve 1 attach state
+		 *       Bit 6: Reserve 2 attach state
+		 *       Bit 15-7: Reserved(0)
+		 *       Bit 16: Folio keyboard location
+		 *         (valid if folio attached)
+		 *         0: keyboard is NOT folded onto the back
+		 *         1: keyboard is folded onto the back of the system
+		 *       Bit 31-17: Reserved(0)
+		 */
+		switch (res) {
+		case 1:
+			tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_GTOP_ANY_TYPE;
+			if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x200))
+				in_tablet_mode = hotkey_gtop_any_type_get_tablet_mode(res);
+			type = "GTOP";
+			break;
+		case 4:
+			tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_GTOP_X1_TABLET_TYPE;
+			if (acpi_evalf(hkey_handle, &res, "GTOP", "qdd", 0x200))
+				in_tablet_mode = hotkey_gtop_x1_tablet_type_get_tablet_mode(res);
+			type = "GTOP";
+			break;
+		default:
+			pr_err("unsupported GTOP type, please report this to %s\n", TPACPI_MAIL);
+			break;
+		}
+	} else if (acpi_evalf(hkey_handle, &res, "GMMS", "qdd", 0)) {
 		int has_tablet_mode;
 
 		in_tablet_mode = hotkey_gmms_get_tablet_mode(res,
@@ -3989,6 +4072,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:
+	case TP_HKEY_EV_KBD_COVER_DETACH:
+		tpacpi_input_send_tabletsw();
+		hotkey_tablet_mode_notify_change();
+		*send_acpi_ev = false;
+		return true;
 
 	default:
 		return false;
-- 
2.30.1



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5916 bytes --]