diff mbox series

[1/1] asus-wmi: Add support for ROG X13 tablet mode

Message ID 20220803063734.765614-1-luke@ljones.dev (mailing list archive)
State Changes Requested, archived
Headers show
Series [1/1] asus-wmi: Add support for ROG X13 tablet mode | expand

Commit Message

Luke D. Jones Aug. 3, 2022, 6:37 a.m. UTC
Add quirk for ASUS ROG X13 Flow 2-in-1 to enable tablet mode with
lid flip (all screen rotations).

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-nb-wmi.c         | 16 ++++++++++++++++
 drivers/platform/x86/asus-wmi.c            | 16 +++++++++++++++-
 drivers/platform/x86/asus-wmi.h            |  1 +
 include/linux/platform_data/x86/asus-wmi.h |  1 +
 4 files changed, 33 insertions(+), 1 deletion(-)

Comments

Hans de Goede Aug. 4, 2022, 1:51 p.m. UTC | #1
Hi,

On 8/3/22 08:37, Luke D. Jones wrote:
> Add quirk for ASUS ROG X13 Flow 2-in-1 to enable tablet mode with
> lid flip (all screen rotations).
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/asus-nb-wmi.c         | 16 ++++++++++++++++
>  drivers/platform/x86/asus-wmi.c            | 16 +++++++++++++++-
>  drivers/platform/x86/asus-wmi.h            |  1 +
>  include/linux/platform_data/x86/asus-wmi.h |  1 +
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index 478dd300b9c9..1ce8924d0474 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -123,6 +123,12 @@ static struct quirk_entry quirk_asus_use_lid_flip_devid = {
>  	.use_lid_flip_devid = true,
>  };
>  
> +static struct quirk_entry quirk_asus_gv301 = {
> +	.wmi_backlight_set_devstate = true,
> +	.use_lid_flip_devid = true,
> +	.enodev_as_tablet_mode = true,
> +};
> +
>  static int dmi_matched(const struct dmi_system_id *dmi)
>  {
>  	pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -471,6 +477,15 @@ static const struct dmi_system_id asus_quirks[] = {
>  		},
>  		.driver_data = &quirk_asus_use_lid_flip_devid,
>  	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG FLOW X13",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "GV301Q"),
> +		},
> +		.driver_data = &quirk_asus_gv301,
> +	},
>  	{},
>  };
>  
> @@ -581,6 +596,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
>  	{ KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
>  	{ KE_IGNORE, 0xC6, },  /* Ambient Light Sensor notification */
>  	{ KE_KEY, 0xFA, { KEY_PROG2 } },           /* Lid flip action */
> +	{ KE_KEY, 0xBD, { KEY_PROG2 } },           /* Lid flip action on rog flow laptops */

asus_wmi_handle_event_code() will never get to the part where it parses
the keymap since it has:

	if (asus->driver->quirks->use_lid_flip_devid &&
			(code == NOTIFY_LID_FLIP || code == NOTIFY_LID_FLIP_GV301)) {
 		lid_flip_tablet_mode_get_state(asus);
  		return;
  	}

after this patch. The old 0xFA mapping is there from before we had LID switch
reporting on devices using ASUS_WMI_DEVID_LID_FLIP. I don't believe adding
an extra entry for this is necessary; nor is it a good idea since then
userspace might become to rely on these events which we don't want.



>  	{ KE_END, 0},
>  };
>  
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 62ce198a3463..0458e9107ca7 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -68,6 +68,7 @@ module_param(fnlock_default, bool, 0444);
>  #define NOTIFY_KBD_FBM			0x99
>  #define NOTIFY_KBD_TTP			0xae
>  #define NOTIFY_LID_FLIP			0xfa
> +#define NOTIFY_LID_FLIP_GV301	0xbd
>  
>  #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
>  
> @@ -516,6 +517,12 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
>  
>  	if (asus->driver->quirks->use_lid_flip_devid) {
>  		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +		if (result < 0)
> +			result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GV301_LID_FLIP);
> +
> +		if (result == -ENODEV && asus->driver->quirks->enodev_as_tablet_mode)
> +			result = 1;
> +

Looking at the handling of this here.

>  		if (result < 0)
>  			asus->driver->quirks->use_lid_flip_devid = 0;
>  		if (result >= 0) {
> @@ -553,6 +560,12 @@ static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus)
>  {
>  	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
>  
> +	if (result < 0)
> +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GV301_LID_FLIP);
> +
> +	if (result == -ENODEV && asus->driver->quirks->enodev_as_tablet_mode)
> +		result = 1;
> +

And here.

>  	if (result >= 0) {
>  		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
>  		input_sync(asus->inputdev);
> @@ -3094,7 +3107,8 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
>  		return;
>  	}
>  
> -	if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
> +	if (asus->driver->quirks->use_lid_flip_devid &&
> +			(code == NOTIFY_LID_FLIP || code == NOTIFY_LID_FLIP_GV301)) {
>  		lid_flip_tablet_mode_get_state(asus);
>  		return;
>  	}

and here. This really just is an entirely different code flow from the
devices using ASUS_WMI_DEVID_LID_FLIP.

I think it would be better to instead of the enodev_as_tablet_mode quirk, to
do a preparation patch 1/2 adding a:

enum asus_wmi_tablet_switch_mode {
	asus_wmi_no_tablet_switch,
	asus_wmi_kbd_dock_devid,
	asus_Wmi_lid_flip_devid,
	asus_wmi_gv301_lid_flip_devid, /* to be added in patch 2/2 */
};

and then in the quirks struct replace:

  	bool use_kbd_dock_devid;
  	bool use_lid_flip_devid;

with:

	enum asus_wmi_tablet_switch_mode tablet_switch_mode;

Adjust the quirks to set this to the right value and then where
the current code has the following pattern:

        if (asus->driver->quirks->use_kbd_dock_devid) {
		<kbd_dock_devid handling>;
	}
	
        if (asus->driver->quirks->use_lid_flip_devid) {
		<lid_flip_devid handling>;
	}

replace this with:

        switch (asus->driver->quirks->tablet_switch_mode) {
	case asus_wmi_no_tablet_switch:
		break;
	case asus_wmi_kbd_dock_devid:
		<kbd_dock_devid handling>;
		break;
	case asus_Wmi_lid_flip_devid:
		<lid_flip_devid handling>;
		break;
	}

And then in patch 2/2 add asus_wmi_gv301_lid_flip_devid to the enum
and extend the switch-cases with the necessary handling for the new
tablet-mode-switch type.

Regards,

Hans



> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index b302415bf1d9..ac9023aae838 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -35,6 +35,7 @@ struct quirk_entry {
>  	bool wmi_force_als_set;
>  	bool use_kbd_dock_devid;
>  	bool use_lid_flip_devid;
> +	bool enodev_as_tablet_mode;
>  	int wapf;
>  	/*
>  	 * For machines with AMD graphic chips, it will send out WMI event
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index a571b47ff362..79bd06628a8b 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -64,6 +64,7 @@
>  #define ASUS_WMI_DEVID_PANEL_OD		0x00050019
>  #define ASUS_WMI_DEVID_CAMERA		0x00060013
>  #define ASUS_WMI_DEVID_LID_FLIP		0x00060062
> +#define ASUS_WMI_DEVID_GV301_LID_FLIP	0x00060077
>  
>  /* Storage */
>  #define ASUS_WMI_DEVID_CARDREADER	0x00080013
Luke D. Jones Aug. 4, 2022, 10:20 p.m. UTC | #2
Excellent review as always, thank you.

Per your advice I've rewritten the patches and will have some people 
test them.

I've attached the patches and also pasted.

Trying to reply via git is a bit frustrating so below is a paste of the 
first half of the new patch, followed by second:

 From e66a77b93c9d60687b96bb72aa876e261dea8535 Mon Sep 17 00:00:00 2001
 From: "Luke D. Jones" <luke@ljones.dev>
Date: Fri, 5 Aug 2022 09:39:18 +1200
Subject: [PATCH 1/2] asus-wmi: Adjust tablet/lidflip handling to use 
enum

Due to multiple types of tablet/lidflip, the existing code for
handlign these events is refactored to use an enum for each type.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-nb-wmi.c | 13 +++-----
 drivers/platform/x86/asus-wmi.c | 53 +++++++++++++++++++++---------
 drivers/platform/x86/asus-wmi.h | 9 +++--
 3 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c 
b/drivers/platform/x86/asus-nb-wmi.c
index a81dc4b191b7..3a93e056c4e1 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -115,12 +115,12 @@ static struct quirk_entry quirk_asus_forceals = {
 };

 static struct quirk_entry quirk_asus_use_kbd_dock_devid = {
- .use_kbd_dock_devid = true,
+ .tablet_switch_mode = asus_wmi_kbd_dock_devid,
 };

 static struct quirk_entry quirk_asus_use_lid_flip_devid = {
  .wmi_backlight_set_devstate = true,
- .use_lid_flip_devid = true,
+ .tablet_switch_mode = asus_wmi_lid_flip_devid,
 };

 static int dmi_matched(const struct dmi_system_id *dmi)
@@ -492,16 +492,13 @@ static void asus_nb_wmi_quirks(struct 
asus_wmi_driver *driver)

  switch (tablet_mode_sw) {
  case 0:
- quirks->use_kbd_dock_devid = false;
- quirks->use_lid_flip_devid = false;
+ quirks->tablet_switch_mode = asus_wmi_no_tablet_switch;
   break;
  case 1:
- quirks->use_kbd_dock_devid = true;
- quirks->use_lid_flip_devid = false;
+ quirks->tablet_switch_mode = asus_wmi_kbd_dock_devid;
   break;
  case 2:
- quirks->use_kbd_dock_devid = false;
- quirks->use_lid_flip_devid = true;
+ quirks->tablet_switch_mode = asus_wmi_lid_flip_devid;
   break;
  }

diff --git a/drivers/platform/x86/asus-wmi.c 
b/drivers/platform/x86/asus-wmi.c
index 0e7fbed8a50d..a32e99205697 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -504,7 +504,10 @@ static int asus_wmi_input_init(struct asus_wmi 
*asus)
  if (err)
   goto err_free_dev;

- if (asus->driver->quirks->use_kbd_dock_devid) {
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ break;
+ case asus_wmi_kbd_dock_devid:
   result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_DOCK);
   if (result >= 0) {
    input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
@@ -512,12 +515,11 @@ static int asus_wmi_input_init(struct asus_wmi 
*asus)
   } else if (result != -ENODEV) {
    pr_err("Error checking for keyboard-dock: %d\n", result);
   }
- }
-
- if (asus->driver->quirks->use_lid_flip_devid) {
+ break;
+ case asus_wmi_lid_flip_devid:
   result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
   if (result < 0)
- asus->driver->quirks->use_lid_flip_devid = 0;
+ asus->driver->quirks->tablet_switch_mode = asus_wmi_no_tablet_switch;
   if (result >= 0) {
    input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
    input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
@@ -526,6 +528,7 @@ static int asus_wmi_input_init(struct asus_wmi 
*asus)
   } else {
    pr_err("Error checking for lid-flip: %d\n", result);
   }
+ break;
  }

  err = input_register_device(asus->inputdev);
@@ -3083,20 +3086,26 @@ static void asus_wmi_handle_event_code(int 
code, struct asus_wmi *asus)
   return;
  }

- if (asus->driver->quirks->use_kbd_dock_devid && code == 
NOTIFY_KBD_DOCK_CHANGE) {
- result = asus_wmi_get_devstate_simple(asus,
- ASUS_WMI_DEVID_KBD_DOCK);
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ break;
+ case asus_wmi_kbd_dock_devid:
+ if (code == NOTIFY_KBD_DOCK_CHANGE) {
+ result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_DOCK);
   if (result >= 0) {
    input_report_switch(asus->inputdev, SW_TABLET_MODE,
- !result);
+ !result);
    input_sync(asus->inputdev);
   }
   return;
- }
-
- if (asus->driver->quirks->use_lid_flip_devid && code == 
NOTIFY_LID_FLIP) {
- lid_flip_tablet_mode_get_state(asus);
- return;
+ }
+ break;
+ case asus_wmi_lid_flip_devid:
+ if (code == NOTIFY_LID_FLIP) {
+ lid_flip_tablet_mode_get_state(asus);
+ return;
+ }
+ break;
  }

  if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
@@ -3731,8 +3740,14 @@ static int asus_hotk_resume(struct device 
*device)
  if (asus_wmi_has_fnlock_key(asus))
   asus_wmi_fnlock_update(asus);

- if (asus->driver->quirks->use_lid_flip_devid)
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ case asus_wmi_kbd_dock_devid:
+ break;
+ case asus_wmi_lid_flip_devid:
   lid_flip_tablet_mode_get_state(asus);
+ break;
+ }

  return 0;
 }
@@ -3773,8 +3788,14 @@ static int asus_hotk_restore(struct device 
*device)
  if (asus_wmi_has_fnlock_key(asus))
   asus_wmi_fnlock_update(asus);

- if (asus->driver->quirks->use_lid_flip_devid)
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ case asus_wmi_kbd_dock_devid:
+ break;
+ case asus_wmi_lid_flip_devid:
   lid_flip_tablet_mode_get_state(asus);
+ break;
+ }

  return 0;
 }
diff --git a/drivers/platform/x86/asus-wmi.h 
b/drivers/platform/x86/asus-wmi.h
index b302415bf1d9..413920bad0c6 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -25,6 +25,12 @@ struct module;
 struct key_entry;
 struct asus_wmi;

+enum asus_wmi_tablet_switch_mode {
+ asus_wmi_no_tablet_switch,
+ asus_wmi_kbd_dock_devid,
+ asus_wmi_lid_flip_devid,
+};
+
 struct quirk_entry {
  bool hotplug_wireless;
  bool scalar_panel_brightness;
@@ -33,8 +39,7 @@ struct quirk_entry {
  bool wmi_backlight_native;
  bool wmi_backlight_set_devstate;
  bool wmi_force_als_set;
- bool use_kbd_dock_devid;
- bool use_lid_flip_devid;
+ enum asus_wmi_tablet_switch_mode tablet_switch_mode;
  int wapf;
  /*
   * For machines with AMD graphic chips, it will send out WMI event
diff mbox series

Patch

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index 478dd300b9c9..1ce8924d0474 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -123,6 +123,12 @@  static struct quirk_entry quirk_asus_use_lid_flip_devid = {
 	.use_lid_flip_devid = true,
 };
 
+static struct quirk_entry quirk_asus_gv301 = {
+	.wmi_backlight_set_devstate = true,
+	.use_lid_flip_devid = true,
+	.enodev_as_tablet_mode = true,
+};
+
 static int dmi_matched(const struct dmi_system_id *dmi)
 {
 	pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -471,6 +477,15 @@  static const struct dmi_system_id asus_quirks[] = {
 		},
 		.driver_data = &quirk_asus_use_lid_flip_devid,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUS ROG FLOW X13",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "GV301Q"),
+		},
+		.driver_data = &quirk_asus_gv301,
+	},
 	{},
 };
 
@@ -581,6 +596,7 @@  static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
 	{ KE_IGNORE, 0xC6, },  /* Ambient Light Sensor notification */
 	{ KE_KEY, 0xFA, { KEY_PROG2 } },           /* Lid flip action */
+	{ KE_KEY, 0xBD, { KEY_PROG2 } },           /* Lid flip action on rog flow laptops */
 	{ KE_END, 0},
 };
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 62ce198a3463..0458e9107ca7 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -68,6 +68,7 @@  module_param(fnlock_default, bool, 0444);
 #define NOTIFY_KBD_FBM			0x99
 #define NOTIFY_KBD_TTP			0xae
 #define NOTIFY_LID_FLIP			0xfa
+#define NOTIFY_LID_FLIP_GV301	0xbd
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
 
@@ -516,6 +517,12 @@  static int asus_wmi_input_init(struct asus_wmi *asus)
 
 	if (asus->driver->quirks->use_lid_flip_devid) {
 		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
+		if (result < 0)
+			result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GV301_LID_FLIP);
+
+		if (result == -ENODEV && asus->driver->quirks->enodev_as_tablet_mode)
+			result = 1;
+
 		if (result < 0)
 			asus->driver->quirks->use_lid_flip_devid = 0;
 		if (result >= 0) {
@@ -553,6 +560,12 @@  static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus)
 {
 	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
 
+	if (result < 0)
+		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GV301_LID_FLIP);
+
+	if (result == -ENODEV && asus->driver->quirks->enodev_as_tablet_mode)
+		result = 1;
+
 	if (result >= 0) {
 		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
 		input_sync(asus->inputdev);
@@ -3094,7 +3107,8 @@  static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
-	if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
+	if (asus->driver->quirks->use_lid_flip_devid &&
+			(code == NOTIFY_LID_FLIP || code == NOTIFY_LID_FLIP_GV301)) {
 		lid_flip_tablet_mode_get_state(asus);
 		return;
 	}
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index b302415bf1d9..ac9023aae838 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -35,6 +35,7 @@  struct quirk_entry {
 	bool wmi_force_als_set;
 	bool use_kbd_dock_devid;
 	bool use_lid_flip_devid;
+	bool enodev_as_tablet_mode;
 	int wapf;
 	/*
 	 * For machines with AMD graphic chips, it will send out WMI event
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..79bd06628a8b 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -64,6 +64,7 @@ 
 #define ASUS_WMI_DEVID_PANEL_OD		0x00050019
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
 #define ASUS_WMI_DEVID_LID_FLIP		0x00060062
+#define ASUS_WMI_DEVID_GV301_LID_FLIP	0x00060077
 
 /* Storage */
 #define ASUS_WMI_DEVID_CARDREADER	0x00080013