Bug 7570

Created attachment 9596 [details]
dmesg output

Look at a call trace for more hints.

Hello,

Does suspend to disk (i.e. S4) work as expected with patches from #7122 applied?

Yes, to great extend it does and under certain conditions I am able to restore a
correct behaviour of all fans. Please read bug 7122, comment 61 for more
details. A quite obvious difference is that there is no call trace in dmesg
output in case of S4.

Created attachment 9757 [details]
fan/thermal resume handlers fix

As mentioned in the last comment from bug #7122, disabling fan resume handler
solves the problem. The reason is the implementation of the fan resume handler:

'force_power_state'flag being set, disables the check if the required power
state is the same as the current one. In that case the list of power resources
being enabled is the same as the list of power resources being disabled, and
follows to consequent enabling and disabling of these resources.
The attached patch should fix the issue in fan resume handler, and also updates
thermal resume handler. It fixed the problem for my nx6125.

So far so good. I can use S3 and S4 states in various combinations under
different cpu workloads without any negative impact on a fan subsystem. Now the
fan works as it is expected.

The only minor 'but' is that I can still notice the very same call trace in
dmesg output produced by the might_sleep() macro.  I'm not in position to decide
whether this is indicative of some bug, or someone simply left a macro call for
debugging purposes. 

Sorry I don't get it. Why is this:
+	if (state == ACPI_STATE_D0) {
+		acpi_bus_set_power(device->handle, ACPI_STATE_D3);
+	} else {
+		acpi_bus_set_power(device->handle, ACPI_STATE_D0);
+	}
 	acpi_bus_set_power(device->handle, state);

in acpi_fan_add and acpi_fan_resume?
I understand that it makes sense to either set it to D0 or D3 instead of 
setting the state from bus_get_power(..), to avoid a D1/D2 state the BIOS could 
have set the fan while suspending or whatever might confuse later.
But why again calling acpi_bus_set_power(device->handle, state);?

I also wonder why bus_set_power needs to be called at all?
If state and kernel power variables are out of sync, shouldn't there something 
be like check_power() in power.c, similar as in thermal.c which gets invoked at 
resume (or notification, or whenever out of sync can happen)?

1. acpi_bus_set_power() has the check if the device already in the target 
state, so we cannot set it to that state directly.
2. acpi_power_transition() function executes _ON methods for power resources 
associated with target device power state, and then executes _OFF methods for 
power resources associated with current device power state, so if we skip the 
check in acpi_bus_set_power(), the power resources associated with the required 
power state will turned off.
3. Sometimes after resume the fan state reported doesn't match with the actual 
device state, so the only way to bring the consistency is to set the device in 
reported state.
4. The acpi_thermal_active() function uses ACPI_STATE_D0/ACPI_STATE_D3 to 
operate the fans, so I assumed that it's ok to use the same technique to bring 
the fans to appropriate state.
I'm agree that it's not very clean and safe approach, maybe we want to try to 
find the lowest power state available?

Created attachment 9802 [details]
hopefully more clean fix patch

Here is another patch to fix the issue. Works for me, and use only one call to
interpreter.

Alexander, could you try it also?

Thomas, does this patch looks better for you? Please take in account my
comments in the post above.

Works.

Excuse my ingnorance, but what is so dangerous in calling acpi_bus_set_power()
twice in a row? I skimmed through a body of the function and haven't noticed
anything special.

Thanks for reply.

The code executes much slower in the interpreter, so if there is possibility 
to avoid or reduce it, better to do so.

Well, I'm closing this bug, please re-open, if there are any problems.

Created attachment 10023 [details]
the same patch against kernel 2.6.20-rc3 (possibly for all kernels since 2.6.19)

the same patch against kernel 2.6.20-rc3 ( and possibly for all kernels since
2.6.19)
the patch is on top of patch 10022 from bug 7122
http://bugzilla.kernel.org/attachment.cgi?id=10022&action=view

Created attachment 10024 [details]
the same patch against kernel 2.6.20-rc3 (possibly for all kernels since 2.6.19)

the same patch against kernel 2.6.20-rc3 ( and possibly for all kernels since
2.6.19)
the patch is on top of patch 10022 from bug 7122
http://bugzilla.kernel.org/attachment.cgi?id=10022&action=view

patch from comment #8, as refreshed in comment #12 applied to acpi-test

Created attachment 10472 [details]
Additional fix for the same issue

Here is the update on the issue for the latest kernel.

patch from comment #8, as refreshed in comment #12 shipped in Linux-2.6.21-rc1
patch from comment #14 applied to acpi-test

patch from comment #14 shipped in 2.6.21-rc3-git6
closed