* [PATCH v11 0/1] asus-wmi: Add support for custom fan curves @ 2021-09-07 23:22 Luke D. Jones 2021-09-07 23:22 ` [PATCH v11] " Luke D. Jones 0 siblings, 1 reply; 15+ messages in thread From: Luke D. Jones @ 2021-09-07 23:22 UTC (permalink / raw) To: linux-kernel Cc: hdegoede, pobrn, hadess, linux, platform-driver-x86, Luke D. Jones Current version is v11, this is to be considered the final patch *unless* there is something horribly wrong. The patch has been quite heavily tested now and all the edge-cases found (with the resources available). Add support for custom fan curves found on some ASUS ROG laptops. - V1 + Initial patch work - V2 + Don't fail and remove wmi driver if error from asus_wmi_evaluate_method_buf() if error is -ENODEV - V3 + Store the "default" fan curves + Call throttle_thermal_policy_write() if a curve is erased to ensure that the factory default for a profile is applied again - V4 + Do not apply default curves by default. Testers have found that the default curves don't quite match actual no-curve behaviours + Add method to enable/disable curves for each profile - V5 + Remove an unrequired function left over from previous iterations + Ensure default curves are applied if user writes " " to a curve path + Rename "active_fan_curve_profiles" to "enabled_fan_curve_profiles" to better reflect the behavious of this setting + Move throttle_thermal_policy_write_*pu_curves() and rename to fan_curve_*pu_write() + Merge fan_curve_check_valid() and fan_curve_write() + Remove some leftover debug statements - V6 + Refactor data structs to store array or u8 instead of strings. This affects the entire patch except the enabled_fan_curves block + Use sysfs_match_string in enabled_fan_curve block + Add some extra comments to describe things + Allow some variation in how fan curve input can be formatted + Use SENSOR_DEVICE_ATTR_2_RW() to reduce the amount of lines per fan+profile combo drastically - V7 + Further refactor to use pwm1_auto_point1_temp + pwm1_auto_point1_pwm format, creating two blocks of attributes for CPU and GPU fans + Remove storing of defualt curves and method to reset them. The factory defaults are still populated in to structs on module load so users have a starting point - V8 + Make asus_wmi_evaluate_method_buf() safe + Take in to account machines that do not have throttle_thermal_policy but do have a single custom fan curve. These machines can't use a throttle_thermal mode change to reset the fans to factory default if fan curve is disabled so we need to write their stored default back. In some cases this is also needed due to mistakes in ASUS ACPI tables. + Formatting tidy and dev_err() use + Extra comments to make certain things (such as above) more clear + Give generated hwmon a more descriptive `name asus_custom_fan_curve` -V9 + Cleanup and remove per-profile setting + Call `asus_fan_set_auto()` if method supported to ensure fan state is reset on these models + Add extra case (3) to related `pwm<N>_enable`s for fan curves to reset the used curve to factory default + Related to the above is that if throttle_thermal_policy is supported then the fetched factory default curve is correct for the current throttle_thermal_policy_mode + Ensure that if throttle_thermal_policy_mode is changed then fan_curve is set to disabled. + Ensure the same for pwm1_enable_store() - V10 - Better handling of conditions in asus_wmi_evaluate_method_buf() - Correct a mistaken conversion to percentage for temperature - Remove unused function - Formating corrections - Update or remove various comments - Update commit message to better reflect purpose of patch -V11 - Remove fan_curve_verify() as this prevented easily adjusting a fan curve and there is no good way to give user feedback on fan-curve validity unless checked in userspace Luke D. Jones (1): asus-wmi: Add support for custom fan curves drivers/platform/x86/asus-wmi.c | 650 ++++++++++++++++++++- include/linux/platform_data/x86/asus-wmi.h | 2 + 2 files changed, 644 insertions(+), 8 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-07 23:22 [PATCH v11 0/1] asus-wmi: Add support for custom fan curves Luke D. Jones @ 2021-09-07 23:22 ` Luke D. Jones 2021-09-08 12:32 ` kernel test robot ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Luke D. Jones @ 2021-09-07 23:22 UTC (permalink / raw) To: linux-kernel Cc: hdegoede, pobrn, hadess, linux, platform-driver-x86, Luke D. Jones 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 two ACPI methods. This patch adds two pwm<N> attributes to the hwmon sysfs, pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the name `asus_custom_fan_curve`. There is no safety check of the set fan curves - this must be done in userspace. The fans have settings [1,2,3] under pwm<N>_enable: 1. Enable and write settings out 2. Disable and use factory fan mode 3. Same as 2, additionally restoring default factory curve. Use of 2 means that the curve the user has set is still stored and won't be erased, but the laptop will be using its default auto-fan mode. Re-enabling the manual mode then activates the curves again. Notes: - pwm<N>_enable = 0 is an invalid setting. - pwm is actually a percentage and is scaled on writing to device. Signed-off-by: Luke D. Jones <luke@ljones.dev> --- drivers/platform/x86/asus-wmi.c | 609 ++++++++++++++++++++- include/linux/platform_data/x86/asus-wmi.h | 2 + 2 files changed, 603 insertions(+), 8 deletions(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index cc5811844012..15c01fad30a1 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -106,8 +106,17 @@ module_param(fnlock_default, bool, 0444); #define WMI_EVENT_MASK 0xFFFF +#define FAN_CURVE_POINTS 8 +#define FAN_CURVE_BUF_LEN (FAN_CURVE_POINTS * 2) +#define FAN_CURVE_DEV_CPU 0x00 +#define FAN_CURVE_DEV_GPU 0x01 +/* Mask to determine if setting temperature or percentage */ +#define FAN_CURVE_PWM_MASK 0x04 + 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 +131,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 +183,19 @@ 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 { + bool enabled; + u8 temps[FAN_CURVE_POINTS]; + u8 percents[FAN_CURVE_POINTS]; + u8 default_temps[FAN_CURVE_POINTS]; + u8 default_percents[FAN_CURVE_POINTS]; +}; + struct asus_wmi { int dsts_id; int spec; @@ -220,6 +243,10 @@ struct asus_wmi { bool throttle_thermal_policy_available; u8 throttle_thermal_policy_mode; + bool cpu_fan_curve_available; + bool gpu_fan_curve_available; + struct fan_curve_data custom_fan_curves[2]; + struct platform_profile_handler platform_profile_handler; bool platform_profile_support; @@ -285,6 +312,103 @@ 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; + 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; + + switch (obj->type) { + case ACPI_TYPE_BUFFER: + if (obj->buffer.length > size) + err = -ENOSPC; + if (obj->buffer.length == 0) + err = -ENODATA; + + memcpy(ret_buffer, obj->buffer.pointer, obj->buffer.length); + break; + case ACPI_TYPE_INTEGER: + err = (u32)obj->integer.value; + + if (err == ASUS_WMI_UNSUPPORTED_METHOD) + err = -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 (err == 0) + err = -ENODATA; + break; + default: + err = -ENODATA; + break; + } + + kfree(obj); + + if (err) + return err; + + return 0; +} + static int asus_wmi_evaluate_method_agfn(const struct acpi_buffer args) { struct acpi_buffer input; @@ -1806,6 +1930,13 @@ static ssize_t pwm1_enable_store(struct device *dev, } asus->fan_pwm_mode = state; + + /* Must set to disabled if mode is toggled */ + if (asus->cpu_fan_curve_available) + asus->custom_fan_curves[FAN_CURVE_DEV_CPU].enabled = false; + if (asus->gpu_fan_curve_available) + asus->custom_fan_curves[FAN_CURVE_DEV_GPU].enabled = false; + return count; } @@ -1953,9 +2084,9 @@ static int fan_boost_mode_check_present(struct asus_wmi *asus) static int fan_boost_mode_write(struct asus_wmi *asus) { - int err; - u8 value; u32 retval; + u8 value; + int err; value = asus->fan_boost_mode; @@ -2013,10 +2144,10 @@ static ssize_t fan_boost_mode_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); u8 mask = asus->fan_boost_mode_mask; + u8 new_mode; + int result; result = kstrtou8(buf, 10, &new_mode); if (result < 0) { @@ -2043,6 +2174,456 @@ 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 fan_curve_copy_from_buf(struct fan_curve_data *data, u8 *buf) +{ + int i; + + for (i = 0; i < FAN_CURVE_POINTS; i++) { + data->temps[i] = buf[i]; + data->default_temps[i] = buf[i]; + } + + for (i = 0; i < FAN_CURVE_POINTS; i++) { + data->percents[i] = + 255 * buf[i + FAN_CURVE_POINTS] / 100; + data->default_percents[i] = + 255 * buf[i + FAN_CURVE_POINTS] / 100; + } +} + +static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev) +{ + struct fan_curve_data *curves; + u8 buf[FAN_CURVE_BUF_LEN]; + int fan_idx = 0; + u8 mode = 0; + int err; + + if (asus->throttle_thermal_policy_available) + mode = asus->throttle_thermal_policy_mode; + /* DEVID_<C/G>PU_FAN_CURVE is switched for OVERBOOST vs SILENT */ + if (mode == 2) + mode = 1; + if (mode == 1) + mode = 2; + + if (fan_dev == ASUS_WMI_DEVID_GPU_FAN_CURVE) + fan_idx = FAN_CURVE_DEV_GPU; + + curves = &asus->custom_fan_curves[fan_idx]; + err = asus_wmi_evaluate_method_buf(asus->dsts_id, fan_dev, mode, buf, + FAN_CURVE_BUF_LEN); + if (err) + return err; + + fan_curve_copy_from_buf(curves, buf); + + return 0; +} + +/* + * Check if capability exists, and populate defaults. + */ +static int fan_curve_check_present(struct asus_wmi *asus, bool *available, + u32 fan_dev) +{ + int err; + + *available = false; + + err = fan_curve_get_factory_default(asus, fan_dev); + if (err) { + if (err == -ENODEV) + return 0; + return err; + } + + *available = true; + return 0; +} + +static struct fan_curve_data *fan_curve_data_select(struct asus_wmi *asus, + struct device_attribute *attr) +{ + /* Determine which fan the attribute is for */ + int nr = to_sensor_dev_attr_2(attr)->nr; + int fan = nr & FAN_CURVE_DEV_GPU; + + return &asus->custom_fan_curves[fan]; +} + +static ssize_t fan_curve_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct asus_wmi *asus = dev_get_drvdata(dev); + /* Determine if temperature or pwm */ + int nr = to_sensor_dev_attr_2(attr)->nr; + struct fan_curve_data *data; + int value, index; + + data = fan_curve_data_select(asus, attr); + index = to_sensor_dev_attr_2(attr)->index; + + if (nr & FAN_CURVE_PWM_MASK) + value = data->percents[index]; + else + value = data->temps[index]; + + return sysfs_emit(buf, "%d\n", value); +} + +/* + * "fan_dev" is the related WMI method such as ASUS_WMI_DEVID_CPU_FAN_CURVE. + */ +static int fan_curve_write(struct asus_wmi *asus, + struct device_attribute *attr, u32 fan_dev) +{ + struct fan_curve_data *data = fan_curve_data_select(asus, attr); + u32 arg1 = 0, arg2 = 0, arg3 = 0, arg4 = 0; + u8 *percents = data->percents; + u8 *temps = data->temps; + int ret, i, shift = 0; + + for (i = 0; i < FAN_CURVE_POINTS / 2; i++) { + arg1 += (temps[i]) << shift; + arg2 += (temps[i + 4]) << shift; + /* Scale to percentage for device */ + arg3 += (100 * percents[i] / 255) << shift; + arg4 += (100 * percents[i + 4] / 255) << shift; + shift += 8; + } + + return asus_wmi_evaluate_method5(ASUS_WMI_METHODID_DEVS, fan_dev, arg1, + arg2, arg3, arg4, &ret); +} + +/* + * Called on curve enable/disable. This should be the only way to write out the + * fan curves. This avoids potential lockups on write to ACPI for every change. + */ +static int fan_curve_write_data(struct asus_wmi *asus, struct device_attribute *attr) +{ + int err; + + if (asus->cpu_fan_curve_available) { + err = fan_curve_write(asus, attr, ASUS_WMI_DEVID_CPU_FAN_CURVE); + if (err) + return err; + } + + if (asus->gpu_fan_curve_available) { + err = fan_curve_write(asus, attr, ASUS_WMI_DEVID_GPU_FAN_CURVE); + 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 asus_wmi *asus = dev_get_drvdata(dev); + struct fan_curve_data *data; + 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; + + data = fan_curve_data_select(asus, attr); + + err = kstrtou8(buf, 10, &value); + if (err < 0) + return err; + + if (pwm) { + old_value = data->percents[index]; + data->percents[index] = value; + } else { + old_value = data->temps[index]; + data->temps[index] = value; + } + + /* + * Mark as disabled so the user has to explicitly enable to apply a + * changed fan curve. This prevents potential lockups from writing out + * many changes as one-write-per-change. + */ + data->enabled = false; + + return count; +} + +static ssize_t fan_curve_enable_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct asus_wmi *asus = dev_get_drvdata(dev); + struct fan_curve_data *data = fan_curve_data_select(asus, attr); + + return sysfs_emit(buf, "%d\n", data->enabled); +} + +static int fan_curve_set_default(struct asus_wmi *asus) +{ + int err; + + err = fan_curve_get_factory_default( + asus, ASUS_WMI_DEVID_CPU_FAN_CURVE); + if (err) + return err; + + err = fan_curve_get_factory_default( + asus, ASUS_WMI_DEVID_GPU_FAN_CURVE); + if (err) + return err; + return 0; +} + +static ssize_t fan_curve_enable_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct asus_wmi *asus = dev_get_drvdata(dev); + struct fan_curve_data *data; + int value; + int err; + + data = fan_curve_data_select(asus, attr); + + err = kstrtoint(buf, 10, &value); + if (err < 0) + return err; + + switch (value) { + case 1: + data->enabled = true; + break; + case 2: + data->enabled = false; + break; + /* + * Auto + reset the fan curve data to defaults. Make it an explicit + * option so that users don't accidentally overwrite a set fan curve. + */ + case 3: + err = fan_curve_set_default(asus); + if (err) + return err; + data->enabled = false; + break; + default: + return -EINVAL; + }; + + /* + * For machines with throttle this is the only way to reset fans to + * default mode of operation (does not erase curve data). + */ + if (asus->throttle_thermal_policy_available && !data->enabled) { + err = throttle_thermal_policy_write(asus); + if (err) + return err; + } + /* Similar is true for laptops with this fan */ + if (asus->fan_type == FAN_TYPE_SPEC83) { + err = asus_fan_set_auto(asus); + if (err) + return err; + } + /* + * Machines without either need to write their defaults back always. + * This is more of a safeguard against ASUS faulty ACPI tables. + */ + if (!asus->throttle_thermal_policy_available + && asus->fan_type != FAN_TYPE_SPEC83 && !data->enabled) { + err = fan_curve_set_default(asus); + if (err) + return err; + err = fan_curve_write_data(asus, attr); + if (err) + return err; + } + + if (data->enabled) { + err = fan_curve_write_data(asus, attr); + if (err) + return err; + } + + return count; +} + +/* CPU */ +static SENSOR_DEVICE_ATTR_RW(pwm1_enable, fan_curve_enable, FAN_CURVE_DEV_CPU); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point1_temp, fan_curve, + FAN_CURVE_DEV_CPU, 0); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point2_temp, fan_curve, + FAN_CURVE_DEV_CPU, 1); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point3_temp, fan_curve, + FAN_CURVE_DEV_CPU, 2); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point4_temp, fan_curve, + FAN_CURVE_DEV_CPU, 3); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point5_temp, fan_curve, + FAN_CURVE_DEV_CPU, 4); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point6_temp, fan_curve, + FAN_CURVE_DEV_CPU, 5); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point7_temp, fan_curve, + FAN_CURVE_DEV_CPU, 6); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point8_temp, fan_curve, + FAN_CURVE_DEV_CPU, 7); + +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point1_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 0); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point2_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 1); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point3_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 2); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point4_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 3); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point5_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 4); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point6_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 5); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point7_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 6); +static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point8_pwm, fan_curve, + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 7); + +/* GPU */ +static SENSOR_DEVICE_ATTR_RW(pwm2_enable, fan_curve_enable, FAN_CURVE_DEV_GPU); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point1_temp, fan_curve, + FAN_CURVE_DEV_GPU, 0); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point2_temp, fan_curve, + FAN_CURVE_DEV_GPU, 1); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point3_temp, fan_curve, + FAN_CURVE_DEV_GPU, 2); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point4_temp, fan_curve, + FAN_CURVE_DEV_GPU, 3); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point5_temp, fan_curve, + FAN_CURVE_DEV_GPU, 4); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point6_temp, fan_curve, + FAN_CURVE_DEV_GPU, 5); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point7_temp, fan_curve, + FAN_CURVE_DEV_GPU, 6); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point8_temp, fan_curve, + FAN_CURVE_DEV_GPU, 7); + +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point1_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 0); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point2_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 1); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point3_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 2); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point4_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 3); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point5_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 4); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point6_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 5); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point7_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 6); +static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point8_pwm, fan_curve, + FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 7); + +static struct attribute *asus_fan_curve_attr[] = { + /* CPU */ + &sensor_dev_attr_pwm1_enable.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point1_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point2_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point3_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point4_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point5_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point6_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point7_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point8_temp.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point2_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point3_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point4_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point5_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point6_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point7_pwm.dev_attr.attr, + &sensor_dev_attr_pwm1_auto_point8_pwm.dev_attr.attr, + /* GPU */ + &sensor_dev_attr_pwm2_enable.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point1_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point2_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point3_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point4_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point5_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point6_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point7_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point8_temp.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point2_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point3_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point4_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point5_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point6_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point7_pwm.dev_attr.attr, + &sensor_dev_attr_pwm2_auto_point8_pwm.dev_attr.attr, + NULL +}; + +static umode_t asus_fan_curve_is_visible(struct kobject *kobj, + struct attribute *attr, int idx) +{ + struct device *dev = container_of(kobj, struct device, kobj); + struct asus_wmi *asus = dev_get_drvdata(dev->parent); + + if (asus->cpu_fan_curve_available) + return 0644; + + if (asus->gpu_fan_curve_available) + return 0644; + + return 0; +} + +static const struct attribute_group asus_fan_curve_attr_group = { + .is_visible = asus_fan_curve_is_visible, + .attrs = asus_fan_curve_attr, +}; +__ATTRIBUTE_GROUPS(asus_fan_curve_attr); + +/* + * Must be initialised after throttle_thermal_policy_check_present() as + * we check the status of throttle_thermal_policy_available during init. + */ +static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus) +{ + struct device *dev = &asus->platform_device->dev; + struct device *hwmon; + int err; + + err = fan_curve_check_present(asus, &asus->cpu_fan_curve_available, + ASUS_WMI_DEVID_CPU_FAN_CURVE); + if (err) + return err; + + err = fan_curve_check_present(asus, &asus->gpu_fan_curve_available, + ASUS_WMI_DEVID_GPU_FAN_CURVE); + if (err) + return err; + + hwmon = devm_hwmon_device_register_with_groups( + dev, "asus_custom_fan_curve", asus, asus_fan_curve_attr_groups); + + if (IS_ERR(hwmon)) { + dev_err(dev, + "Could not register asus_custom_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 +2634,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 +2673,12 @@ static int throttle_thermal_policy_write(struct asus_wmi *asus) return -EIO; } + /* Must set to disabled if mode is toggled */ + if (asus->cpu_fan_curve_available) + asus->custom_fan_curves[FAN_CURVE_DEV_CPU].enabled = false; + if (asus->gpu_fan_curve_available) + asus->custom_fan_curves[FAN_CURVE_DEV_GPU].enabled = false; + return 0; } @@ -2904,7 +3491,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 +3625,10 @@ static int asus_wmi_add(struct platform_device *pdev) if (err) goto fail_hwmon; + err = asus_wmi_custom_fan_curve_init(asus); + if (err) + goto fail_custom_fan_curve; + err = asus_wmi_led_init(asus); if (err) goto fail_leds; @@ -3109,6 +3700,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(); @@ -3134,6 +3726,7 @@ static int asus_wmi_remove(struct platform_device *device) asus_wmi_debugfs_exit(asus); asus_wmi_sysfs_exit(asus->platform_device); asus_fan_set_auto(asus); + throttle_thermal_policy_set_default(asus); asus_wmi_battery_exit(asus); if (asus->platform_profile_support) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-07 23:22 ` [PATCH v11] " Luke D. Jones @ 2021-09-08 12:32 ` kernel test robot 2021-09-08 17:49 ` kernel test robot 2021-09-28 11:36 ` Bastien Nocera 2 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2021-09-08 12:32 UTC (permalink / raw) To: Luke D. Jones, linux-kernel Cc: kbuild-all, hdegoede, pobrn, hadess, linux, platform-driver-x86, Luke D. Jones [-- Attachment #1: Type: text/plain, Size: 2879 bytes --] Hi "Luke, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on next-20210908] [cannot apply to linux/master v5.14] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Luke-D-Jones/asus-wmi-Add-support-for-custom-fan-curves/20210908-072520 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 626bf91a292e2035af5b9d9cce35c5c138dfe06d config: x86_64-rhel-8.3-kselftests (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/3f17887090cfe852531995edb96ad6a3580a6a55 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Luke-D-Jones/asus-wmi-Add-support-for-custom-fan-curves/20210908-072520 git checkout 3f17887090cfe852531995edb96ad6a3580a6a55 # save the attached .config to linux build tree make W=1 ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/platform/x86/asus-wmi.c: In function 'fan_curve_store': >> drivers/platform/x86/asus-wmi.c:2332:12: warning: variable 'old_value' set but not used [-Wunused-but-set-variable] 2332 | u8 value, old_value; | ^~~~~~~~~ vim +/old_value +2332 drivers/platform/x86/asus-wmi.c 2325 2326 static ssize_t fan_curve_store(struct device *dev, 2327 struct device_attribute *attr, const char *buf, 2328 size_t count) 2329 { 2330 struct asus_wmi *asus = dev_get_drvdata(dev); 2331 struct fan_curve_data *data; > 2332 u8 value, old_value; 2333 int err; 2334 2335 int index = to_sensor_dev_attr_2(attr)->index; 2336 int nr = to_sensor_dev_attr_2(attr)->nr; 2337 int pwm = nr & FAN_CURVE_PWM_MASK; 2338 2339 data = fan_curve_data_select(asus, attr); 2340 2341 err = kstrtou8(buf, 10, &value); 2342 if (err < 0) 2343 return err; 2344 2345 if (pwm) { 2346 old_value = data->percents[index]; 2347 data->percents[index] = value; 2348 } else { 2349 old_value = data->temps[index]; 2350 data->temps[index] = value; 2351 } 2352 2353 /* 2354 * Mark as disabled so the user has to explicitly enable to apply a 2355 * changed fan curve. This prevents potential lockups from writing out 2356 * many changes as one-write-per-change. 2357 */ 2358 data->enabled = false; 2359 2360 return count; 2361 } 2362 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 42021 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-07 23:22 ` [PATCH v11] " Luke D. Jones 2021-09-08 12:32 ` kernel test robot @ 2021-09-08 17:49 ` kernel test robot 2021-09-28 11:36 ` Bastien Nocera 2 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2021-09-08 17:49 UTC (permalink / raw) To: Luke D. Jones, linux-kernel Cc: kbuild-all, hdegoede, pobrn, hadess, linux, platform-driver-x86, Luke D. Jones [-- Attachment #1: Type: text/plain, Size: 2907 bytes --] Hi "Luke, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on next-20210908] [cannot apply to linux/master v5.14] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Luke-D-Jones/asus-wmi-Add-support-for-custom-fan-curves/20210908-072520 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 626bf91a292e2035af5b9d9cce35c5c138dfe06d config: i386-allyesconfig (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/3f17887090cfe852531995edb96ad6a3580a6a55 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Luke-D-Jones/asus-wmi-Add-support-for-custom-fan-curves/20210908-072520 git checkout 3f17887090cfe852531995edb96ad6a3580a6a55 # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/platform/x86/asus-wmi.c: In function 'fan_curve_store': >> drivers/platform/x86/asus-wmi.c:2332:12: error: variable 'old_value' set but not used [-Werror=unused-but-set-variable] 2332 | u8 value, old_value; | ^~~~~~~~~ cc1: all warnings being treated as errors vim +/old_value +2332 drivers/platform/x86/asus-wmi.c 2325 2326 static ssize_t fan_curve_store(struct device *dev, 2327 struct device_attribute *attr, const char *buf, 2328 size_t count) 2329 { 2330 struct asus_wmi *asus = dev_get_drvdata(dev); 2331 struct fan_curve_data *data; > 2332 u8 value, old_value; 2333 int err; 2334 2335 int index = to_sensor_dev_attr_2(attr)->index; 2336 int nr = to_sensor_dev_attr_2(attr)->nr; 2337 int pwm = nr & FAN_CURVE_PWM_MASK; 2338 2339 data = fan_curve_data_select(asus, attr); 2340 2341 err = kstrtou8(buf, 10, &value); 2342 if (err < 0) 2343 return err; 2344 2345 if (pwm) { 2346 old_value = data->percents[index]; 2347 data->percents[index] = value; 2348 } else { 2349 old_value = data->temps[index]; 2350 data->temps[index] = value; 2351 } 2352 2353 /* 2354 * Mark as disabled so the user has to explicitly enable to apply a 2355 * changed fan curve. This prevents potential lockups from writing out 2356 * many changes as one-write-per-change. 2357 */ 2358 data->enabled = false; 2359 2360 return count; 2361 } 2362 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 65422 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-07 23:22 ` [PATCH v11] " Luke D. Jones 2021-09-08 12:32 ` kernel test robot 2021-09-08 17:49 ` kernel test robot @ 2021-09-28 11:36 ` Bastien Nocera 2021-09-28 11:43 ` Luke Jones 2021-09-28 11:44 ` Hans de Goede 2 siblings, 2 replies; 15+ messages in thread From: Bastien Nocera @ 2021-09-28 11:36 UTC (permalink / raw) To: Luke D. Jones, linux-kernel; +Cc: hdegoede, pobrn, linux, platform-driver-x86 On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote: > 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 two ACPI methods. > > This patch adds two pwm<N> attributes to the hwmon sysfs, > pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the > name `asus_custom_fan_curve`. There is no safety check of the set > fan curves - this must be done in userspace. > > The fans have settings [1,2,3] under pwm<N>_enable: > 1. Enable and write settings out > 2. Disable and use factory fan mode > 3. Same as 2, additionally restoring default factory curve. > > Use of 2 means that the curve the user has set is still stored and > won't be erased, but the laptop will be using its default auto-fan > mode. Re-enabling the manual mode then activates the curves again. > > Notes: > - pwm<N>_enable = 0 is an invalid setting. > - pwm is actually a percentage and is scaled on writing to device. I was trying to update: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 but I don't understand what files I need to check for what values to detect whether custom fan curves were used. Can you help me out here? Also, was this patch accepted in the pdx86 tree? Cheers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:36 ` Bastien Nocera @ 2021-09-28 11:43 ` Luke Jones 2021-09-28 11:56 ` Hans de Goede 2021-09-28 11:44 ` Hans de Goede 1 sibling, 1 reply; 15+ messages in thread From: Luke Jones @ 2021-09-28 11:43 UTC (permalink / raw) To: Bastien Nocera; +Cc: linux-kernel, hdegoede, pobrn, linux, platform-driver-x86 Sure, the path is similar to /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where hwmon4 will likely be different depending on init, and pwm2_enable is the second fan if it exists. The values are 1,2,3 - where 1 = fan curve enabled/manual, 2 = auto. 3 here is custom extra that writes default curve back then defaults to 2. As I understand it, this should be adhering to the accepted kernel standard, so if you use this for ASUS laptops, then it should carry over to other brands that implement it also. Hope this is helpful, Luke. On Tue, Sep 28 2021 at 13:36:57 +0200, Bastien Nocera <hadess@hadess.net> wrote: > On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote: >> 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 two ACPI methods. >> >> This patch adds two pwm<N> attributes to the hwmon sysfs, >> pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the >> name `asus_custom_fan_curve`. There is no safety check of the set >> fan curves - this must be done in userspace. >> >> The fans have settings [1,2,3] under pwm<N>_enable: >> 1. Enable and write settings out >> 2. Disable and use factory fan mode >> 3. Same as 2, additionally restoring default factory curve. >> >> Use of 2 means that the curve the user has set is still stored and >> won't be erased, but the laptop will be using its default auto-fan >> mode. Re-enabling the manual mode then activates the curves again. >> >> Notes: >> - pwm<N>_enable = 0 is an invalid setting. >> - pwm is actually a percentage and is scaled on writing to device. > > I was trying to update: > https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 > but I don't understand what files I need to check for what values to > detect whether custom fan curves were used. > > Can you help me out here? > > Also, was this patch accepted in the pdx86 tree? > > Cheers > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:43 ` Luke Jones @ 2021-09-28 11:56 ` Hans de Goede 2021-09-28 11:59 ` Luke Jones 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-09-28 11:56 UTC (permalink / raw) To: Luke Jones, Bastien Nocera Cc: linux-kernel, pobrn, linux, platform-driver-x86 Hi, On 9/28/21 1:43 PM, Luke Jones wrote: > Sure, the path is similar to /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where hwmon4 will likely be different depending on init, and pwm2_enable is the second fan if it exists. The values are 1,2,3 - where 1 = fan curve enabled/manual, 2 = auto. 3 here is custom extra that writes default curve back then defaults to 2. > > As I understand it, this should be adhering to the accepted kernel standard, so if you use this for ASUS laptops, then it should carry over to other brands that implement it also. Ah, so this is a bit different then how I thought this would work (this is probably better though). <snip> >>> The fans have settings [1,2,3] under pwm<N>_enable: >>> 1. Enable and write settings out >>> 2. Disable and use factory fan mode >>> 3. Same as 2, additionally restoring default factory curve. Quoting Documentation/hwmon/sysfs-interface.rst `pwm[1-*]_enable` Fan speed control method: - 0: no fan speed control (i.e. fan at full speed) - 1: manual fan speed control enabled (using `pwm[1-*]`) - 2+: automatic fan speed control enabled 1 normally means the fans runs at a fixed speed, but you are using it for the custom/manual profile, which is still a temp<->pwm table, right? I guess this make sense since full manual control is not supported and this keeps "2" aka auto as being the normal factory auto setting which is good. Bastien is this interface usable for p-p-d ? I guess that it is a bit annoying that you need to figure out the # in the hwmon# part of the path, but there will be only one hwmon child. You could also go through /sys/class/hwmon but then you really have no idea which one to use. Ideally we would have some way to indicate that there is a hmwon class-dev associated with /sys/firmware/acpi/platform_profile but as we mentioned before we should defer coming up with a generic solution for this until we have more then 1 user, so that we hopefully get the generic solution right in one go. Regards, Hans >>> >>> Use of 2 means that the curve the user has set is still stored and >>> won't be erased, but the laptop will be using its default auto-fan >>> mode. Re-enabling the manual mode then activates the curves again. >>> >>> Notes: >>> - pwm<N>_enable = 0 is an invalid setting. >>> - pwm is actually a percentage and is scaled on writing to device. >> >> I was trying to update: >> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 >> but I don't understand what files I need to check for what values to >> detect whether custom fan curves were used. >> >> Can you help me out here? >> >> Also, was this patch accepted in the pdx86 tree? >> >> Cheers >> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:56 ` Hans de Goede @ 2021-09-28 11:59 ` Luke Jones 2021-09-28 12:03 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Luke Jones @ 2021-09-28 11:59 UTC (permalink / raw) To: Hans de Goede Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 On Tue, Sep 28 2021 at 13:56:05 +0200, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 9/28/21 1:43 PM, Luke Jones wrote: >> Sure, the path is similar to >> /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where >> hwmon4 will likely be different depending on init, and pwm2_enable >> is the second fan if it exists. The values are 1,2,3 - where 1 = fan >> curve enabled/manual, 2 = auto. 3 here is custom extra that writes >> default curve back then defaults to 2. >> >> As I understand it, this should be adhering to the accepted kernel >> standard, so if you use this for ASUS laptops, then it should carry >> over to other brands that implement it also. > > Ah, so this is a bit different then how I thought this would work > (this is probably better though). > > <snip> > >>>> The fans have settings [1,2,3] under pwm<N>_enable: >>>> 1. Enable and write settings out >>>> 2. Disable and use factory fan mode >>>> 3. Same as 2, additionally restoring default factory curve. > > Quoting Documentation/hwmon/sysfs-interface.rst > > `pwm[1-*]_enable` > Fan speed control method: > > - 0: no fan speed control (i.e. fan at full speed) > - 1: manual fan speed control enabled (using > `pwm[1-*]`) > - 2+: automatic fan speed control enabled > > 1 normally means the fans runs at a fixed speed, but you are using it > for the custom/manual profile, which is still a temp<->pwm table, > right? > > I guess this make sense since full manual control is not supported > and this keeps "2" aka auto as being the normal factory auto > setting which is good. > > Bastien is this interface usable for p-p-d ? > > I guess that it is a bit annoying that you need to figure out > the # in the hwmon# part of the path, but there will be only > one hwmon child. > > You could also go through /sys/class/hwmon but then you really > have no idea which one to use. Ideally we would have some way > to indicate that there is a hmwon class-dev associated with > /sys/firmware/acpi/platform_profile but as we mentioned before > we should defer coming up with a generic solution for this > until we have more then 1 user, so that we hopefully get the > generic solution right in one go. If it's at all helpful, I named the interface as "asus_custom_fan_curve". I use this to verify I have the correct hwmon for asusctl. Open to suggestions on that. > > Regards, > > Hans > > > > > >>>> >>>> Use of 2 means that the curve the user has set is still stored >>>> and >>>> won't be erased, but the laptop will be using its default >>>> auto-fan >>>> mode. Re-enabling the manual mode then activates the curves >>>> again. >>>> >>>> Notes: >>>> - pwm<N>_enable = 0 is an invalid setting. >>>> - pwm is actually a percentage and is scaled on writing to >>>> device. >>> >>> I was trying to update: >>> >>> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 >>> but I don't understand what files I need to check for what values >>> to >>> detect whether custom fan curves were used. >>> >>> Can you help me out here? >>> >>> Also, was this patch accepted in the pdx86 tree? >>> >>> Cheers >>> >> >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:59 ` Luke Jones @ 2021-09-28 12:03 ` Hans de Goede 2021-09-28 12:15 ` Luke Jones 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-09-28 12:03 UTC (permalink / raw) To: Luke Jones Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 Hi, On 9/28/21 1:59 PM, Luke Jones wrote: > > > On Tue, Sep 28 2021 at 13:56:05 +0200, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi, >> >> On 9/28/21 1:43 PM, Luke Jones wrote: >>> Sure, the path is similar to /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where hwmon4 will likely be different depending on init, and pwm2_enable is the second fan if it exists. The values are 1,2,3 - where 1 = fan curve enabled/manual, 2 = auto. 3 here is custom extra that writes default curve back then defaults to 2. >>> >>> As I understand it, this should be adhering to the accepted kernel standard, so if you use this for ASUS laptops, then it should carry over to other brands that implement it also. >> >> Ah, so this is a bit different then how I thought this would work >> (this is probably better though). >> >> <snip> >> >>>>> The fans have settings [1,2,3] under pwm<N>_enable: >>>>> 1. Enable and write settings out >>>>> 2. Disable and use factory fan mode >>>>> 3. Same as 2, additionally restoring default factory curve. >> >> Quoting Documentation/hwmon/sysfs-interface.rst >> >> `pwm[1-*]_enable` >> Fan speed control method: >> >> - 0: no fan speed control (i.e. fan at full speed) >> - 1: manual fan speed control enabled (using `pwm[1-*]`) >> - 2+: automatic fan speed control enabled >> >> 1 normally means the fans runs at a fixed speed, but you are using it >> for the custom/manual profile, which is still a temp<->pwm table, >> right? >> >> I guess this make sense since full manual control is not supported >> and this keeps "2" aka auto as being the normal factory auto >> setting which is good. >> >> Bastien is this interface usable for p-p-d ? >> >> I guess that it is a bit annoying that you need to figure out >> the # in the hwmon# part of the path, but there will be only >> one hwmon child. >> >> You could also go through /sys/class/hwmon but then you really >> have no idea which one to use. Ideally we would have some way >> to indicate that there is a hmwon class-dev associated with >> /sys/firmware/acpi/platform_profile but as we mentioned before >> we should defer coming up with a generic solution for this >> until we have more then 1 user, so that we hopefully get the >> generic solution right in one go. > > If it's at all helpful, I named the interface as "asus_custom_fan_curve". I use this to verify I have the correct hwmon for asusctl. Open to suggestions on that. Ah yes, that means the interface could be looked up through /sys/class/hwmon too, that is probably the safest route to go then, as the /sys/devices/platform/asus-nb-wmi/ path might change if we e.g. ever convert the asus-wmi code to use the new wmi bus interface. Now that you have confirmed that any writes to /sys/firmware/acpi/platform_profile override any custom profiles I wonder if p-p-d needs to take this into account at all though. The easiest way to deal with this in p-p-d, is just to not deal with it at all... If that turns out to be a bad idea, we can always reconsider and add some special handling to p-p-d for this later. Regards, Hans ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 12:03 ` Hans de Goede @ 2021-09-28 12:15 ` Luke Jones 2021-09-28 14:11 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Luke Jones @ 2021-09-28 12:15 UTC (permalink / raw) To: Hans de Goede Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 On Tue, Sep 28 2021 at 14:03:54 +0200, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 9/28/21 1:59 PM, Luke Jones wrote: >> >> >> On Tue, Sep 28 2021 at 13:56:05 +0200, Hans de Goede >> <hdegoede@redhat.com> wrote: >>> Hi, >>> >>> On 9/28/21 1:43 PM, Luke Jones wrote: >>>> Sure, the path is similar to >>>> /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where >>>> hwmon4 will likely be different depending on init, and pwm2_enable >>>> is the second fan if it exists. The values are 1,2,3 - where 1 = >>>> fan curve enabled/manual, 2 = auto. 3 here is custom extra that >>>> writes default curve back then defaults to 2. >>>> >>>> As I understand it, this should be adhering to the accepted >>>> kernel standard, so if you use this for ASUS laptops, then it >>>> should carry over to other brands that implement it also. >>> >>> Ah, so this is a bit different then how I thought this would work >>> (this is probably better though). >>> >>> <snip> >>> >>>>>> The fans have settings [1,2,3] under pwm<N>_enable: >>>>>> 1. Enable and write settings out >>>>>> 2. Disable and use factory fan mode >>>>>> 3. Same as 2, additionally restoring default factory curve. >>> >>> Quoting Documentation/hwmon/sysfs-interface.rst >>> >>> `pwm[1-*]_enable` >>> Fan speed control method: >>> >>> - 0: no fan speed control (i.e. fan at full speed) >>> - 1: manual fan speed control enabled (using >>> `pwm[1-*]`) >>> - 2+: automatic fan speed control enabled >>> >>> 1 normally means the fans runs at a fixed speed, but you are using >>> it >>> for the custom/manual profile, which is still a temp<->pwm table, >>> right? >>> >>> I guess this make sense since full manual control is not supported >>> and this keeps "2" aka auto as being the normal factory auto >>> setting which is good. >>> >>> Bastien is this interface usable for p-p-d ? >>> >>> I guess that it is a bit annoying that you need to figure out >>> the # in the hwmon# part of the path, but there will be only >>> one hwmon child. >>> >>> You could also go through /sys/class/hwmon but then you really >>> have no idea which one to use. Ideally we would have some way >>> to indicate that there is a hmwon class-dev associated with >>> /sys/firmware/acpi/platform_profile but as we mentioned before >>> we should defer coming up with a generic solution for this >>> until we have more then 1 user, so that we hopefully get the >>> generic solution right in one go. >> >> If it's at all helpful, I named the interface as >> "asus_custom_fan_curve". I use this to verify I have the correct >> hwmon for asusctl. Open to suggestions on that. > > Ah yes, that means the interface could be looked up through > /sys/class/hwmon > too, that is probably the safest route to go then, as the > /sys/devices/platform/asus-nb-wmi/ path might change if we e.g. ever > convert the asus-wmi code to use the new wmi bus interface. Oh man... can you link me to relevant bits? I'll stick this on my to-do for future. There will be more patches from me over time so this might be good to have done? > > Now that you have confirmed that any writes to > /sys/firmware/acpi/platform_profile override any custom profiles > I wonder if p-p-d needs to take this into account at all though. > > The easiest way to deal with this in p-p-d, is just to not deal > with it at all... If that turns out to be a bad idea, we > can always reconsider and add some special handling to p-p-d for > this later. I believe Bastiens concern here will be that manual control can still be enabled and ppd won't be aware of it without a check. For example the user may switch profile then re-enable the fan-curve. If some problem arises due to manual control of fan then there is no way for ppd to determine if manual was enabled without actually looking. This does mean the pwm name here is set in stone. But also means it's special-cased I guess. Perhaps a check for pwm<N>_enable, then check for the pairs of pwm<N>_auto_<xX> that come with it? Ciao, Luke. > > Regards, > > Hans > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 12:15 ` Luke Jones @ 2021-09-28 14:11 ` Hans de Goede 0 siblings, 0 replies; 15+ messages in thread From: Hans de Goede @ 2021-09-28 14:11 UTC (permalink / raw) To: Luke Jones Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 Hi, On 9/28/21 2:15 PM, Luke Jones wrote: > > > On Tue, Sep 28 2021 at 14:03:54 +0200, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi, >> >> On 9/28/21 1:59 PM, Luke Jones wrote: >>> >>> >>> On Tue, Sep 28 2021 at 13:56:05 +0200, Hans de Goede <hdegoede@redhat.com> wrote: >>>> Hi, >>>> >>>> On 9/28/21 1:43 PM, Luke Jones wrote: >>>>> Sure, the path is similar to /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where hwmon4 will likely be different depending on init, and pwm2_enable is the second fan if it exists. The values are 1,2,3 - where 1 = fan curve enabled/manual, 2 = auto. 3 here is custom extra that writes default curve back then defaults to 2. >>>>> >>>>> As I understand it, this should be adhering to the accepted kernel standard, so if you use this for ASUS laptops, then it should carry over to other brands that implement it also. >>>> >>>> Ah, so this is a bit different then how I thought this would work >>>> (this is probably better though). >>>> >>>> <snip> >>>> >>>>>>> The fans have settings [1,2,3] under pwm<N>_enable: >>>>>>> 1. Enable and write settings out >>>>>>> 2. Disable and use factory fan mode >>>>>>> 3. Same as 2, additionally restoring default factory curve. >>>> >>>> Quoting Documentation/hwmon/sysfs-interface.rst >>>> >>>> `pwm[1-*]_enable` >>>> Fan speed control method: >>>> >>>> - 0: no fan speed control (i.e. fan at full speed) >>>> - 1: manual fan speed control enabled (using `pwm[1-*]`) >>>> - 2+: automatic fan speed control enabled >>>> >>>> 1 normally means the fans runs at a fixed speed, but you are using it >>>> for the custom/manual profile, which is still a temp<->pwm table, >>>> right? >>>> >>>> I guess this make sense since full manual control is not supported >>>> and this keeps "2" aka auto as being the normal factory auto >>>> setting which is good. >>>> >>>> Bastien is this interface usable for p-p-d ? >>>> >>>> I guess that it is a bit annoying that you need to figure out >>>> the # in the hwmon# part of the path, but there will be only >>>> one hwmon child. >>>> >>>> You could also go through /sys/class/hwmon but then you really >>>> have no idea which one to use. Ideally we would have some way >>>> to indicate that there is a hmwon class-dev associated with >>>> /sys/firmware/acpi/platform_profile but as we mentioned before >>>> we should defer coming up with a generic solution for this >>>> until we have more then 1 user, so that we hopefully get the >>>> generic solution right in one go. >>> >>> If it's at all helpful, I named the interface as "asus_custom_fan_curve". I use this to verify I have the correct hwmon for asusctl. Open to suggestions on that. >> >> Ah yes, that means the interface could be looked up through /sys/class/hwmon >> too, that is probably the safest route to go then, as the >> /sys/devices/platform/asus-nb-wmi/ path might change if we e.g. ever >> convert the asus-wmi code to use the new wmi bus interface. > > Oh man... can you link me to relevant bits? I'll stick this on my to-do for future. There will be more patches from me over time so this might be good to have done? This is not something which we have made mandatory for old code to switch too. The idea is that you get one wmi_device per guid under: /sys/bus/wmi/devices/ And then the old platform_drivers (which do typically use "wmi:GUID" as modalias) are converted to wmi_drivers which bind to a wmi_device and can make calls on that using e.g. : wmidev_evaluate_method(wmi_dev, ...) instead of: wmi_evaluate_method(GUID, ...) Grep for module_wmi_driver in drivers/platform/x86 for drivers which have been converted to use this. I see that the asus code uses 3 GUIDs: ASUS_WMI_MGMT_GUID ASUS_NB_WMI_EVENT_GUID EEEPC_WMI_EVENT_GUID With the first one being shared and the modalias-es for the asus-nb-wmi resp eeepc-wmi drivers actually pointing to the 2 EVENT_GUIDs. So you could change things to have the 2 drivers bind to the 2 different event_guids, they can then also have a notify callback as part of their driver structure instead of having to call wmi_install_notify_handler() / wmi_remove_notify_handler() ### Actually, never mind, switching to the new way of doing things will move all the sysfs attributes from /sys/devices/platform/asus-nb-wmi/... to some thing like: /sys/devices/platform/PNP0C14:07/wmi_bus/wmi_bus-PNP0C14:07/D931B4CF-F54E-4D07-9420-42858CC6A234 So this would be a clear userspace API break :| IOW we are stuck with using the old way of doing things in asus-wmi. Regards, Hans > >> >> Now that you have confirmed that any writes to >> /sys/firmware/acpi/platform_profile override any custom profiles >> I wonder if p-p-d needs to take this into account at all though. >> >> The easiest way to deal with this in p-p-d, is just to not deal >> with it at all... If that turns out to be a bad idea, we >> can always reconsider and add some special handling to p-p-d for >> this later. > > I believe Bastiens concern here will be that manual control can still be enabled and ppd won't be aware of it without a check. For example the user may switch profile then re-enable the fan-curve. If some problem arises due to manual control of fan then there is no way for ppd to determine if manual was enabled without actually looking. > > This does mean the pwm name here is set in stone. But also means it's special-cased I guess. Perhaps a check for pwm<N>_enable, then check for the pairs of pwm<N>_auto_<xX> that come with it? > > Ciao, > Luke. > >> >> Regards, >> >> Hans >> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:36 ` Bastien Nocera 2021-09-28 11:43 ` Luke Jones @ 2021-09-28 11:44 ` Hans de Goede 2021-09-28 11:56 ` Luke Jones 1 sibling, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-09-28 11:44 UTC (permalink / raw) To: Bastien Nocera, Luke D. Jones, linux-kernel Cc: pobrn, linux, platform-driver-x86 Hi, On 9/28/21 1:36 PM, Bastien Nocera wrote: > On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote: >> 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 two ACPI methods. >> >> This patch adds two pwm<N> attributes to the hwmon sysfs, >> pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the >> name `asus_custom_fan_curve`. There is no safety check of the set >> fan curves - this must be done in userspace. >> >> The fans have settings [1,2,3] under pwm<N>_enable: >> 1. Enable and write settings out >> 2. Disable and use factory fan mode >> 3. Same as 2, additionally restoring default factory curve. >> >> Use of 2 means that the curve the user has set is still stored and >> won't be erased, but the laptop will be using its default auto-fan >> mode. Re-enabling the manual mode then activates the curves again. >> >> Notes: >> - pwm<N>_enable = 0 is an invalid setting. >> - pwm is actually a percentage and is scaled on writing to device. > > I was trying to update: > https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 > but I don't understand what files I need to check for what values to > detect whether custom fan curves were used. > > Can you help me out here? How to deal with this is actually one of my remaining questions too. I've not looked at the new code closely yet, but if I understand things correctly, the now code basically only allows to set 1 custom profile and setting that profile overrides the last profile set through /sys/firmware/acpi/platform_profile. And any write to /sys/firmware/acpi/platform_profile will overwrite / replace the last custom set profile (if any) with the one matching the requested platform-profile. So basically users of custom fan profiles are expected to disable power-profiles-daemon or at least to refrain from making any platform_profile changes. And if power-profile-daemon is actually active and makes a change then any custom settings will be thrown away, IOW p-p-d will always win. So I believe that it no longer needs to check for custom profiles, since any time it requests a standard profile that will overwrite any custom profile which may be present. Luke, do I have that right ? > Also, was this patch accepted in the pdx86 tree? No, I still need to find/make some time to review it and I still have the same question as you :) Regards, Hans ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:44 ` Hans de Goede @ 2021-09-28 11:56 ` Luke Jones 2021-09-28 11:59 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Luke Jones @ 2021-09-28 11:56 UTC (permalink / raw) To: Hans de Goede Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 Hmm, A change via /sys/firmware/acpi/platform_profile does disable the fan-curve until a user re-enables it. It doesn't wipe the actual curve setting though, I thought that would be bad UX, but yes the curve is definitely disabled on profile change and will remain disabled until turned on again. At which point another profile change will disable it again. And as stated in previous reply use of /sys/devices/platform/asus-nb-wmi/hwmon/hwmon<N>/pwm<N>_enable to check the status is stabilised (1 = manual fan). Looking at it with fresh eyes I just spotted some things I can clean up further. Very sorry, there'll be a v15 :( Many thanks, Luke. On Tue, Sep 28 2021 at 13:44:32 +0200, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 9/28/21 1:36 PM, Bastien Nocera wrote: >> On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote: >>> 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 two ACPI methods. >>> >>> This patch adds two pwm<N> attributes to the hwmon sysfs, >>> pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the >>> name `asus_custom_fan_curve`. There is no safety check of the set >>> fan curves - this must be done in userspace. >>> >>> The fans have settings [1,2,3] under pwm<N>_enable: >>> 1. Enable and write settings out >>> 2. Disable and use factory fan mode >>> 3. Same as 2, additionally restoring default factory curve. >>> >>> Use of 2 means that the curve the user has set is still stored and >>> won't be erased, but the laptop will be using its default auto-fan >>> mode. Re-enabling the manual mode then activates the curves again. >>> >>> Notes: >>> - pwm<N>_enable = 0 is an invalid setting. >>> - pwm is actually a percentage and is scaled on writing to device. >> >> I was trying to update: >> >> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 >> but I don't understand what files I need to check for what values to >> detect whether custom fan curves were used. >> >> Can you help me out here? > > How to deal with this is actually one of my remaining questions too. > > I've not looked at the new code closely yet, but if I understand > things correctly, the now code basically only allows to set 1 > custom profile and setting that profile overrides the last > profile set through /sys/firmware/acpi/platform_profile. > > And any write to /sys/firmware/acpi/platform_profile will > overwrite / replace the last custom set profile (if any) with > the one matching the requested platform-profile. > > So basically users of custom fan profiles are expected to > disable power-profiles-daemon or at least to refrain from > making any platform_profile changes. > > And if power-profile-daemon is actually active and > makes a change then any custom settings will be thrown away, > IOW p-p-d will always win. So I believe that it no longer needs > to check for custom profiles, since any time it requests a > standard profile that will overwrite any custom profile > which may be present. > > Luke, do I have that right ? > >> Also, was this patch accepted in the pdx86 tree? > > No, I still need to find/make some time to review it and > I still have the same question as you :) > > Regards, > > Hans > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:56 ` Luke Jones @ 2021-09-28 11:59 ` Hans de Goede 2021-09-28 12:01 ` Luke Jones 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-09-28 11:59 UTC (permalink / raw) To: Luke Jones Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 Hi, On 9/28/21 1:56 PM, Luke Jones wrote: > Hmm, > A change via /sys/firmware/acpi/platform_profile does disable the fan-curve until a user re-enables it. Ah ok, so did get that part right :) So basically any write to /sys/firmware/acpi/platform_profile will reset the pwm1_enable to "2" if it was not "2" already. > It doesn't wipe the actual curve setting though, I thought that would be bad UX, Ok that is fine. > but yes the curve is definitely disabled on profile change and will remain disabled until turned on again. At which point another profile change will disable it again. > > And as stated in previous reply use of /sys/devices/platform/asus-nb-wmi/hwmon/hwmon<N>/pwm<N>_enable to check the status is stabilised (1 = manual fan). > > Looking at it with fresh eyes I just spotted some things I can clean up further. Very sorry, there'll be a v15 :( No worries, maybe wait a bit with posting v15 till Bastien has a chance to way in on this discussion though. Regards, Hans > On Tue, Sep 28 2021 at 13:44:32 +0200, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi, >> >> On 9/28/21 1:36 PM, Bastien Nocera wrote: >>> On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote: >>>> 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 two ACPI methods. >>>> >>>> This patch adds two pwm<N> attributes to the hwmon sysfs, >>>> pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the >>>> name `asus_custom_fan_curve`. There is no safety check of the set >>>> fan curves - this must be done in userspace. >>>> >>>> The fans have settings [1,2,3] under pwm<N>_enable: >>>> 1. Enable and write settings out >>>> 2. Disable and use factory fan mode >>>> 3. Same as 2, additionally restoring default factory curve. >>>> >>>> Use of 2 means that the curve the user has set is still stored and >>>> won't be erased, but the laptop will be using its default auto-fan >>>> mode. Re-enabling the manual mode then activates the curves again. >>>> >>>> Notes: >>>> - pwm<N>_enable = 0 is an invalid setting. >>>> - pwm is actually a percentage and is scaled on writing to device. >>> >>> I was trying to update: >>> >>> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 >>> but I don't understand what files I need to check for what values to >>> detect whether custom fan curves were used. >>> >>> Can you help me out here? >> >> How to deal with this is actually one of my remaining questions too. >> >> I've not looked at the new code closely yet, but if I understand >> things correctly, the now code basically only allows to set 1 >> custom profile and setting that profile overrides the last >> profile set through /sys/firmware/acpi/platform_profile. >> >> And any write to /sys/firmware/acpi/platform_profile will >> overwrite / replace the last custom set profile (if any) with >> the one matching the requested platform-profile. >> >> So basically users of custom fan profiles are expected to >> disable power-profiles-daemon or at least to refrain from >> making any platform_profile changes. >> >> And if power-profile-daemon is actually active and >> makes a change then any custom settings will be thrown away, >> IOW p-p-d will always win. So I believe that it no longer needs >> to check for custom profiles, since any time it requests a >> standard profile that will overwrite any custom profile >> which may be present. >> >> Luke, do I have that right ? >> >>> Also, was this patch accepted in the pdx86 tree? >> >> No, I still need to find/make some time to review it and >> I still have the same question as you :) >> >> Regards, >> >> Hans >> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v11] asus-wmi: Add support for custom fan curves 2021-09-28 11:59 ` Hans de Goede @ 2021-09-28 12:01 ` Luke Jones 0 siblings, 0 replies; 15+ messages in thread From: Luke Jones @ 2021-09-28 12:01 UTC (permalink / raw) To: Hans de Goede Cc: Bastien Nocera, linux-kernel, pobrn, linux, platform-driver-x86 On Tue, Sep 28 2021 at 13:59:31 +0200, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 9/28/21 1:56 PM, Luke Jones wrote: >> Hmm, >> A change via /sys/firmware/acpi/platform_profile does disable the >> fan-curve until a user re-enables it. > > Ah ok, so did get that part right :) > > So basically any write to /sys/firmware/acpi/platform_profile > will reset the pwm1_enable to "2" if it was not "2" already. > >> It doesn't wipe the actual curve setting though, I thought that >> would be bad UX, > > Ok that is fine. > >> but yes the curve is definitely disabled on profile change and will >> remain disabled until turned on again. At which point another >> profile change will disable it again. >> >> And as stated in previous reply use of >> /sys/devices/platform/asus-nb-wmi/hwmon/hwmon<N>/pwm<N>_enable to >> check the status is stabilised (1 = manual fan). >> >> Looking at it with fresh eyes I just spotted some things I can >> clean up further. Very sorry, there'll be a v15 :( > > No worries, maybe wait a bit with posting v15 till Bastien has a > chance > to way in on this discussion though. No problem at all. Very little will change except for code clean up :) > > Regards, > > Hans > > > >> On Tue, Sep 28 2021 at 13:44:32 +0200, Hans de Goede >> <hdegoede@redhat.com> wrote: >>> Hi, >>> >>> On 9/28/21 1:36 PM, Bastien Nocera wrote: >>>> On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote: >>>>> 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 two ACPI methods. >>>>> >>>>> This patch adds two pwm<N> attributes to the hwmon sysfs, >>>>> pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of >>>>> the >>>>> name `asus_custom_fan_curve`. There is no safety check of the >>>>> set >>>>> fan curves - this must be done in userspace. >>>>> >>>>> The fans have settings [1,2,3] under pwm<N>_enable: >>>>> 1. Enable and write settings out >>>>> 2. Disable and use factory fan mode >>>>> 3. Same as 2, additionally restoring default factory curve. >>>>> >>>>> Use of 2 means that the curve the user has set is still stored >>>>> and >>>>> won't be erased, but the laptop will be using its default >>>>> auto-fan >>>>> mode. Re-enabling the manual mode then activates the curves >>>>> again. >>>>> >>>>> Notes: >>>>> - pwm<N>_enable = 0 is an invalid setting. >>>>> - pwm is actually a percentage and is scaled on writing to >>>>> device. >>>> >>>> I was trying to update: >>>> >>>> >>>> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80 >>>> but I don't understand what files I need to check for what >>>> values to >>>> detect whether custom fan curves were used. >>>> >>>> Can you help me out here? >>> >>> How to deal with this is actually one of my remaining questions >>> too. >>> >>> I've not looked at the new code closely yet, but if I understand >>> things correctly, the now code basically only allows to set 1 >>> custom profile and setting that profile overrides the last >>> profile set through /sys/firmware/acpi/platform_profile. >>> >>> And any write to /sys/firmware/acpi/platform_profile will >>> overwrite / replace the last custom set profile (if any) with >>> the one matching the requested platform-profile. >>> >>> So basically users of custom fan profiles are expected to >>> disable power-profiles-daemon or at least to refrain from >>> making any platform_profile changes. >>> >>> And if power-profile-daemon is actually active and >>> makes a change then any custom settings will be thrown away, >>> IOW p-p-d will always win. So I believe that it no longer needs >>> to check for custom profiles, since any time it requests a >>> standard profile that will overwrite any custom profile >>> which may be present. >>> >>> Luke, do I have that right ? >>> >>>> Also, was this patch accepted in the pdx86 tree? >>> >>> No, I still need to find/make some time to review it and >>> I still have the same question as you :) >>> >>> Regards, >>> >>> Hans >>> >> >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-09-28 14:11 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-07 23:22 [PATCH v11 0/1] asus-wmi: Add support for custom fan curves Luke D. Jones 2021-09-07 23:22 ` [PATCH v11] " Luke D. Jones 2021-09-08 12:32 ` kernel test robot 2021-09-08 17:49 ` kernel test robot 2021-09-28 11:36 ` Bastien Nocera 2021-09-28 11:43 ` Luke Jones 2021-09-28 11:56 ` Hans de Goede 2021-09-28 11:59 ` Luke Jones 2021-09-28 12:03 ` Hans de Goede 2021-09-28 12:15 ` Luke Jones 2021-09-28 14:11 ` Hans de Goede 2021-09-28 11:44 ` Hans de Goede 2021-09-28 11:56 ` Luke Jones 2021-09-28 11:59 ` Hans de Goede 2021-09-28 12:01 ` Luke Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).