Add support for vivobook fan profiles wmi call on the ASUS VIVOBOOK
to adjust power limits.
These fan profiles have a different device id than the ROG series
and different order. This reorders the existing modes and adds a new
full speed mode available on these laptops.
As part of keeping the patch clean the throttle_thermal_policy_available
boolean stored in the driver struct is removed and
throttle_thermal_policy_dev is used in place (as on init it is zeroed).
Changelog:
- V1
- fixes grammar in description
- reorders macros
- adds throttle_thermal_policy_max_mode() helper function
- V2
- removes unnecessary braces in throttle_thermal_policy_max_mode()
- V3
- fixes a use before declaration compilation error caused by reordering of local
variables in throttle_thermal_policy_store() function
drivers/platform/x86/asus-wmi.c | 93 ++++++++++++----------
include/linux/platform_data/x86/asus-wmi.h | 1 +
2 files changed, 51 insertions(+), 43 deletions(-)
--
2.44.0
Add support for vivobook fan profiles wmi call on the ASUS VIVOBOOK
to adjust power limits.
These fan profiles have a different device id than the ROG series
and different order. This reorders the existing modes and adds a new
full speed mode available on these laptops.
As part of keeping the patch clean the throttle_thermal_policy_available
boolean stored in the driver struct is removed and
throttle_thermal_policy_dev is used in place (as on init it is zeroed).
Signed-off-by: Mohamed Ghanmi <[email protected]>
Co-developed-by: Luke D. Jones <[email protected]>
Signed-off-by: Luke D. Jones <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
---
drivers/platform/x86/asus-wmi.c | 93 ++++++++++++----------
include/linux/platform_data/x86/asus-wmi.h | 1 +
2 files changed, 51 insertions(+), 43 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3c61d75a3..1f54596ca 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -97,6 +97,11 @@ module_param(fnlock_default, bool, 0444);
#define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST 1
#define ASUS_THROTTLE_THERMAL_POLICY_SILENT 2
+#define ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO 0
+#define ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO 1
+#define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO 2
+#define ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED 3
+
#define USB_INTEL_XUSB2PR 0xD0
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
@@ -293,8 +298,8 @@ struct asus_wmi {
u32 kbd_rgb_dev;
bool kbd_rgb_state_available;
- bool throttle_thermal_policy_available;
u8 throttle_thermal_policy_mode;
+ u32 throttle_thermal_policy_dev;
bool cpu_fan_curve_available;
bool gpu_fan_curve_available;
@@ -3152,7 +3157,7 @@ static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
int err, fan_idx;
u8 mode = 0;
- if (asus->throttle_thermal_policy_available)
+ if (asus->throttle_thermal_policy_dev)
mode = asus->throttle_thermal_policy_mode;
/* DEVID_<C/G>PU_FAN_CURVE is switched for OVERBOOST vs SILENT */
if (mode == 2)
@@ -3359,7 +3364,7 @@ static ssize_t fan_curve_enable_store(struct device *dev,
* 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) {
+ if (asus->throttle_thermal_policy_dev) {
err = throttle_thermal_policy_write(asus);
if (err)
return err;
@@ -3576,8 +3581,8 @@ static const struct attribute_group asus_fan_curve_attr_group = {
__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.
+ * Must be initialised after throttle_thermal_policy_dev is set as
+ * we check the status of throttle_thermal_policy_dev during init.
*/
static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
{
@@ -3618,38 +3623,37 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
}
/* Throttle thermal policy ****************************************************/
-
-static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
+static u8 throttle_thermal_policy_max_mode(struct asus_wmi *asus)
{
- u32 result;
- int err;
-
- asus->throttle_thermal_policy_available = false;
-
- err = asus_wmi_get_devstate(asus,
- ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
- &result);
- if (err) {
- if (err == -ENODEV)
- return 0;
- return err;
- }
-
- if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
- asus->throttle_thermal_policy_available = true;
-
- return 0;
+ if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO)
+ return ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED;
+ else
+ return ASUS_THROTTLE_THERMAL_POLICY_SILENT;
}
static int throttle_thermal_policy_write(struct asus_wmi *asus)
{
- int err;
- u8 value;
+ u8 value = asus->throttle_thermal_policy_mode;
u32 retval;
+ bool vivo;
+ int err;
- value = asus->throttle_thermal_policy_mode;
+ vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
+ if (vivo) {
+ switch (value) {
+ case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
+ value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
+ break;
+ case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
+ value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
+ break;
+ case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
+ value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
+ break;
+ }
+ }
- err = asus_wmi_set_devstate(ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
+ err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev,
value, &retval);
sysfs_notify(&asus->platform_device->dev.kobj, NULL,
@@ -3679,7 +3683,7 @@ static int throttle_thermal_policy_write(struct asus_wmi *asus)
static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
{
- if (!asus->throttle_thermal_policy_available)
+ if (!asus->throttle_thermal_policy_dev)
return 0;
asus->throttle_thermal_policy_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
@@ -3689,9 +3693,10 @@ static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
{
u8 new_mode = asus->throttle_thermal_policy_mode + 1;
+ u8 max_mode = throttle_thermal_policy_max_mode(asus);
int err;
- if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
+ if (new_mode > max_mode)
new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
asus->throttle_thermal_policy_mode = new_mode;
@@ -3722,6 +3727,7 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
const char *buf, size_t count)
{
struct asus_wmi *asus = dev_get_drvdata(dev);
+ u8 max_mode = throttle_thermal_policy_max_mode(asus);
u8 new_mode;
int result;
int err;
@@ -3730,7 +3736,7 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
if (result < 0)
return result;
- if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
+ if (new_mode > max_mode)
return -EINVAL;
asus->throttle_thermal_policy_mode = new_mode;
@@ -3747,7 +3753,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
return count;
}
-// Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
+/*
+ * Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
+ * Throttle thermal policy vivobook : 0 - default, 1 - silent, 2 - overboost, 3 - fullspeed
+ */
static DEVICE_ATTR_RW(throttle_thermal_policy);
/* Platform profile ***********************************************************/
@@ -3813,7 +3822,7 @@ static int platform_profile_setup(struct asus_wmi *asus)
* Not an error if a component platform_profile relies on is unavailable
* so early return, skipping the setup of platform_profile.
*/
- if (!asus->throttle_thermal_policy_available)
+ if (!asus->throttle_thermal_policy_dev)
return 0;
dev_info(dev, "Using throttle_thermal_policy for platform_profile support\n");
@@ -4228,7 +4237,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
if (code == NOTIFY_KBD_FBM || code == NOTIFY_KBD_TTP) {
if (asus->fan_boost_mode_available)
fan_boost_mode_switch_next(asus);
- if (asus->throttle_thermal_policy_available)
+ if (asus->throttle_thermal_policy_dev)
throttle_thermal_policy_switch_next(asus);
return;
@@ -4436,7 +4445,7 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
else if (attr == &dev_attr_fan_boost_mode.attr)
ok = asus->fan_boost_mode_available;
else if (attr == &dev_attr_throttle_thermal_policy.attr)
- ok = asus->throttle_thermal_policy_available;
+ ok = asus->throttle_thermal_policy_dev != 0;
else if (attr == &dev_attr_ppt_pl2_sppt.attr)
devid = ASUS_WMI_DEVID_PPT_PL2_SPPT;
else if (attr == &dev_attr_ppt_pl1_spl.attr)
@@ -4745,16 +4754,15 @@ static int asus_wmi_add(struct platform_device *pdev)
else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE2))
asus->kbd_rgb_dev = ASUS_WMI_DEVID_TUF_RGB_MODE2;
+ if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY))
+ asus->throttle_thermal_policy_dev = ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY;
+ else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO))
+ asus->throttle_thermal_policy_dev = ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
+
err = fan_boost_mode_check_present(asus);
if (err)
goto fail_fan_boost_mode;
- err = throttle_thermal_policy_check_present(asus);
- if (err)
- goto fail_throttle_thermal_policy;
- else
- throttle_thermal_policy_set_default(asus);
-
err = platform_profile_setup(asus);
if (err)
goto fail_platform_profile_setup;
@@ -4849,7 +4857,6 @@ static int asus_wmi_add(struct platform_device *pdev)
fail_input:
asus_wmi_sysfs_exit(asus->platform_device);
fail_sysfs:
-fail_throttle_thermal_policy:
fail_custom_fan_curve:
fail_platform_profile_setup:
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 3eb5cd677..982a63774 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -64,6 +64,7 @@
#define ASUS_WMI_DEVID_SCREENPAD_LIGHT 0x00050032
#define ASUS_WMI_DEVID_FAN_BOOST_MODE 0x00110018
#define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
+#define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO 0x00110019
/* Misc */
#define ASUS_WMI_DEVID_PANEL_OD 0x00050019
--
2.44.0
Hi,
On 4/21/24 9:43 PM, Mohamed Ghanmi wrote:
> Add support for vivobook fan profiles wmi call on the ASUS VIVOBOOK
> to adjust power limits.
>
> These fan profiles have a different device id than the ROG series
> and different order. This reorders the existing modes and adds a new
> full speed mode available on these laptops.
>
> As part of keeping the patch clean the throttle_thermal_policy_available
> boolean stored in the driver struct is removed and
> throttle_thermal_policy_dev is used in place (as on init it is zeroed).
>
> Signed-off-by: Mohamed Ghanmi <[email protected]>
> Co-developed-by: Luke D. Jones <[email protected]>
> Signed-off-by: Luke D. Jones <[email protected]>
> Reviewed-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/platform/x86/asus-wmi.c | 93 ++++++++++++----------
> include/linux/platform_data/x86/asus-wmi.h | 1 +
> 2 files changed, 51 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 3c61d75a3..1f54596ca 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -97,6 +97,11 @@ module_param(fnlock_default, bool, 0444);
> #define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST 1
> #define ASUS_THROTTLE_THERMAL_POLICY_SILENT 2
>
> +#define ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO 0
> +#define ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO 1
> +#define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO 2
> +#define ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED 3
> +
> #define USB_INTEL_XUSB2PR 0xD0
> #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
>
> @@ -293,8 +298,8 @@ struct asus_wmi {
> u32 kbd_rgb_dev;
> bool kbd_rgb_state_available;
>
> - bool throttle_thermal_policy_available;
> u8 throttle_thermal_policy_mode;
> + u32 throttle_thermal_policy_dev;
>
> bool cpu_fan_curve_available;
> bool gpu_fan_curve_available;
> @@ -3152,7 +3157,7 @@ static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
> int err, fan_idx;
> u8 mode = 0;
>
> - if (asus->throttle_thermal_policy_available)
> + if (asus->throttle_thermal_policy_dev)
> mode = asus->throttle_thermal_policy_mode;
> /* DEVID_<C/G>PU_FAN_CURVE is switched for OVERBOOST vs SILENT */
> if (mode == 2)
> @@ -3359,7 +3364,7 @@ static ssize_t fan_curve_enable_store(struct device *dev,
> * 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) {
> + if (asus->throttle_thermal_policy_dev) {
> err = throttle_thermal_policy_write(asus);
> if (err)
> return err;
> @@ -3576,8 +3581,8 @@ static const struct attribute_group asus_fan_curve_attr_group = {
> __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.
> + * Must be initialised after throttle_thermal_policy_dev is set as
> + * we check the status of throttle_thermal_policy_dev during init.
> */
> static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
> {
> @@ -3618,38 +3623,37 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
> }
>
> /* Throttle thermal policy ****************************************************/
> -
> -static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
> +static u8 throttle_thermal_policy_max_mode(struct asus_wmi *asus)
> {
> - u32 result;
> - int err;
> -
> - asus->throttle_thermal_policy_available = false;
> -
> - err = asus_wmi_get_devstate(asus,
> - ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
> - &result);
> - if (err) {
> - if (err == -ENODEV)
> - return 0;
> - return err;
> - }
> -
> - if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
> - asus->throttle_thermal_policy_available = true;
> -
> - return 0;
> + if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO)
> + return ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED;
> + else
> + return ASUS_THROTTLE_THERMAL_POLICY_SILENT;
> }
>
> static int throttle_thermal_policy_write(struct asus_wmi *asus)
> {
> - int err;
> - u8 value;
> + u8 value = asus->throttle_thermal_policy_mode;
> u32 retval;
> + bool vivo;
> + int err;
>
> - value = asus->throttle_thermal_policy_mode;
> + vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> + if (vivo) {
> + switch (value) {
> + case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> + value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
> + break;
> + case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> + value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
> + break;
> + case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> + value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
> + break;
> + }
> + }
>
> - err = asus_wmi_set_devstate(ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
> + err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev,
> value, &retval);
>
> sysfs_notify(&asus->platform_device->dev.kobj, NULL,
> @@ -3679,7 +3683,7 @@ static int throttle_thermal_policy_write(struct asus_wmi *asus)
>
> static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
> {
> - if (!asus->throttle_thermal_policy_available)
> + if (!asus->throttle_thermal_policy_dev)
> return 0;
>
> asus->throttle_thermal_policy_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> @@ -3689,9 +3693,10 @@ static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
> static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
> {
> u8 new_mode = asus->throttle_thermal_policy_mode + 1;
> + u8 max_mode = throttle_thermal_policy_max_mode(asus);
> int err;
>
> - if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
> + if (new_mode > max_mode)
> new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
>
> asus->throttle_thermal_policy_mode = new_mode;
> @@ -3722,6 +3727,7 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> const char *buf, size_t count)
> {
> struct asus_wmi *asus = dev_get_drvdata(dev);
> + u8 max_mode = throttle_thermal_policy_max_mode(asus);
> u8 new_mode;
> int result;
> int err;
> @@ -3730,7 +3736,7 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> if (result < 0)
> return result;
>
> - if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
> + if (new_mode > max_mode)
> return -EINVAL;
>
> asus->throttle_thermal_policy_mode = new_mode;
> @@ -3747,7 +3753,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> return count;
> }
>
> -// Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> +/*
> + * Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> + * Throttle thermal policy vivobook : 0 - default, 1 - silent, 2 - overboost, 3 - fullspeed
> + */
throttle_thermal_policy_write() always expects normal (non vivobook) values and
then translates those to vivo values, so this comment is not correct.
The only difference is that vivobook also has fullspeed, but the way userspace
sees it 1/2 or silent/overspeed are not swapped, since the swapping is taking
care of in throttle_thermal_policy_write().
Also the new fullspeed is not exported through the platform_profile interface,
for setting values this is somewhat ok, but fullspeed can be set through
sysfs, and this will then cause asus_wmi_platform_profile_get() to fail
with -EINVAL, so this need to be fixed. Either map fullspeed to
PLATFORM_PROFILE_PERFORMANCE in asus_wmi_platform_profile_get(), or add
a new platform_profile value for this.
Regards,
Hans
> static DEVICE_ATTR_RW(throttle_thermal_policy);
>
> /* Platform profile ***********************************************************/
> @@ -3813,7 +3822,7 @@ static int platform_profile_setup(struct asus_wmi *asus)
> * Not an error if a component platform_profile relies on is unavailable
> * so early return, skipping the setup of platform_profile.
> */
> - if (!asus->throttle_thermal_policy_available)
> + if (!asus->throttle_thermal_policy_dev)
> return 0;
>
> dev_info(dev, "Using throttle_thermal_policy for platform_profile support\n");
> @@ -4228,7 +4237,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> if (code == NOTIFY_KBD_FBM || code == NOTIFY_KBD_TTP) {
> if (asus->fan_boost_mode_available)
> fan_boost_mode_switch_next(asus);
> - if (asus->throttle_thermal_policy_available)
> + if (asus->throttle_thermal_policy_dev)
> throttle_thermal_policy_switch_next(asus);
> return;
>
> @@ -4436,7 +4445,7 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
> else if (attr == &dev_attr_fan_boost_mode.attr)
> ok = asus->fan_boost_mode_available;
> else if (attr == &dev_attr_throttle_thermal_policy.attr)
> - ok = asus->throttle_thermal_policy_available;
> + ok = asus->throttle_thermal_policy_dev != 0;
> else if (attr == &dev_attr_ppt_pl2_sppt.attr)
> devid = ASUS_WMI_DEVID_PPT_PL2_SPPT;
> else if (attr == &dev_attr_ppt_pl1_spl.attr)
> @@ -4745,16 +4754,15 @@ static int asus_wmi_add(struct platform_device *pdev)
> else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE2))
> asus->kbd_rgb_dev = ASUS_WMI_DEVID_TUF_RGB_MODE2;
>
> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY))
> + asus->throttle_thermal_policy_dev = ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY;
> + else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO))
> + asus->throttle_thermal_policy_dev = ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> +
> err = fan_boost_mode_check_present(asus);
> if (err)
> goto fail_fan_boost_mode;
>
> - err = throttle_thermal_policy_check_present(asus);
> - if (err)
> - goto fail_throttle_thermal_policy;
> - else
> - throttle_thermal_policy_set_default(asus);
> -
> err = platform_profile_setup(asus);
> if (err)
> goto fail_platform_profile_setup;
> @@ -4849,7 +4857,6 @@ static int asus_wmi_add(struct platform_device *pdev)
> fail_input:
> asus_wmi_sysfs_exit(asus->platform_device);
> fail_sysfs:
> -fail_throttle_thermal_policy:
> fail_custom_fan_curve:
> fail_platform_profile_setup:
> 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 3eb5cd677..982a63774 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -64,6 +64,7 @@
> #define ASUS_WMI_DEVID_SCREENPAD_LIGHT 0x00050032
> #define ASUS_WMI_DEVID_FAN_BOOST_MODE 0x00110018
> #define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
> +#define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO 0x00110019
>
> /* Misc */
> #define ASUS_WMI_DEVID_PANEL_OD 0x00050019
On Mon, 29 Apr 2024, at 10:20 PM, Hans de Goede wrote:
> Hi,
>
> On 4/21/24 9:43 PM, Mohamed Ghanmi wrote:
> > Add support for vivobook fan profiles wmi call on the ASUS VIVOBOOK
> > to adjust power limits.
> >
> > These fan profiles have a different device id than the ROG series
> > and different order. This reorders the existing modes and adds a new
> > full speed mode available on these laptops.
> >
> > As part of keeping the patch clean the throttle_thermal_policy_available
> > boolean stored in the driver struct is removed and
> > throttle_thermal_policy_dev is used in place (as on init it is zeroed).
> >
> > Signed-off-by: Mohamed Ghanmi <[email protected]>
> > Co-developed-by: Luke D. Jones <[email protected]>
> > Signed-off-by: Luke D. Jones <[email protected]>
> > Reviewed-by: Ilpo Järvinen <[email protected]>
> > ---
> > drivers/platform/x86/asus-wmi.c | 93 ++++++++++++----------
> > include/linux/platform_data/x86/asus-wmi.h | 1 +
> > 2 files changed, 51 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 3c61d75a3..1f54596ca 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -97,6 +97,11 @@ module_param(fnlock_default, bool, 0444);
> > #define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST 1
> > #define ASUS_THROTTLE_THERMAL_POLICY_SILENT 2
> >
> > +#define ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO 0
> > +#define ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO 1
> > +#define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO 2
> > +#define ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED 3
> > +
> > #define USB_INTEL_XUSB2PR 0xD0
> > #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
> >
> > @@ -293,8 +298,8 @@ struct asus_wmi {
> > u32 kbd_rgb_dev;
> > bool kbd_rgb_state_available;
> >
> > - bool throttle_thermal_policy_available;
> > u8 throttle_thermal_policy_mode;
> > + u32 throttle_thermal_policy_dev;
> >
> > bool cpu_fan_curve_available;
> > bool gpu_fan_curve_available;
> > @@ -3152,7 +3157,7 @@ static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
> > int err, fan_idx;
> > u8 mode = 0;
> >
> > - if (asus->throttle_thermal_policy_available)
> > + if (asus->throttle_thermal_policy_dev)
> > mode = asus->throttle_thermal_policy_mode;
> > /* DEVID_<C/G>PU_FAN_CURVE is switched for OVERBOOST vs SILENT */
> > if (mode == 2)
> > @@ -3359,7 +3364,7 @@ static ssize_t fan_curve_enable_store(struct device *dev,
> > * 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) {
> > + if (asus->throttle_thermal_policy_dev) {
> > err = throttle_thermal_policy_write(asus);
> > if (err)
> > return err;
> > @@ -3576,8 +3581,8 @@ static const struct attribute_group asus_fan_curve_attr_group = {
> > __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.
> > + * Must be initialised after throttle_thermal_policy_dev is set as
> > + * we check the status of throttle_thermal_policy_dev during init.
> > */
> > static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
> > {
> > @@ -3618,38 +3623,37 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
> > }
> >
> > /* Throttle thermal policy ****************************************************/
> > -
> > -static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
> > +static u8 throttle_thermal_policy_max_mode(struct asus_wmi *asus)
> > {
> > - u32 result;
> > - int err;
> > -
> > - asus->throttle_thermal_policy_available = false;
> > -
> > - err = asus_wmi_get_devstate(asus,
> > - ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
> > - &result);
> > - if (err) {
> > - if (err == -ENODEV)
> > - return 0;
> > - return err;
> > - }
> > -
> > - if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
> > - asus->throttle_thermal_policy_available = true;
> > -
> > - return 0;
> > + if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO)
> > + return ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED;
> > + else
> > + return ASUS_THROTTLE_THERMAL_POLICY_SILENT;
> > }
> >
> > static int throttle_thermal_policy_write(struct asus_wmi *asus)
> > {
> > - int err;
> > - u8 value;
> > + u8 value = asus->throttle_thermal_policy_mode;
> > u32 retval;
> > + bool vivo;
> > + int err;
> >
> > - value = asus->throttle_thermal_policy_mode;
> > + vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> > + if (vivo) {
> > + switch (value) {
> > + case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> > + value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
> > + break;
> > + case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> > + value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
> > + break;
> > + case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> > + value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
> > + break;
> > + }
> > + }
> >
> > - err = asus_wmi_set_devstate(ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
> > + err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev,
> > value, &retval);
> >
> > sysfs_notify(&asus->platform_device->dev.kobj, NULL,
> > @@ -3679,7 +3683,7 @@ static int throttle_thermal_policy_write(struct asus_wmi *asus)
> >
> > static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
> > {
> > - if (!asus->throttle_thermal_policy_available)
> > + if (!asus->throttle_thermal_policy_dev)
> > return 0;
> >
> > asus->throttle_thermal_policy_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> > @@ -3689,9 +3693,10 @@ static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
> > static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
> > {
> > u8 new_mode = asus->throttle_thermal_policy_mode + 1;
> > + u8 max_mode = throttle_thermal_policy_max_mode(asus);
> > int err;
> >
> > - if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
> > + if (new_mode > max_mode)
> > new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> >
> > asus->throttle_thermal_policy_mode = new_mode;
> > @@ -3722,6 +3727,7 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> > const char *buf, size_t count)
> > {
> > struct asus_wmi *asus = dev_get_drvdata(dev);
> > + u8 max_mode = throttle_thermal_policy_max_mode(asus);
> > u8 new_mode;
> > int result;
> > int err;
> > @@ -3730,7 +3736,7 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> > if (result < 0)
> > return result;
> >
> > - if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
> > + if (new_mode > max_mode)
> > return -EINVAL;
> >
> > asus->throttle_thermal_policy_mode = new_mode;
> > @@ -3747,7 +3753,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> > return count;
> > }
> >
> > -// Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> > +/*
> > + * Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> > + * Throttle thermal policy vivobook : 0 - default, 1 - silent, 2 - overboost, 3 - fullspeed
> > + */
>
> throttle_thermal_policy_write() always expects normal (non vivobook) values and
> then translates those to vivo values, so this comment is not correct.
>
> The only difference is that vivobook also has fullspeed, but the way userspace
> sees it 1/2 or silent/overspeed are not swapped, since the swapping is taking
> care of in throttle_thermal_policy_write().
>
> Also the new fullspeed is not exported through the platform_profile interface,
> for setting values this is somewhat ok, but fullspeed can be set through
> sysfs, and this will then cause asus_wmi_platform_profile_get() to fail
> with -EINVAL, so this need to be fixed. Either map fullspeed to
> PLATFORM_PROFILE_PERFORMANCE in asus_wmi_platform_profile_get(), or add
> a new platform_profile value for this.
>
I would much prefer if "fullspeed" was not included at all unless it was an individual setting. It very rarely contributes anything good to the driver, and most certainly won't be of value in the platform_profile.
Otherwise, what is the status on this?
>
> > static DEVICE_ATTR_RW(throttle_thermal_policy);
> >
> > /* Platform profile ***********************************************************/
> > @@ -3813,7 +3822,7 @@ static int platform_profile_setup(struct asus_wmi *asus)
> > * Not an error if a component platform_profile relies on is unavailable
> > * so early return, skipping the setup of platform_profile.
> > */
> > - if (!asus->throttle_thermal_policy_available)
> > + if (!asus->throttle_thermal_policy_dev)
> > return 0;
> >
> > dev_info(dev, "Using throttle_thermal_policy for platform_profile support\n");
> > @@ -4228,7 +4237,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> > if (code == NOTIFY_KBD_FBM || code == NOTIFY_KBD_TTP) {
> > if (asus->fan_boost_mode_available)
> > fan_boost_mode_switch_next(asus);
> > - if (asus->throttle_thermal_policy_available)
> > + if (asus->throttle_thermal_policy_dev)
> > throttle_thermal_policy_switch_next(asus);
> > return;
> >
> > @@ -4436,7 +4445,7 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
> > else if (attr == &dev_attr_fan_boost_mode.attr)
> > ok = asus->fan_boost_mode_available;
> > else if (attr == &dev_attr_throttle_thermal_policy.attr)
> > - ok = asus->throttle_thermal_policy_available;
> > + ok = asus->throttle_thermal_policy_dev != 0;
> > else if (attr == &dev_attr_ppt_pl2_sppt.attr)
> > devid = ASUS_WMI_DEVID_PPT_PL2_SPPT;
> > else if (attr == &dev_attr_ppt_pl1_spl.attr)
> > @@ -4745,16 +4754,15 @@ static int asus_wmi_add(struct platform_device *pdev)
> > else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE2))
> > asus->kbd_rgb_dev = ASUS_WMI_DEVID_TUF_RGB_MODE2;
> >
> > + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY))
> > + asus->throttle_thermal_policy_dev = ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY;
> > + else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO))
> > + asus->throttle_thermal_policy_dev = ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> > +
> > err = fan_boost_mode_check_present(asus);
> > if (err)
> > goto fail_fan_boost_mode;
> >
> > - err = throttle_thermal_policy_check_present(asus);
> > - if (err)
> > - goto fail_throttle_thermal_policy;
> > - else
> > - throttle_thermal_policy_set_default(asus);
> > -
> > err = platform_profile_setup(asus);
> > if (err)
> > goto fail_platform_profile_setup;
> > @@ -4849,7 +4857,6 @@ static int asus_wmi_add(struct platform_device *pdev)
> > fail_input:
> > asus_wmi_sysfs_exit(asus->platform_device);
> > fail_sysfs:
> > -fail_throttle_thermal_policy:
> > fail_custom_fan_curve:
> > fail_platform_profile_setup:
> > 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 3eb5cd677..982a63774 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -64,6 +64,7 @@
> > #define ASUS_WMI_DEVID_SCREENPAD_LIGHT 0x00050032
> > #define ASUS_WMI_DEVID_FAN_BOOST_MODE 0x00110018
> > #define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
> > +#define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO 0x00110019
> >
> > /* Misc */
> > #define ASUS_WMI_DEVID_PANEL_OD 0x00050019
>
>
On Mon, 3 Jun 2024, Luke Jones wrote:
> On Mon, 29 Apr 2024, at 10:20 PM, Hans de Goede wrote:
> > On 4/21/24 9:43 PM, Mohamed Ghanmi wrote:
> > > Add support for vivobook fan profiles wmi call on the ASUS VIVOBOOK
> > > to adjust power limits.
> > >
> > > These fan profiles have a different device id than the ROG series
> > > and different order. This reorders the existing modes and adds a new
> > > full speed mode available on these laptops.
> > >
> > > As part of keeping the patch clean the throttle_thermal_policy_available
> > > boolean stored in the driver struct is removed and
> > > throttle_thermal_policy_dev is used in place (as on init it is zeroed).
> > >
> > > Signed-off-by: Mohamed Ghanmi <[email protected]>
> > > Co-developed-by: Luke D. Jones <[email protected]>
> > > Signed-off-by: Luke D. Jones <[email protected]>
> > > Reviewed-by: Ilpo Järvinen <[email protected]>
> > > ---
> > > drivers/platform/x86/asus-wmi.c | 93 ++++++++++++----------
> > > include/linux/platform_data/x86/asus-wmi.h | 1 +
> > > 2 files changed, 51 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > index 3c61d75a3..1f54596ca 100644
> > > --- a/drivers/platform/x86/asus-wmi.c
> > > +++ b/drivers/platform/x86/asus-wmi.c
> > > @@ -3747,7 +3753,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> > > return count;
> > > }
> > >
> > > -// Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> > > +/*
> > > + * Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> > > + * Throttle thermal policy vivobook : 0 - default, 1 - silent, 2 - overboost, 3 - fullspeed
> > > + */
> >
> > throttle_thermal_policy_write() always expects normal (non vivobook) values and
> > then translates those to vivo values, so this comment is not correct.
> >
> > The only difference is that vivobook also has fullspeed, but the way userspace
> > sees it 1/2 or silent/overspeed are not swapped, since the swapping is taking
> > care of in throttle_thermal_policy_write().
> >
> > Also the new fullspeed is not exported through the platform_profile interface,
> > for setting values this is somewhat ok, but fullspeed can be set through
> > sysfs, and this will then cause asus_wmi_platform_profile_get() to fail
> > with -EINVAL, so this need to be fixed. Either map fullspeed to
> > PLATFORM_PROFILE_PERFORMANCE in asus_wmi_platform_profile_get(), or add
> > a new platform_profile value for this.
> >
>
> I would much prefer if "fullspeed" was not included at all unless it was
> an individual setting. It very rarely contributes anything good to the
> driver, and most certainly won't be of value in the platform_profile.
>
> Otherwise, what is the status on this?
Hi,
I was expecting an update that addresses Hans' review comment.
Luke, are you arguing that his comment is not valid and v3 is fine?
(In any case, I've summoned v3 back from archive into active patches in
patchwork so this doesn't get forgotten if v4 is not needed).
--
i.
On Mon, 3 Jun 2024, at 10:29 PM, Ilpo Järvinen wrote:
> On Mon, 3 Jun 2024, Luke Jones wrote:
> > On Mon, 29 Apr 2024, at 10:20 PM, Hans de Goede wrote:
> > > On 4/21/24 9:43 PM, Mohamed Ghanmi wrote:
> > > > Add support for vivobook fan profiles wmi call on the ASUS VIVOBOOK
> > > > to adjust power limits.
> > > >
> > > > These fan profiles have a different device id than the ROG series
> > > > and different order. This reorders the existing modes and adds a new
> > > > full speed mode available on these laptops.
> > > >
> > > > As part of keeping the patch clean the throttle_thermal_policy_available
> > > > boolean stored in the driver struct is removed and
> > > > throttle_thermal_policy_dev is used in place (as on init it is zeroed).
> > > >
> > > > Signed-off-by: Mohamed Ghanmi <[email protected]>
> > > > Co-developed-by: Luke D. Jones <[email protected]>
> > > > Signed-off-by: Luke D. Jones <[email protected]>
> > > > Reviewed-by: Ilpo Järvinen <[email protected]>
> > > > ---
> > > > drivers/platform/x86/asus-wmi.c | 93 ++++++++++++----------
> > > > include/linux/platform_data/x86/asus-wmi.h | 1 +
> > > > 2 files changed, 51 insertions(+), 43 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > > index 3c61d75a3..1f54596ca 100644
> > > > --- a/drivers/platform/x86/asus-wmi.c
> > > > +++ b/drivers/platform/x86/asus-wmi.c
>
> > > > @@ -3747,7 +3753,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> > > > return count;
> > > > }
> > > >
> > > > -// Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> > > > +/*
> > > > + * Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
> > > > + * Throttle thermal policy vivobook : 0 - default, 1 - silent, 2 - overboost, 3 - fullspeed
> > > > + */
> > >
> > > throttle_thermal_policy_write() always expects normal (non vivobook) values and
> > > then translates those to vivo values, so this comment is not correct.
> > >
> > > The only difference is that vivobook also has fullspeed, but the way userspace
> > > sees it 1/2 or silent/overspeed are not swapped, since the swapping is taking
> > > care of in throttle_thermal_policy_write().
> > >
> > > Also the new fullspeed is not exported through the platform_profile interface,
> > > for setting values this is somewhat ok, but fullspeed can be set through
> > > sysfs, and this will then cause asus_wmi_platform_profile_get() to fail
> > > with -EINVAL, so this need to be fixed. Either map fullspeed to
> > > PLATFORM_PROFILE_PERFORMANCE in asus_wmi_platform_profile_get(), or add
> > > a new platform_profile value for this.
> > >
> >
> > I would much prefer if "fullspeed" was not included at all unless it was
> > an individual setting. It very rarely contributes anything good to the
> > driver, and most certainly won't be of value in the platform_profile.
> >
> > Otherwise, what is the status on this?
>
> Hi,
>
> I was expecting an update that addresses Hans' review comment.
>
> Luke, are you arguing that his comment is not valid and v3 is fine?
>
> (In any case, I've summoned v3 back from archive into active patches in
> patchwork so this doesn't get forgotten if v4 is not needed).
>
> --
> i.
Hi.
I am saying I would like to see ASUS_THROTTLE_THERMAL_POLICY_FULLSPEED removed, or placed somewhere else such as in <sysfs>/asus-nb-wmi/hwmon/hwmon3/pwm1_enable.
It certainly shouldn't be included in platform_profile and I'm not keen on seeing it in thorttle_thermal_policy either.
A lot of this reasoning is now coming from the refactor I've just done of asus-wmi and the "features" such as this one to place them under firmware_attributes class and begin deprecation of them in asus_wmi. From what I've achieved so far with it it is much *much* more suitable than this ad-hoc style of adding features I've been doing here (I'll submit this work soon, it repalces the last patch series I did).
In light of the above I'm considering the possibility of removing throttle_thermal_policy completely to wholly favour the use of platform_profile. It doesn't make all that much sense to have two different things manipulating the same thing - and as such I don't think the "full speed fan" setting fits at all with platform_profile as it is not a performance tweak.
Mohamed, I would be happy to include the Vivo support so long as:
1. the fullspeed setting is removed
2. the modes map correctly to platform_profile
I hope this makes sense. Very sorry about the previous brief response (I was recovering from surgery trauma).
Cheers,
Luke.