Simple driver to enable control of the fan in ASUS laptops. So far this
has only been tested in ASUS Zenbook Prime UX31A, but according to some
online reference [1], it should work in other models as well.
The implementation is very straight-forward, the only caveat is that the
fan speed needs to be saved after it has been manually changed because
it won't be reported properly until it goes back to 'auto' mode.
[1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
Signed-off-by: Felipe Contreras <[email protected]>
---
drivers/platform/x86/asus-wmi.c | 105 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 19c313b..5fdfed6 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -78,6 +78,7 @@ MODULE_LICENSE("GPL");
#define ASUS_WMI_METHODID_GPID 0x44495047 /* Get Panel ID?? (Resol) */
#define ASUS_WMI_METHODID_QMOD 0x444F4D51 /* Quiet MODe */
#define ASUS_WMI_METHODID_SPLV 0x4C425053 /* Set Panel Light Value */
+#define ASUS_WMI_METHODID_AGFN 0x4E464741
#define ASUS_WMI_METHODID_SFUN 0x4E554653 /* FUNCtionalities */
#define ASUS_WMI_METHODID_SDSP 0x50534453 /* Set DiSPlay output */
#define ASUS_WMI_METHODID_GDSP 0x50534447 /* Get DiSPlay output */
@@ -155,6 +156,16 @@ struct bios_args {
u32 arg1;
} __packed;
+struct fan_args {
+ u16 mfun;
+ u16 sfun;
+ u16 len;
+ u8 stas;
+ u8 err;
+ u8 fan;
+ u32 speed;
+} __packed;
+
/*
* <platform>/ - debugfs root directory
* dev_id - current dev_id
@@ -205,6 +216,9 @@ struct asus_wmi {
struct asus_rfkill gps;
struct asus_rfkill uwb;
+ struct thermal_cooling_device *fan;
+ int fan_speed;
+
struct hotplug_slot *hotplug_slot;
struct mutex hotplug_lock;
struct mutex wmi_lock;
@@ -1022,6 +1036,90 @@ exit:
}
/*
+ * Fan
+ */
+
+static int fan_speed(struct asus_wmi *asus, int write, int fan, unsigned long *speed)
+{
+ struct fan_args args = {
+ .len = sizeof(args),
+ .mfun = 0x13,
+ .sfun = write ? 0x07 : 0x06,
+ .fan = fan,
+ .speed = write ? *speed : 0,
+ };
+ int r;
+ u32 value;
+
+ if (!write && asus->fan_speed >= 0) {
+ *speed = asus->fan_speed;
+ return 0;
+ }
+
+ r = asus_wmi_evaluate_method(ASUS_WMI_METHODID_AGFN, virt_to_phys(&args), 0, &value);
+ if (r || value || args.err)
+ return -1;
+
+ if (write)
+ asus->fan_speed = fan > 0 ? *speed : -1;
+ else
+ *speed = args.speed;
+
+ return 0;
+}
+
+static int fan_set_auto(struct asus_wmi *asus)
+{
+ unsigned long speed = 0;
+ return fan_speed(asus, 1, 0, &speed);
+}
+
+static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+ *state = 0xff;
+ return 0;
+}
+
+static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+ return fan_speed(cdev->devdata, 0, 1, state);
+}
+
+static int fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
+{
+ return fan_speed(cdev->devdata, 1, 1, &state);
+}
+
+static const struct thermal_cooling_device_ops fan_cooling_ops = {
+ .get_max_state = fan_get_max_state,
+ .get_cur_state = fan_get_cur_state,
+ .set_cur_state = fan_set_cur_state,
+};
+
+static int asus_wmi_fan_init(struct asus_wmi *asus)
+{
+ struct thermal_cooling_device *cdev;
+
+ if (fan_set_auto(asus))
+ return 0;
+
+ cdev = thermal_cooling_device_register("Fan", asus, &fan_cooling_ops);
+ if (IS_ERR(cdev))
+ return PTR_ERR(cdev);
+ asus->fan = cdev;
+ return 0;
+}
+
+static void asus_wmi_fan_exit(struct asus_wmi *asus)
+{
+ if (!asus->fan)
+ return;
+ fan_set_auto(asus);
+ thermal_cooling_device_unregister(asus->fan);
+ asus->fan = NULL;
+}
+
+/*
* Hwmon device
*/
static ssize_t asus_hwmon_pwm1(struct device *dev,
@@ -1796,6 +1894,10 @@ static int asus_wmi_add(struct platform_device *pdev)
if (err)
goto fail_rfkill;
+ err = asus_wmi_fan_init(asus);
+ if (err)
+ goto fail_fan;
+
if (asus->driver->quirks->wmi_backlight_power)
acpi_video_dmi_promote_vendor();
if (!acpi_video_backlight_support()) {
@@ -1830,6 +1932,8 @@ fail_debugfs:
fail_wmi_handler:
asus_wmi_backlight_exit(asus);
fail_backlight:
+ asus_wmi_fan_exit(asus);
+fail_fan:
asus_wmi_rfkill_exit(asus);
fail_rfkill:
asus_wmi_led_exit(asus);
@@ -1855,6 +1959,7 @@ static int asus_wmi_remove(struct platform_device *device)
asus_wmi_hwmon_exit(asus);
asus_wmi_led_exit(asus);
asus_wmi_rfkill_exit(asus);
+ asus_wmi_fan_exit(asus);
asus_wmi_debugfs_exit(asus);
asus_wmi_platform_exit(asus);
--
1.8.4-fc
On Tue, Oct 8, 2013 at 7:48 AM, Felipe Contreras
<[email protected]> wrote:
> Simple driver to enable control of the fan in ASUS laptops. So far this
> has only been tested in ASUS Zenbook Prime UX31A, but according to some
> online reference [1], it should work in other models as well.
>
> The implementation is very straight-forward, the only caveat is that the
> fan speed needs to be saved after it has been manually changed because
> it won't be reported properly until it goes back to 'auto' mode.
>
> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
>
> Signed-off-by: Felipe Contreras <[email protected]>
> + r = asus_wmi_evaluate_method(ASUS_WMI_METHODID_AGFN, virt_to_phys(&args), 0, &value);
I don't like using virt_to_phys() here, but it seems that's what the
ACPI code expects.
Method (AGFN, 1, Serialized)
{
If (LEqual (Arg0, Zero))
{
Return (GNBF)
}
Store (Zero, Local0)
OperationRegion (\PARM, SystemMemory, Arg0, 0x08)
Field (PARM, DWordAcc, NoLock, Preserve)
{
MFUN, 16,
SFUN, 16,
LEN, 16,
STAS, 8,
EROR, 8
}
Any suggestions?
--
Felipe Contreras
On Thu, 2013-10-10 at 16:59 +0100, Corentin Chary wrote:
>
> This doesn't really seems to be related to wmi, and is likely to be
> available only on a subset of models. Maybe it should a separate
> driver instead ?
This version seems to be implemented entirely in WMI, and it's using the
same WMI GUID as asus-wmi - implementing it here seems appropriate. I am
concerned about the phys/virt thing, though. The ACPI interpreter is
running in the kernel, not the hardware - are we artificially limiting
SystemMemory opregions to physical addresses?
--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Thu, Oct 10, 2013 at 10:59 AM, Corentin Chary
<[email protected]> wrote:
>
>
>
> On Tue, Oct 8, 2013 at 1:48 PM, Felipe Contreras
> <[email protected]> wrote:
>>
>> Simple driver to enable control of the fan in ASUS laptops. So far this
>> has only been tested in ASUS Zenbook Prime UX31A, but according to some
>> online reference [1], it should work in other models as well.
>>
>> The implementation is very straight-forward, the only caveat is that the
>> fan speed needs to be saved after it has been manually changed because
>> it won't be reported properly until it goes back to 'auto' mode.
>
> This doesn't really seems to be related to wmi, and is likely to be
> available only on a subset of models. Maybe it should a separate driver
> instead ?
The first patch I sent was standalone, and you said it should be wmi,
and now I do it wmi, and you said it should be separate. Which is it?
--
Felipe Contreras
On Thu, Oct 10, 2013 at 11:06 AM, Matthew Garrett
<[email protected]> wrote:
> On Thu, 2013-10-10 at 16:59 +0100, Corentin Chary wrote:
>
>>
>> This doesn't really seems to be related to wmi, and is likely to be
>> available only on a subset of models. Maybe it should a separate
>> driver instead ?
>
> This version seems to be implemented entirely in WMI, and it's using the
> same WMI GUID as asus-wmi - implementing it here seems appropriate. I am
> concerned about the phys/virt thing, though. The ACPI interpreter is
> running in the kernel, not the hardware - are we artificially limiting
> SystemMemory opregions to physical addresses?
I don't see anything in acpi_ex_system_memory_space_handler() that
takes into consideration virtual addresses.
--
Felipe Contreras
On Sun, Oct 13, 2013 at 04:29:34AM -0500, Felipe Contreras wrote:
> I don't see anything in acpi_ex_system_memory_space_handler() that
> takes into consideration virtual addresses.
The spec doesn't seem to constrain it to physical addresses (it just
refers to "Control Methods read and write data to locations in address
spaces (for example, System memory and System I/O)", so I'd lean towards
changing the behaviour of acpica rather than adding virt_to_phys().
--
Matthew Garrett | [email protected]
On Sun, Oct 13, 2013 at 10:17 AM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 13, 2013 at 04:29:34AM -0500, Felipe Contreras wrote:
>
>> I don't see anything in acpi_ex_system_memory_space_handler() that
>> takes into consideration virtual addresses.
>
> The spec doesn't seem to constrain it to physical addresses (it just
> refers to "Control Methods read and write data to locations in address
> spaces (for example, System memory and System I/O)", so I'd lean towards
> changing the behaviour of acpica rather than adding virt_to_phys().
And I assume you are not going to do that. Isn't acpica code outside
of the scope of Linux kernel development? If not, how do I go about
adding that capability.
Also, I find it weird that this the first driver that needs this.
Finally, I'm much more interested on what happens next, because I want
to link this driver to the thermal framework, so when temperature gets
too high, the fan gets an increased speed, because right now it seems
it's always on low speed, temperature gets high, and instead the CPU
gets throttled, which is not desirable.
Maybe this driver should be added to the staging area.
--
Felipe Contreras
On Sun, Oct 13, 2013 at 09:31:25PM -0500, Felipe Contreras wrote:
> On Sun, Oct 13, 2013 at 10:17 AM, Matthew Garrett <[email protected]> wrote:
> > The spec doesn't seem to constrain it to physical addresses (it just
> > refers to "Control Methods read and write data to locations in address
> > spaces (for example, System memory and System I/O)", so I'd lean towards
> > changing the behaviour of acpica rather than adding virt_to_phys().
>
> And I assume you are not going to do that. Isn't acpica code outside
> of the scope of Linux kernel development? If not, how do I go about
> adding that capability.
I wasn't planning on it, no. Just write the code and submit it to
linux-acpi, and Cc: Bob Moore.
> Also, I find it weird that this the first driver that needs this.
The normal way to do this would be for the ASL to just define a buffer
within the argument, rather than assigning it to an operation region. I
haven't seen anyone take this approach before.
> Finally, I'm much more interested on what happens next, because I want
> to link this driver to the thermal framework, so when temperature gets
> too high, the fan gets an increased speed, because right now it seems
> it's always on low speed, temperature gets high, and instead the CPU
> gets throttled, which is not desirable.
It wouldn't be appropriate to alter the firmware behaviour by default,
but yeah, that's the kind of thing that the thermal framework exists to
do.
> Maybe this driver should be added to the staging area.
I don't think you can easily register multiple drivers for the same WMI
device.
--
Matthew Garrett | [email protected]
On Mon, Oct 14, 2013 at 10:52 AM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 13, 2013 at 09:31:25PM -0500, Felipe Contreras wrote:
>> On Sun, Oct 13, 2013 at 10:17 AM, Matthew Garrett <[email protected]> wrote:
>> > The spec doesn't seem to constrain it to physical addresses (it just
>> > refers to "Control Methods read and write data to locations in address
>> > spaces (for example, System memory and System I/O)", so I'd lean towards
>> > changing the behaviour of acpica rather than adding virt_to_phys().
>>
>> And I assume you are not going to do that. Isn't acpica code outside
>> of the scope of Linux kernel development? If not, how do I go about
>> adding that capability.
>
> I wasn't planning on it, no. Just write the code and submit it to
> linux-acpi, and Cc: Bob Moore.
I might give it a try.
>> Also, I find it weird that this the first driver that needs this.
>
> The normal way to do this would be for the ASL to just define a buffer
> within the argument, rather than assigning it to an operation region. I
> haven't seen anyone take this approach before.
>
>> Finally, I'm much more interested on what happens next, because I want
>> to link this driver to the thermal framework, so when temperature gets
>> too high, the fan gets an increased speed, because right now it seems
>> it's always on low speed, temperature gets high, and instead the CPU
>> gets throttled, which is not desirable.
>
> It wouldn't be appropriate to alter the firmware behaviour by default,
> but yeah, that's the kind of thing that the thermal framework exists to
> do.
Well, how do I do that? The driver is up and running, and I can
manually set different fan speeds, however nothing seems to happen
automatically when the temperature increases.
>> Maybe this driver should be added to the staging area.
>
> I don't think you can easily register multiple drivers for the same WMI
> device.
I don't mean this one, I mean the standalone one. Actually, the first
one I sent doesn't require all this system memory stuff.
--
Felipe Contreras
On Mon, Oct 14, 2013 at 06:18:36PM -0500, Felipe Contreras wrote:
> On Mon, Oct 14, 2013 at 10:52 AM, Matthew Garrett <[email protected]> wrote:
> > It wouldn't be appropriate to alter the firmware behaviour by default,
> > but yeah, that's the kind of thing that the thermal framework exists to
> > do.
>
> Well, how do I do that? The driver is up and running, and I can
> manually set different fan speeds, however nothing seems to happen
> automatically when the temperature increases.
The easiest is to just do it from userspace. I think Intel have some
code for doing this, but I haven't looked at the thermal code for years.
> > I don't think you can easily register multiple drivers for the same WMI
> > device.
>
> I don't mean this one, I mean the standalone one. Actually, the first
> one I sent doesn't require all this system memory stuff.
Banging EC registers directly is the wrong thing to do. Going via WMI is
correct.
--
Matthew Garrett | [email protected]
On Mon, Oct 14, 2013 at 6:22 PM, Matthew Garrett <[email protected]> wrote:
> On Mon, Oct 14, 2013 at 06:18:36PM -0500, Felipe Contreras wrote:
>> On Mon, Oct 14, 2013 at 10:52 AM, Matthew Garrett <[email protected]> wrote:
>> > It wouldn't be appropriate to alter the firmware behaviour by default,
>> > but yeah, that's the kind of thing that the thermal framework exists to
>> > do.
>>
>> Well, how do I do that? The driver is up and running, and I can
>> manually set different fan speeds, however nothing seems to happen
>> automatically when the temperature increases.
>
> The easiest is to just do it from userspace. I think Intel have some
> code for doing this, but I haven't looked at the thermal code for years.
That defeats the purpose of the whole thermal binding infrastructure.
>> > I don't think you can easily register multiple drivers for the same WMI
>> > device.
>>
>> I don't mean this one, I mean the standalone one. Actually, the first
>> one I sent doesn't require all this system memory stuff.
>
> Banging EC registers directly is the wrong thing to do. Going via WMI is
> correct.
I'm not going to bother arguing against your absolutist rhetoric. The
fact is one patch can be applied, the other can't. Besides, nobody
said anything about banging EC registers directly.
--
Felipe Contreras
On Mon, Oct 14, 2013 at 06:27:33PM -0500, Felipe Contreras wrote:
> On Mon, Oct 14, 2013 at 6:22 PM, Matthew Garrett <[email protected]> wrote:
> > The easiest is to just do it from userspace. I think Intel have some
> > code for doing this, but I haven't looked at the thermal code for years.
>
> That defeats the purpose of the whole thermal binding infrastructure.
Not really, but I agree it's not ideal. If there's a mechanism to get
the system temperature via WMI then you could easily construct your own
thermal zone and associated cooling device, but otherwise you'd have to
provide a mechanism for exporting either the CPU information from
coretemp or the thermal zones from ACPI.
> >> > I don't think you can easily register multiple drivers for the same WMI
> >> > device.
> >>
> >> I don't mean this one, I mean the standalone one. Actually, the first
> >> one I sent doesn't require all this system memory stuff.
> >
> > Banging EC registers directly is the wrong thing to do. Going via WMI is
> > correct.
>
> I'm not going to bother arguing against your absolutist rhetoric. The
> fact is one patch can be applied, the other can't. Besides, nobody
> said anything about banging EC registers directly.
I'm sorry, you're right - you're calling ACPI methods directly instead.
This is still incorrect. The platform provides an exported interface,
and you should use that exported interface.
As long as Corentin doesn't object, I'm happy to merge this driver in
its current form (including virt_to_phys()) providing that it's wrapped
in CONFIG_STAGING, and assuming that you'll do the supporting work in
acpica. I'll pull it out again in 6 months or so if that hasn't been
fixed up. Fair?
--
Matthew Garrett | [email protected]
On Mon, Oct 14, 2013 at 6:34 PM, Matthew Garrett <[email protected]> wrote:
> On Mon, Oct 14, 2013 at 06:27:33PM -0500, Felipe Contreras wrote:
>> On Mon, Oct 14, 2013 at 6:22 PM, Matthew Garrett <[email protected]> wrote:
>> > The easiest is to just do it from userspace. I think Intel have some
>> > code for doing this, but I haven't looked at the thermal code for years.
>>
>> That defeats the purpose of the whole thermal binding infrastructure.
>
> Not really, but I agree it's not ideal.
So what's the point of thermal_zone_bind_cooling_device?
>> >> > I don't think you can easily register multiple drivers for the same WMI
>> >> > device.
>> >>
>> >> I don't mean this one, I mean the standalone one. Actually, the first
>> >> one I sent doesn't require all this system memory stuff.
>> >
>> > Banging EC registers directly is the wrong thing to do. Going via WMI is
>> > correct.
>>
>> I'm not going to bother arguing against your absolutist rhetoric. The
>> fact is one patch can be applied, the other can't. Besides, nobody
>> said anything about banging EC registers directly.
>
> I'm sorry, you're right - you're calling ACPI methods directly instead.
> This is still incorrect. The platform provides an exported interface,
> and you should use that exported interface.
Maybe, but I can call those same methods outside asus-wmi.
> As long as Corentin doesn't object, I'm happy to merge this driver in
> its current form (including virt_to_phys()) providing that it's wrapped
> in CONFIG_STAGING, and assuming that you'll do the supporting work in
> acpica. I'll pull it out again in 6 months or so if that hasn't been
> fixed up. Fair?
That's fair, I'll give that a try soon.
--
Felipe Contreras