| Message ID | 20200423215709.72993-1-larsh@apache.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v4] thinkpad_acpi: Add support for dual fan control on select models | expand |
On Fri, Apr 24, 2020 at 12:57 AM Lars <larsh@apache.org> wrote: > > This adds dual fan control for the following models: > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > Both fans are controlled together as if they were a single fan. > > Tested on an X1 Extreme Gen1, an X1 Extreme Gen2, and a P50. > > The patch is defensive, it adds only specific supported machines, and falls > back to the old behavior if both fans cannot be controlled. > > Background: > I tested the BIOS default behavior on my X1E gen2 and both fans are always > changed together. So rather than adding controls for each fan, this controls > both fans together as the BIOS would do. > > This was inspired by a discussion on dual fan support for the thinkfan tool > [1]. > All BIOS ids are taken from there. The X1E gen2 id is verified on my machine. > > Thanks to GitHub users voidworker and civic9 for the earlier patches and BIOS > ids, and to users peter-stoll and sassman for testing the patch on their > machines. > Pushed to my review and testing queue, thank you! > [1]: https://github.com/vmatare/thinkfan/issues/58 > > Signed-off-by: Lars <larsh@apache.org> > --- > drivers/platform/x86/thinkpad_acpi.c | 43 ++++++++++++++++++++++++---- > 1 file changed, 37 insertions(+), 6 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index da794dcfdd92..9e0f65e567be 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -318,6 +318,7 @@ static struct { > u32 uwb:1; > u32 fan_ctrl_status_undef:1; > u32 second_fan:1; > + u32 second_fan_ctl:1; > u32 beep_needs_two_args:1; > u32 mixer_no_level_control:1; > u32 battery_force_primary:1; > @@ -8325,11 +8326,19 @@ 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_ctl) { > + if (!fan_select_fan2() || > + !acpi_evalf(sfan_handle, NULL, NULL, "vd", level)) { > + pr_warn("Couldn't set 2nd fan level, disabling support\n"); > + tp_features.second_fan_ctl = 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,15 @@ 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_ctl) { > + if (!fan_select_fan2() || > + !acpi_ec_write(fan_status_offset, level)) { > + pr_warn("Couldn't set 2nd fan level, disabling support\n"); > + tp_features.second_fan_ctl = 0; > + } > + fan_select_fan1(); > + > + } > if (!acpi_ec_write(fan_status_offset, level)) > return -EIO; > else > @@ -8764,6 +8782,7 @@ static const struct attribute_group fan_attr_group = { > > #define TPACPI_FAN_Q1 0x0001 /* Unitialized HFSP */ > #define TPACPI_FAN_2FAN 0x0002 /* EC 0x31 bit 0 selects fan2 */ > +#define TPACPI_FAN_2CTL 0x0004 /* selects fan2 control */ > > static const struct tpacpi_quirk fan_quirk_table[] __initconst = { > TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1), > @@ -8772,6 +8791,13 @@ 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_2CTL), /* P70 */ > + TPACPI_Q_LNV3('N', '1', 'E', TPACPI_FAN_2CTL), /* P50 */ > + TPACPI_Q_LNV3('N', '1', 'T', TPACPI_FAN_2CTL), /* P71 */ > + TPACPI_Q_LNV3('N', '1', 'U', TPACPI_FAN_2CTL), /* P51 */ > + TPACPI_Q_LNV3('N', '2', 'C', TPACPI_FAN_2CTL), /* P52 / P72 */ > + TPACPI_Q_LNV3('N', '2', 'E', TPACPI_FAN_2CTL), /* P1 / X1 Extreme (1st gen) */ > + TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2CTL), /* P1 / X1 Extreme (2nd gen) */ > }; > > static int __init fan_init(struct ibm_init_struct *iibm) > @@ -8789,6 +8815,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) > fan_watchdog_maxinterval = 0; > tp_features.fan_ctrl_status_undef = 0; > tp_features.second_fan = 0; > + tp_features.second_fan_ctl = 0; > fan_control_desired_level = 7; > > if (tpacpi_is_ibm()) { > @@ -8813,8 +8840,12 @@ 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"); > + } > + if (quirks & TPACPI_FAN_2CTL) { > + tp_features.second_fan = 1; > + tp_features.second_fan_ctl = 1; > + pr_info("secondary fan control enabled\n"); > } > } else { > pr_err("ThinkPad ACPI EC access misbehaving, fan status and control unavailable\n"); > -- > 2.25.3 >
On Fri, Apr 24, 2020 at 12:57 AM Lars <larsh@apache.org> wrote: > > This adds dual fan control for the following models: > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > Both fans are controlled together as if they were a single fan. > > Tested on an X1 Extreme Gen1, an X1 Extreme Gen2, and a P50. > > The patch is defensive, it adds only specific supported machines, and falls > back to the old behavior if both fans cannot be controlled. > > Background: > I tested the BIOS default behavior on my X1E gen2 and both fans are always > changed together. So rather than adding controls for each fan, this controls > both fans together as the BIOS would do. > > This was inspired by a discussion on dual fan support for the thinkfan tool > [1]. > All BIOS ids are taken from there. The X1E gen2 id is verified on my machine. > > Thanks to GitHub users voidworker and civic9 for the earlier patches and BIOS > ids, and to users peter-stoll and sassman for testing the patch on their > machines. > > [1]: https://github.com/vmatare/thinkfan/issues/58 > > Signed-off-by: Lars <larsh@apache.org> One question though, is Lars your real name here? [1] [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
Hi Andy, my full name is Lars Hofhansl. Should I send a new post? Just in case, I hereby: Signed-off-by: Lars Hofhansl <larsh@apache.org> -- Lars On Friday, April 24, 2020, 4:12:05 AM PDT, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: On Fri, Apr 24, 2020 at 12:57 AM Lars <larsh@apache.org> wrote: > > This adds dual fan control for the following models: > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > Both fans are controlled together as if they were a single fan. > > Tested on an X1 Extreme Gen1, an X1 Extreme Gen2, and a P50. > > The patch is defensive, it adds only specific supported machines, and falls > back to the old behavior if both fans cannot be controlled. > > Background: > I tested the BIOS default behavior on my X1E gen2 and both fans are always > changed together. So rather than adding controls for each fan, this controls > both fans together as the BIOS would do. > > This was inspired by a discussion on dual fan support for the thinkfan tool > [1]. > All BIOS ids are taken from there. The X1E gen2 id is verified on my machine. > > Thanks to GitHub users voidworker and civic9 for the earlier patches and BIOS > ids, and to users peter-stoll and sassman for testing the patch on their > machines. > > [1]: https://github.com/vmatare/thinkfan/issues/58 > > Signed-off-by: Lars <larsh@apache.org> One question though, is Lars your real name here? [1] [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
On Mon, Apr 27, 2020 at 10:44 AM larsh@apache.org <larsh@apache.org> wrote: > > > Hi Andy, > > my full name is Lars Hofhansl. > Should I send a new post? > > Just in case, I hereby: > > Signed-off-by: Lars Hofhansl <larsh@apache.org> No need for this one, I will update locally, thanks! > > -- Lars > > On Friday, April 24, 2020, 4:12:05 AM PDT, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > > > > On Fri, Apr 24, 2020 at 12:57 AM Lars <larsh@apache.org> wrote: > > > > This adds dual fan control for the following models: > > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > > > Both fans are controlled together as if they were a single fan. > > > > Tested on an X1 Extreme Gen1, an X1 Extreme Gen2, and a P50. > > > > The patch is defensive, it adds only specific supported machines, and falls > > back to the old behavior if both fans cannot be controlled. > > > > Background: > > I tested the BIOS default behavior on my X1E gen2 and both fans are always > > changed together. So rather than adding controls for each fan, this controls > > both fans together as the BIOS would do. > > > > This was inspired by a discussion on dual fan support for the thinkfan tool > > [1]. > > All BIOS ids are taken from there. The X1E gen2 id is verified on my machine. > > > > Thanks to GitHub users voidworker and civic9 for the earlier patches and BIOS > > ids, and to users peter-stoll and sassman for testing the patch on their > > machines. > > > > [1]: https://github.com/vmatare/thinkfan/issues/58 > > > > Signed-off-by: Lars <larsh@apache.org> > > One question though, is Lars your real name here? [1] > > > [1]: > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin > > -- > With Best Regards, > Andy Shevchenko >
On Thu, Apr 23, 2020 at 23:57:59 CEST, Lars <larsh@apache.org> wrote: > This adds dual fan control for the following models: > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > Both fans are controlled together as if they were a single fan. > [...] > Background: > I tested the BIOS default behavior on my X1E gen2 and both fans are always > changed together. So rather than adding controls for each fan, this controls > both fans together as the BIOS would do. Hi Lars, why have you chosen to control multiple fans in this way? I know that BIOS controls both fans together, but the EC has the capabilities to control both fans independently, so maybe it can be convenient to expose this feature. Distinti saluti/Best regards, Dario Messina
Hi Dario, it just seemed an unnecessary complication for the user. This way existing tools (like thinkfan) would just continue to work. I could not think of any use case on these laptops where you actually want to control the fan independently. It would also be hard to expose this through the /proc/acpi/ibm/fan interface. -- Lars On Monday, April 27, 2020, 11:41:22 AM PDT, Dario Messina <nanodario@gmail.com> wrote: On Thu, Apr 23, 2020 at 23:57:59 CEST, Lars <larsh@apache.org> wrote: > This adds dual fan control for the following models: > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > Both fans are controlled together as if they were a single fan. > [...] > Background: > I tested the BIOS default behavior on my X1E gen2 and both fans are always > changed together. So rather than adding controls for each fan, this controls > both fans together as the BIOS would do. Hi Lars, why have you chosen to control multiple fans in this way? I know that BIOS controls both fans together, but the EC has the capabilities to control both fans independently, so maybe it can be convenient to expose this feature. Distinti saluti/Best regards, Dario Messina
pon., 27 kwi 2020 o 20:41 Dario Messina <nanodario@gmail.com> napisał(a): > > On Thu, Apr 23, 2020 at 23:57:59 CEST, Lars <larsh@apache.org> wrote: > > This adds dual fan control for the following models: > > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > > > Both fans are controlled together as if they were a single fan. > > [...] > > Background: > > I tested the BIOS default behavior on my X1E gen2 and both fans are always > > changed together. So rather than adding controls for each fan, this controls > > both fans together as the BIOS would do. > Hi Lars, why have you chosen to control multiple fans in this way? > I know that BIOS controls both fans together, but the EC has the capabilities > to control both fans independently, so maybe it can be convenient to expose > this feature. +1 Previous version of the patch [1] allows to control both fans independently. However some software like thinkfan is not ready to control two fans. But I also think this feature should be at least optionally exposed. [1] https://github.com/civic9/thinkpad_acpi.2ndfan.patch/blob/master/thinkpad_acpi.2ndfan.patch/thinkpad_acpi.2ndfan.patch
Do you have a use case for that behavior? The previous patch broke the /proc interface, didn't not work with the current version of thinkfan (but a a version with multi-fan support is in the works), and it had hard to track internal mutable state. The proposed change is clean on all these fronts. I'm not a fan of surprising the user with unnecessarily complex behavior (but perhaps this can be added as an option in the future.) -- Lars On Tuesday, April 28, 2020, 2:18:42 PM PDT, civic9 <civic9@gmail.com> wrote: pon., 27 kwi 2020 o 20:41 Dario Messina <nanodario@gmail.com> napisał(a): > > On Thu, Apr 23, 2020 at 23:57:59 CEST, Lars <larsh@apache.org> wrote: > > This adds dual fan control for the following models: > > P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. > > > > Both fans are controlled together as if they were a single fan. > > [...] > > Background: > > I tested the BIOS default behavior on my X1E gen2 and both fans are always > > changed together. So rather than adding controls for each fan, this controls > > both fans together as the BIOS would do. > Hi Lars, why have you chosen to control multiple fans in this way? > I know that BIOS controls both fans together, but the EC has the capabilities > to control both fans independently, so maybe it can be convenient to expose > this feature. +1 Previous version of the patch [1] allows to control both fans independently. However some software like thinkfan is not ready to control two fans. But I also think this feature should be at least optionally exposed. [1] https://github.com/civic9/thinkpad_acpi.2ndfan.patch/blob/master/thinkpad_acpi.2ndfan.patch/thinkpad_acpi.2ndfan.patch
On 29.04.20 02:20, larsh@apache.org wrote: > Do you have a use case for that behavior? > > The previous patch broke the /proc interface, didn't not work with the current version of thinkfan > (but a a version with multi-fan support is in the works), and it had hard to track internal mutable state. > > The proposed change is clean on all these fronts. > > I'm not a fan of surprising the user with unnecessarily complex behavior (but perhaps this can be added as an option in the future.) I concur to keep the patch as is. Any additional functionality could be added later on, if deemed necessary. Stefan > -- Lars > > On Tuesday, April 28, 2020, 2:18:42 PM PDT, civic9 <civic9@gmail.com> wrote: > > > > > > pon., 27 kwi 2020 o 20:41 Dario Messina <nanodario@gmail.com> napisał(a): >> >> On Thu, Apr 23, 2020 at 23:57:59 CEST, Lars <larsh@apache.org> wrote: >>> This adds dual fan control for the following models: >>> P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. >>> >>> Both fans are controlled together as if they were a single fan. >>> [...] >>> Background: >>> I tested the BIOS default behavior on my X1E gen2 and both fans are always >>> changed together. So rather than adding controls for each fan, this controls >>> both fans together as the BIOS would do. >> Hi Lars, why have you chosen to control multiple fans in this way? >> I know that BIOS controls both fans together, but the EC has the capabilities >> to control both fans independently, so maybe it can be convenient to expose >> this feature. > > +1 > Previous version of the patch [1] allows to control both fans independently. > However some software like thinkfan is not ready to control two fans. > But I also think this feature should be at least optionally exposed. > > > [1] > https://github.com/civic9/thinkpad_acpi.2ndfan.patch/blob/master/thinkpad_acpi.2ndfan.patch/thinkpad_acpi.2ndfan.patch >
On Wed, 29 Apr 2020, Stefan Assmann wrote: > On 29.04.20 02:20, larsh@apache.org wrote: > > Do you have a use case for that behavior? > > > > The previous patch broke the /proc interface, didn't not work with the current version of thinkfan > > (but a a version with multi-fan support is in the works), and it had hard to track internal mutable state. > > > > The proposed change is clean on all these fronts. > > > > I'm not a fan of surprising the user with unnecessarily complex behavior (but perhaps this can be added as an option in the future.) > > I concur to keep the patch as is. Any additional functionality could be > added later on, if deemed necessary. Yes, but let's avoid changing exposed APIs as much as possible... Anyway, the correct interface *when exposing both fans* is: 1. Use the "hwmon" sysfs interface to expose each fan separately, and control them separately. 1a. it is quite acceptable to control them in group by default, and have a module parameter to select grouped, or separate behavior. 2. /proc/acpi/ibm/fan shall control both of them at the same time, and report data from the first fan. THIS INTERFACE IS FROZEN, LET'S NOT MESS WITH IT. Also, "fail-safe" for this is to have the two fans on automatic, and to enable both fans. All of that said, *it is very much acceptable* to merge a first set of patches that controls both fans simultaneously and exposes the fan group as if it were just the main fan, and later improve on the patch to expose the second fan separately (provided safe group behavior is maintained, see above).
Thanks for looking. Agreed. This changes leaves /proc/acpi/ibm/fan entirely untouched (and actually behave as expected). That's by design. > 1. Use the "hwmon" sysfs interface to expose each fan separately, and > control them separately. > > 1a. it is quite acceptable to control them in group by default, and have > a module parameter to select grouped, or separate behavior. Agree as well. I would add, though, that that would need a more involved refactor, since there would be two different ways to control the fans now. > Also, "fail-safe" for this is to have the two fans on automatic, and to > enable both fans. I thought I hadn't touched that part. Lemme double check. > [...] later improve on the patch to > expose the second fan separately (provided safe group behavior is > maintained, see above). Yep. -- Lars On Thursday, April 30, 2020, 2:40:57 PM PDT, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote: On Wed, 29 Apr 2020, Stefan Assmann wrote: > On 29.04.20 02:20, larsh@apache.org wrote: > > Do you have a use case for that behavior? > > > > The previous patch broke the /proc interface, didn't not work with the current version of thinkfan > > (but a a version with multi-fan support is in the works), and it had hard to track internal mutable state. > > > > The proposed change is clean on all these fronts. > > > > I'm not a fan of surprising the user with unnecessarily complex behavior (but perhaps this can be added as an option in the future.) > > I concur to keep the patch as is. Any additional functionality could be > added later on, if deemed necessary. Yes, but let's avoid changing exposed APIs as much as possible... Anyway, the correct interface *when exposing both fans* is: 1. Use the "hwmon" sysfs interface to expose each fan separately, and control them separately. 1a. it is quite acceptable to control them in group by default, and have a module parameter to select grouped, or separate behavior. 2. /proc/acpi/ibm/fan shall control both of them at the same time, and report data from the first fan. THIS INTERFACE IS FROZEN, LET'S NOT MESS WITH IT. Also, "fail-safe" for this is to have the two fans on automatic, and to enable both fans. All of that said, *it is very much acceptable* to merge a first set of patches that controls both fans simultaneously and exposes the fan group as if it were just the main fan, and later improve on the patch to expose the second fan separately (provided safe group behavior is maintained, see above).
śr., 29 kwi 2020 o 02:21 larsh@apache.org <larsh@apache.org> napisał(a): > > Do you have a use case for that behavior? I work very often with just one fan enabled at the lowest level. It is inaudible for me and it does its job for not too heavy usage. If the second one is also enabled I can hear them. We love Linux for such small extra features too.
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index da794dcfdd92..9e0f65e567be 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -318,6 +318,7 @@ static struct { u32 uwb:1; u32 fan_ctrl_status_undef:1; u32 second_fan:1; + u32 second_fan_ctl:1; u32 beep_needs_two_args:1; u32 mixer_no_level_control:1; u32 battery_force_primary:1; @@ -8325,11 +8326,19 @@ 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_ctl) { + if (!fan_select_fan2() || + !acpi_evalf(sfan_handle, NULL, NULL, "vd", level)) { + pr_warn("Couldn't set 2nd fan level, disabling support\n"); + tp_features.second_fan_ctl = 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,15 @@ 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_ctl) { + if (!fan_select_fan2() || + !acpi_ec_write(fan_status_offset, level)) { + pr_warn("Couldn't set 2nd fan level, disabling support\n"); + tp_features.second_fan_ctl = 0; + } + fan_select_fan1(); + + } if (!acpi_ec_write(fan_status_offset, level)) return -EIO; else @@ -8764,6 +8782,7 @@ static const struct attribute_group fan_attr_group = { #define TPACPI_FAN_Q1 0x0001 /* Unitialized HFSP */ #define TPACPI_FAN_2FAN 0x0002 /* EC 0x31 bit 0 selects fan2 */ +#define TPACPI_FAN_2CTL 0x0004 /* selects fan2 control */ static const struct tpacpi_quirk fan_quirk_table[] __initconst = { TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1), @@ -8772,6 +8791,13 @@ 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_2CTL), /* P70 */ + TPACPI_Q_LNV3('N', '1', 'E', TPACPI_FAN_2CTL), /* P50 */ + TPACPI_Q_LNV3('N', '1', 'T', TPACPI_FAN_2CTL), /* P71 */ + TPACPI_Q_LNV3('N', '1', 'U', TPACPI_FAN_2CTL), /* P51 */ + TPACPI_Q_LNV3('N', '2', 'C', TPACPI_FAN_2CTL), /* P52 / P72 */ + TPACPI_Q_LNV3('N', '2', 'E', TPACPI_FAN_2CTL), /* P1 / X1 Extreme (1st gen) */ + TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2CTL), /* P1 / X1 Extreme (2nd gen) */ }; static int __init fan_init(struct ibm_init_struct *iibm) @@ -8789,6 +8815,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) fan_watchdog_maxinterval = 0; tp_features.fan_ctrl_status_undef = 0; tp_features.second_fan = 0; + tp_features.second_fan_ctl = 0; fan_control_desired_level = 7; if (tpacpi_is_ibm()) { @@ -8813,8 +8840,12 @@ 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"); + } + if (quirks & TPACPI_FAN_2CTL) { + tp_features.second_fan = 1; + tp_features.second_fan_ctl = 1; + pr_info("secondary fan control enabled\n"); } } else { pr_err("ThinkPad ACPI EC access misbehaving, fan status and control unavailable\n");
This adds dual fan control for the following models: P50, P51, P52, P70, P71, P72, P1 gen1, X1E gen1, P2 gen2, and X1E gen2. Both fans are controlled together as if they were a single fan. Tested on an X1 Extreme Gen1, an X1 Extreme Gen2, and a P50. The patch is defensive, it adds only specific supported machines, and falls back to the old behavior if both fans cannot be controlled. Background: I tested the BIOS default behavior on my X1E gen2 and both fans are always changed together. So rather than adding controls for each fan, this controls both fans together as the BIOS would do. This was inspired by a discussion on dual fan support for the thinkfan tool [1]. All BIOS ids are taken from there. The X1E gen2 id is verified on my machine. Thanks to GitHub users voidworker and civic9 for the earlier patches and BIOS ids, and to users peter-stoll and sassman for testing the patch on their machines. [1]: https://github.com/vmatare/thinkfan/issues/58 Signed-off-by: Lars <larsh@apache.org> --- drivers/platform/x86/thinkpad_acpi.c | 43 ++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 6 deletions(-)