[PATCH v5 0/2] platform/x86 asus_wmi: Support of ASUS TUF laptops on Ryzen CPUs

[PATCH v5 0/2] platform/x86 asus_wmi: Support of ASUS TUF laptops on Ryzen CPUs

[Leonid Maksymchuk]

Hi,

this patch series adds support of Throttle themal policy ACPI device to
existing asus_wmi platform driver. Support of this device is required
for ASUS TUF laptops on Ryzen CPUs to properly work on Linux.

v2: fixed indentation.
v3: patches 2/3 and 3/3 are refactored.
v4: patch 2/3 are simplified.
v5: add new device instead of merging with fan boost mode

Leonid Maksymchuk (2):
  platform/x86 asus_wmi: Support throttle thermal policy
  platform/x86 asus_wmi: Set throttle thermal policy to default

 drivers/platform/x86/asus-wmi.c            | 128 +++++++++++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   1 +
 2 files changed, 129 insertions(+)

-- 
1.8.3.1

[PATCH v5 1/2] platform/x86 asus_wmi: Support throttle thermal policy

[Leonid Maksymchuk]

Throttle thermal policy ACPI device is used to control CPU cooling and
throttling. This patch adds sysfs entry for setting current mode and
Fn+F5 hotkey that switches to next.

Policy modes:
* 0x00 - default
* 0x01 - overboost
* 0x02 - silent

Signed-off-by: Leonid Maksymchuk <leonmaxx@gmail.com>
---
 drivers/platform/x86/asus-wmi.c            | 117 +++++++++++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   1 +
 2 files changed, 118 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 821b08e..88faea6 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -61,6 +61,7 @@
 #define NOTIFY_KBD_BRTDWN		0xc5
 #define NOTIFY_KBD_BRTTOGGLE		0xc7
 #define NOTIFY_KBD_FBM			0x99
+#define NOTIFY_KBD_TTP			0xae
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
 
@@ -81,6 +82,10 @@
 #define ASUS_FAN_BOOST_MODE_SILENT_MASK		0x02
 #define ASUS_FAN_BOOST_MODES_MASK		0x03
 
+#define ASUS_THROTTLE_THERMAL_POLICY_DEFAULT	0
+#define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST	1
+#define ASUS_THROTTLE_THERMAL_POLICY_SILENT	2
+
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
@@ -198,6 +203,9 @@ struct asus_wmi {
 	u8 fan_boost_mode_mask;
 	u8 fan_boost_mode;
 
+	bool throttle_thermal_policy_available;
+	u8 throttle_thermal_policy_mode;
+
 	// The RSOC controls the maximum charging percentage.
 	bool battery_rsoc_available;
 
@@ -1724,6 +1732,102 @@ static ssize_t fan_boost_mode_store(struct device *dev,
 // Fan boost mode: 0 - normal, 1 - overboost, 2 - silent
 static DEVICE_ATTR_RW(fan_boost_mode);
 
+/* Throttle thermal policy ****************************************************/
+
+static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->throttle_thermal_policy_available = false;
+
+	err = asus_wmi_get_devstate(asus,
+				    ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
+				    &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	if ((result & ASUS_WMI_DSTS_PRESENCE_BIT))
+		asus->throttle_thermal_policy_available = true;
+
+	return 0;
+}
+
+static int throttle_thermal_policy_write(struct asus_wmi *asus)
+{
+	int err;
+	u8 value;
+	u32 retval;
+
+	value = asus->throttle_thermal_policy_mode;
+
+	pr_info("Set throttle thermal policy: %u\n", value);
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
+				    value, &retval);
+	if (err) {
+		pr_warn("Failed to set throttle thermal policy: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn("Failed to set throttle thermal policy (retval): 0x%x\n",
+			retval);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
+{
+	u8 new_mode = asus->throttle_thermal_policy_mode + 1;
+
+	if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
+		new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
+
+	asus->throttle_thermal_policy_mode = new_mode;
+	return throttle_thermal_policy_write(asus);
+}
+
+static ssize_t throttle_thermal_policy_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n",
+			 asus->throttle_thermal_policy_mode);
+}
+
+static ssize_t throttle_thermal_policy_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	int result;
+	u8 new_mode;
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	result = kstrtou8(buf, 10, &new_mode);
+	if (result < 0) {
+		pr_warn("Trying to store invalid value\n");
+		return result;
+	}
+
+	if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
+		return -EINVAL;
+
+	asus->throttle_thermal_policy_mode = new_mode;
+	throttle_thermal_policy_write(asus);
+
+	return count;
+}
+
+// Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
+static DEVICE_ATTR_RW(throttle_thermal_policy);
+
 /* Backlight ******************************************************************/
 
 static int read_backlight_power(struct asus_wmi *asus)
@@ -2005,6 +2109,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	if (asus->throttle_thermal_policy_available && code == NOTIFY_KBD_TTP) {
+		throttle_thermal_policy_switch_next(asus);
+		return;
+	}
+
 	if (is_display_toggle(code) && asus->driver->quirks->no_display_toggle)
 		return;
 
@@ -2155,6 +2264,7 @@ static ssize_t cpufv_store(struct device *dev, struct device_attribute *attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
+	&dev_attr_throttle_thermal_policy.attr,
 	NULL
 };
 
@@ -2178,6 +2288,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		devid = ASUS_WMI_DEVID_ALS_ENABLE;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
 		ok = asus->fan_boost_mode_available;
+	else if (attr == &dev_attr_throttle_thermal_policy.attr)
+		ok = asus->throttle_thermal_policy_available;
 
 	if (devid != -1)
 		ok = !(asus_wmi_get_devstate_simple(asus, devid) < 0);
@@ -2437,6 +2549,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_fan_boost_mode;
 
+	err = throttle_thermal_policy_check_present(asus);
+	if (err)
+		goto fail_throttle_thermal_policy;
+
 	err = asus_wmi_sysfs_init(asus->platform_device);
 	if (err)
 		goto fail_sysfs;
@@ -2521,6 +2637,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_input:
 	asus_wmi_sysfs_exit(asus->platform_device);
 fail_sysfs:
+fail_throttle_thermal_policy:
 fail_fan_boost_mode:
 fail_platform:
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 60249e2..d39fc65 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -58,6 +58,7 @@
 #define ASUS_WMI_DEVID_LIGHT_SENSOR	0x00050022 /* ?? */
 #define ASUS_WMI_DEVID_LIGHTBAR		0x00050025
 #define ASUS_WMI_DEVID_FAN_BOOST_MODE	0x00110018
+#define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
-- 
1.8.3.1

[PATCH v5 2/2] platform/x86 asus_wmi: Set throttle thermal policy to default

[Leonid Maksymchuk]

ASUS TUF FX705DY/FX505DY starts in silent mode and under heavy
CPU load it overheats and drops CPU frequency to 399MHz and stays
at it until reboot [1]. Set throttle thermal policy to default
to avoid overheating and throttlig.

[1] Link: https://bugzilla.kernel.org/show_bug.cgi?id=203733

Signed-off-by: Leonid Maksymchuk <leonmaxx@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 88faea6..fe571d1 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1782,6 +1782,15 @@ static int throttle_thermal_policy_write(struct asus_wmi *asus)
 	return 0;
 }
 
+static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
+{
+	if (!asus->throttle_thermal_policy_available)
+		return 0;
+
+	asus->throttle_thermal_policy_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
+	return throttle_thermal_policy_write(asus);
+}
+
 static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
 {
 	u8 new_mode = asus->throttle_thermal_policy_mode + 1;
@@ -2552,6 +2561,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 	err = throttle_thermal_policy_check_present(asus);
 	if (err)
 		goto fail_throttle_thermal_policy;
+	else
+		throttle_thermal_policy_set_default(asus);
 
 	err = asus_wmi_sysfs_init(asus->platform_device);
 	if (err)
-- 
1.8.3.1

Re: [PATCH v5 0/2] platform/x86 asus_wmi: Support of ASUS TUF laptops on Ryzen CPUs

[Andy Shevchenko]

On Sun, Nov 24, 2019 at 4:07 PM Leonid Maksymchuk <leonmaxx@gmail.com> wrote:
>
> Hi,
>
> this patch series adds support of Throttle themal policy ACPI device to
> existing asus_wmi platform driver. Support of this device is required
> for ASUS TUF laptops on Ryzen CPUs to properly work on Linux.
>

In both patches you forgot a colon after platform/x86 prefix.

> v2: fixed indentation.
> v3: patches 2/3 and 3/3 are refactored.
> v4: patch 2/3 are simplified.
> v5: add new device instead of merging with fan boost mode
>
> Leonid Maksymchuk (2):
>   platform/x86 asus_wmi: Support throttle thermal policy
>   platform/x86 asus_wmi: Set throttle thermal policy to default
>
>  drivers/platform/x86/asus-wmi.c            | 128 +++++++++++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   1 +
>  2 files changed, 129 insertions(+)
>
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/2] platform/x86 asus_wmi: Support throttle thermal policy

[Andy Shevchenko]

On Sun, Nov 24, 2019 at 4:07 PM Leonid Maksymchuk <leonmaxx@gmail.com> wrote:
>
> Throttle thermal policy ACPI device is used to control CPU cooling and

> throttling. This patch adds sysfs entry for setting current mode and
> Fn+F5 hotkey that switches to next.

sysfs means ABI. ABI must be documented.

> Policy modes:
> * 0x00 - default
> * 0x01 - overboost
> * 0x02 - silent

> +static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
> +{
> +       u32 result;
> +       int err;
> +
> +       asus->throttle_thermal_policy_available = false;
> +
> +       err = asus_wmi_get_devstate(asus,
> +                                   ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
> +                                   &result);
> +       if (err) {
> +               if (err == -ENODEV)
> +                       return 0;

> +               else

Redundant.

> +                       return err;
> +       }
> +

> +       if ((result & ASUS_WMI_DSTS_PRESENCE_BIT))

Too many parentheses.

> +               asus->throttle_thermal_policy_available = true;
> +
> +       return 0;
> +}

> +       pr_info("Set throttle thermal policy: %u\n", value);

Do we really need this message?

> +       return scnprintf(buf, PAGE_SIZE, "%d\n",
> +                        asus->throttle_thermal_policy_mode);

Can it be one line?

> +       result = kstrtou8(buf, 10, &new_mode);
> +       if (result < 0) {

> +               pr_warn("Trying to store invalid value\n");

Redundant. By error code user space will get a message.

> +               return result;
> +       }



-- 
With Best Regards,
Andy Shevchenko