vmatare / thinkfan

The minimalist fan control program
GNU General Public License v3.0
541 stars 62 forks source link

handling of multiple fans on Lenovo P50 #58

Open sassmann opened 6 years ago

sassmann commented 6 years ago

The P50 has 2 fans, lm-sensors shows:

thinkpad-isa-0000
Adapter: ISA adapter
fan1:        2316 RPM
fan2:        2332 RPM

in sysfs there is

# ls -al /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon3/
total 0
drwxr-xr-x 3 root root    0 Aug 14 13:39 .
drwxr-xr-x 3 root root    0 Aug 14 10:01 ..
lrwxrwxrwx 1 root root    0 Aug 14 10:01 device -> ../../../thinkpad_hwmon
-r--r--r-- 1 root root 4096 Aug 14 13:39 fan1_input
-r--r--r-- 1 root root 4096 Aug 14 10:01 fan2_input
-r--r--r-- 1 root root 4096 Aug 14 10:01 name
drwxr-xr-x 2 root root    0 Aug 14 13:39 power
-rw-r--r-- 1 root root 4096 Aug 14 13:39 pwm1
-rw-r--r-- 1 root root 4096 Aug 14 13:39 pwm1_enable
lrwxrwxrwx 1 root root    0 Aug 14 10:01 subsystem -> ../../../../../class/hwmon
-rw-r--r-- 1 root root 4096 Aug 14 10:01 uevent

It seems that thinkfan is only able to control fan1, fan2 keeps constant ~2300 RPM. thinkfan.conf used

tp_fan /proc/acpi/ibm/fan
hwmon /sys/devices/platform/coretemp.0/hwmon/hwmon4/temp1_input
(0,     0,      65)
(1,     55,     82)
(2,     65,     88)
(6,     82,     92)
(7,     70,     32767)

Is there a way to control both fans?

vmatare commented 6 years ago

No, currently thinkfan is designed to control one fan only. In theory, you can use dangerous mode (the -D switch that disables all sanity checks) to allow for two instances of thinkfan to be running at the same time, one for each fan. But I've never tried that, it might work or not. However, I am currently working on multifan support, but my spare time is limited so it might take a while.

vmatare commented 6 years ago

Oh btw, looking at your hwmon directory listing, I see that you only seem to have a pwm1 file, but not pwm2. This indicates that your hwmon driver can only control one fan, so that would be an additional issue that you'd have to resolve with the driver developer (thinkpad_acpi I guess).

ericvhileman commented 5 years ago

Would love to have this ability for the x1 extreme as well. Unsure why lm-sensors only detects one fan for me though...

vmatare commented 5 years ago

The thinkpad_acpi kernel module is probably responsible for detecting the fans. Before thinkfan can use them, they will have to be supported by thinkpad_acpi, so I guess you'll have to contact those authors/maintainers for that.

LukeFernandes commented 5 years ago

Hi there, just wondering if this is still in the pipeline? There are quite a few ThinkPad dual fan laptops now, especially in the P series, and without this functionality thinkfan is less useful than it could be.

vmatare commented 4 years ago

Well, are they supported by thinkpad_acpi or any other hwmon driver? Because if not then you have to talk to these developers first ;-)

bssb commented 4 years ago

I also have the issue where only 1 of the 2 fans is detected by thinkpad_acpi (on my P53), so I filed a bug here: https://bugzilla.kernel.org/show_bug.cgi?id=205405

voidworker commented 4 years ago

There are more and more dual-fan thinkpad laptops.

I use old thinkpad_acpi patch for 2-fan thinkpads, P50 and P70. I can't adapt it to P53, because I understand almost nothing in the code. But maybe someone can help P53 owners one day... TIP: if dmideocode | grep N1 output is non-empty, there is a chance this patch works for you.

I don't know why this patch has not been accepted into kernel, but I'm using it and it works (kernel version is 4.15):

# ls -al /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon3/
total 0
drwxr-xr-x 3 root root    0 Nov  8 22:33 .
drwxr-xr-x 3 root root    0 Oct 29 20:48 ..
lrwxrwxrwx 1 root root    0 Oct 29 20:48 device -> ../../../thinkpad_hwmon
-r--r--r-- 1 root root 4096 Oct 29 20:48 fan1_input
-r--r--r-- 1 root root 4096 Oct 29 20:48 fan2_input
-r--r--r-- 1 root root 4096 Oct 29 20:48 name
drwxr-xr-x 2 root root    0 Oct 29 20:50 power
-rw-r--r-- 1 root root 4096 Oct  8 22:33 pwm1
-rw-r--r-- 1 root root 4096 Oct  8 20:40 pwm1_enable
-rw-r--r-- 1 root root 4096 Oct  8 22:33 pwm2
-rw-r--r-- 1 root root 4096 Oct  8 20:40 pwm2_enable
lrwxrwxrwx 1 root root    0 Oct 29 20:50 subsystem -> ../../../../../class/hwmon
-rw-r--r-- 1 root root 4096 Oct 29 20:48 uevent

fancontrol now has support for (>1)-fan configurations, so I have to use it. (Also I have to rebuild thinkpad_acpi kernel module every time I upgrade kernel) The patch: https://pastebin.com/eVPcnvfK History: https://sourceforge.net/p/ibm-acpi/mailman/message/35005000/ my patched thinkpad_acpi.c https://gist.github.com/voidworker/12294117db5eb0ce663159a96e965093

TIP: If you are dummy like me, you can use tools like meld to compare vanilla and patched files and merge changes into vanilla version. But not all of them! There are some minor changes in vanilla since this patch was created.

bssb commented 4 years ago

@voidworker Wow thank you very much for this! I'll take a look and see if I can get it working with the P53.

ghost commented 4 years ago

I merged the patch posted by @voidworker and reinstalled the thinkpad_acpi module. The left fan (which was controlled by thinkfan) is now following the BIOS curve while the right one (which was previously unavailable) can now be set. The problem is that sensors still only lists fan1 which is now the right fan. The left one is now hidden and cannot be controlled anymore.

sensors:

thinkpad-isa-0000
Adapter: ISA adapter
fan1:           0 RPM <-- the left fan was spinning when this measurement was taken

ls -la /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon6:

lrwxrwxrwx     0 root  1 Dez  1:30  device -> ../../../thinkpad_hwmon
.r--r--r--  4.1k root  1 Dez  1:36  fan1_input
.r--r--r--  4.1k root  1 Dez  1:36  name
drwxr-xr-x     - root  1 Dez  1:36  power
.rw-r--r--  4.1k root  1 Dez  1:36  pwm1
.rw-r--r--  4.1k root  1 Dez  1:36  pwm1_enable
lrwxrwxrwx     0 root  1 Dez  1:36  subsystem -> ../../../../../class/hwmon
.r--r--r--  4.1k root  1 Dez  1:36  temp1_input
.r--r--r--  4.1k root  1 Dez  1:36  temp2_input
.r--r--r--  4.1k root  1 Dez  1:36  temp3_input
.r--r--r--  4.1k root  1 Dez  1:36  temp4_input
.r--r--r--  4.1k root  1 Dez  1:36  temp5_input
.r--r--r--  4.1k root  1 Dez  1:36  temp6_input
.r--r--r--  4.1k root  1 Dez  1:36  temp7_input
.r--r--r--  4.1k root  1 Dez  1:36  temp8_input
.r--r--r--  4.1k root  1 Dez  1:36  temp9_input
.r--r--r--  4.1k root  1 Dez  1:36  temp10_input
.r--r--r--  4.1k root  1 Dez  1:36  temp11_input
.r--r--r--  4.1k root  1 Dez  1:36  temp12_input
.r--r--r--  4.1k root  1 Dez  1:36  temp13_input
.r--r--r--  4.1k root  1 Dez  1:36  temp14_input
.r--r--r--  4.1k root  1 Dez  1:36  temp15_input
.r--r--r--  4.1k root  1 Dez  1:36  temp16_input
.rw-r--r--  4.1k root  1 Dez  1:36  uevent

I will try to understand the code which has been modified but if anyone manages to find a solution, please share.

I'm using this on an X1 Extreme Gen 2 by the way.

voidworker commented 4 years ago

I am sorry for my lack of expertise. Maybe it's worth asking the author of the patch? By the way, I have correct detection on my P50:

thinkpad-isa-0000
Adapter: ISA adapter
fan1:        1940 RPM
fan2:        2207 RPM
tobomobo commented 4 years ago

Hello guys. I am am a noob so please excuse me if this makes no sense.

Pop! OS supports Dual Fan Control on my Thinkpad P1 Gen 2 (X1 Extreme) but I want to use Manjaro KDE. Is it possible to extract the fan control from them?

Thanks in advance :)

bssb commented 4 years ago

@tobomobo If I were you I would stick with what's already working. Just install KDE on your PopOS.

tobomobo commented 4 years ago

@tobomobo If I were you I would stick with what's already working. Just install KDE on your PopOS.

I would love to stay on an Arch based Distro because of the AUR packages .

But thanks for the quick reply 😊

bssb commented 4 years ago

AUR is nice and convenient but it's not worth giving up the functionality of one of your fans...

tobomobo commented 4 years ago

I mean they work both. One fan is just a little bit louder and never stops spinning no matter what.

I think I can live with that, because I am mostly in an office Space where my Fan noise isn't the biggest issue.

Am So., 8. Dez. 2019 um 23:15 Uhr schrieb B notifications@github.com:

AUR is nice and convenient but it's not worth giving up the functionality of one of your fans...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vmatare/thinkfan/issues/58?email_source=notifications&email_token=ANY7FCVYKEWZ3HPDTZMZGK3QXVWZRA5CNFSM4FPRD7Q2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGHLSZI#issuecomment-563001701, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANY7FCWV7QJUCUP5ZCNIPULQXVWZRANCNFSM4FPRD7QQ .

civic9 commented 4 years ago

@voidworker Thanks for your info. After minor modifications it works for me on Arch, kernel 5.4.7, Thinkpad X1 Extreme 1st gen. If anyone is interested: https://github.com/civic9/thinkpad_acpi.2ndfan.patch

I use it with two instances of thinkfan, as @vmatare suggested. Everything works fine. Finally X1 extreme has become usable for me.

ghost commented 4 years ago

@civic9 Thank you for this! I was able to read both fans by changing the BIOS string in the fan_quirk_table from N2E to N2O.

Could you share your thinkfan config file? I am having some issues setting the correct speed. Also, how are you running two instances automatically? Did you modify the systemd service files?

civic9 commented 4 years ago

@niciuffo x1 extreme speed values: 0-31 (off), 32-63 - level 1, 64-95 - level 2, 96-127 - level 3, 128-159 - level 4, 160-191 - level 5, 192 - 255 - level 6 (max). my thinkpad.conf:

hwmon /sys/class/hwmon/hwmon6/temp1_input
pwm_fan /sys/class/hwmon/hwmon5/pwm1

(0,     0,      67)
(32,    65,     75)
(64,    73,     80)
(96,    78,     85)
(128,   83,     90)
(160,   88,     95)
(255,   93,     32767)

For 2nd thinkfan instance I created /etc/thinkfan2.conf (as above, but pwm2 in pwm_fan path) and /etc/systemd/system/thinkfan2.service:

[Unit]
Description=simple and lightweight fan control program - 2nd fan
Wants=lm_sensors.service
After=lm_sensors.service

[Service]
Type=forking
ExecStart=/usr/bin/thinkfan -b0 -c /etc/thinkfan2.conf -D
ExecReload=pkill -HUP thinkfan

[Install]
WantedBy=multi-user.target
Also=thinkfan-wakeup.service

In logs it shows a warning/error about existing pid file, but works fine.

sassmann commented 4 years ago

@civic9 Thank you for this! I was able to read both fans by changing the BIOS string in the fan_quirk_table from N2E to N2O.

@niciuffo Where exactly did you make that change?

ghost commented 4 years ago

@sassmann sorry forgot to reply, did you manage to fix this? You shouldn't have to change anything as the the string has been added on commit 2294baf.

sassmann commented 4 years ago

Ah, my bad. The patch isn't upstream yet.

tobomobo commented 4 years ago

Hello there. Noob alert.

Is someone willing to explain step by step how to do this? (Using a P1 Gen2)

Thanks in advance! :)

lhofhansl commented 4 years ago

Hey all. I looked at the Kernel code and found a better solution. Instead of creating two fan control devices I just have the one device set both fan controls. That way a single thinkfan instance it sufficient. And you rarely want different levels for the fans anyway. (and the code is simpler).

Tried on my x1 extreme gen2, and it works beautifully.

Here's the patch:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da794dcfdd92..282b97f39074 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -8326,6 +8326,11 @@ static int fan_set_level(int level)
    switch (fan_control_access_mode) {
    case TPACPI_FAN_WR_ACPI_SFAN:
        if (level >= 0 && level <= 7) {
+           fan_select_fan2();
+           if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
+               return -EIO;
+
+           fan_select_fan1();
            if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
                return -EIO;
        } else
@@ -8346,10 +8351,18 @@ static int fan_set_level(int level)
        else if (level & TP_EC_FAN_AUTO)
            level |= 4; /* safety min speed 4 */

+       fan_select_fan2();
        if (!acpi_ec_write(fan_status_offset, level))
            return -EIO;
        else
            tp_features.fan_ctrl_status_undef = 0;
+
+       fan_select_fan1();
+       if (!acpi_ec_write(fan_status_offset, level))
+           return -EIO;
+       else
+           tp_features.fan_ctrl_status_undef = 0;
+
        break;

    default:
@@ -8772,6 +8785,7 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
    TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
    TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
    TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
+   TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2FAN),  /* X1 Extreme (2nd gen) */
 };

 static int __init fan_init(struct ibm_init_struct *iibm)
@@ -8813,8 +8827,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
                fan_quirk1_setup();
            if (quirks & TPACPI_FAN_2FAN) {
                tp_features.second_fan = 1;
-               dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
-                   "secondary fan support enabled\n");
+               pr_info("secondary fan support enabled\n");
            }
        } else {
            pr_err("ThinkPad ACPI EC access misbehaving, fan status and control unavailable\n");
tobomobo commented 4 years ago

Hey all. I looked at the Kernel code and found a better solution. Instead of creating two fan control devices I just have the one device set both fan controls. That way a single thinkfan instance it sufficient. And you rarely want different levels for the fans anyway. (and the code is simpler).

Tried on my x1 extreme gen2, and it works beautifully.

Here's the patch:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da794dcfdd92..282b97f39074 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -8326,6 +8326,11 @@ static int fan_set_level(int level)
  switch (fan_control_access_mode) {
  case TPACPI_FAN_WR_ACPI_SFAN:
      if (level >= 0 && level <= 7) {
+         fan_select_fan2();
+         if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
+             return -EIO;
+
+         fan_select_fan1();
          if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
              return -EIO;
      } else
@@ -8346,10 +8351,18 @@ static int fan_set_level(int level)
      else if (level & TP_EC_FAN_AUTO)
          level |= 4; /* safety min speed 4 */

+     fan_select_fan2();
      if (!acpi_ec_write(fan_status_offset, level))
          return -EIO;
      else
          tp_features.fan_ctrl_status_undef = 0;
+
+     fan_select_fan1();
+     if (!acpi_ec_write(fan_status_offset, level))
+         return -EIO;
+     else
+         tp_features.fan_ctrl_status_undef = 0;
+
      break;

  default:
@@ -8772,6 +8785,7 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
  TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
  TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
  TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
+ TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2FAN),  /* X1 Extreme (2nd gen) */
 };

 static int __init fan_init(struct ibm_init_struct *iibm)
@@ -8813,8 +8827,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
              fan_quirk1_setup();
          if (quirks & TPACPI_FAN_2FAN) {
              tp_features.second_fan = 1;
-             dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
-                 "secondary fan support enabled\n");
+             pr_info("secondary fan support enabled\n");
          }
      } else {
          pr_err("ThinkPad ACPI EC access misbehaving, fan status and control unavailable\n");

Hey thanks for improving the solution but unfortunately I am still stuck.

Could you explain to me how to apply this patch or link me to the right tutorial?

Help is greatly appreciated! 😊

lhofhansl commented 4 years ago

You have to rebuild the thinkpad_acpi kernel module. With my change that's all you need. Thinkfan will see a single fan, any changes will apply ton both fans.

I'll add detailed instructions later. @civic9 has a patch script linked above, I got the BIOS quirks from there. Thanks @civic9 !

For the actual change I looked into tp-thinkfan (for Windows) and just added the right parts to the patch.

lhofhansl commented 4 years ago

Update patch:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da794dcfdd92..bd24642ba0f6 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -8320,13 +8320,18 @@ static int fan2_get_speed(unsigned int *speed)

 static int fan_set_level(int level)
 {
+   bool result1, result2;
+
    if (!fan_control_allowed)
        return -EPERM;

    switch (fan_control_access_mode) {
    case TPACPI_FAN_WR_ACPI_SFAN:
        if (level >= 0 && level <= 7) {
-           if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
+           result2 = fan_select_fan2() && acpi_evalf(sfan_handle, NULL, NULL, "vd", level);
+           result1 = fan_select_fan1() && acpi_evalf(sfan_handle, NULL, NULL, "vd", level);
+
+           if (!result1 || !result2)
                return -EIO;
        } else
            return -EINVAL;
@@ -8346,7 +8351,10 @@ static int fan_set_level(int level)
        else if (level & TP_EC_FAN_AUTO)
            level |= 4; /* safety min speed 4 */

-       if (!acpi_ec_write(fan_status_offset, level))
+       result2 = fan_select_fan2() && acpi_ec_write(fan_status_offset, level);
+       result1 = fan_select_fan1() && acpi_ec_write(fan_status_offset, level);
+
+       if (!result1 || !result2)
            return -EIO;
        else
            tp_features.fan_ctrl_status_undef = 0;
@@ -8772,6 +8780,14 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
    TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
    TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
    TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
+   TPACPI_Q_LNV3('N', '1', 'D', TPACPI_FAN_2FAN),  /* P70 */
+   TPACPI_Q_LNV3('N', '1', 'T', TPACPI_FAN_2FAN),  /* P71 */
+   TPACPI_Q_LNV3('N', '2', 'C', TPACPI_FAN_2FAN),  /* P72 */
+   TPACPI_Q_LNV3('N', '1', 'E', TPACPI_FAN_2FAN),  /* P50 */
+   TPACPI_Q_LNV3('N', '1', 'U', TPACPI_FAN_2FAN),  /* P51 */
+   TPACPI_Q_LNV3('N', '2', 'C', TPACPI_FAN_2FAN),  /* P52 */
+   TPACPI_Q_LNV3('N', '2', 'E', TPACPI_FAN_2FAN),  /* X1 Extreme (1st gen) */
+   TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2FAN),  /* X1 Extreme (2nd gen) */
 };

 static int __init fan_init(struct ibm_init_struct *iibm)
@@ -8813,8 +8829,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
                fan_quirk1_setup();
            if (quirks & TPACPI_FAN_2FAN) {
                tp_features.second_fan = 1;
-               dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
-                   "secondary fan support enabled\n");
+               pr_info("secondary fan support enabled\n");
            }
        } else {
            pr_err("ThinkPad ACPI EC access misbehaving, fan status and control unavailable\n");

Added all the known bios version (again thanks @civic9 ), and made the error handling slightly more robust.

For instructions... It's really best to look at @civic9 's build script: https://github.com/civic9/thinkpad_acpi.2ndfan.patch/blob/master/build_install.sh

All I would do here is to write out the very same instructions. You can do all these commands manually.

Cut the patch above into some file and patch it into the kernel source (as shown the script).

tobomobo commented 4 years ago

Thanks for you help.

I am new to this. So sorry if this is a head bump question haha.

My problem is, when I start the scrip and the kernel gets pulled the terminal asks me different things. The first one is, if I want to "Compile also drivers which will not load" and many others. Do I just close out now or do I have to press yes to some things in terminal?

EDIT: after manually entering the commands from the script I get these two errors:

cp: cannot stat '/usr/lib/modules/5.5.13-1-MANJARO/build/.config': No such file or directory

cp: cannot stat '/usr/lib/modules/5.5.13-1-MANJARO/build/Module.symvers': No such file or directory

lhofhansl commented 4 years ago

No problem at all!

For all the questions... It's the first time you do make oldconfig just take all the defaults, it doesn't matter for this module.

Manjaro is based off Arch, so in theory it should mostly work the same. (I have Fedora and building the kernel and installing kernel modules is almost the same).

I assume that happens when you do depmod -a...? Can you tell me which directory is missing in the path (/usr/lib/modules/5.5.13-1-MANJARO/build/.config)? What's the output of $ ls /usr/lib/modules/ ?

You might need install a package to have that. On Fedora is kernel-core, but I do not know how it work on Arch/Manjaro.

oyhel commented 4 years ago

When trying to apply the patch (using build_install.sh from civic9) I get the error below. Could anyone help? Noob here obviously...

...
AR      drivers/platform/x86/built-in.a

  WARNING: Symbol version dump ./Module.symvers
           is missing; modules will have no dependencies and modversions.

  Building modules, stage 2.
  MODPOST 0 modules
xz: drivers/platform/x86/thinkpad_acpi.ko: No such file or directory
...

edit: so i managed to find the Module.symvers and copying the file to the root of the source made the warning disappear. However, thinkpad_acpi.ko is not being generated.

  AR      drivers/platform/x86/built-in.a
  Building modules, stage 2.
  MODPOST 0 modules
xz: drivers/platform/x86/thinkpad_acpi.ko: No such file or directory
mkdir: cannot create directory ‘/usr/lib/modules/5.4.24-1-MANJARO/updates’: File exists
cp: cannot stat 'drivers/platform/x86/thinkpad_acpi.ko.xz': No such file or directory

edit: SOLVED: Turned out to be a problem using the refind bootloader. When booting using grub the script worked expect for a small edit to the build script where I had to manually specify the location of the patch file and thinkpad_acpi.c (#patch -p1 < ../../../../thinkpad_acpi.2ndfan.patch/thinkpad_acpi.2ndfan.patch did not work).

edit: forked and updated with lhofhansl patch: https://github.com/oyhel/thinkpad_acpi.2ndfan.patch

confirmed working with X1 Extreme using thinkfan-git (thinkfan-git 1.0.2.r16.7027a2d-1)

Fan profile in /etc/thinkfan.conf: (0, 0, 67) (1, 65, 75) (2, 73, 80) (3, 78, 85) (4, 83, 90) (5, 88, 95) (7, 93, 32767)

edit: @lhofhansl patch now also pushed.

lhofhansl commented 4 years ago

Thanks @oyhel , I think you forgot to actually merge my change :) (at least I do not see it in your repo)

lhofhansl commented 4 years ago

I'll work with @civic9 to make a single patch, credit all the right people, and then try to get it into the Kernel.

lhofhansl commented 4 years ago

This is the Kernel patch I want to propose:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da794dcfdd92..8d46b4c2bde8 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -8325,11 +8325,20 @@ static int fan_set_level(int level)

    switch (fan_control_access_mode) {
    case TPACPI_FAN_WR_ACPI_SFAN:
-       if (level >= 0 && level <= 7) {
-           if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
-               return -EIO;
-       } else
+       if (((level < 0) || (level > 7)))
            return -EINVAL;
+
+       if (tp_features.second_fan) {
+           if (!fan_select_fan2() ||
+               !acpi_evalf(sfan_handle, NULL, NULL, "vd", level)) {
+               fan_select_fan1();
+               pr_warn("Couldn't set 2nd fan level, disabling support\n");
+               tp_features.second_fan = 0;
+           }
+           fan_select_fan1();
+       }
+       if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
+           return -EIO;
        break;

    case TPACPI_FAN_WR_ACPI_FANS:
@@ -8346,6 +8355,16 @@ static int fan_set_level(int level)
        else if (level & TP_EC_FAN_AUTO)
            level |= 4; /* safety min speed 4 */

+       if (tp_features.second_fan) {
+           if (!fan_select_fan2() ||
+               !acpi_ec_write(fan_status_offset, level)) {
+               fan_select_fan1();
+               pr_warn("Couldn't set 2nd fan level, disabling support\n");
+               tp_features.second_fan = 0;
+           }
+           fan_select_fan1();
+
+       }
        if (!acpi_ec_write(fan_status_offset, level))
            return -EIO;
        else
@@ -8772,6 +8791,9 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
    TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
    TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
    TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
+   TPACPI_Q_LNV3('N', '2', 'C', TPACPI_FAN_2FAN),  /* P52 / P72 */
+   TPACPI_Q_LNV3('N', '2', 'E', TPACPI_FAN_2FAN),  /* X1 Extreme (1st gen) */
+   TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2FAN),  /* X1 Extreme (2nd gen) */
 };

 static int __init fan_init(struct ibm_init_struct *iibm)
@@ -8813,8 +8835,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
                fan_quirk1_setup();
            if (quirks & TPACPI_FAN_2FAN) {
                tp_features.second_fan = 1;
-               dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
-                   "secondary fan support enabled\n");
+               pr_info("secondary fan support enabled\n");
            }
        } else {
            pr_err("ThinkPad ACPI EC access misbehaving, fan status and control unavailable\n");
BeiNacht commented 4 years ago

Working with X1 Extreme Gen1 and Kernel 5.6.3

Much better solution. Had many problems with running multiple running thinkfan instances.

lhofhansl commented 4 years ago

So it seems we have confirmed that this works at least on the X1E and X1E gen2.

Is there anybody out here who tried or can try it on other models (P52/P72) and older models? Would be good to have a confirmed list to post to the Kernel mailing list.

lhofhansl commented 4 years ago

I'm afraid without any further reports of working laptops the Kernel folks (perhaps rightly so) will just ignore my patch.

lhofhansl commented 4 years ago

@sassmann do you still have your P50? Could you try the patch?

sassmann commented 4 years ago

@lhofhansl really appreciate your effort. I'm planning to test the latest patch soon. Is the patch available somewhere or have you posted it somewhere already, I'm not sure how to properly extract it from the github comment above.

vmatare commented 4 years ago

Guys, I haven't been tracking the progress on the thinkpad_acpi front, I hope I'll get to that soon. Just wanted to remind you that thinkfan has a multifan branch which adds support for multiple fans. It's been working well for me for a while now on a large desktop system with 4 temperature sensors and 7 fans.

lhofhansl commented 4 years ago

@vmatare Thanks. I suppose my proposal of treating the fans as a single control is then not necessary. Without a lot of refactoring it is just seemed cleaner than the initial patch (the proc interface does not work anymore for example).

vmatare commented 4 years ago

@lhofhansl As far as I can see, the sysfs interface of thinkpad_acpi supports all features by now. If not, then it's probably in the process of being ported, so I'd consider the /proc/acpi/ibm interface dead, at least in the long run. If thinkpad_acpi does support multiple fans via /sys/class/hwmon by now (does it?), we're fine and backporting it to /proc/acpi/ibm is unnecessary and unlikely to happen, precisely because we're going to be moving away from these non-standard interfaces.

lhofhansl commented 4 years ago

@vmatare Unfortunately thinkpad_acpi does not support (1) reading multiple fans on new H/W (that can be fixed by just adding more BIOS versions of the quirks), and (2) controlling multiple fans on any H/W (only one of the fans is changed, the other continues to do whatever the BIOS is default is).

So on Thinkpads multi-fan support is pretty much not functional unless we get a Kernel patch in. :(

Maybe I'll look at the initial multi-fan control patch again. Do you think that is better? All my tests with the default BIOS curve always had both fans handled in exactly the same way (rev up/down exactly the same time, and same level). That is on my X1 Extreme Gen2.

sassmann commented 4 years ago

@lhofhansl where is the latest version of the patch stored?

peter-stoll commented 4 years ago

Since the P1 (Gen 1 and Gen 2 ) is almost identical to the X1E (Gen 1 and Gen 2 resp.) , can you please add the P1 (gen 1 and 2) to the list of supported machines? I own a P1 Gen 2 and can provide information about that device and testing if you tell me what you need.

ratcashdev commented 4 years ago

The acpi-patch finally made my laptop silent (X1E Gen 1, kernel 5.6.3) ! Thanks a lot for it On a related note, can someone explain, why do I only see 2 real temp measurements when doing:

$ cat /proc/acpi/ibm/thermal
temperatures:   65 65 0 1 0 0 0 0 0 0 1 0 0 0 0 0

The results is pretty much the same for cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon7/temp*_input However, s-tui is able to show temps per cpu core, package, etc. Is using temp1_input enough in the config? Does anyone have a more elaborate config for X1E/P1?

lhofhansl commented 4 years ago

@sassmann It's the patch I added in my "This is the Kernel patch I want to propose:" comment above. Also see @andy-shev 's referenced commit above.

@peter-stoll The P1/X1E gen1 are supported via this existing quirk TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN), the patch extends their functionality to fan control (instead of just querying)

@ratcashdev good to know! Thanks for testing with a Gen 1 machine!

LukeFernandes commented 4 years ago

I'm afraid without any further reports of working laptops the Kernel folks (perhaps rightly so) will just ignore my patch.

I have a P72 and would like to confirm this is working. Thanks for all the hard work. I've been keeping an eye on this since I commented last year.

However, I'm not very good with Linux - if my kernel is v 4.19 can I still apply the patch? Any Arch/Manjaro specific tips?

LukeFernandes commented 4 years ago

cp: cannot stat '/usr/lib/modules/5.5.13-1-MANJARO/build/.config': No such file or directory

cp: cannot stat '/usr/lib/modules/5.5.13-1-MANJARO/build/Module.symvers': No such file or directory

I have the same issue as @tobomobo above...

lhofhansl commented 4 years ago

@tobomobo @LukeFernandes I do not know any specifics about Manjaro. :(

If you build the modules the first time, perhaps you'll need to run make config first. (You can just hit enter through all the defaults - doesn't matter for the module) After that make oldconfig as in the script is all you need.

Does that help?

Re: 4.19... Lemme try (or have you managed to apply the patch?)

sassmann commented 4 years ago

@lhofhansl I've successfully tested the patch with my P50. Had to add the quirk for P50 to the fan_quirk_table. Patch can be found at https://github.com/sassmann/linux/commit/5440eb26d7ba1995f39df577830cfd3e9faaf581