2022-09-16 00:56:57

by Luke D. Jones

[permalink] [raw]
Subject: [PATCH] asus-wmi: Expand support of GPU fan to read RPM and label

The previously added patch to add support for pwm change for TUF laptops
also is usuable for more than TUF. The same method `0x00110014` is
used to read the fan RPM.

Add two extra attributes for reading fan2 plus fan2 label.

Signed-off-by: Luke D. Jones <[email protected]>
---
drivers/platform/x86/asus-wmi.c | 36 +++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ae46af731de9..7fe6ce25da0a 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -72,6 +72,7 @@ module_param(fnlock_default, bool, 0444);

#define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)

+#define ASUS_GPU_FAN_DESC "gpu_fan"
#define ASUS_FAN_DESC "cpu_fan"
#define ASUS_FAN_MFUN 0x13
#define ASUS_FAN_SFUN_READ 0x06
@@ -2078,6 +2079,30 @@ static ssize_t asus_hwmon_temp1(struct device *dev,
}

/* GPU fan on modern ROG laptops */
+static ssize_t fan2_input_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+ int value;
+ int ret;
+
+ ret = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_GPU_FAN_CTRL, &value);
+ if (ret < 0)
+ return ret;
+
+ value &= 0xffff;
+
+ return sysfs_emit(buf, "%d\n", value < 0 ? -1 : value * 100);
+}
+
+static ssize_t fan2_label_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%s\n", ASUS_GPU_FAN_DESC);
+}
+
static ssize_t pwm2_enable_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -2127,9 +2152,12 @@ static ssize_t pwm2_enable_store(struct device *dev,
/* Fan1 */
static DEVICE_ATTR_RW(pwm1);
static DEVICE_ATTR_RW(pwm1_enable);
-static DEVICE_ATTR_RW(pwm2_enable);
static DEVICE_ATTR_RO(fan1_input);
static DEVICE_ATTR_RO(fan1_label);
+/* Fan2 - GPU fan */
+static DEVICE_ATTR_RW(pwm2_enable);
+static DEVICE_ATTR_RO(fan2_input);
+static DEVICE_ATTR_RO(fan2_label);

/* Temperature */
static DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL);
@@ -2140,6 +2168,8 @@ static struct attribute *hwmon_attributes[] = {
&dev_attr_pwm2_enable.attr,
&dev_attr_fan1_input.attr,
&dev_attr_fan1_label.attr,
+ &dev_attr_fan2_input.attr,
+ &dev_attr_fan2_label.attr,

&dev_attr_temp1_input.attr,
NULL
@@ -2160,7 +2190,9 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
|| attr == &dev_attr_pwm1_enable.attr) {
if (asus->fan_type == FAN_TYPE_NONE)
return 0;
- } else if (attr == &dev_attr_pwm2_enable.attr) {
+ } else if (attr == &dev_attr_fan2_input.attr
+ || attr == &dev_attr_fan2_label.attr
+ || attr == &dev_attr_pwm2_enable.attr) {
if (asus->gpu_fan_type == FAN_TYPE_NONE)
return 0;
} else if (attr == &dev_attr_temp1_input.attr) {
--
2.37.3


2022-09-17 17:24:36

by Barnabás Pőcze

[permalink] [raw]
Subject: Re: [PATCH] asus-wmi: Expand support of GPU fan to read RPM and label

Hi


2022. szeptember 16., péntek 2:46 keltezéssel, Luke D. Jones írta:

> The previously added patch to add support for pwm change for TUF laptops
> also is usuable for more than TUF. The same method `0x00110014` is
> used to read the fan RPM.
>
> Add two extra attributes for reading fan2 plus fan2 label.
>
> Signed-off-by: Luke D. Jones <[email protected]>
> ---
> drivers/platform/x86/asus-wmi.c | 36 +++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ae46af731de9..7fe6ce25da0a 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -72,6 +72,7 @@ module_param(fnlock_default, bool, 0444);
> [...]
> +static ssize_t fan2_input_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct asus_wmi *asus = dev_get_drvdata(dev);
> + int value;
> + int ret;
> +
> + ret = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_GPU_FAN_CTRL, &value);
> + if (ret < 0)
> + return ret;
> +
> + value &= 0xffff;
> +
> + return sysfs_emit(buf, "%d\n", value < 0 ? -1 : value * 100);

Can `value` can be negative here? I am not sure because of the `value &= 0xffff` part.
And maybe it would be better to return -ENODATA or something similar instead of printing "-1".


> +}
> [...]


Regards,
Barnabás Pőcze

2022-09-19 13:26:49

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] asus-wmi: Expand support of GPU fan to read RPM and label

Hi Luke,

On 9/16/22 01:46, Luke D. Jones wrote:
> The previously added patch to add support for pwm change for TUF laptops
> also is usuable for more than TUF. The same method `0x00110014` is
> used to read the fan RPM.
>
> Add two extra attributes for reading fan2 plus fan2 label.
>
> Signed-off-by: Luke D. Jones <[email protected]>
> ---
> drivers/platform/x86/asus-wmi.c | 36 +++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ae46af731de9..7fe6ce25da0a 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -72,6 +72,7 @@ module_param(fnlock_default, bool, 0444);
>
> #define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)
>
> +#define ASUS_GPU_FAN_DESC "gpu_fan"
> #define ASUS_FAN_DESC "cpu_fan"
> #define ASUS_FAN_MFUN 0x13
> #define ASUS_FAN_SFUN_READ 0x06
> @@ -2078,6 +2079,30 @@ static ssize_t asus_hwmon_temp1(struct device *dev,
> }
>
> /* GPU fan on modern ROG laptops */
> +static ssize_t fan2_input_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct asus_wmi *asus = dev_get_drvdata(dev);
> + int value;
> + int ret;
> +
> + ret = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_GPU_FAN_CTRL, &value);
> + if (ret < 0)
> + return ret;
> +
> + value &= 0xffff;
> +
> + return sysfs_emit(buf, "%d\n", value < 0 ? -1 : value * 100);

As already mentioned since you & with 0xffff above the sign bit can never be
set, so the value is never less then < 0, so I have simplified this to:

return sysfs_emit(buf, "%d\n", value * 100);

while merging.

> +}
> +
> +static ssize_t fan2_label_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%s\n", ASUS_GPU_FAN_DESC);
> +}

And here I have done s/sprintf/sysfs_emit/ with those changes
I've applied this patch to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> +
> static ssize_t pwm2_enable_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -2127,9 +2152,12 @@ static ssize_t pwm2_enable_store(struct device *dev,
> /* Fan1 */
> static DEVICE_ATTR_RW(pwm1);
> static DEVICE_ATTR_RW(pwm1_enable);
> -static DEVICE_ATTR_RW(pwm2_enable);
> static DEVICE_ATTR_RO(fan1_input);
> static DEVICE_ATTR_RO(fan1_label);
> +/* Fan2 - GPU fan */
> +static DEVICE_ATTR_RW(pwm2_enable);
> +static DEVICE_ATTR_RO(fan2_input);
> +static DEVICE_ATTR_RO(fan2_label);
>
> /* Temperature */
> static DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL);
> @@ -2140,6 +2168,8 @@ static struct attribute *hwmon_attributes[] = {
> &dev_attr_pwm2_enable.attr,
> &dev_attr_fan1_input.attr,
> &dev_attr_fan1_label.attr,
> + &dev_attr_fan2_input.attr,
> + &dev_attr_fan2_label.attr,
>
> &dev_attr_temp1_input.attr,
> NULL
> @@ -2160,7 +2190,9 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
> || attr == &dev_attr_pwm1_enable.attr) {
> if (asus->fan_type == FAN_TYPE_NONE)
> return 0;
> - } else if (attr == &dev_attr_pwm2_enable.attr) {
> + } else if (attr == &dev_attr_fan2_input.attr
> + || attr == &dev_attr_fan2_label.attr
> + || attr == &dev_attr_pwm2_enable.attr) {
> if (asus->gpu_fan_type == FAN_TYPE_NONE)
> return 0;
> } else if (attr == &dev_attr_temp1_input.attr) {

2022-09-20 00:44:03

by Luke D. Jones

[permalink] [raw]
Subject: Re: [PATCH] asus-wmi: Expand support of GPU fan to read RPM and label

On Mon, 2022-09-19 at 13:18 +0100, Hans de Goede wrote:
> Hi Luke,
>
> On 9/16/22 01:46, Luke D. Jones wrote:
> > The previously added patch to add support for pwm change for TUF
> > laptops
> > also is usuable for more than TUF. The same method `0x00110014` is
> > used to read the fan RPM.
> >
> > Add two extra attributes for reading fan2 plus fan2 label.
> >
> > Signed-off-by: Luke D. Jones <[email protected]>
> > ---
> >  drivers/platform/x86/asus-wmi.c | 36
> > +++++++++++++++++++++++++++++++--
> >  1 file changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c
> > b/drivers/platform/x86/asus-wmi.c
> > index ae46af731de9..7fe6ce25da0a 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -72,6 +72,7 @@ module_param(fnlock_default, bool, 0444);
> >  
> >  #define ASUS_WMI_FNLOCK_BIOS_DISABLED  BIT(0)
> >  
> > +#define ASUS_GPU_FAN_DESC              "gpu_fan"
> >  #define ASUS_FAN_DESC                  "cpu_fan"
> >  #define ASUS_FAN_MFUN                  0x13
> >  #define ASUS_FAN_SFUN_READ             0x06
> > @@ -2078,6 +2079,30 @@ static ssize_t asus_hwmon_temp1(struct
> > device *dev,
> >  }
> >  
> >  /* GPU fan on modern ROG laptops */
> > +static ssize_t fan2_input_show(struct device *dev,
> > +                                       struct device_attribute
> > *attr,
> > +                                       char *buf)
> > +{
> > +       struct asus_wmi *asus = dev_get_drvdata(dev);
> > +       int value;
> > +       int ret;
> > +
> > +       ret = asus_wmi_get_devstate(asus,
> > ASUS_WMI_DEVID_GPU_FAN_CTRL, &value);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       value &= 0xffff;
> > +
> > +       return sysfs_emit(buf, "%d\n", value < 0 ? -1 : value *
> > 100);
>
> As already mentioned since you & with 0xffff above the sign bit can
> never be
> set, so the value is never less then < 0, so I have simplified this
> to:
>
>         return sysfs_emit(buf, "%d\n", value * 100);
>
> while merging.

Thanks Hans really appreciate it, sorry i missed that. My workload is a
bit too large at the moment :)

Cheers,
Luke.

>
> > +}
> > +
> > +static ssize_t fan2_label_show(struct device *dev,
> > +                                         struct device_attribute
> > *attr,
> > +                                         char *buf)
> > +{
> > +       return sprintf(buf, "%s\n", ASUS_GPU_FAN_DESC);
> > +}
>
> And here I have done s/sprintf/sysfs_emit/ with those changes
> I've applied this patch to my review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> Note it will show up in my review-hans branch once I've pushed my
> local branch there, which might take a while.
>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
>
> Regards,
>
> Hans
>
>
>
> > +
> >  static ssize_t pwm2_enable_show(struct device *dev,
> >                                 struct device_attribute *attr,
> >                                 char *buf)
> > @@ -2127,9 +2152,12 @@ static ssize_t pwm2_enable_store(struct
> > device *dev,
> >  /* Fan1 */
> >  static DEVICE_ATTR_RW(pwm1);
> >  static DEVICE_ATTR_RW(pwm1_enable);
> > -static DEVICE_ATTR_RW(pwm2_enable);
> >  static DEVICE_ATTR_RO(fan1_input);
> >  static DEVICE_ATTR_RO(fan1_label);
> > +/* Fan2 - GPU fan */
> > +static DEVICE_ATTR_RW(pwm2_enable);
> > +static DEVICE_ATTR_RO(fan2_input);
> > +static DEVICE_ATTR_RO(fan2_label);
> >  
> >  /* Temperature */
> >  static DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL);
> > @@ -2140,6 +2168,8 @@ static struct attribute *hwmon_attributes[] =
> > {
> >         &dev_attr_pwm2_enable.attr,
> >         &dev_attr_fan1_input.attr,
> >         &dev_attr_fan1_label.attr,
> > +       &dev_attr_fan2_input.attr,
> > +       &dev_attr_fan2_label.attr,
> >  
> >         &dev_attr_temp1_input.attr,
> >         NULL
> > @@ -2160,7 +2190,9 @@ static umode_t
> > asus_hwmon_sysfs_is_visible(struct kobject *kobj,
> >             || attr == &dev_attr_pwm1_enable.attr) {
> >                 if (asus->fan_type == FAN_TYPE_NONE)
> >                         return 0;
> > -       } else if (attr == &dev_attr_pwm2_enable.attr) {
> > +       } else if (attr == &dev_attr_fan2_input.attr
> > +           || attr == &dev_attr_fan2_label.attr
> > +           || attr == &dev_attr_pwm2_enable.attr) {
> >                 if (asus->gpu_fan_type == FAN_TYPE_NONE)
> >                         return 0;
> >         } else if (attr == &dev_attr_temp1_input.attr) {
>