On Thu, Nov 14, 2019 at 10:14:08PM +0100, Giovanni Mascellani wrote: > On some Dell laptops the BIOS directly controls fan speed, ignoring > SET_FAN commands. Also, the BIOS controller often happens to be buggy, > failing to increase fan speed when the CPU is under heavy load and > setting fan at full speed even when the CPU is idle and relatively > cool. > > Disable BIOS fan control on such laptops as soon as a SET_FAN command > is issued, and re-enable it at module unloading, so that a userspace > controller like i8kmon can take control of the fan. > > There is a missing feature: interaction with PM; I think that when > suspending on RAM the fans should be stopped (this BIOS doesn't always > do this automatically, neither when fan control is enabled nor when it > is disabled); when recovering from hibernation to disk, also, the > command to disable BIOS fan control should be issued again, because > the computer will have had a power cycle in the meantime. > > I don't know how to implement these two actions; can someone suggest a > way? Also, I would be happy to know from more experienced people if > these actions are sensible. > I think this is too drastic: Not everyone wants to use this driver for fan control, and even automatic switching to manual mode on write operations may be unexpected. I can see two possibilities: Either add a pwm1_enable attribute to be able to set manual/automatic fan control, or add a module parameter to enable manual fan speed control on affected systems. As for PM support, this would have to be implemented using the standard PM functions. Guenter > Signed-off-by: Giovanni Mascellani <gio@xxxxxxxxxx> > --- > drivers/hwmon/dell-smm-hwmon.c | 64 ++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c > index 4212d022d253..6d72e207076f 100644 > --- a/drivers/hwmon/dell-smm-hwmon.c > +++ b/drivers/hwmon/dell-smm-hwmon.c > @@ -32,6 +32,12 @@ > > #include <linux/i8k.h> > > +/* > + * The code for enabling and disabling BIOS fan control were found by > + * trial and error with the program at > + * https://github.com/clopez/dellfan. > + */ > + > #define I8K_SMM_FN_STATUS 0x0025 > #define I8K_SMM_POWER_STATUS 0x0069 > #define I8K_SMM_SET_FAN 0x01a3 > @@ -43,6 +49,8 @@ > #define I8K_SMM_GET_TEMP_TYPE 0x11a3 > #define I8K_SMM_GET_DELL_SIG1 0xfea3 > #define I8K_SMM_GET_DELL_SIG2 0xffa3 > +#define I8K_SMM_DISABLE_BIOS 0x30a3 > +#define I8K_SMM_ENABLE_BIOS 0x31a3 > > #define I8K_FAN_MULT 30 > #define I8K_FAN_MAX_RPM 30000 > @@ -68,6 +76,8 @@ static uint i8k_pwm_mult; > static uint i8k_fan_max = I8K_FAN_HIGH; > static bool disallow_fan_type_call; > static bool disallow_fan_support; > +static bool disable_bios; > +static bool bios_disabled; > > #define I8K_HWMON_HAVE_TEMP1 (1 << 0) > #define I8K_HWMON_HAVE_TEMP2 (1 << 1) > @@ -419,6 +429,26 @@ static int i8k_get_power_status(void) > return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY; > } > > +/* > + * Disable BIOS fan control. > + */ > +static int i8k_disable_bios(void) > +{ > + struct smm_regs regs = { .eax = I8K_SMM_DISABLE_BIOS, .ebx = 0 }; > + > + return i8k_smm(®s) ? : regs.eax & 0xff; > +} > + > +/* > + * Enable BIOS fan control. > + */ > +static int i8k_enable_bios(void) > +{ > + struct smm_regs regs = { .eax = I8K_SMM_ENABLE_BIOS, .ebx = 0 }; > + > + return i8k_smm(®s) ? : regs.eax & 0xff; > +} > + > /* > * Procfs interface > */ > @@ -488,6 +518,13 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg) > if (copy_from_user(&speed, argp + 1, sizeof(int))) > return -EFAULT; > > + if (disable_bios && !bios_disabled) { > + val = i8k_disable_bios(); > + if (val < 0) > + return val; > + bios_disabled = true; > + } > + > val = i8k_set_fan(val, speed); > break; > > @@ -1135,6 +1172,22 @@ static struct dmi_system_id i8k_blacklist_fan_support_dmi_table[] __initdata = { > { } > }; > > +/* > + * On some machines the BIOS disregards all SET_FAN commands unless it > + * is explicitly disabled. > + * See bug: https://bugzilla.kernel.org/show_bug.cgi?id=200949 > + */ > +static struct dmi_system_id i8k_disable_bios_dmi_table[] __initdata = { > + { > + .ident = "Dell Precision 5530", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Precision 5530"), > + }, > + }, > + { } > +}; > + > /* > * Probe for the presence of a supported laptop. > */ > @@ -1169,6 +1222,11 @@ static int __init i8k_probe(void) > disallow_fan_type_call = true; > } > > + if (dmi_check_system(i8k_disable_bios_dmi_table)) { > + pr_warn("broken Dell BIOS detected, will disable BIOS fan control\n"); The above is your interpretation: Since there is a command to enable/disable BIOS fan control, we can not just claim that it is broken because we don't like how it works. > + disable_bios = true; > + } > + > strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION), > sizeof(bios_version)); > strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL), > @@ -1241,6 +1299,12 @@ static void __exit i8k_exit(void) > { > hwmon_device_unregister(i8k_hwmon_dev); > i8k_exit_procfs(); > + > + if (bios_disabled) { > + pr_warn("re-enabling BIOS fan control\n"); I don't see the point of this warning. > + if (i8k_enable_bios()) > + pr_warn("could not re-enable BIOS fan control\n"); > + } > } > > module_init(i8k_init); > -- > 2.24.0 >
