Bug 11331

Summary: Fan speed setting reset after suspend to disk/ram on Thinkpad X31
Product: Drivers Reporter: Richard Hartmann (richih.mailinglist)
Component: PlatformAssignee: Henrique de Moraes Holschuh (hmh)
Status: CLOSED CODE_FIX    
Severity: enhancement CC: acpi-bugzilla, bunk, hmh
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.26-1 Subsystem:
Regression: --- Bisected commit-id:
Bug Depends on:    
Bug Blocks: 7216, 56331    
Attachments: Preserve (selectively) the fan level on resume
Preserve (selectively) the fan level on resume (v2)

Description Richard Hartmann 2008-08-14 06:45:43 UTC
Distribution: Debian unstable
Hardware Environment: Thinkpad X31
Software Environment: Debian unstable
Problem Description:
After waking the system from suspend to disk/ram, the fan speed is reset to the default value. As I understand things, the value should be cached and re-set to what I last set it to.

Steps to reproduce:

# echo "options thinkpad_acpi fan_control=1" >> /etc/modprobe.d/options
# shutdown -r now

# echo level 7 >! /proc/acpi/ibm/fan #fan will spin on fastest level, you will hear this
# cat /proc/acpi/ibm/fan
status:         enabled
speed:          4107
level:          7
commands:       level <level> (<level> is 0-7, auto, disengaged, full-speed)
commands:       enable, disable
commands:       watchdog <timeout> (<timeout> is 0 (off), 1-120 (seconds))
commands:       speed <speed> (<speed> is 0-65535)
# s2ram || s2disk
press Fn-F4 or power button to wake system

# cat /proc/acpi/ibm/fan
status:         enabled
speed:          0
level:          auto
commands:       level <level> (<level> is 0-7, auto, disengaged, full-speed)
commands:       enable, disable
commands:       watchdog <timeout> (<timeout> is 0 (off), 1-120 (seconds))
commands:       speed <speed> (<speed> is 0-65535)
Comment 1 ykzhao 2008-08-14 18:35:50 UTC
Hi, Richard
    Will you please attach the output of acpidump?

Thanks.
Comment 2 Zhang Rui 2008-08-14 20:15:12 UTC
Re-assign to Henrique. :)
Comment 3 Richard Hartmann 2008-08-15 14:56:48 UTC
On Fri, Aug 15, 2008 at 03:35,  <bugme-daemon@bugzilla.kernel.org> wrote:

>    Will you please attach the output of acpidump?

Will do so asap. Sunday at the latest.


Richard
Comment 4 Henrique de Moraes Holschuh 2008-08-15 17:35:44 UTC
Fan status is not reset on wakeup on purpose.

It would be DANGEROUS to overide the fan state at resume, except maybe if we do it in such a way that we only RAISE the fan speed (i.e. basically we can restore level 7 and also the PWM locked at 100% condition ("level disengaged"), but that's it).

It is less confusing to just not override it at all on resume.  It will remain on whatever the ACPI DSDT set it to (so far, that will be "auto" or "level 7", but it could be different on future BIOS releases and future thinkpad models).

So, unless you have a good reason to want the PWM off restore or level 7 restore, I am afraid I will have to "reject" this bug as "WILL_NOT_FIX" :-(
Comment 5 Richard Hartmann 2008-08-16 07:28:34 UTC
On Sat, Aug 16, 2008 at 02:35,  <bugme-daemon@bugzilla.kernel.org> wrote:

> hmh@hmh.eng.br changed:

> It would be DANGEROUS to overide the fan state at resume, except maybe if we
> do
> it in such a way that we only RAISE the fan speed (i.e. basically we can
> restore level 7 and also the PWM locked at 100% condition ("level
> disengaged"),
> but that's it).

I would argue that the user has set his speed setting on purpose, so chances
of this happening are not that great. On the other hand, they might simply
forget that they changed it to a fixed value and be in different
conditions, now.
So yes, it is a good idea to only raise the value.


> It is less confusing to just not override it at all on resume.  It will
> remain
> on whatever the ACPI DSDT set it to (so far, that will be "auto" or "level
> 7",
> but it could be different on future BIOS releases and future thinkpad
> models).

Well, a single printk could solve this problem.


> So, unless you have a good reason to want the PWM off restore or level 7
> restore, I am afraid I will have to "reject" this bug as "WILL_NOT_FIX" :-(

My reason is that I don't like my machine becoming too warm. I prefer a
little noise over a higher temperature. Thus, I keep my laptop at level 4.
In warmer conditions or when CPU usage is high, I keep it at 7.

I would like to ask you to at least keep level 7 if level 7 was set before.
Personally, I would not mind restoring from 4 to 7, but that would probably
be confusing to everyone who does not know of this intended behaviour.
Ideally, it would simply restore from 4 to 4, though.

I agree that off is not a good level to restore to, btw.


Richard

PS: Attached is the acpidump that Yakui asked for.
Comment 6 Zhang Rui 2008-09-01 18:40:54 UTC
Hi, henrique,
what's the status of this bug?
are we going to make some changes in the thinkpad_acpi driver?
e.g. reset fan speed to Max(cached value, default value after resume).
Comment 7 Henrique de Moraes Holschuh 2008-09-02 06:40:26 UTC
I will change thinkpad-acpi, it is in the TODO list.  It will reset fan speed IF AND ONLY IF it was set to max or PWM OFF (which is a lot faster than max) before suspend.  Otherwise, it will NOT touch whatever the firmware set on resume.

But that patch might take a while to get ready.
Comment 8 Henrique de Moraes Holschuh 2008-09-02 06:45:44 UTC
BTW, I have marked this as an enhancement.  This is not a problem in thinkpad-acpi, we are currently respecting the firmware decisions regarding fan speed when something massive happens to system state (i.e. it spends some time suspended, and wakes up for whatever unknown reason), which is, BTW, the only thing that the vendor supports.
Comment 9 Henrique de Moraes Holschuh 2008-09-02 07:00:59 UTC
Reminders (requirements the patch needs to address):

1. Be safe in future thinkpads where max > 7 or PWM OFF changes
   * cannot be autodetected with 100% reliability, but we can
   bail out if we get anything unusual in the HFSP register, like
   a level above 7 or an unknown bit set.

2. thinkpad-acpi will have to be verbose when it resets fan speed

3. Restore only (speed 7, auto off) and (any, auto off, PWM locked at 100%)
   but with the safety net (should the PWM locked bit be ignored) set to
   level 7, as done in the fan watchdog.
Comment 10 Richard Hartmann 2008-09-03 06:04:06 UTC
> I will change thinkpad-acpi, it is in the TODO list.  It will reset fan speed
> IF AND ONLY IF it was set to max or PWM OFF (which is a lot faster than max)
> before suspend.  Otherwise, it will NOT touch whatever the firmware set on
> resume.

So PWN OFF just uses whatever the true max speed of the fan is?
Interesting, how do you set that?


Richard
Comment 11 Henrique de Moraes Holschuh 2008-09-03 20:04:06 UTC
You set pwm1_enable to 0 on the thinkpad_hwmon attributes.  See the thinkpad-acpi documentation in your kernel's Documentation/laptops/ directory, as well as the hwmon documentation in Documentation/hwmon/sysfs-interface.

And if continuous use of that mode that damages your fan, the MOSFET fan driver (which requires an extremely expensive motherboard change to repair, as far as Lenovo is concerned), or fills up your thinkpad with dust, it is your own fault. Operation of a thinkpad outside of fan mode "auto" (pwm1_enable = 2) is done on your own risk, and might void your warranty.
Comment 12 Henrique de Moraes Holschuh 2008-09-03 20:05:35 UTC
(read my last post as "please just use level 7 unless you're desperate").
Comment 13 Richard Hartmann 2008-09-04 03:16:21 UTC
> (read my last post as "please just use level 7 unless you're desperate").

I'm not, I just wanted to know. Thanks :)


Richard
Comment 14 Henrique de Moraes Holschuh 2008-10-04 20:49:21 UTC
Created attachment 18170 [details]
Preserve (selectively) the fan level on resume

It is ready, tested, and in my tree.  It will be pushed out eventually, but I am not yet sure if it will be for 2.6.28 or 2.6.29.
Comment 15 Henrique de Moraes Holschuh 2008-10-04 20:50:10 UTC
Resolving this as CODE_FIX.  The bug can be closed when the patch hits mainline.
Comment 16 Henrique de Moraes Holschuh 2008-10-06 10:38:42 UTC
Created attachment 18182 [details]
Preserve (selectively) the fan level on resume (v2)

Fix issue on the T43 and a few others with the "HFSP register unitialized by DSDT" quirk.  It would always resume with fan set to level 7 if you didn't touch the fan at all since boot.  Not dangerous at all, but rather annoying.
Comment 17 Len Brown 2008-10-16 17:56:08 UTC
applied to acpi-test.

FYI, Henrique, it is less editing for me if
you attach patches to bugzilla that you export with
"git format-patch", rather than "git show"
because they look like e-mail messages which
git-am understands.

thanks,
-Len
Comment 18 Henrique de Moraes Holschuh 2008-10-17 08:22:21 UTC
Wah!  Don't grab my patches directly from bugzilla, I will submit them to you through email the normal way :-)

It is a good thing I bothered to update the patch here for the bug submitter to use it while it went through the ropes to get merged...

I have a small stack of fixes I didn't send you because I thought the window was already closed.  I will flush them to you over the weekend, and that includes this patch.
Comment 19 Len Brown 2008-10-17 11:01:26 UTC
Henrique,
Unless you want me to drop it completely, I'll keep the patch
in comment #16 in acpi-test.  It is on a branch that will just
be in the test tree and will not go upstgream.

Yes, I'm still merging and have not pushed to 2.6.28 yet,
so if you send me a batch of low enough risk patches
in the next 3 days, it is possible they can still make 2.6.28.
(note that not all patches can be so late, but yours are
 specific to a driver, and thus inherently lower risk than
 patches that touch more systems)
Comment 20 Henrique de Moraes Holschuh 2008-10-17 12:51:57 UTC
You don't have to drop the patch in this particular case, it is not incorrect. I just never expected patches to be fished from bugzilla into acpi-test, so I got surprised by your notice :-)

I will check whatever is already in acpi-test before I send you the stuff, and adjust accordingly.  If I see any patches that need to be dropped, I will tell you.

In this case, I was going to let the patch cook into some thinkpad-acpi devel releases before I sent it to you for acpi-test, since I didn't expect it to go into 2.6.28.  But I did test the patch in a box that is likely to trigger every corner case in fan-control, and it seemed to work fine, so I guess it is ok if it stays in acpi-test.
Comment 21 Len Brown 2008-10-24 22:57:36 UTC
shipped in linux-2.6.28-rc1
closed
commit 75700e53cd14ccc7a5a42547497dff11fe209186
Author: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Date:   Sat Oct 18 14:23:52 2008 -0300

    ACPI: thinkpad-acpi: attempt to preserve fan state on resume
Comment 22 Richard Hartmann 2008-10-25 06:34:25 UTC
Thanks!