Bug 11331

Hi, Richard
    Will you please attach the output of acpidump?

Thanks.

Re-assign to Henrique. :)

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

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" :-(

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.

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).

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.

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.

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.

> 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

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.

(read my last post as "please just use level 7 unless you're desperate").

> (read my last post as "please just use level 7 unless you're desperate").

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


Richard

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.

Resolving this as CODE_FIX.  The bug can be closed when the patch hits mainline.

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.

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

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.

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)

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.

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

Thanks!