BeardOverflow / msi-ec

GNU General Public License v2.0
149 stars 45 forks source link

Implement Fan Thermal Curve #143

Open Waujito opened 4 months ago

Waujito commented 4 months ago

Hi! I hope this PR will be helpful as a reference implementation of the fan thermal curve in further development. So, I got tired of always having hot keys, unstable MControlCenter (it seems to have a tendency to switch to msi-ec, so it needs this feature too) and had implemented the curve myself in this module. This implementation still needs a lot of testing and is only available for my laptop now, but I think there is enough information to port it to other devices (idk about WMI1 but WMI2 seems to be ok). So, how it is: The module creates /cpu/curve and /gpu/curve attributes, which are plain text files with curve data. The data is stored in the following order: s0 t1 s1 t2 s2 ... t(n-1) s(n-1) t(n) s(n)\n where s is a fan speed, t is a temperature. Note that there is no t0 because it is less than t1 (in fact there is likely a field for t0 in EC, which defaults to 0 but I think the behavior for temperatures less than it is undefined, so it is safer to just omit it). Previously I used to think that a more sysfs-ish way to represent the curve is a separate attribute group with s0 t1 ... sn tn attributes but this involves partial updates, while the user expects atomicity. Also there were thoughts about bin_attribute with curve representation in binary format (without spaces and each character represents curves parameter) but this involves too much overhead and is hard to read/write for the user.

Also about the implementation: It is different from other attributes because it is stored in memory and syncs with EC lazily. I did this because of #138 where we confirmed that the fan speed is broken on some devices when a non-default curve is in EC in non-advanced fan_mode. So on such devices curve fallback to default values is available: in non-advanced mode the curve keeps its default value, but the end user can virtually change it. /*/curve attributes are available but do not affect EC (they goes on when advanced mode is enabled).

heeen commented 2 months ago

this PR appears to work for my 16V5EMS1.108 GS66-12UHS. as for the interface, maybe a leading t0=0 in the curve file would make it easier to read for humans

Waujito commented 2 months ago

I have been using the curve for about 2 months and can say that it works fine. The only noticeable thing is curve override after suspend. But I don't think this is driver's problem, as we can easily fix it in userspace, like this:

[Unit]
Description=Restart curve after resume
After=suspend.target

[Service]
Type=simple
ExecStart=/bin/systemctl --no-block restart custom_curve.service

[Install]
WantedBy=suspend.target

custom_curve is just a service that pushes the curve to msi-ec.

About that leading t0=0 I think I can add it, but the driver should check it to be 0 always.

heeen commented 2 months ago

would you mind posting your systemd service files?

Waujito commented 2 months ago

would you mind posting your systemd service files?

They are just simple scripts to set up and off the curve: /etc/systemd/system/custom_curve.service

[Unit]
Description=CustomCurve

[Service]
Type=oneshot
ExecStart=/bin/curve_apply
RemainAfterExit=true
ExecStop=/bin/curve_stop
ExecReload=/bin/curve_apply

[Install]
WantedBy=multi-user.target

/etc/systemd/system/cscurve_suspend.service

[Unit]
Description=Reset curve after resume
After=suspend.target

[Service]
Type=simple
ExecStart=/bin/systemctl --no-block restart custom_curve.service

[Install]
WantedBy=suspend.target

/bin/curve_apply

#! /bin/bash
set -e
curve="0 50 41 55 55 60 65 65 85 68 100 80 120"
echo $curve > /sys/devices/platform/msi-ec/cpu/curve
echo $curve > /sys/devices/platform/msi-ec/gpu/curve
echo advanced > /sys/devices/platform/msi-ec/fan_mode

/bin/curve_stop

#!/bin/bash
set -e
echo auto > /sys/devices/platform/msi-ec/fan_mode