login
Search
Hi Rafael, Matthew,
my laptop changed behavior during poweroff recently (after upgrading
from .33 to .34-rc1). The system seems to be powered off (status display
is off) at first glance but when I close the lid then I can hear a noise
which sounds like HDD parking and when I open lid again it starts
booting without poweroff button (same like when I suspend to RAM).
I have bisected that down to the following commit:
commit 9630bdd9b15d2f489c646d8bc04b60e53eb5ec78
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date: Wed Feb 17 23:41:07 2010 +0100
ACPI: Use GPE reference counting to support shared GPEs
ACPI GPEs may map to multiple devices. The current GPE interface
only provides a mechanism for enabling and disabling GPEs, making
it difficult to change the state of GPEs at runtime without extensive
cooperation between devices.
Add an API to allow devices to indicate whether or not they want
their device's GPE to be enabled for both runtime and wakeup events.
Remove the old GPE type handling entirely, which gets rid of various
quirks, like the implicit disabling with GPE type setting. This
requires a small amount of rework in order to ensure that non-wake
GPEs are enabled by default to preserve existing behaviour.
Based on patches from Matthew Garrett <mjg@redhat.com>.
Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
When I revert this commit, the system behaves like before. I am not sure
which kind of information might be useful so I have attached only config
file for now. The laptop is Futjitsu Siemens Lifebook S Series.
Let me know what else might help for further investigation.
Thanks
and
best regards
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
Hm. At a guess, we're somehow managing to leave the lid GPE enabled at poweroff. Can you send the output of the acpidump command? -- Matthew Garrett | mjg59@srcf.ucam.org --
Sure, see attached. -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
Thanks. Please also send the contents of /proc/acpi/wakeup. Rafael --
Sorry for late reply, but I was AFK during holiday. See attached. I have checked both kernels (with and without reverted 9630bdd) and the output looks pretty much same. 2.6.34-rc3, however, -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
Well, I fail to see the possible failure scenario, so let's first try a blind shot. Please check if this patch helps: https://patchwork.kernel.org/patch/86238/ Rafael --
No change, sorry. I have tried that on top of 0fdf867 which is not the most recent one (does it make sense to retest on the most recent one -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic --
There were no other changes in that area I know of, but the current git contains the above patch, so I'd recommend to use it for further testing. I'll try to prepare a debug patch for this issue. Rafael --
Rafael, I have the exact same "lid close = powering back on" regression on a Fujitsu-Siemens Lifebook S6420. Totally missed this thread before, but I'm glad that I'm not alone with this one. Happy to test any patches you may come up with; will an acpidump do for you or do you need additional data? Regards, -- Tony Vroon UNIX systems administrator London Internet Exchange Ltd, Trinity Court, Trinity Street, Peterborough, PE1 1DA Registered in England number 3137929 E-Mail: tony@linx.net
I need to figure out what actually happens and I'll need you to try a debug patch, but it's not ready yet. Rafael --
Please check if the patch below changes anything. Rafael --- drivers/acpi/wakeup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: linux-2.6/drivers/acpi/wakeup.c =================================================================== --- linux-2.6.orig/drivers/acpi/wakeup.c +++ linux-2.6/drivers/acpi/wakeup.c @@ -71,9 +71,9 @@ void acpi_enable_wakeup_device(u8 sleep_ || sleep_state > (u32) dev->wakeup.sleep_state) continue; - /* The wake-up power should have been enabled already. */ + /* The wake-up power should have been enabled already. acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, - ACPI_GPE_ENABLE); + ACPI_GPE_ENABLE);*/ } } --
That didn't change the behaviour for me, sorry. (I made sure to go through a full power down session before trying the patched kernel) Regards, -- Tony Vroon UNIX systems administrator London Internet Exchange Ltd, Trinity Court, Trinity Street, Peterborough, PE1 1DA Registered in England number 3137929 E-Mail: tony@linx.net
Thanks for testing. So it looks like we don't disable the GPE during power off. I'll try to figure out what's going on, please stay tuned. Rafael --
Can you please check if the patch below changes the behavior?
Rafael
---
drivers/acpi/wakeup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
--- linux-2.6.orig/drivers/acpi/wakeup.c
+++ linux-2.6/drivers/acpi/wakeup.c
@@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list);
+ u8 action = ACPI_GPE_ENABLE;
if (!dev->wakeup.flags.valid)
continue;
if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
|| sleep_state > (u32) dev->wakeup.sleep_state)
- continue;
+ action = ACPI_GPE_DISABLE;
- /* The wake-up power should have been enabled already. */
acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
- ACPI_GPE_ENABLE);
+ action);
}
}
--
Unfortunately, it didn't help either (I have tried on top of the fresh -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic --
Unfortunately didn't help as well...
Just for reference:
diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
index 248b473..f23c08f 100644
--- a/drivers/acpi/wakeup.c
+++ b/drivers/acpi/wakeup.c
@@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list);
- u8 action = ACPI_GPE_ENABLE;
+ u8 action = ACPI_GPE_DISABLE;
if (!dev->wakeup.flags.valid)
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
That probably means the chipset enables the GPEs by itself _after_ we've disabled them in acpi_enable_wakeup_device(). Unfortunately, I can't reproduce the issue on any of my test boxes and it's hard to find the source of the problem staring at the code. Rafael --
Are there any debug options I can turn on to provide some information? Btw. what exactly does this mean? In what state is the laptop while it is turned off and GPE is enabled? -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic --
We can only check what the kernel tells us before power off, but all that seems If a GPE is enabled, then some part of the chipset has power provided so that it can signal wakeup. I'll look into it a bit more later today. Thanks, Rafael --
Please try the patch below. It kind of restores the previous behavior,
let's see if it changes anything.
Rafael
---
drivers/acpi/acpica/evgpeblk.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c
+++ linux-2.6/drivers/acpi/acpica/evgpeblk.c
@@ -364,7 +364,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob
union acpi_operand_object *pkg_desc;
union acpi_operand_object *obj_desc;
u32 gpe_number;
- acpi_status status;
+ acpi_status status = AE_OK;
ACPI_FUNCTION_TRACE(ev_match_prw_and_gpe);
@@ -439,13 +439,15 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob
if (gpe_device == target_gpe_device) {
gpe_event_info = acpi_ev_gpeblk_event_info(gpe_block,
gpe_number);
- if (gpe_event_info)
+ if (gpe_event_info) {
+ status = acpi_ev_disable_gpe(gpe_event_info);
gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
+ }
}
cleanup:
acpi_ut_remove_reference(pkg_desc);
- return_ACPI_STATUS(AE_OK);
+ return_ACPI_STATUS(status);
}
/*******************************************************************************
--
If it doesn't help, please try to comment out the acpi_enable_gpe() in drivers/acpi/wakeup.c:acpi_wakeup_device_init() and retest (if you haven't done that already). Rafael --
Again, no success. Just to make sure that I didn't screw anything. I
have used just the following patch on top of the clean rc5 (your patch
has failed with some rejects but I guess that the following should do
the same):
commit 65eafe4a504e3bb2c13b4feb8590dc52e7439baa
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date: Sun Apr 25 04:35:42 2010 +0200
diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
index fef7219..b6e1e0c 100644
--- a/drivers/acpi/acpica/evgpeblk.c
+++ b/drivers/acpi/acpica/evgpeblk.c
@@ -367,7 +367,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle obj_handle,
union acpi_operand_object *pkg_desc;
union acpi_operand_object *obj_desc;
u32 gpe_number;
- acpi_status status;
+ acpi_status status = AE_OK;
ACPI_FUNCTION_TRACE(ev_match_prw_and_gpe);
@@ -447,12 +447,13 @@ acpi_ev_match_prw_and_gpe(acpi_handle obj_handle,
gpe_block->
block_base_number];
+ status = acpi_ev_disable_gpe(gpe_event_info);
gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
}
cleanup:
acpi_ut_remove_reference(pkg_desc);
- return_ACPI_STATUS(AE_OK);
+ return_ACPI_STATUS(status);
}
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
Perhaps you can review this description and help me better understand the symptoms? Michal, Re: poweroff Before the regression you could hear the HDD park as a result of the "poweroff" command without touching the lid, and after the regression you don't hear that sound until after you close the lid -- no matter how long you wait to close the lid? During this period after poweroff and LCD goes out, but before lid close, are there any other signs of life? eg. does caps lock button light the caps LED? How about during this period if you press the power button and hold it for 5 seconds, does that cause the HDD parking sound? If yes, then the regression is that poweroff never completes, though perhaps a lid event allows it to complete. When you say 'when I open the lid again i starts booting'. Is that a cold boot from scratch, or a resume from suspend? Does the new boot start when the lid is closed, or when it is subsequently opened? Thanks for sending your /proc/acpi/wakeup, it shows that LID S4 *enabled which means that the LID is not an S5 wakeup device. So if you really do get down to S5, the LID should not be able to wake the system. So if it is, then we are never getting to S5. Tony, Please send the /proc/acpi/wakeup file before and after the regression. Does the system reach a complete poweroff before the lid is closed? If yes, what happens when you press the power button w/o closing the lid? If you reached S5, it should give you a cold boot. If you did not reach S5, it will have no effect. thanks, Len Brown, Intel Open Source Technology Center --
Hi Len, It is hard to tell whether this sound comes from HDD and I think that it is actually something different as I've realized that I've heard this kind of sound only when system was suspended to RAM and lid was closed. Yes. Also the main difference is that the laptop turns on automatically No signs of life at all. The small display which shows the status (charging, battery status, caps lock, num lock, etc.) is turned off. No sound. Even when I closed the lid. Nevertheless, laptop doesn't I have not seen start during lid being closed. As I already said, the Thanks! -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic --
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Holding down the power button for 5 seconds has no audible or visible effects, but does indeed properly shut it down. Closing or opening the lid afterwards does not cause a phantom power-up. A short push of the power button seems to start the machine up normally (from a cold boot; just like when the lid is closed/opened after a linux I don't have this file in my /proc filesystem, sorry. Regards, - -- Tony Vroon UNIX systems administrator London Internet Exchange Ltd, Trinity Court, Trinity Street, Peterborough, PE1 1DA Registered in England number 3137929 E-Mail: tony@linx.net -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkvZmdYACgkQp5vW4rUFj5oUxgCdGPnfQsSE9jhjSzzar/kQbw95 Er4AmgLGStZlrvGZltqsU/REs2srO95A =6VLH -----END PGP SIGNATURE----- --
Thanks for testing. Did you try the second thing, ie. try to comment out the acpi_enable_gpe() in drivers/acpi/wakeup.c:acpi_wakeup_device_init() and retest (if you haven't done that already)? Rafael --
I have tried it just now and still no success.
commit 1a7e7c9ac82aa2de185fa2f686417a5eb7765420
Author: Michal Hocko <mhocko@suse.cz>
Date: Tue Apr 27 08:22:23 2010 +0200
disable acpi_enable_gpe in acpi_wakeup_device_init
as suggested by Rafael.
diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
index 4b9d339..a0f4859 100644
--- a/drivers/acpi/wakeup.c
+++ b/drivers/acpi/wakeup.c
@@ -113,8 +113,9 @@ int __init acpi_wakeup_device_init(void)
if (!dev->wakeup.flags.always_enabled ||
dev->wakeup.state.enabled)
continue;
- acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
+ /*acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
ACPI_GPE_TYPE_WAKE);
+ */
dev->wakeup.state.enabled = 1;
}
mutex_unlock(&acpi_device_lock);
On top of the previous patch. If I understand that correctly then this
is really strange because this patch disables GPE completely, doesn't
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
Kind of, but this only happens for devices that are not handled by the button driver which now has become our primary suspect. Rafael --
I'm thinking at the moment that we probably enable something at the hardware level that we didn't before. Why it is not _disabled_ before power off although there are all of the necessary things in the code remains to be a mystery to me. Rafael --
