Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading
@ 2019-11-12 21:41 Giovanni Mascellani
  2019-11-12 22:28 ` Valdis Klētnieks
  0 siblings, 1 reply; 5+ messages in thread
From: Giovanni Mascellani @ 2019-11-12 21:41 UTC (permalink / raw)
  To: kernelnewbies; +Cc: Giovanni Mascellani

Hello,

I would like to submit the following patch for inclusion in Linux. Since
it is my first Linux patch, I would like to have a preliminary review
here, if possible.

I have a question in particular: I would like the functions
i8k_disable_bios() and i8k_enable_bios() to be called when the computer
is about to enter or is recovering suspension to RAM, or is recovering
from hibernation to disk. How can I do that?

The original commit message follows.

Thanks, Giovanni.


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 at module loading, and
re-enable it at module unloading, so that a userspace controller like
i8kmon can take control of the fan.

Signed-off-by: Giovanni Mascellani <gio@debian.org>
---
 drivers/hwmon/dell-smm-hwmon.c | 61 ++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 4212d022d253..6110558c8609 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,7 @@ 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;
 
 #define I8K_HWMON_HAVE_TEMP1	(1 << 0)
 #define I8K_HWMON_HAVE_TEMP2	(1 << 1)
@@ -419,6 +428,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(&regs) ? : 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(&regs) ? : regs.eax & 0xff;
+}
+
 /*
  * Procfs interface
  */
@@ -1135,6 +1164,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 +1214,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, disabling BIOS fan control\n");
+		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),
@@ -1218,6 +1268,11 @@ static int __init i8k_probe(void)
 		i8k_fan_mult = fan_mult;
 	}
 
+	if (disable_bios) {
+		if (i8k_disable_bios())
+			pr_warn("could not disable BIOS fan control\n");
+	}
+
 	return 0;
 }
 
@@ -1241,6 +1296,12 @@ static void __exit i8k_exit(void)
 {
 	hwmon_device_unregister(i8k_hwmon_dev);
 	i8k_exit_procfs();
+
+	if (disable_bios) {
+		pr_warn("re-enabling BIOS fan control\n");
+		if (i8k_enable_bios())
+			pr_warn("could not re-enable BIOS fan control\n");
+	}
 }
 
 module_init(i8k_init);
-- 
2.24.0


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading
  2019-11-12 21:41 [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading Giovanni Mascellani
@ 2019-11-12 22:28 ` Valdis Klētnieks
  2019-11-13  7:17   ` Giovanni Mascellani
  0 siblings, 1 reply; 5+ messages in thread
From: Valdis Klētnieks @ 2019-11-12 22:28 UTC (permalink / raw)
  To: Giovanni Mascellani; +Cc: kernelnewbies

[-- Attachment #1.1: Type: text/plain, Size: 464 bytes --]

On Tue, 12 Nov 2019 22:41:28 +0100, Giovanni Mascellani said:

> Disable BIOS fan control on such laptops at module loading, and
> re-enable it at module unloading, so that a userspace controller like
> i8kmon can take control of the fan.

The code as written appears to make the disabling unconditional,
which means that if a userspace controller *isn't* started things will
get toasty really fast. Probably need a module parameter or something
to control that.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading
  2019-11-12 22:28 ` Valdis Klētnieks
@ 2019-11-13  7:17   ` Giovanni Mascellani
  2019-11-13  8:15     ` Ilie Halip
  0 siblings, 1 reply; 5+ messages in thread
From: Giovanni Mascellani @ 2019-11-13  7:17 UTC (permalink / raw)
  To: kernelnewbies

[-- Attachment #1.1.1: Type: text/plain, Size: 668 bytes --]

Hi,

Il 12/11/19 23:28, Valdis Klētnieks ha scritto:
> The code as written appears to make the disabling unconditional,
> which means that if a userspace controller *isn't* started things will
> get toasty really fast. Probably need a module parameter or something
> to control that.

Good point, thank you. If I add a module parameter, how would you
suggest to make it interact with the dmi_table? Remove the dmi_table
altogether? Send the BIOS disabling code only when both the dmi_table
and the module parameter are enabled?

Thanks, Giovanni.
-- 
Giovanni Mascellani <g.mascellani@gmail.com>
Postdoc researcher - Université Libre de Bruxelles


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading
  2019-11-13  7:17   ` Giovanni Mascellani
@ 2019-11-13  8:15     ` Ilie Halip
  2019-11-13  8:31       ` Giovanni Mascellani
  0 siblings, 1 reply; 5+ messages in thread
From: Ilie Halip @ 2019-11-13  8:15 UTC (permalink / raw)
  To: Giovanni Mascellani; +Cc: kernelnewbies

> Send the BIOS disabling code only when both the dmi_table
> and the module parameter are enabled?

The module parameter value, when specified, should be used.
When it is not specified, you probably still want to check the
dmi_table.

Coincidentally, I have a Dell laptop model which might be
affected by a similar issue. I'd be happy to test your patch.

I.H.

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading
  2019-11-13  8:15     ` Ilie Halip
@ 2019-11-13  8:31       ` Giovanni Mascellani
  0 siblings, 0 replies; 5+ messages in thread
From: Giovanni Mascellani @ 2019-11-13  8:31 UTC (permalink / raw)
  To: kernelnewbies

[-- Attachment #1.1.1: Type: text/plain, Size: 1384 bytes --]

Hi,

Il 13/11/19 09:15, Ilie Halip ha scritto:
> The module parameter value, when specified, should be used.
> When it is not specified, you probably still want to check the
> dmi_table.

Hmm, that does not solve the problem mentioned by Valdis: if there is no
userspace daemon piloting the fan, the module should not do anything, so
that the BIOS still has a chance to regulate the fan (which might not
mean much: the BIOS seems to be very buggy, and sometimes it won't speed
up the fan even when all the cores are busy compiling C++ for minutes
and very hot; but at least that's not the kernel module's fault).

Maybe another approach is to only disable the BIOS code when the first
SET_FAN command is received, so I know that there is someone in
userspace. Of course I am not protected from the possibility that the
userspace daemon sends wrong commands or crashes, but I am not protected
anyway. I rely on other lower level measures (such as, the CPU powering
down then it crosses a critical threshold) to protect the hardware from
this scenario.

Is this a reasonable solution?

> Coincidentally, I have a Dell laptop model which might be
> affected by a similar issue. I'd be happy to test your patch.

That would be good!

Thanks, Giovanni.
-- 
Giovanni Mascellani <g.mascellani@gmail.com>
Postdoc researcher - Université Libre de Bruxelles


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 21:41 [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control upon loading Giovanni Mascellani
2019-11-12 22:28 ` Valdis Klētnieks
2019-11-13  7:17   ` Giovanni Mascellani
2019-11-13  8:15     ` Ilie Halip
2019-11-13  8:31       ` Giovanni Mascellani

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org
	public-inbox-index kernelnewbies

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git