All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luke Jones <luke@ljones.dev>
To: Hans de Goede <hdegoede@redhat.com>
Cc: markgross@kernel.org, platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] asus-wmi: Add support for ROG X13 tablet mode
Date: Fri, 05 Aug 2022 10:20:49 +1200	[thread overview]
Message-ID: <PE34GR.UO4Z6YSS29Z83@ljones.dev> (raw)
In-Reply-To: <d9d79f9b-f3ab-c07e-9e18-5760ff828487@redhat.com>

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

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
-- 
2.37.1



 From b5c6f4dc0be17d3f57b2f6f90e9d01027c03a5e1 Mon Sep 17 00:00:00 2001
 From: "Luke D. Jones" <luke@ljones.dev>
Date: Fri, 5 Aug 2022 10:15:14 +1200
Subject: [PATCH 2/2] asus-wmi: Add support for ROG X13 tablet mode

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 | 17 +++++++++-
 drivers/platform/x86/asus-wmi.c | 36 ++++++++++++++++++++++
 drivers/platform/x86/asus-wmi.h | 1 +
 include/linux/platform_data/x86/asus-wmi.h | 1 +
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c 
b/drivers/platform/x86/asus-nb-wmi.c
index 3a93e056c4e1..4aeaac92296f 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -123,6 +123,11 @@ static struct quirk_entry 
quirk_asus_use_lid_flip_devid = {
  .tablet_switch_mode = asus_wmi_lid_flip_devid,
 };

+static struct quirk_entry quirk_asus_tablet_mode = {
+ .wmi_backlight_set_devstate = true,
+ .tablet_switch_mode = asus_wmi_lid_flip_rog_devid,
+};
+
 static int dmi_matched(const struct dmi_system_id *dmi)
 {
  pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -471,6 +476,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_tablet_mode,
+ },
  {},
 };

@@ -574,7 +588,8 @@ static const struct key_entry asus_nb_wmi_keymap[] 
= {
  { KE_KEY, 0xC4, { KEY_KBDILLUMUP } },
  { KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
  { KE_IGNORE, 0xC6, }, /* Ambient Light Sensor notification */
- { KE_KEY, 0xFA, { KEY_PROG2 } }, /* Lid flip action */
+ { KE_KEY, 0xFA, { KEY_PROG2 } }, /* Lid flip action */
+ { KE_KEY, 0xBD, { KEY_PROG2 } }, /* Lid flip action on ROG xflow 
laptops */
  { KE_END, 0},
 };

diff --git a/drivers/platform/x86/asus-wmi.c 
b/drivers/platform/x86/asus-wmi.c
index a32e99205697..51610bd6b1c4 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_ROG 0xbd

 #define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)

@@ -529,6 +530,19 @@ static int asus_wmi_input_init(struct asus_wmi 
*asus)
    pr_err("Error checking for lid-flip: %d\n", result);
   }
   break;
+ case asus_wmi_lid_flip_rog_devid:
+ result = asus_wmi_get_devstate_simple(asus, 
ASUS_WMI_DEVID_LID_FLIP_ROG);
+ if (result < 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);
+ } else if (result == -ENODEV) {
+ pr_err("This device has lid-flip-rog quirk but got ENODEV checking 
it. This is a bug.");
+ } else {
+ pr_err("Error checking for lid-flip: %d\n", result);
+ }
+ break;
  }

  err = input_register_device(asus->inputdev);
@@ -562,6 +576,16 @@ static void lid_flip_tablet_mode_get_state(struct 
asus_wmi *asus)
  }
 }

+static void lid_flip_rog_tablet_mode_get_state(struct asus_wmi *asus)
+{
+ int result = asus_wmi_get_devstate_simple(asus, 
ASUS_WMI_DEVID_LID_FLIP_ROG);
+
+ if (result >= 0) {
+ input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
+ input_sync(asus->inputdev);
+ }
+}
+
 /* dGPU 
********************************************************************/
 static int dgpu_disable_check_present(struct asus_wmi *asus)
 {
@@ -3106,6 +3130,12 @@ static void asus_wmi_handle_event_code(int code, 
struct asus_wmi *asus)
    return;
   }
   break;
+ case asus_wmi_lid_flip_rog_devid:
+ if (code == NOTIFY_LID_FLIP_ROG) {
+ lid_flip_rog_tablet_mode_get_state(asus);
+ return;
+ }
+ break;
  }

  if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
@@ -3747,6 +3777,9 @@ static int asus_hotk_resume(struct device *device)
  case asus_wmi_lid_flip_devid:
   lid_flip_tablet_mode_get_state(asus);
   break;
+ case asus_wmi_lid_flip_rog_devid:
+ lid_flip_rog_tablet_mode_get_state(asus);
+ break;
  }

  return 0;
@@ -3795,6 +3828,9 @@ static int asus_hotk_restore(struct device 
*device)
  case asus_wmi_lid_flip_devid:
   lid_flip_tablet_mode_get_state(asus);
   break;
+ case asus_wmi_lid_flip_rog_devid:
+ lid_flip_rog_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 413920bad0c6..0187f13d2414 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -29,6 +29,7 @@ enum asus_wmi_tablet_switch_mode {
  asus_wmi_no_tablet_switch,
  asus_wmi_kbd_dock_devid,
  asus_wmi_lid_flip_devid,
+ asus_wmi_lid_flip_rog_devid,
 };

 struct quirk_entry {
diff --git a/include/linux/platform_data/x86/asus-wmi.h 
b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..d54458431600 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_LID_FLIP_ROG 0x00060077

 /* Storage */
 #define ASUS_WMI_DEVID_CARDREADER 0x00080013
-- 
2.37.1



On Thu, Aug 4 2022 at 15:51:22 +0200, Hans de Goede 
<hdegoede@redhat.com> wrote:
> 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
> 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1: 0002-asus-wmi-Add-support-for-ROG-X13-tablet-mode.patch --]
[-- Type: text/x-patch, Size: 5589 bytes --]

From b5c6f4dc0be17d3f57b2f6f90e9d01027c03a5e1 Mon Sep 17 00:00:00 2001
From: "Luke D. Jones" <luke@ljones.dev>
Date: Fri, 5 Aug 2022 10:15:14 +1200
Subject: [PATCH 2/2] asus-wmi: Add support for ROG X13 tablet mode

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         | 17 +++++++++-
 drivers/platform/x86/asus-wmi.c            | 36 ++++++++++++++++++++++
 drivers/platform/x86/asus-wmi.h            |  1 +
 include/linux/platform_data/x86/asus-wmi.h |  1 +
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index 3a93e056c4e1..4aeaac92296f 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -123,6 +123,11 @@ static struct quirk_entry quirk_asus_use_lid_flip_devid = {
 	.tablet_switch_mode = asus_wmi_lid_flip_devid,
 };
 
+static struct quirk_entry quirk_asus_tablet_mode = {
+	.wmi_backlight_set_devstate = true,
+	.tablet_switch_mode = asus_wmi_lid_flip_rog_devid,
+};
+
 static int dmi_matched(const struct dmi_system_id *dmi)
 {
 	pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -471,6 +476,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_tablet_mode,
+	},
 	{},
 };
 
@@ -574,7 +588,8 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_KEY, 0xC4, { KEY_KBDILLUMUP } },
 	{ KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
 	{ KE_IGNORE, 0xC6, },  /* Ambient Light Sensor notification */
-	{ KE_KEY, 0xFA, { KEY_PROG2 } },           /* Lid flip action */
+	{ KE_KEY, 0xFA, { KEY_PROG2 } }, /* Lid flip action */
+	{ KE_KEY, 0xBD, { KEY_PROG2 } }, /* Lid flip action on ROG xflow laptops */
 	{ KE_END, 0},
 };
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index a32e99205697..51610bd6b1c4 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_ROG		0xbd
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
 
@@ -529,6 +530,19 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
 			pr_err("Error checking for lid-flip: %d\n", result);
 		}
 		break;
+	case asus_wmi_lid_flip_rog_devid:
+		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP_ROG);
+		if (result < 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);
+		} else if (result == -ENODEV) {
+			pr_err("This device has lid-flip-rog quirk but got ENODEV checking it. This is a bug.");
+		} else {
+			pr_err("Error checking for lid-flip: %d\n", result);
+		}
+		break;
 	}
 
 	err = input_register_device(asus->inputdev);
@@ -562,6 +576,16 @@ static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus)
 	}
 }
 
+static void lid_flip_rog_tablet_mode_get_state(struct asus_wmi *asus)
+{
+	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP_ROG);
+
+	if (result >= 0) {
+		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
+		input_sync(asus->inputdev);
+	}
+}
+
 /* dGPU ********************************************************************/
 static int dgpu_disable_check_present(struct asus_wmi *asus)
 {
@@ -3106,6 +3130,12 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 			return;
 		}
 		break;
+	case asus_wmi_lid_flip_rog_devid:
+		if (code == NOTIFY_LID_FLIP_ROG) {
+			lid_flip_rog_tablet_mode_get_state(asus);
+			return;
+		}
+		break;
 	}
 
 	if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
@@ -3747,6 +3777,9 @@ static int asus_hotk_resume(struct device *device)
 	case asus_wmi_lid_flip_devid:
 		lid_flip_tablet_mode_get_state(asus);
 		break;
+	case asus_wmi_lid_flip_rog_devid:
+		lid_flip_rog_tablet_mode_get_state(asus);
+		break;
 	}
 
 	return 0;
@@ -3795,6 +3828,9 @@ static int asus_hotk_restore(struct device *device)
 	case asus_wmi_lid_flip_devid:
 		lid_flip_tablet_mode_get_state(asus);
 		break;
+	case asus_wmi_lid_flip_rog_devid:
+		lid_flip_rog_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 413920bad0c6..0187f13d2414 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -29,6 +29,7 @@ enum asus_wmi_tablet_switch_mode {
 	asus_wmi_no_tablet_switch,
 	asus_wmi_kbd_dock_devid,
 	asus_wmi_lid_flip_devid,
+	asus_wmi_lid_flip_rog_devid,
 };
 
 struct quirk_entry {
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..d54458431600 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_LID_FLIP_ROG	0x00060077
 
 /* Storage */
 #define ASUS_WMI_DEVID_CARDREADER	0x00080013
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.2: 0001-asus-wmi-Adjust-tablet-lidflip-handling-to-use-enum.patch --]
[-- Type: text/x-patch, Size: 6097 bytes --]

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
-- 
2.37.1


      reply	other threads:[~2022-08-04 22:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-03  6:37 [PATCH 1/1] asus-wmi: Add support for ROG X13 tablet mode Luke D. Jones
2022-08-04 13:51 ` Hans de Goede
2022-08-04 22:20   ` Luke Jones [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PE34GR.UO4Z6YSS29Z83@ljones.dev \
    --to=luke@ljones.dev \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.