[PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[Luke D. Jones]

This patch series adds support for some functions that are found on newer
ASUS gaming laptops:

- Panel overdrive: Some laptops can drive the LCD matrix slightly faster
  to eliminate or reduce ghosting artifacts

- dGPU disable: ASUS added a function in ACPI to disable or enable the dGPU
  which removes it from the PCI bus. Presumably this was to help prevent
  Windows apps from using the dGPU when the user didn't want them to but
  because of how it works it also means that when rebooted to Linux the dGPU
  no-longer exits. This patch enables a user to echo 0/1 to a WMI path to
  re-enable it (or disable, but the drivers *must* be unloaded first).

- eGPU enable: The ASUS x-flow lpatop has an iGPU, a dGPU, and an optional
  eGPU. This patch enables the user to echo 0/1 to a WMI path to enable or
  disable the eGPU. In ACPI this also appears to remove the dGPU from the
  PCI bus.

All of the above patches have been tested over the course of a few months.
There is a small possibility of user error perhaps, where the user tries to
enable or disable the dGPU/eGPU while drivers are loaded which would cause
a system hang, but it is expected that almost all users would be using the
`asusctl` daemon and dbus methods to manage the above which then eliminates
these issues.

Luke D. Jones (3):
  asus-wmi: Add panel overdrive functionality
  asus-wmi: Add dgpu disable method
  asus-wmi: Add egpu enable method

 drivers/platform/x86/asus-wmi.c            | 289 +++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   7 +
 2 files changed, 296 insertions(+)

--
2.31.1

[PATCH v3 1/3] asus-wmi: Add panel overdrive functionality

[Luke D. Jones]

Some ASUS ROG laptops have the ability to drive the display panel
a higher rate to eliminate or reduce ghosting.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ebaeb7bb80f5..cbf91a9134fd 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -216,6 +216,9 @@ struct asus_wmi {
 	// The RSOC controls the maximum charging percentage.
 	bool battery_rsoc_available;
 
+	bool panel_overdrive_available;
+	bool panel_overdrive;
+
 	struct hotplug_slot hotplug_slot;
 	struct mutex hotplug_lock;
 	struct mutex wmi_lock;
@@ -1221,6 +1224,87 @@ static int asus_wmi_rfkill_init(struct asus_wmi *asus)
 	return result;
 }
 
+/* Panel Overdrive ************************************************************/
+static int panel_od_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->panel_overdrive_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_PANEL_OD, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
+		asus->panel_overdrive_available = true;
+		asus->panel_overdrive = result & ASUS_WMI_DSTS_STATUS_BIT;
+	}
+
+	return 0;
+}
+
+static int panel_od_write(struct asus_wmi *asus)
+{
+	u32 retval;
+	u8 value;
+	int err;
+
+	/* Don't rely on type conversion */
+	value = asus->panel_overdrive ? 1 : 0;
+
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_PANEL_OD, value, &retval);
+
+	if (err) {
+		pr_warn("Failed to set panel overdrive: %d\n", err);
+		return err;
+	}
+
+	if (retval > 1 || retval < 0) {
+		pr_warn("Failed to set panel overdrive (retval): 0x%x\n", retval);
+		return -EIO;
+	}
+
+	sysfs_notify(&asus->platform_device->dev.kobj, NULL, "panel_od");
+
+	return 0;
+}
+
+static ssize_t panel_od_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", asus->panel_overdrive);
+}
+
+static ssize_t panel_od_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	bool overdrive;
+	int result;
+
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	result = kstrtobool(buf, &overdrive);
+	if (result)
+		return result;
+
+	asus->panel_overdrive = overdrive;
+	result = panel_od_write(asus);
+
+	if (result)
+		return result;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(panel_od);
+
 /* Quirks *********************************************************************/
 
 static void asus_wmi_set_xusb2pr(struct asus_wmi *asus)
@@ -2332,6 +2416,7 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
 	&dev_attr_throttle_thermal_policy.attr,
+	&dev_attr_panel_od.attr,
 	NULL
 };
 
@@ -2357,6 +2442,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		ok = asus->fan_boost_mode_available;
 	else if (attr == &dev_attr_throttle_thermal_policy.attr)
 		ok = asus->throttle_thermal_policy_available;
+	else if (attr == &dev_attr_panel_od.attr)
+		ok = asus->panel_overdrive_available;
 
 	if (devid != -1)
 		ok = !(asus_wmi_get_devstate_simple(asus, devid) < 0);
@@ -2622,6 +2709,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	else
 		throttle_thermal_policy_set_default(asus);
 
+	err = panel_od_check_present(asus);
+	if (err)
+		goto fail_panel_od;
+
 	err = asus_wmi_sysfs_init(asus->platform_device);
 	if (err)
 		goto fail_sysfs;
@@ -2709,6 +2800,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_throttle_thermal_policy:
 fail_fan_boost_mode:
 fail_platform:
+fail_panel_od:
 	kfree(asus);
 	return err;
 }
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 2f274cf52805..428aea701c7b 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -61,6 +61,7 @@
 #define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
 
 /* Misc */
+#define ASUS_WMI_DEVID_PANEL_OD		0x00050019
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
 #define ASUS_WMI_DEVID_LID_FLIP		0x00060062
 
-- 
2.31.1

[PATCH v3 2/3] asus-wmi: Add dgpu disable method

[Luke D. Jones]

In Windows the ASUS Armory Crate program can enable or disable the
dGPU via a WMI call. This functions much the same as various Linux
methods in software where the dGPU is removed from the device tree.

However the WMI call saves the state of dGPU (enabled or not) and
this then changes the dGPU visibility in Linux with no way for
Linux users to re-enable it. We expose the WMI method so users can
see and change the dGPU ACPI state.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c            | 98 ++++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |  3 +
 2 files changed, 101 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index cbf91a9134fd..bee22a12bf3d 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -210,6 +210,9 @@ struct asus_wmi {
 	u8 fan_boost_mode_mask;
 	u8 fan_boost_mode;
 
+	bool dgpu_disable_available;
+	bool dgpu_disable;
+
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
@@ -427,6 +430,93 @@ static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus)
 	}
 }
 
+/* dGPU ********************************************************************/
+static int dgpu_disable_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->dgpu_disable_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_DGPU, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
+		asus->dgpu_disable_available = true;
+		asus->dgpu_disable = result & ASUS_WMI_DSTS_STATUS_BIT;
+	}
+
+	return 0;
+}
+
+static int dgpu_disable_write(struct asus_wmi *asus)
+{
+	u32 retval;
+	u8 value;
+	int err;
+
+	/* Don't rely on type conversion */
+	value = asus->dgpu_disable ? 1 : 0;
+
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_DGPU, value, &retval);
+	if (err) {
+		pr_warn("Failed to set dgpu disable: %d\n", err);
+		return err;
+	}
+
+	if (retval > 1 || retval < 0) {
+		pr_warn("Failed to set dgpu disable (retval): 0x%x\n", retval);
+		return -EIO;
+	}
+
+	sysfs_notify(&asus->platform_device->dev.kobj, NULL, "dgpu_disable");
+
+	return 0;
+}
+
+static ssize_t dgpu_disable_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+	u8 mode = asus->dgpu_disable;
+
+	return sysfs_emit(buf, "%d\n", mode);
+}
+
+/*
+ * A user may be required to store the value twice, typcial store first, then
+ * rescan PCI bus to activate power, then store a second time to save correctly.
+ * The reason for this is that an extra code path in the ACPI is enabled when
+ * the device and bus are powered.
+ */
+static ssize_t dgpu_disable_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	bool disable;
+	int result;
+
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	result = kstrtobool(buf, &disable);
+	if (result)
+		return result;
+
+	asus->dgpu_disable = disable;
+
+	result = dgpu_disable_write(asus);
+	if (result)
+		return result;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(dgpu_disable);
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -2412,6 +2502,7 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_camera.attr,
 	&dev_attr_cardr.attr,
 	&dev_attr_touchpad.attr,
+	&dev_attr_dgpu_disable.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
@@ -2438,6 +2529,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		devid = ASUS_WMI_DEVID_LID_RESUME;
 	else if (attr == &dev_attr_als_enable.attr)
 		devid = ASUS_WMI_DEVID_ALS_ENABLE;
+	else if (attr == &dev_attr_dgpu_disable.attr)
+		ok = asus->dgpu_disable_available;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
 		ok = asus->fan_boost_mode_available;
 	else if (attr == &dev_attr_throttle_thermal_policy.attr)
@@ -2699,6 +2792,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_platform;
 
+	err = dgpu_disable_check_present(asus);
+	if (err)
+		goto fail_dgpu_disable;
+
 	err = fan_boost_mode_check_present(asus);
 	if (err)
 		goto fail_fan_boost_mode;
@@ -2799,6 +2896,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_sysfs:
 fail_throttle_thermal_policy:
 fail_fan_boost_mode:
+fail_dgpu_disable:
 fail_platform:
 fail_panel_od:
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 428aea701c7b..a528f9d0e4b7 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -90,6 +90,9 @@
 /* Keyboard dock */
 #define ASUS_WMI_DEVID_KBD_DOCK		0x00120063
 
+/* dgpu on/off */
+#define ASUS_WMI_DEVID_DGPU		0x00090020
+
 /* DSTS masks */
 #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
 #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
-- 
2.31.1

[PATCH v3 3/3] asus-wmi: Add egpu enable method

[Luke D. Jones]

The X13 Flow laptops can utilise an external GPU. This requires
toggling an ACPI method which will first disable the internal
dGPU, and then enable the eGPU.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c            | 99 ++++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |  3 +
 2 files changed, 102 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index bee22a12bf3d..90a6a0d00deb 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -210,6 +210,9 @@ struct asus_wmi {
 	u8 fan_boost_mode_mask;
 	u8 fan_boost_mode;
 
+	bool egpu_enable_available; // 0 = enable
+	bool egpu_enable;
+
 	bool dgpu_disable_available;
 	bool dgpu_disable;
 
@@ -517,6 +520,94 @@ static ssize_t dgpu_disable_store(struct device *dev,
 
 static DEVICE_ATTR_RW(dgpu_disable);
 
+/* eGPU ********************************************************************/
+static int egpu_enable_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->egpu_enable_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_EGPU, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
+		asus->egpu_enable_available = true;
+		asus->egpu_enable = result & ASUS_WMI_DSTS_STATUS_BIT;
+	}
+
+	return 0;
+}
+
+static int egpu_enable_write(struct asus_wmi *asus)
+{
+	u32 retval;
+	u8 value;
+	int err;
+
+	/* Don't rely on type conversion */
+	value = asus->egpu_enable ? 1 : 0;
+
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_EGPU, value, &retval);
+
+	if (err) {
+		pr_warn("Failed to set egpu disable: %d\n", err);
+		return err;
+	}
+
+	if (retval > 1 || retval < 0) {
+		pr_warn("Failed to set egpu disable (retval): 0x%x\n", retval);
+		return -EIO;
+	}
+
+	sysfs_notify(&asus->platform_device->dev.kobj, NULL, "egpu_enable");
+
+	return 0;
+}
+
+static ssize_t egpu_enable_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+	bool mode = asus->egpu_enable;
+
+	return sysfs_emit(buf, "%d\n", mode);
+}
+
+/* The ACPI call to enable the eGPU also disables the internal dGPU */
+static ssize_t egpu_enable_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	bool enable;
+	int result;
+
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	result = kstrtobool(buf, &enable);
+	if (result)
+		return result;
+
+	asus->egpu_enable = enable;
+
+	result = egpu_enable_write(asus);
+	if (result)
+		return result;
+
+	/* Ensure that the kernel status of dgpu is updated */
+	result = dgpu_disable_check_present(asus);
+	if (result)
+		return result;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(egpu_enable);
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -2502,6 +2593,7 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_camera.attr,
 	&dev_attr_cardr.attr,
 	&dev_attr_touchpad.attr,
+	&dev_attr_egpu_enable.attr,
 	&dev_attr_dgpu_disable.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
@@ -2529,6 +2621,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		devid = ASUS_WMI_DEVID_LID_RESUME;
 	else if (attr == &dev_attr_als_enable.attr)
 		devid = ASUS_WMI_DEVID_ALS_ENABLE;
+	else if (attr == &dev_attr_egpu_enable.attr)
+		ok = asus->egpu_enable_available;
 	else if (attr == &dev_attr_dgpu_disable.attr)
 		ok = asus->dgpu_disable_available;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
@@ -2792,6 +2886,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_platform;
 
+	err = egpu_enable_check_present(asus);
+	if (err)
+		goto fail_egpu_enable;
+
 	err = dgpu_disable_check_present(asus);
 	if (err)
 		goto fail_dgpu_disable;
@@ -2896,6 +2994,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_sysfs:
 fail_throttle_thermal_policy:
 fail_fan_boost_mode:
+fail_egpu_enable:
 fail_dgpu_disable:
 fail_platform:
 fail_panel_od:
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a528f9d0e4b7..17dc5cb6f3f2 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -90,6 +90,9 @@
 /* Keyboard dock */
 #define ASUS_WMI_DEVID_KBD_DOCK		0x00120063
 
+/* dgpu on/off */
+#define ASUS_WMI_DEVID_EGPU		0x00090019
+
 /* dgpu on/off */
 #define ASUS_WMI_DEVID_DGPU		0x00090020
 
-- 
2.31.1

Re: [PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[Hans de Goede]

Hi Luke,

On 8/7/21 4:36 AM, Luke D. Jones wrote:
> This patch series adds support for some functions that are found on newer
> ASUS gaming laptops:
> 
> - Panel overdrive: Some laptops can drive the LCD matrix slightly faster
>   to eliminate or reduce ghosting artifacts
> 
> - dGPU disable: ASUS added a function in ACPI to disable or enable the dGPU
>   which removes it from the PCI bus. Presumably this was to help prevent
>   Windows apps from using the dGPU when the user didn't want them to but
>   because of how it works it also means that when rebooted to Linux the dGPU
>   no-longer exits. This patch enables a user to echo 0/1 to a WMI path to
>   re-enable it (or disable, but the drivers *must* be unloaded first).
> 
> - eGPU enable: The ASUS x-flow lpatop has an iGPU, a dGPU, and an optional
>   eGPU. This patch enables the user to echo 0/1 to a WMI path to enable or
>   disable the eGPU. In ACPI this also appears to remove the dGPU from the
>   PCI bus.
> 
> All of the above patches have been tested over the course of a few months.
> There is a small possibility of user error perhaps, where the user tries to
> enable or disable the dGPU/eGPU while drivers are loaded which would cause
> a system hang, but it is expected that almost all users would be using the
> `asusctl` daemon and dbus methods to manage the above which then eliminates
> these issues.

Thank you for the new version, all 3 patches look good to me, but I miss
a changelog in this cover-letter.

Specifically I'm wondering what happened to the following,
which you wrote about in the v1 patch-set thread:

"""
Proper enable of the dGPU again as far as my testing goes works such that:
1. call the ACPI method
2. rescan PCI bus to ensure the device is powered
3. call the ACPI method again to save the setting

But it appears that recent work in-kernel for many things AMD related has broken this for us...
"""

?

Regards,

Hans



> 
> Luke D. Jones (3):
>   asus-wmi: Add panel overdrive functionality
>   asus-wmi: Add dgpu disable method
>   asus-wmi: Add egpu enable method
> 
>  drivers/platform/x86/asus-wmi.c            | 289 +++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   7 +
>  2 files changed, 296 insertions(+)
> 
> --
> 2.31.1
> 

Re: [PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[Luke Jones]

On Mon, Aug 9 2021 at 11:18:38 +0200, Hans de Goede 
<hdegoede@redhat.com> wrote:
> Hi Luke,
> 
> On 8/7/21 4:36 AM, Luke D. Jones wrote:
>>  This patch series adds support for some functions that are found on 
>> newer
>>  ASUS gaming laptops:
>> 
>>  - Panel overdrive: Some laptops can drive the LCD matrix slightly 
>> faster
>>    to eliminate or reduce ghosting artifacts
>> 
>>  - dGPU disable: ASUS added a function in ACPI to disable or enable 
>> the dGPU
>>    which removes it from the PCI bus. Presumably this was to help 
>> prevent
>>    Windows apps from using the dGPU when the user didn't want them 
>> to but
>>    because of how it works it also means that when rebooted to Linux 
>> the dGPU
>>    no-longer exits. This patch enables a user to echo 0/1 to a WMI 
>> path to
>>    re-enable it (or disable, but the drivers *must* be unloaded 
>> first).
>> 
>>  - eGPU enable: The ASUS x-flow lpatop has an iGPU, a dGPU, and an 
>> optional
>>    eGPU. This patch enables the user to echo 0/1 to a WMI path to 
>> enable or
>>    disable the eGPU. In ACPI this also appears to remove the dGPU 
>> from the
>>    PCI bus.
>> 
>>  All of the above patches have been tested over the course of a few 
>> months.
>>  There is a small possibility of user error perhaps, where the user 
>> tries to
>>  enable or disable the dGPU/eGPU while drivers are loaded which 
>> would cause
>>  a system hang, but it is expected that almost all users would be 
>> using the
>>  `asusctl` daemon and dbus methods to manage the above which then 
>> eliminates
>>  these issues.
> 
> Thank you for the new version, all 3 patches look good to me, but I 
> miss
> a changelog in this cover-letter.
> 
> Specifically I'm wondering what happened to the following,
> which you wrote about in the v1 patch-set thread:
> 
> """
> Proper enable of the dGPU again as far as my testing goes works such 
> that:
> 1. call the ACPI method
> 2. rescan PCI bus to ensure the device is powered
> 3. call the ACPI method again to save the setting
> 
> But it appears that recent work in-kernel for many things AMD related 
> has broken this for us...
> """

Apologies, I've been a bit too busy to remember some things.

The changes are mostly to satisfy review. The dGPU patch has removed the
dual call to the ACPI method, it was not working as expected. I will 
revisit
this when 5.14 kernel is released.

I'll be sure to remember the changelog next time I submit a patch, 
sorry.

Regards,
Luke.

> 
> ?
> 
> Regards,
> 
> Hans
> 
> 
> 
>> 
>>  Luke D. Jones (3):
>>    asus-wmi: Add panel overdrive functionality
>>    asus-wmi: Add dgpu disable method
>>    asus-wmi: Add egpu enable method
>> 
>>   drivers/platform/x86/asus-wmi.c            | 289 
>> +++++++++++++++++++++
>>   include/linux/platform_data/x86/asus-wmi.h |   7 +
>>   2 files changed, 296 insertions(+)
>> 
>>  --
>>  2.31.1
>> 
> 

Re: [PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[Hans de Goede]

<Added platform-driver-x86@vger.kernel.org to the Cc>

On 8/9/21 11:49 PM, Luke Jones wrote:
> 
> 
> On Mon, Aug 9 2021 at 11:18:38 +0200, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi Luke,
>>
>> On 8/7/21 4:36 AM, Luke D. Jones wrote:
>>>  This patch series adds support for some functions that are found on newer
>>>  ASUS gaming laptops:
>>>
>>>  - Panel overdrive: Some laptops can drive the LCD matrix slightly faster
>>>    to eliminate or reduce ghosting artifacts
>>>
>>>  - dGPU disable: ASUS added a function in ACPI to disable or enable the dGPU
>>>    which removes it from the PCI bus. Presumably this was to help prevent
>>>    Windows apps from using the dGPU when the user didn't want them to but
>>>    because of how it works it also means that when rebooted to Linux the dGPU
>>>    no-longer exits. This patch enables a user to echo 0/1 to a WMI path to
>>>    re-enable it (or disable, but the drivers *must* be unloaded first).
>>>
>>>  - eGPU enable: The ASUS x-flow lpatop has an iGPU, a dGPU, and an optional
>>>    eGPU. This patch enables the user to echo 0/1 to a WMI path to enable or
>>>    disable the eGPU. In ACPI this also appears to remove the dGPU from the
>>>    PCI bus.
>>>
>>>  All of the above patches have been tested over the course of a few months.
>>>  There is a small possibility of user error perhaps, where the user tries to
>>>  enable or disable the dGPU/eGPU while drivers are loaded which would cause
>>>  a system hang, but it is expected that almost all users would be using the
>>>  `asusctl` daemon and dbus methods to manage the above which then eliminates
>>>  these issues.
>>
>> Thank you for the new version, all 3 patches look good to me, but I miss
>> a changelog in this cover-letter.
>>
>> Specifically I'm wondering what happened to the following,
>> which you wrote about in the v1 patch-set thread:
>>
>> """
>> Proper enable of the dGPU again as far as my testing goes works such that:
>> 1. call the ACPI method
>> 2. rescan PCI bus to ensure the device is powered
>> 3. call the ACPI method again to save the setting
>>
>> But it appears that recent work in-kernel for many things AMD related has broken this for us...
>> """
> 
> Apologies, I've been a bit too busy to remember some things.
> 
> The changes are mostly to satisfy review. The dGPU patch has removed the
> dual call to the ACPI method, it was not working as expected. I will revisit
> this when 5.14 kernel is released.

Ok, so from my pov these patches are ready for merging now, but since this
is still somewhat of an open question, I wonder if they are also ready
for merging from your pov, or if you want to fist sort this out ?

> I'll be sure to remember the changelog next time I submit a patch, sorry.

No problem.

Regards,

Hans



>>>  Luke D. Jones (3):
>>>    asus-wmi: Add panel overdrive functionality
>>>    asus-wmi: Add dgpu disable method
>>>    asus-wmi: Add egpu enable method
>>>
>>>   drivers/platform/x86/asus-wmi.c            | 289 +++++++++++++++++++++
>>>   include/linux/platform_data/x86/asus-wmi.h |   7 +
>>>   2 files changed, 296 insertions(+)
>>>
>>>  --
>>>  2.31.1
>>>
>>
> 
> 

Re: [PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[Luke Jones]

On Tue, Aug 10 2021 at 09:46:13 +0200, Hans de Goede 
<hdegoede@redhat.com> wrote:
> <Added platform-driver-x86@vger.kernel.org to the Cc>
> 
> On 8/9/21 11:49 PM, Luke Jones wrote:
>> 
>> 
>>  On Mon, Aug 9 2021 at 11:18:38 +0200, Hans de Goede 
>> <hdegoede@redhat.com> wrote:
>>>  Hi Luke,
>>> 
>>>  On 8/7/21 4:36 AM, Luke D. Jones wrote:
>>>>   This patch series adds support for some functions that are found 
>>>> on newer
>>>>   ASUS gaming laptops:
>>>> 
>>>>   - Panel overdrive: Some laptops can drive the LCD matrix 
>>>> slightly faster
>>>>     to eliminate or reduce ghosting artifacts
>>>> 
>>>>   - dGPU disable: ASUS added a function in ACPI to disable or 
>>>> enable the dGPU
>>>>     which removes it from the PCI bus. Presumably this was to help 
>>>> prevent
>>>>     Windows apps from using the dGPU when the user didn't want 
>>>> them to but
>>>>     because of how it works it also means that when rebooted to 
>>>> Linux the dGPU
>>>>     no-longer exits. This patch enables a user to echo 0/1 to a 
>>>> WMI path to
>>>>     re-enable it (or disable, but the drivers *must* be unloaded 
>>>> first).
>>>> 
>>>>   - eGPU enable: The ASUS x-flow lpatop has an iGPU, a dGPU, and 
>>>> an optional
>>>>     eGPU. This patch enables the user to echo 0/1 to a WMI path to 
>>>> enable or
>>>>     disable the eGPU. In ACPI this also appears to remove the dGPU 
>>>> from the
>>>>     PCI bus.
>>>> 
>>>>   All of the above patches have been tested over the course of a 
>>>> few months.
>>>>   There is a small possibility of user error perhaps, where the 
>>>> user tries to
>>>>   enable or disable the dGPU/eGPU while drivers are loaded which 
>>>> would cause
>>>>   a system hang, but it is expected that almost all users would be 
>>>> using the
>>>>   `asusctl` daemon and dbus methods to manage the above which then 
>>>> eliminates
>>>>   these issues.
>>> 
>>>  Thank you for the new version, all 3 patches look good to me, but 
>>> I miss
>>>  a changelog in this cover-letter.
>>> 
>>>  Specifically I'm wondering what happened to the following,
>>>  which you wrote about in the v1 patch-set thread:
>>> 
>>>  """
>>>  Proper enable of the dGPU again as far as my testing goes works 
>>> such that:
>>>  1. call the ACPI method
>>>  2. rescan PCI bus to ensure the device is powered
>>>  3. call the ACPI method again to save the setting
>>> 
>>>  But it appears that recent work in-kernel for many things AMD 
>>> related has broken this for us...
>>>  """
>> 
>>  Apologies, I've been a bit too busy to remember some things.
>> 
>>  The changes are mostly to satisfy review. The dGPU patch has 
>> removed the
>>  dual call to the ACPI method, it was not working as expected. I 
>> will revisit
>>  this when 5.14 kernel is released.
> 
> Ok, so from my pov these patches are ready for merging now, but since 
> this
> is still somewhat of an open question, I wonder if they are also ready
> for merging from your pov, or if you want to fist sort this out ?

I'm definitely okay with merging now. The dgpu patch can at least 
provide
us a hint for the user that the dGPU may have been disabled in Windows 
and
recommend that they use Windows + Armoury Crate to enable it again.

The eGPU patch itself appears to work well enough and relies on the dGPU
patch.

If we're both fine with the patches lets go ahead :)

Many thanks,
Luke.

> 
>>  I'll be sure to remember the changelog next time I submit a patch, 
>> sorry.
> 
> No problem.
> 
> Regards,
> 
> Hans
> 
> 
> 
>>>>   Luke D. Jones (3):
>>>>     asus-wmi: Add panel overdrive functionality
>>>>     asus-wmi: Add dgpu disable method
>>>>     asus-wmi: Add egpu enable method
>>>> 
>>>>    drivers/platform/x86/asus-wmi.c            | 289 
>>>> +++++++++++++++++++++
>>>>    include/linux/platform_data/x86/asus-wmi.h |   7 +
>>>>    2 files changed, 296 insertions(+)
>>>> 
>>>>   --
>>>>   2.31.1
>>>> 
>>> 
>> 
>> 
> 

Re: [PATCH v3 0/3] Support for ASUS egpu, dpgu disable, panel overdrive

[Hans de Goede]

Hi,

On 8/7/21 4:36 AM, Luke D. Jones wrote:
> This patch series adds support for some functions that are found on newer
> ASUS gaming laptops:
> 
> - Panel overdrive: Some laptops can drive the LCD matrix slightly faster
>   to eliminate or reduce ghosting artifacts
> 
> - dGPU disable: ASUS added a function in ACPI to disable or enable the dGPU
>   which removes it from the PCI bus. Presumably this was to help prevent
>   Windows apps from using the dGPU when the user didn't want them to but
>   because of how it works it also means that when rebooted to Linux the dGPU
>   no-longer exits. This patch enables a user to echo 0/1 to a WMI path to
>   re-enable it (or disable, but the drivers *must* be unloaded first).
> 
> - eGPU enable: The ASUS x-flow lpatop has an iGPU, a dGPU, and an optional
>   eGPU. This patch enables the user to echo 0/1 to a WMI path to enable or
>   disable the eGPU. In ACPI this also appears to remove the dGPU from the
>   PCI bus.
> 
> All of the above patches have been tested over the course of a few months.
> There is a small possibility of user error perhaps, where the user tries to
> enable or disable the dGPU/eGPU while drivers are loaded which would cause
> a system hang, but it is expected that almost all users would be using the
> `asusctl` daemon and dbus methods to manage the above which then eliminates
> these issues.

Thank you for your patch-series, I've applied the series 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


> 
> Luke D. Jones (3):
>   asus-wmi: Add panel overdrive functionality
>   asus-wmi: Add dgpu disable method
>   asus-wmi: Add egpu enable method
> 
>  drivers/platform/x86/asus-wmi.c            | 289 +++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   7 +
>  2 files changed, 296 insertions(+)
> 
> --
> 2.31.1
>