Hi
I think I found a regression in 2.6.24-git. After waking up from suspend
2 ram, the fan of my laptop turns constantly at highest speed. It didn't
do this in 2.6.24.
I bisected it down to this commit:
commit c95d47a868f35cd47643d116a3c680cdaa954df8
Author: Rafael J. Wysocki <[email protected]>
Date: Tue Jan 8 00:05:21 2008 +0100
ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
Reverting it resolves the problem for me, but I can't say if this makes
any sense.
My machine is a Zepto laptop with Intel Santa-Rosa chipset and I'm
running
a x86_64 Ubuntu Gutsy.
Mirco
On Feb 10, 2008 9:21 AM, Mirco Tischler <[email protected]> wrote:
> Hi
>
> I think I found a regression in 2.6.24-git. After waking up from suspend
> 2 ram, the fan of my laptop turns constantly at highest speed. It didn't
> do this in 2.6.24.
>
> I bisected it down to this commit:
>
> commit c95d47a868f35cd47643d116a3c680cdaa954df8
> Author: Rafael J. Wysocki <[email protected]>
> Date: Tue Jan 8 00:05:21 2008 +0100
>
> ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
>
> Reverting it resolves the problem for me, but I can't say if this makes
> any sense.
>
> My machine is a Zepto laptop with Intel Santa-Rosa chipset and I'm
> running
> a x86_64 Ubuntu Gutsy.
I just noticed the same problem. After about ten minutes, it seems
like all the ACPI events got 'unstuck' -- my screen's backlight went
up and down a couple of times (and battery/AC indicator flipped a
couple of times) responding to previous power loss and gain events, I
think, and the fans dropped down from high speed to low.
I was chalking it up to another charming bug on my HP nx6125, but if
someone else is seeing it too...
I'm on Ubuntu, x86_64, and git as of a couple days ago as well. I
haven't tried reverting the patch.
On Sunday, 10 of February 2008, Mirco Tischler wrote:
> Hi
Hi,
> I think I found a regression in 2.6.24-git. After waking up from suspend
> 2 ram, the fan of my laptop turns constantly at highest speed. It didn't
> do this in 2.6.24.
>
> I bisected it down to this commit:
>
> commit c95d47a868f35cd47643d116a3c680cdaa954df8
> Author: Rafael J. Wysocki <[email protected]>
> Date: Tue Jan 8 00:05:21 2008 +0100
>
> ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
>
> Reverting it resolves the problem for me, but I can't say if this makes
> any sense.
Well, _GTS and _BFS are nops on all machines known to me. However, there's
one more change in there that may break things in (very speculative) theory.
Can you apply the appended patch on top of the current mainline and tetest?
Thanks,
Rafael
---
drivers/acpi/hardware/hwsleep.c | 72 ++++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 36 deletions(-)
Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
@@ -494,12 +494,47 @@ acpi_status acpi_leave_sleep_state_prep(
struct acpi_object_list arg_list;
union acpi_object arg;
acpi_status status;
+
+ ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
+
+ /* Execute the _BFS method */
+
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = sleep_state;
+
+ status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
+ if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+ ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
+ }
+
+ return_ACPI_STATUS(status);
+}
+
+/*******************************************************************************
+ *
+ * FUNCTION: acpi_leave_sleep_state
+ *
+ * PARAMETERS: sleep_state - Which sleep state we just exited
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
+ * Called with interrupts ENABLED.
+ *
+ ******************************************************************************/
+acpi_status acpi_leave_sleep_state(u8 sleep_state)
+{
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
+ acpi_status status;
struct acpi_bit_register_info *sleep_type_reg_info;
struct acpi_bit_register_info *sleep_enable_reg_info;
u32 PM1Acontrol;
u32 PM1Bcontrol;
- ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
+ ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
/*
* Set SLP_TYPE and SLP_EN to state S0.
@@ -546,41 +581,6 @@ acpi_status acpi_leave_sleep_state_prep(
}
}
- /* Execute the _BFS method */
-
- arg_list.count = 1;
- arg_list.pointer = &arg;
- arg.type = ACPI_TYPE_INTEGER;
- arg.integer.value = sleep_state;
-
- status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
- if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
- ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
- }
-
- return_ACPI_STATUS(status);
-}
-
-/*******************************************************************************
- *
- * FUNCTION: acpi_leave_sleep_state
- *
- * PARAMETERS: sleep_state - Which sleep state we just exited
- *
- * RETURN: Status
- *
- * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
- * Called with interrupts ENABLED.
- *
- ******************************************************************************/
-acpi_status acpi_leave_sleep_state(u8 sleep_state)
-{
- struct acpi_object_list arg_list;
- union acpi_object arg;
- acpi_status status;
-
- ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
-
/* Ensure enter_sleep_state_prep -> enter_sleep_state ordering */
acpi_gbl_sleep_type_a = ACPI_SLEEP_TYPE_INVALID;
On Sunday, 10 of February 2008, Ray Lee wrote:
> On Feb 10, 2008 9:21 AM, Mirco Tischler <[email protected]> wrote:
> > Hi
> >
> > I think I found a regression in 2.6.24-git. After waking up from suspend
> > 2 ram, the fan of my laptop turns constantly at highest speed. It didn't
> > do this in 2.6.24.
> >
> > I bisected it down to this commit:
> >
> > commit c95d47a868f35cd47643d116a3c680cdaa954df8
> > Author: Rafael J. Wysocki <[email protected]>
> > Date: Tue Jan 8 00:05:21 2008 +0100
> >
> > ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
> >
> > Reverting it resolves the problem for me, but I can't say if this makes
> > any sense.
> >
> > My machine is a Zepto laptop with Intel Santa-Rosa chipset and I'm
> > running a x86_64 Ubuntu Gutsy.
>
> I just noticed the same problem. After about ten minutes, it seems
> like all the ACPI events got 'unstuck' -- my screen's backlight went
> up and down a couple of times (and battery/AC indicator flipped a
> couple of times) responding to previous power loss and gain events, I
> think, and the fans dropped down from high speed to low.
>
> I was chalking it up to another charming bug on my HP nx6125, but if
> someone else is seeing it too...
>
> I'm on Ubuntu, x86_64, and git as of a couple days ago as well. I
> haven't tried reverting the patch.
I've just sent a patch to Mirco, please test it too.
Thanks,
Rafael
On Sunday, 10 of February 2008, Rafaek J. Wysocki wrote:
> Can you apply the appended patch on top of the current mainline and tetest?
>
> Thanks,
> Rafael
>
Sorry, that doesn't fix it.
But I'm pretty sure it is related to that commit. Bisecting went
straight forward to it and reverting it fixes the bug.
@Ray
I'm not sure you have the same problem. After waking up from s2ram, only
my fan runs at highest speed and keeps on, at least for half an hour.
All other acpi-related stuff works as usual. No sign of a stuck
ACPI-system.
Thanks,
Mirco
On Feb 11, 2008 7:56 AM, Mirco Tischler <[email protected]> wrote:
> On Sunday, 10 of February 2008, Rafaek J. Wysocki wrote:
>
> > Can you apply the appended patch on top of the current mainline and tetest?
> >
> > Thanks,
> > Rafael
> >
>
> Sorry, that doesn't fix it.
> But I'm pretty sure it is related to that commit. Bisecting went
> straight forward to it and reverting it fixes the bug.
>
> @Ray
> I'm not sure you have the same problem. After waking up from s2ram, only
> my fan runs at highest speed and keeps on, at least for half an hour.
> All other acpi-related stuff works as usual. No sign of a stuck
> ACPI-system.
Yeah, it may just be another bug in this HP laptop.
Regardless, Rafael, I'm using a kernel with the patch applied, and fan
control after a couple of suspends and resumes is looking fine. But
given how inconsistent this laptop is on resume, I may have just had a
string of bad luck, and now several good cases in a row, I don't know.
I'll run with this for a while and see if anything crops up again.
On Monday, 11 of February 2008, Mirco Tischler wrote:
> On Sunday, 10 of February 2008, Rafaek J. Wysocki wrote:
>
> > Can you apply the appended patch on top of the current mainline and tetest?
> >
> > Thanks,
> > Rafael
> >
>
> Sorry, that doesn't fix it.
> But I'm pretty sure it is related to that commit. Bisecting went
> straight forward to it and reverting it fixes the bug.
Can you send me the output of acpidump from your system, please?
Rafael
On Monday, 11 of February 2008, Mirco Tischler wrote:
> On Sunday, 10 of February 2008, Rafaek J. Wysocki wrote:
>
> > Can you apply the appended patch on top of the current mainline and tetest?
> >
> > Thanks,
> > Rafael
> >
>
> Sorry, that doesn't fix it.
> But I'm pretty sure it is related to that commit. Bisecting went
> straight forward to it and reverting it fixes the bug.
BTW, does the kernel compile for you after reverting this patch?
Rafael
> Can you send me the output of acpidump from your system, please?
I atach the output.
> BTW, does the kernel compile for you after reverting this patch?
>
> Rafael
Yes, the kernel still compiles and runs on 2.6.25-rc1 with the commit
reverted.
I just tested it again.
Mirco
On Monday, 11 of February 2008, Mirco Tischler wrote:
> > Can you send me the output of acpidump from your system, please?
>
> I atach the output.
>
> > BTW, does the kernel compile for you after reverting this patch?
> >
> > Rafael
>
> Yes, the kernel still compiles and runs on 2.6.25-rc1 with the commit
> reverted.
> I just tested it again.
Well, this is strange, because one function introduced by this commit is
referred to by the subsequent commits. Can you send me the patch reverting
this commit that you apply on top of 2.6.25-rc1, please?
Thanks,
Rafael
On Monday, 11 of February 2008, Mirco Tischler wrote:
> > Can you send me the output of acpidump from your system, please?
>
> I atach the output.
>
> > BTW, does the kernel compile for you after reverting this patch?
> >
> > Rafael
>
> Yes, the kernel still compiles and runs on 2.6.25-rc1 with the commit
> reverted.
> I just tested it again.
To eliminate the possible conincidence, can you test the current mainline
with the patch from:
http://lkml.org/lkml/2008/2/11/472
applied, please?
Thanks,
Rafael
On Di, 2008-02-12 at 00:05 +0100, Rafael J. Wysocki wrote:
> Well, this is strange, because one function introduced by this commit
is
> referred to by the subsequent commits. Can you send me the patch
reverting
> this commit that you apply on top of 2.6.25-rc1, please?
>
> Thanks,
> Rafael
Here is the diff after reverting the patch.
Mirco
On Tuesday, 12 of February 2008, Mirco Tischler wrote:
> On Di, 2008-02-12 at 00:05 +0100, Rafael J. Wysocki wrote:
> > Well, this is strange, because one function introduced by this commit
> is
> > referred to by the subsequent commits. Can you send me the patch
> reverting
> > this commit that you apply on top of 2.6.25-rc1, please?
> >
> > Thanks,
> > Rafael
>
> Here is the diff after reverting the patch.
OK, thanks.
Since _GTS and _BFS don't seem to be defined in your box's BIOS, please
try to apply the appended patch on top of the revert and see if that breaks
things again.
Thanks,
Rafael
---
drivers/acpi/hardware/hwsleep.c | 11 -----------
1 file changed, 11 deletions(-)
Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
@@ -199,11 +199,6 @@ acpi_status acpi_enter_sleep_state_prep(
return_ACPI_STATUS(status);
}
- status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
- if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
- return_ACPI_STATUS(status);
- }
-
/* Setup the argument to _SST */
switch (sleep_state) {
@@ -554,12 +549,6 @@ acpi_status acpi_leave_sleep_state(u8 sl
ACPI_EXCEPTION((AE_INFO, status, "During Method _SST"));
}
- arg.integer.value = sleep_state;
- status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
- if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
- ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
- }
-
/*
* GPEs must be enabled before _WAK is called as GPEs
* might get fired there
Am Dienstag, den 12.02.2008, 01:13 +0100 schrieb Rafael J. Wysocki:
> Since _GTS and _BFS don't seem to be defined in your box's BIOS, please
> try to apply the appended patch on top of the revert and see if that breaks
> things again.
>
> Thanks,
> Rafael
>
> ---
> drivers/acpi/hardware/hwsleep.c | 11 -----------
> 1 file changed, 11 deletions(-)
>
> Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> @@ -199,11 +199,6 @@ acpi_status acpi_enter_sleep_state_prep(
> return_ACPI_STATUS(status);
> }
>
> - status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
> - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> - return_ACPI_STATUS(status);
> - }
> -
> /* Setup the argument to _SST */
>
> switch (sleep_state) {
> @@ -554,12 +549,6 @@ acpi_status acpi_leave_sleep_state(u8 sl
> ACPI_EXCEPTION((AE_INFO, status, "During Method _SST"));
> }
>
> - arg.integer.value = sleep_state;
> - status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
> - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> - ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
> - }
> -
> /*
> * GPEs must be enabled before _WAK is called as GPEs
> * might get fired there
Yes, that's it. This patch breaks things again.
Thanks
Mirco
On Tuesday, 12 of February 2008, Mirco Tischler wrote:
>
> Am Dienstag, den 12.02.2008, 01:13 +0100 schrieb Rafael J. Wysocki:
> > Since _GTS and _BFS don't seem to be defined in your box's BIOS, please
> > try to apply the appended patch on top of the revert and see if that breaks
> > things again.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > drivers/acpi/hardware/hwsleep.c | 11 -----------
> > 1 file changed, 11 deletions(-)
> >
> > Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> > +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> > @@ -199,11 +199,6 @@ acpi_status acpi_enter_sleep_state_prep(
> > return_ACPI_STATUS(status);
> > }
> >
> > - status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
> > - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > - return_ACPI_STATUS(status);
> > - }
> > -
> > /* Setup the argument to _SST */
> >
> > switch (sleep_state) {
> > @@ -554,12 +549,6 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > ACPI_EXCEPTION((AE_INFO, status, "During Method _SST"));
> > }
> >
> > - arg.integer.value = sleep_state;
> > - status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
> > - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > - ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
> > - }
> > -
> > /*
> > * GPEs must be enabled before _WAK is called as GPEs
> > * might get fired there
> Yes, that's it. This patch breaks things again.
Ouch. I think I know what the problem is.
On top of this patch, please apply the appended one and retest.
Thanks,
Rafael
---
drivers/acpi/hardware/hwsleep.c | 1 +
1 file changed, 1 insertion(+)
Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
@@ -566,6 +566,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
return_ACPI_STATUS(status);
}
+ arg.integer.value = sleep_state;
status = acpi_evaluate_object(NULL, METHOD_NAME__WAK, &arg_list, NULL);
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "During Method _WAK"));
Am Dienstag, den 12.02.2008, 23:18 +0100 schrieb Rafael J. Wysocki:
> Ouch. I think I know what the problem is.
>
> On top of this patch, please apply the appended one and retest.
>
> Thanks,
> Rafael
>
> ---
> drivers/acpi/hardware/hwsleep.c | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> @@ -566,6 +566,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
> return_ACPI_STATUS(status);
> }
>
> + arg.integer.value = sleep_state;
> status = acpi_evaluate_object(NULL, METHOD_NAME__WAK, &arg_list, NULL);
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> ACPI_EXCEPTION((AE_INFO, status, "During Method _WAK"));
Good news. That works perfectly.
Thanks
Mirco
On Wednesday, 13 of February 2008, Mirco Tischler wrote:
>
> Am Dienstag, den 12.02.2008, 23:18 +0100 schrieb Rafael J. Wysocki:
> > Ouch. I think I know what the problem is.
> >
> > On top of this patch, please apply the appended one and retest.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > drivers/acpi/hardware/hwsleep.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> > +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> > @@ -566,6 +566,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > return_ACPI_STATUS(status);
> > }
> >
> > + arg.integer.value = sleep_state;
> > status = acpi_evaluate_object(NULL, METHOD_NAME__WAK, &arg_list, NULL);
> > if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > ACPI_EXCEPTION((AE_INFO, status, "During Method _WAK"));
> Good news. That works perfectly.
Ah, ok. Thanks for testing. :-)
Can you please check if the current mainline with the following patch applied
works on your box?
Thanks,
Rafael
---
drivers/acpi/hardware/hwsleep.c | 1 +
1 file changed, 1 insertion(+)
Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
@@ -616,6 +616,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
return_ACPI_STATUS(status);
}
+ arg.integer.value = sleep_state;
status = acpi_evaluate_object(NULL, METHOD_NAME__WAK, &arg_list, NULL);
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "During Method _WAK"));
On Mi, 2008-02-13 at 00:23 +0100, Rafael J. Wysocki wrote:
> Ah, ok. Thanks for testing. :-)
>
> Can you please check if the current mainline with the following patch applied
> works on your box?
>
> Thanks,
> Rafael
>
> ---
> drivers/acpi/hardware/hwsleep.c | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> @@ -616,6 +616,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
> return_ACPI_STATUS(status);
> }
>
> + arg.integer.value = sleep_state;
> status = acpi_evaluate_object(NULL, METHOD_NAME__WAK, &arg_list, NULL);
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> ACPI_EXCEPTION((AE_INFO, status, "During Method _WAK"));
Yes, works with current mainline too.
Much thanks for your help
Mirco
On Wednesday, 13 of February 2008, Mirco Tischler wrote:
>
> On Mi, 2008-02-13 at 00:23 +0100, Rafael J. Wysocki wrote:
> > Ah, ok. Thanks for testing. :-)
> >
> > Can you please check if the current mainline with the following patch applied
> > works on your box?
> >
> > Thanks,
> > Rafael
> >
> > ---
> > drivers/acpi/hardware/hwsleep.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> > +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> > @@ -616,6 +616,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > return_ACPI_STATUS(status);
> > }
> >
> > + arg.integer.value = sleep_state;
> > status = acpi_evaluate_object(NULL, METHOD_NAME__WAK, &arg_list, NULL);
> > if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > ACPI_EXCEPTION((AE_INFO, status, "During Method _WAK"));
> Yes, works with current mainline too.
>
> Much thanks for your help
Well, thanks for your patience. :-)
It's a well-hidden bug, so it probably would have taken much time to find it
without your ability to reproduce the problem.
Thanks,
Rafael