Re: [PATCH v7] asus-wmi: Add support for custom fan curves

[Luke Jones <luke@ljones.dev>]

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

Hi Barnabás,

I did another refactor using hwmon_device_register_with_info() and 
HWMON_CHANNEL_INFO(). I'm unsure if this is what you were looking for 
so I'm going to attach the patch instead of submitting as a V8 for now.

My main concern as that the use of the above removes the 
pwm1_auto_point1_pwm + pwm1_auto_point1_temp format and gives two 
hwmon<num>, one per cpu/gpu fan with:

device power
fan1_input subsystem
fan2_input temp1_input
fan3_input temp2_input
fan4_input temp3_input
fan5_input temp4_input
fan6_input temp5_input
fan7_input temp6_input
fan8_input temp7_input
in0_enable temp8_input
name uevent

cat -p /sys/devices/platform/asus-nb-wmi/hwmon/hwmon7/name
asus_cpu_fan_custom_curve

I've named the root name of each as descriptive as possible to convey 
exactly what each is

Oh and `sensors` now shows:

asus_cpu_fan_curve-isa-0000
Adapter: ISA adapter
fan1: 8 RPM
fan2: 10 RPM
fan3: 18 RPM
fan4: 20 RPM
fan5: 28 RPM
fan6: 34 RPM
fan7: 44 RPM
fan8: 56 RPM
temp1: +0.0°C
temp2: +0.1°C
temp3: +0.1°C
temp4: +0.1°C
temp5: +0.1°C
temp6: +0.1°C
temp7: +0.1°C
temp8: +0.1°C


 > FYI, the pwmX_enable attributes can be created by the hwmon
 > subsystem itself if you use [devm_]hwmon_device_register_with_info()
 > with appropriately populated `struct hwmon_chip_info`.

So when you say this, did you mean *just* for the pwmX_enable 
attributes?


On Mon, Aug 30 2021 at 21:28:18 +0000, Barnabás Pőcze 
<pobrn@protonmail.com> wrote:
> Hi
> 
> 
> 2021. augusztus 30., hétfő 13:31 keltezéssel, Luke D. Jones írta:
>>  Add support for custom fan curves found on some ASUS ROG laptops.
>> 
>>  These laptops have the ability to set a custom curve for the CPU
>>  and GPU fans via an ACPI method call. This patch enables this,
>>  additionally enabling custom fan curves per-profile, where profile
>>  here means each of the 3 levels of "throttle_thermal_policy".
>> 
>>  This patch adds two blocks of attributes to the hwmon sysfs,
>>  1 block each for CPU and GPU fans.
>> 
>>  When the user switches profiles the associated curve data for that
>>  profile is then show/store enabled to allow users to rotate through
>>  the profiles and set a fan curve for each profile which then
>>  activates on profile switch if enabled.
>> 
>>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
>>  ---
>>   drivers/platform/x86/asus-wmi.c            | 568 
>> ++++++++++++++++++++-
>>   include/linux/platform_data/x86/asus-wmi.h |   2 +
>>   2 files changed, 566 insertions(+), 4 deletions(-)
>> 
>>  diff --git a/drivers/platform/x86/asus-wmi.c 
>> b/drivers/platform/x86/asus-wmi.c
>>  index cc5811844012..b594c2475034 100644
>>  --- a/drivers/platform/x86/asus-wmi.c
>>  +++ b/drivers/platform/x86/asus-wmi.c
>>  [...]
>>  +/*
>>  + * Returns as an error if the method output is not a buffer. 
>> Typically this
> 
> It seems to me it will simply leave the output buffer uninitialized 
> if something
> other than ACPI_TYPE_BUFFER and ACPI_TYPE_INTEGER is encountered and 
> return 0.
> 
> 
>>  + * means that the method called is unsupported.
>>  + */
>>  +static int asus_wmi_evaluate_method_buf(u32 method_id,
>>  +		u32 arg0, u32 arg1, u8 *ret_buffer)
>>  +{
>>  +	struct bios_args args = {
>>  +		.arg0 = arg0,
>>  +		.arg1 = arg1,
>>  +		.arg2 = 0,
>>  +	};
>>  +	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>  +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>  +	acpi_status status;
>>  +	union acpi_object *obj;
>>  +	u32 int_tmp = 0;
>>  +
>>  +	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id,
>>  +				     &input, &output);
>>  +
>>  +	if (ACPI_FAILURE(status))
>>  +		return -EIO;
>>  +
>>  +	obj = (union acpi_object *)output.pointer;
>>  +
>>  +	if (obj && obj->type == ACPI_TYPE_INTEGER) {
>>  +		int_tmp = (u32) obj->integer.value;
>>  +		if (int_tmp == ASUS_WMI_UNSUPPORTED_METHOD)
>>  +			return -ENODEV;
>>  +		return int_tmp;
> 
> Is anything known about the possible values? You are later
> using it as if it was an errno (e.g. in `custom_fan_check_present()`).
> 
> And `obj` is leaked in both of the previous two returns.
> 
> 
>>  +	}
>>  +
>>  +	if (obj && obj->type == ACPI_TYPE_BUFFER)
>>  +		memcpy(ret_buffer, obj->buffer.pointer, obj->buffer.length);
> 
> I would suggest you add a "size_t size" argument to this function, and
> return -ENOSPC/-ENODATA depending on whether the returned buffer is 
> too
> big/small. Maybe return -ENODATA if `obj` is NULL, too.
> 
> 
>>  +
>>  +	kfree(obj);
>>  +
>>  +	return 0;
>>  +}
>>  [...]
>>  +static ssize_t fan_curve_show(struct device *dev,
>>  +				struct device_attribute *attr, char *buf)
>>  +{
>>  +	struct fan_curve_data *data = fan_curve_attr_data_select(dev, 
>> attr);
>>  +	int value;
>>  +
>>  +	int index = to_sensor_dev_attr_2(attr)->index;
>>  +	int nr = to_sensor_dev_attr_2(attr)->nr;
>>  +	int pwm = nr & FAN_CURVE_PWM_MASK;
>>  +
>>  +	if (pwm)
>>  +		value = 255 * data->percents[index] / 100;
>>  +	else
>>  +		value = data->temps[index];
>>  +
>>  +	return scnprintf(buf, PAGE_SIZE, "%d\n", value);
> 
> sysfs_emit()
> 
> 
>>  +}
>>  +
>>  +/*
>>  + * "dev" is the related WMI method such as 
>> ASUS_WMI_DEVID_CPU_FAN_CURVE.
>>  + */
>>  +static int fan_curve_write(struct asus_wmi *asus, u32 dev,
>>  +					struct fan_curve_data *data)
>>  +{
>>  +	int ret, i, shift = 0;
>>  +	u32 arg1, arg2, arg3, arg4;
>>  +
>>  +	arg1 = arg2 = arg3 = arg4 = 0;
>>  +
>>  +	for (i = 0; i < FAN_CURVE_POINTS / 2; i++) {
>>  +		arg1 += data->temps[i] << shift;
>>  +		arg2 += data->temps[i + 4] << shift;
>>  +		arg3 += data->percents[0] << shift;
>>  +		arg4 += data->percents[i + 4] << shift;
>>  +		shift += 8;
>>  +	}
>>  +
>>  +	return asus_wmi_evaluate_method5(ASUS_WMI_METHODID_DEVS, dev,
>>  +					arg1, arg2, arg3, arg4, &ret);
>>  +}
>>  +
>>  +/*
>>  + * Called only by throttle_thermal_policy_write()
>>  + */
> 
> Am I correct in thinking that the firmware does not actually
> support specifying fan curves for each mode, only a single one,
> and the fan curve switching is done by this driver when
> the performance mode is changed?
> 
> 
>>  +static int fan_curve_write_data(struct asus_wmi *asus)
>>  +{
>>  +	struct fan_curve_data *cpu;
>>  +	struct fan_curve_data *gpu;
>>  +	int err, mode;
>>  +
>>  +	mode = asus->throttle_thermal_policy_mode;
>>  +	cpu = &asus->throttle_fan_curves[mode][FAN_CURVE_DEV_CPU];
>>  +	gpu = &asus->throttle_fan_curves[mode][FAN_CURVE_DEV_GPU];
>>  +
>>  +	if (cpu->enabled) {
>>  +		err = fan_curve_write(asus, ASUS_WMI_DEVID_CPU_FAN_CURVE, cpu);
>>  +		if (err)
>>  +			return err;
>>  +	}
>>  +
>>  +	if (gpu->enabled) {
>>  +		err = fan_curve_write(asus, ASUS_WMI_DEVID_GPU_FAN_CURVE, gpu);
>>  +		if (err)
>>  +			return err;
>>  +	}
>>  +
>>  +	return 0;
>>  +}
>>  [...]
>>  +static ssize_t fan_curve_store(struct device *dev,
>>  +				struct device_attribute *attr,
>>  +				const char *buf, size_t count)
>>  +{
>>  +	struct fan_curve_data *data = fan_curve_attr_data_select(dev, 
>> attr);
>>  +	u8 value, old_value;
>>  +	int err;
>>  +
>>  +	int index = to_sensor_dev_attr_2(attr)->index;
>>  +	int nr = to_sensor_dev_attr_2(attr)->nr;
>>  +	int pwm = nr & FAN_CURVE_PWM_MASK;
>>  +
>>  +	err = kstrtou8(buf, 10, &value);
>>  +	if (err < 0)
>>  +		return err;
>>  +
>>  +	if (pwm) {
>>  +		old_value = data->percents[index];
>>  +		data->percents[index] = 100 * value / 255;
>>  +	} else {
>>  +		old_value = data->temps[index];
>>  +		data->temps[index] = value;
>>  +	}
>>  +	/*
>>  +	 * The check here forces writing a curve graph in reverse,
>>  +	 * from highest to lowest.
>>  +	 */
>>  +	err = fan_curve_verify(data);
>>  +	if (err) {
>>  +		if (pwm) {
>>  +			dev_err(dev, "a fan curve percentage was higher than the next 
>> in sequence\n");
>>  +			data->percents[index] = old_value;
>>  +		} else {
>>  +			dev_err(dev, "a fan curve temperature was higher than the next 
>> in sequence\n");
>>  +			data->temps[index] = old_value;
>>  +		}
>>  +		return err;
>>  +	}
> 
> Are such sequences rejected by the firmware itself?
> Or is this just an extra layer of protection?
> 
> 
>>  +
>>  +	return count;
>>  +}
>>  +
>>  +static ssize_t fan_curve_enable_show(struct device *dev,
>>  +				struct device_attribute *attr, char *buf)
>>  +{
>>  +	struct fan_curve_data *data = fan_curve_attr_data_select(dev, 
>> attr);
>>  +
>>  +	return scnprintf(buf, PAGE_SIZE, "%d\n", data->enabled);
> 
> sysfs_emit()
> 
> 
>>  +}
>>  +
>>  +static ssize_t fan_curve_enable_store(struct device *dev,
>>  +				struct device_attribute *attr,
>>  +				const char *buf, size_t count)
>>  +{
>>  +	struct fan_curve_data *data = fan_curve_attr_data_select(dev, 
>> attr);
>>  +	struct asus_wmi *asus = dev_get_drvdata(dev);
>>  +	bool value;
>>  +	int err;
>>  +
>>  +	err = kstrtobool(buf, &value);
>>  +	if (err < 0)
>>  +		return err;
>>  +
>>  +	data->enabled = value;
>>  +	throttle_thermal_policy_write(asus);
>>  +
>>  +	return count;
>>  +}
>>  +
>>  +/* CPU */
>>  +// TODO: enable
>>  +static SENSOR_DEVICE_ATTR_RW(pwm1_enable, fan_curve_enable,
>>  +				FAN_CURVE_DEV_CPU);
> 
> FYI, the pwmX_enable attributes can be created by the hwmon
> subsystem itself if you use [devm_]hwmon_device_register_with_info()
> with appropriately populated `struct hwmon_chip_info`.
> 
> 
>>  [...]
>>  +static const struct attribute_group fan_curve_attribute_group = {
>>  +	.is_visible = fan_curve_sysfs_is_visible,
>>  +	.attrs = fan_curve_attributes
> 
> Small thing, but it is customary to put commas after non-terminating
> entries in initializers / enum definitions.
> 
> 
>>  +};
>>  +__ATTRIBUTE_GROUPS(fan_curve_attribute);
>>  +
>>  +static int asus_wmi_fan_curve_init(struct asus_wmi *asus)
>>  +{
>>  +	struct device *dev = &asus->platform_device->dev;
>>  +	struct device *hwmon;
>>  +
>>  +	hwmon = devm_hwmon_device_register_with_groups(dev, "asus", asus,
>>  +						fan_curve_attribute_groups);
>>  +
>>  +	if (IS_ERR(hwmon)) {
>>  +		pr_err("Could not register asus fan_curve device\n");
> 
> I think `dev_err()` would be better.
> 
> 
>>  +		return PTR_ERR(hwmon);
>>  +	}
>>  +
>>  +	return 0;
>>  +}
>>  [...]
>>  diff --git a/include/linux/platform_data/x86/asus-wmi.h 
>> b/include/linux/platform_data/x86/asus-wmi.h
>>  index 17dc5cb6f3f2..a571b47ff362 100644
>>  --- a/include/linux/platform_data/x86/asus-wmi.h
>>  +++ b/include/linux/platform_data/x86/asus-wmi.h
>>  @@ -77,6 +77,8 @@
>>   #define ASUS_WMI_DEVID_THERMAL_CTRL	0x00110011
>>   #define ASUS_WMI_DEVID_FAN_CTRL		0x00110012 /* deprecated */
>>   #define ASUS_WMI_DEVID_CPU_FAN_CTRL	0x00110013
>>  +#define ASUS_WMI_DEVID_CPU_FAN_CURVE	0x00110024
>>  +#define ASUS_WMI_DEVID_GPU_FAN_CURVE	0x00110025
>> 
>>   /* Power */
>>   #define ASUS_WMI_DEVID_PROCESSOR_STATE	0x00120012
>>  --
>>  2.31.1
> 
> 
> Best regards,
> Barnabás Pőcze


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v8-0001-asus-wmi-Add-support-for-custom-fan-curves.patch --]
[-- Type: text/x-patch, Size: 16921 bytes --]

From 728573afa7eda049a57626cc37ec68733035fdef Mon Sep 17 00:00:00 2001
From: "Luke D. Jones" <luke@ljones.dev>
Date: Sun, 29 Aug 2021 13:21:23 +1200
Subject: [PATCH v8 1/1] asus-wmi: Add support for custom fan curves

Add support for custom fan curves found on some ASUS ROG laptops.

These laptops have the ability to set a custom curve for the CPU
and GPU fans via an ACPI method call. This patch enables this,
additionally enabling custom fan curves per-profile, where profile
here means each of the 3 levels of "throttle_thermal_policy".

This patch adds two blocks of attributes to the hwmon sysfs,
1 block each for CPU and GPU fans.

When the user switches profiles the associated curve data for that
profile is then show/store enabled to allow users to rotate through
the profiles and set a fan curve for each profile which then
activates on profile switch if enabled.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index cc5811844012..f83f153d60ba 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -106,8 +106,16 @@ module_param(fnlock_default, bool, 0444);
 
 #define WMI_EVENT_MASK			0xFFFF
 
+/* The number of ASUS_THROTTLE_THERMAL_POLICY_* available */
+#define FAN_CURVE_PROFILE_NUM	(ASUS_THROTTLE_THERMAL_POLICY_SILENT + 1)
+#define FAN_CURVE_POINTS		8
+#define FAN_CURVE_DEV_CPU		0x00
+#define FAN_CURVE_DEV_GPU		0x01
+
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
+static int throttle_thermal_policy_write(struct asus_wmi *);
+
 static bool ashs_present(void)
 {
 	int i = 0;
@@ -122,7 +130,8 @@ struct bios_args {
 	u32 arg0;
 	u32 arg1;
 	u32 arg2; /* At least TUF Gaming series uses 3 dword input buffer. */
-	u32 arg4;
+	u32 arg3;
+	u32 arg4; /* Some ROG laptops require a full 5 input args */
 	u32 arg5;
 } __packed;
 
@@ -173,6 +182,17 @@ enum fan_type {
 	FAN_TYPE_SPEC83,	/* starting in Spec 8.3, use CPU_FAN_CTRL */
 };
 
+/*
+ * The related ACPI method for testing availability also returns the factory
+ * default fan curves. We save them here so that a user can reset custom
+ * settings if required.
+ */
+struct fan_curve_data {
+	u8 enabled;
+	u8 temps[FAN_CURVE_POINTS];
+	u8 percents[FAN_CURVE_POINTS];
+};
+
 struct asus_wmi {
 	int dsts_id;
 	int spec;
@@ -220,6 +240,11 @@ struct asus_wmi {
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
+	bool cpu_fan_curve_available;
+	bool gpu_fan_curve_available;
+	/* [throttle modes][fan count] */
+	struct fan_curve_data throttle_fan_curves[FAN_CURVE_PROFILE_NUM][2];
+
 	struct platform_profile_handler platform_profile_handler;
 	bool platform_profile_support;
 
@@ -285,6 +310,105 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval)
 }
 EXPORT_SYMBOL_GPL(asus_wmi_evaluate_method);
 
+static int asus_wmi_evaluate_method5(u32 method_id,
+		u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4, u32 *retval)
+{
+	struct bios_args args = {
+		.arg0 = arg0,
+		.arg1 = arg1,
+		.arg2 = arg2,
+		.arg3 = arg3,
+		.arg4 = arg4,
+	};
+	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_status status;
+	union acpi_object *obj;
+	u32 tmp = 0;
+
+	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id,
+				     &input, &output);
+
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	obj = (union acpi_object *)output.pointer;
+	if (obj && obj->type == ACPI_TYPE_INTEGER)
+		tmp = (u32) obj->integer.value;
+
+	if (retval)
+		*retval = tmp;
+
+	kfree(obj);
+
+	if (tmp == ASUS_WMI_UNSUPPORTED_METHOD)
+		return -ENODEV;
+
+	return 0;
+}
+
+/*
+ * Returns as an error if the method output is not a buffer. Typically this
+ * means that the method called is unsupported.
+ */
+static int asus_wmi_evaluate_method_buf(u32 method_id,
+		u32 arg0, u32 arg1, u8 *ret_buffer, size_t size)
+{
+	struct bios_args args = {
+		.arg0 = arg0,
+		.arg1 = arg1,
+		.arg2 = 0,
+	};
+	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_status status;
+	union acpi_object *obj;
+	u32 int_tmp = 0;
+	int err = 0;
+
+	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id,
+				     &input, &output);
+
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	obj = (union acpi_object *)output.pointer;
+
+	if (obj && obj->type == ACPI_TYPE_BUFFER) {
+		if (obj->buffer.length > size)
+			err = -ENOSPC;
+		if (obj->buffer.length == 0)
+			err = -ENODATA;
+		if (err) {
+			kfree(obj);
+			return err;
+		}
+		memcpy(ret_buffer, obj->buffer.pointer, obj->buffer.length);
+	}
+
+	if (obj && obj->type == ACPI_TYPE_INTEGER) {
+		int_tmp = (u32)obj->integer.value;
+
+		kfree(obj);
+		if (int_tmp == ASUS_WMI_UNSUPPORTED_METHOD)
+			return -ENODEV;
+		/*
+		 * At least one method returns a 0 with no buffer if no arg
+		 * is provided, such as ASUS_WMI_DEVID_CPU_FAN_CURVE
+		 */
+		if (int_tmp == 0)
+			return -ENODATA;
+		return int_tmp;
+	}
+
+	kfree(obj);
+
+	if (obj == NULL)
+		return -ENODATA;
+
+	return 0;
+}
+
 static int asus_wmi_evaluate_method_agfn(const struct acpi_buffer args)
 {
 	struct acpi_buffer input;
@@ -2043,6 +2167,357 @@ 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);
 
+/* Custom fan curves per-profile **********************************************/
+
+static void init_fan_curve(struct fan_curve_data *data, u8 *buf)
+{
+	int i;
+
+	for (i = 0; i < FAN_CURVE_POINTS; i++)
+		data->temps[i] = buf[i];
+
+	for (i = 0; i < FAN_CURVE_POINTS; i++)
+		data->percents[i] = buf[i + 8];
+}
+
+/*
+ * Check if the ability to set fan curves on either fan exists, and populate
+ * with system defaults to provide users with a starting point.
+ *
+ * "dev" is either CPU_FAN_CURVE or GPU_FAN_CURVE.
+ */
+static int custom_fan_check_present(struct asus_wmi *asus,
+				    bool *available, u32 dev)
+{
+	struct fan_curve_data *curves;
+	u8 buf[FAN_CURVE_POINTS * 2];
+	int fan_idx = 0;
+	int err;
+
+	*available = false;
+	if (dev == ASUS_WMI_DEVID_GPU_FAN_CURVE)
+		fan_idx = 1;
+
+	/* Balanced default */
+	curves = &asus->throttle_fan_curves
+		[ASUS_THROTTLE_THERMAL_POLICY_DEFAULT][fan_idx];
+	err = asus_wmi_evaluate_method_buf(asus->dsts_id, dev, 0, buf,
+					FAN_CURVE_POINTS * 2);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+	init_fan_curve(curves, buf);
+
+	/*
+	 * Quiet default. The index num for ACPI method does not match the
+	 * throttle_thermal number, same for Performance.
+	 */
+	curves = &asus->throttle_fan_curves
+		[ASUS_THROTTLE_THERMAL_POLICY_SILENT][fan_idx];
+	err = asus_wmi_evaluate_method_buf(asus->dsts_id, dev, 1, buf,
+					FAN_CURVE_POINTS * 2);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+	init_fan_curve(curves, buf);
+
+	/* Performance default */
+	curves = &asus->throttle_fan_curves
+		[ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST][fan_idx];
+	err = asus_wmi_evaluate_method_buf(asus->dsts_id, dev, 2, buf,
+					FAN_CURVE_POINTS * 2);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+	init_fan_curve(curves, buf);
+
+	*available = true;
+	return 0;
+}
+
+/*
+ * "dev" is the related WMI method such as ASUS_WMI_DEVID_CPU_FAN_CURVE.
+ */
+static int fan_curve_write_arg(struct asus_wmi *asus, u32 dev,
+				struct fan_curve_data *data)
+{
+	u32 arg1 = 0, arg2 = 0, arg3 = 0, arg4 = 0;
+	int ret, i, shift = 0;
+
+	for (i = 0; i < FAN_CURVE_POINTS / 2; i++) {
+		arg1 += data->temps[i] << shift;
+		arg2 += data->temps[i + 4] << shift;
+		arg3 += data->percents[0] << shift;
+		arg4 += data->percents[i + 4] << shift;
+		shift += 8;
+	}
+
+	return asus_wmi_evaluate_method5(ASUS_WMI_METHODID_DEVS, dev,
+					arg1, arg2, arg3, arg4, &ret);
+}
+
+/*
+ * Called only by throttle_thermal_policy_write()
+ */
+static int fan_curve_write_data(struct asus_wmi *asus)
+{
+	struct fan_curve_data *cpu;
+	struct fan_curve_data *gpu;
+	int err, mode;
+
+	mode = asus->throttle_thermal_policy_mode;
+	cpu = &asus->throttle_fan_curves[mode][FAN_CURVE_DEV_CPU];
+	gpu = &asus->throttle_fan_curves[mode][FAN_CURVE_DEV_GPU];
+
+	if (cpu->enabled) {
+		err = fan_curve_write_arg(asus, ASUS_WMI_DEVID_CPU_FAN_CURVE, cpu);
+		if (err)
+			return err;
+	}
+
+	if (gpu->enabled) {
+		err = fan_curve_write_arg(asus, ASUS_WMI_DEVID_GPU_FAN_CURVE, gpu);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int fan_curve_verify(struct device *dev,
+				enum hwmon_sensor_types type, u32 fan_dev)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+	u8 mode = asus->throttle_thermal_policy_mode;
+	struct fan_curve_data *data;
+	u8 tmp = 0, prev_tmp = 0;
+	int i;
+
+	switch (type) {
+	case hwmon_temp:
+		data = &asus->throttle_fan_curves[mode][fan_dev];
+		break;
+	case hwmon_fan:
+		data = &asus->throttle_fan_curves[mode][fan_dev];
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	for (i = 0; i < FAN_CURVE_POINTS; i++) {
+		tmp = data->temps[i];
+		if (tmp < prev_tmp)
+			return -EINVAL;
+		prev_tmp = tmp;
+	}
+
+	tmp = prev_tmp = 0;
+	for (i = 0; i < FAN_CURVE_POINTS; i++) {
+		tmp = data->percents[i];
+		if (tmp < prev_tmp)
+			return -EINVAL;
+		prev_tmp = tmp;
+	}
+
+	return 0;
+}
+
+/* ret is a pointer to the requested profile->fan->point */
+static int fan_curve_point_select(struct device *dev,
+		enum hwmon_sensor_types type, int point, u32 fan_dev, u8 **ret)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+	u8 mode = asus->throttle_thermal_policy_mode;
+
+	switch (type) {
+	case hwmon_temp:
+		*ret = &asus->throttle_fan_curves[mode][fan_dev].temps[point];
+		break;
+	case hwmon_fan:
+		*ret = &asus->throttle_fan_curves[mode][fan_dev].percents[point];
+		break;
+	case hwmon_in:
+		*ret = &asus->throttle_fan_curves[mode][fan_dev].enabled;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static int asus_fan_curve_read(struct device *dev, enum hwmon_sensor_types type,
+					int point, long *val, u32 fan_dev)
+{
+	int err;
+	u8 *ret;
+
+	err = fan_curve_point_select(dev, type, point, fan_dev, &ret);
+	if (err)
+		return err;
+	*val = *ret;
+
+	return 0;
+}
+
+static int asus_cpu_fan_curve_read(struct device *dev,
+		enum hwmon_sensor_types type, u32 attr, int point, long *val)
+{
+	return asus_fan_curve_read(dev, type, point, val, FAN_CURVE_DEV_CPU);
+}
+
+static int asus_gpu_fan_curve_read(struct device *dev,
+		enum hwmon_sensor_types type, u32 attr, int point, long *val)
+{
+	return asus_fan_curve_read(dev, type, point, val, FAN_CURVE_DEV_GPU);
+}
+
+
+static int asus_fan_curve_write(struct device *dev,
+				enum hwmon_sensor_types type,
+				u32 attr, int point, long val, u32 fan_dev)
+{
+	u8 value, old_value, *ret;
+	int err;
+
+	err = fan_curve_point_select(dev, type, point, fan_dev, &ret);
+	if (err)
+		return err;
+
+	value = *ret;
+	old_value = value;
+	if (type == hwmon_temp)
+		value = 100 * value / 255;
+
+	/* The check here forces writing a curve graph in reverse for safety */
+	err = fan_curve_verify(dev, type, fan_dev);
+	if (err) {
+		switch (type) {
+		case hwmon_temp:
+			dev_err(dev, "a fan curve temperature was higher than the next in sequence\n");
+			*ret = old_value;
+			break;
+		case hwmon_fan:
+			dev_err(dev, "a fan curve percentage was higher than the next in sequence\n");
+			*ret = old_value;
+			break;
+		default:
+			break;
+		}
+		return err;
+	}
+
+	return 0;
+}
+
+static int asus_cpu_fan_curve_write(struct device *dev,
+		enum hwmon_sensor_types type, u32 attr, int point, long val)
+{
+	return asus_fan_curve_write(dev, type, attr, point, val, FAN_CURVE_DEV_CPU);
+}
+
+static int asus_gpu_fan_curve_write(struct device *dev,
+		enum hwmon_sensor_types type, u32 attr, int point, long val)
+{
+	return asus_fan_curve_write(dev, type, attr, point, val, FAN_CURVE_DEV_GPU);
+}
+
+static umode_t asus_cpu_fan_curve_is_visible(const void *data,
+		enum hwmon_sensor_types type, u32 attr, int point)
+{
+	const struct asus_wmi *asus = data;
+
+	if (asus->cpu_fan_curve_available)
+		return 0644;
+	return 0;
+}
+
+static umode_t asus_gpu_fan_curve_is_visible(const void *data,
+			enum hwmon_sensor_types type, u32 attr, int point)
+{
+	const struct asus_wmi *asus = data;
+
+	if (asus->gpu_fan_curve_available)
+		return 0644;
+	return 0;
+}
+
+static const struct hwmon_ops asus_cpu_fan_curve_ops = {
+	.is_visible = asus_cpu_fan_curve_is_visible,
+	.read = asus_cpu_fan_curve_read,
+	.write = asus_cpu_fan_curve_write,
+};
+
+static const struct hwmon_ops asus_gpu_fan_curve_ops = {
+	.is_visible = asus_gpu_fan_curve_is_visible,
+	.read = asus_gpu_fan_curve_read,
+	.write = asus_gpu_fan_curve_write,
+};
+
+/* Channel number determines CPU or GPU, 0-1 = CPU */
+static const struct hwmon_channel_info *asus_fan_curve_info[] = {
+	HWMON_CHANNEL_INFO(temp,
+		HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT,
+		HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT),
+	HWMON_CHANNEL_INFO(fan,
+		HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT,
+		HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT),
+	HWMON_CHANNEL_INFO(in, HWMON_I_ENABLE),
+	NULL
+};
+
+static const struct hwmon_chip_info asus_cpu_fan_curve_chip_info = {
+	.ops = &asus_cpu_fan_curve_ops,
+	.info = asus_fan_curve_info,
+};
+
+static const struct hwmon_chip_info asus_gpu_fan_curve_chip_info = {
+	.ops = &asus_gpu_fan_curve_ops,
+	.info = asus_fan_curve_info,
+};
+
+static int asus_wmi_fan_curve_init(struct asus_wmi *asus)
+{
+	struct device *dev = &asus->platform_device->dev;
+	struct device *hwmon;
+	int err;
+
+	err = custom_fan_check_present(asus,
+		&asus->cpu_fan_curve_available, ASUS_WMI_DEVID_CPU_FAN_CURVE);
+	if (err)
+		return err;
+
+	err = custom_fan_check_present(asus,
+		&asus->gpu_fan_curve_available, ASUS_WMI_DEVID_GPU_FAN_CURVE);
+	if (err)
+		return err;
+
+	hwmon = hwmon_device_register_with_info(dev,
+					"asus_cpu_fan_custom_curve", asus,
+					&asus_cpu_fan_curve_chip_info, NULL);
+
+	if (IS_ERR(hwmon)) {
+		dev_err(dev, "Could not register asus_cpu_fan_curve device\n");
+		return PTR_ERR(hwmon);
+	}
+
+	hwmon = hwmon_device_register_with_info(dev,
+					"asus_gpu_fan_custom_curve", asus,
+					&asus_gpu_fan_curve_chip_info, NULL);
+
+	if (IS_ERR(hwmon)) {
+		dev_err(dev, "Could not register asus_cpu_fan_curve device\n");
+		return PTR_ERR(hwmon);
+	}
+
+	return 0;
+}
+
 /* Throttle thermal policy ****************************************************/
 
 static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
@@ -2053,8 +2528,8 @@ static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
 	asus->throttle_thermal_policy_available = false;
 
 	err = asus_wmi_get_devstate(asus,
-				    ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
-				    &result);
+		ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
+		&result);
 	if (err) {
 		if (err == -ENODEV)
 			return 0;
@@ -2092,6 +2567,12 @@ static int throttle_thermal_policy_write(struct asus_wmi *asus)
 		return -EIO;
 	}
 
+	if (asus->cpu_fan_curve_available || asus->gpu_fan_curve_available) {
+		err = fan_curve_write_data(asus);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -2904,7 +3385,7 @@ static int show_call(struct seq_file *m, void *data)
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
-	obj = (union acpi_object *)output.pointer;
+	obj = output.pointer;
 	if (obj && obj->type == ACPI_TYPE_INTEGER)
 		seq_printf(m, "%#x(%#x, %#x) = %#x\n", asus->debug.method_id,
 			   asus->debug.dev_id, asus->debug.ctrl_param,
@@ -3038,6 +3519,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_hwmon;
 
+	err = asus_wmi_fan_curve_init(asus);
+	if (err)
+		goto fail_custom_fan_curve;
+
 	err = asus_wmi_led_init(asus);
 	if (err)
 		goto fail_leds;
@@ -3109,6 +3594,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 	asus_wmi_sysfs_exit(asus->platform_device);
 fail_sysfs:
 fail_throttle_thermal_policy:
+fail_custom_fan_curve:
 fail_platform_profile_setup:
 	if (asus->platform_profile_support)
 		platform_profile_remove();
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 17dc5cb6f3f2..a571b47ff362 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -77,6 +77,8 @@
 #define ASUS_WMI_DEVID_THERMAL_CTRL	0x00110011
 #define ASUS_WMI_DEVID_FAN_CTRL		0x00110012 /* deprecated */
 #define ASUS_WMI_DEVID_CPU_FAN_CTRL	0x00110013
+#define ASUS_WMI_DEVID_CPU_FAN_CURVE	0x00110024
+#define ASUS_WMI_DEVID_GPU_FAN_CURVE	0x00110025
 
 /* Power */
 #define ASUS_WMI_DEVID_PROCESSOR_STATE	0x00120012
-- 
2.31.1