On Tue, 2007-07-31 at 15:42 +0200, Rafael J. Wysocki wrote:
> On Tuesday, 31 July 2007 12:42, Thomas Renninger wrote:
> > On Tue, 2007-07-31 at 10:07 +0100, Richard Hughes wrote:
> > > On Tue, 2007-07-31 at 11:08 +0200, Rafael J. Wysocki wrote:
> > > > On Monday, 30 July 2007 17:33, Richard Hughes wrote:
> > > > > On Mon, 2007-07-30 at 17:21 +0200, Thomas Renninger wrote:
> > > > > > On Tue, 2007-07-17 at 12:15 +0100, Richard Hughes wrote:
> > > > > > > On resume we need to refresh the lid status as we will not get an
> > > > > > > event if
> > > > > > > the lid opening was what triggered the suspend.
> > > > > > > This manifests itself in users never getting a "lid open" event
> > > > > > > when a
> > > > > > > suspend happens because of lid close on hardware that supports
> > > > > > > wake on
> > > > > > > lid open. This makes userspace gets very confused indeed.
> > > > > > > Patch inline (and also attached) forces a check of the lid status
> > > > > > > in the
> > > > > > > resume handler.
> > > > > > Is this a general problem on all machines?
> > > > >
> > > > > I've only seen myself it on new ThinkPads such as the T61 and X60,
> > > > > although I've been getting a few bug reports about other IBM laptops.
> > > > >
> > > > > > Or does this only happen if "shutdown" suspend mode is used?
> > > > >
> > > > > No, I don't believe so.
> > > > >
> > > > > > I could imagine a lot machines let it up to OS to check for LID
> > > > > > state
> > > > > > change, then this one should be added.
> > > > >
> > > > > I guess it's up to the BIOS, and I don't think this refresh hurts any
> > > > > machines that implement a notify on resume, and fixes a fair few
> > > > > machines that don't.
> > > >
> > > > AFAICS, the notify doesn't seem to work very well on some machines.
> > >
> > > Agree.
> > >
> > > > Are there any downsides of the $subject patch?
> > >
> > > Not that I've found. I've been testing it on ~6 IBM and non-IBM machines
> > > with no bad effects so far.
> >
> > I just checked a X60 DSDT (couldn't check the SSDTs, but I doubt there
> > is anything related):
> > There are two Notify(\_SB.LID,0x80), both are in GPE handlers.
> > AFAIK there should be one in the _WAK function.
>
> Well, we only enable GPEs after calling _WAK, so this one won't trigger.
>
> Perhaps we should change the code ordering in acpi_leave_sleep_state() to
> enable GPEs before executing _WAK?
>
> > Maybe they try to raise the GPE after wakeup in _WAK by something like
> > this:
> > \VSLD (\_SB.LID._LID ())
> > ....
> > Method (VSLD, 1, NotSerialized)
> > {
> > SMI (0x01, 0x07, Arg0, 0x00, 0x00)
> > }
> > :)
> >
> >
> > Related ACPI Spec parts:
> >
> > 6.3 Device Insertion, Removal, and Status Objects:
> > The Notify command can also be used from the _WAK control method (for
> > more information about _WAK, see section 7.3.7 “\_WAK (System Wake)”) to
> > indicate device changes that may have occurred while the computer was
> > sleeping. For more information about the Notify command,
> > see section 5.6.3 “Device Object Notification.”.”
> >
> > The X60 is definitely not doing this.
> >
> > The transition from Working to Sleep state is described very detailed,
> > but I couldn't find (just overseen?) a detailed description about the
> > transition from Sleep State to working state.
> > In detail I searched for whether first the GPEs should get enabled and
> > then _WAK is called or the other way around (the latter is currently
> > implemented).
> > Maybe enabling GPEs before calling _WAK will also fix this
>
> Well, my thought above. :-)
>
> > (and is the way it should be done or at least the way M$ is doing it?).
>
> I don't know ...
>
> > Richard, could you give attached patch a try, pls.
> > Also check that platform suspend mode is used. AFAIK this isn't called
> > at all in suspend mode.
> >
> > Thanks,
> >
> > Thomas
> >
> > -----------------------
> >
> > Enable GPEs before calling _WAK on resume
> >
> > Signed-off-by: Thomas Renninger <[EMAIL PROTECTED]>
> >
> > ---
> > drivers/acpi/hardware/hwsleep.c | 31 +++++++++++++++++--------------
> > 1 file changed, 17 insertions(+), 14 deletions(-)
> >
> > Index: linux-2.6.22.1/drivers/acpi/hardware/hwsleep.c
> > ===================================================================
> > --- linux-2.6.22.1.orig/drivers/acpi/hardware/hwsleep.c
> > +++ linux-2.6.22.1/drivers/acpi/hardware/hwsleep.c
> > @@ -562,6 +562,23 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > arg_list.pointer = &arg;
> > arg.type = ACPI_TYPE_INTEGER;
> >
> > + /*
> > + * GPEs must be enabled before _WAK is called as GPEs
> > + * might get fired there
> > + *
> > + * Restore the GPEs:
> > + * 1) Disable/Clear all GPEs
> > + * 2) Enable all runtime GPEs
> > + */
> > + status = acpi_hw_disable_all_gpes();
> > + if (ACPI_FAILURE(status)) {
> > + return_ACPI_STATUS(status);
> > + }
> > + status = acpi_hw_enable_all_runtime_gpes();
> > + if (ACPI_FAILURE(status)) {
> > + return_ACPI_STATUS(status);
> > + }
> > +
>
> I wouldn't move that before _BFS, just in case someone actually implements it.
Yes you are right, thanks.
(ACPI spec):
-----------
OSPM will execute the _BFS control method before performing any other
physical I/O or enabling any interrupt servicing upon returning
from a sleeping state.
-----------
Now it makes sense to have _BFS and _WAK, before it had not made a
difference from BIOS programmer point of view to use _BFS or _WAK.
With some luck this fixes some other things, I remember a weird bug on
(X60?) thinkpad:
If you suspend to RAM you can wakeup with the blue FN key, after doing a
suspend to disk and then doing a suspend to RAM the blue FN key does not
wake the machine anymore from STR :)
Attached an updated patch (Rafael, I added your Acked from comments
above. I just moved GPE enabling between _BFS and _WAK as you
suggested, pls scream if you still find something bad).
Len, can you commit this one, pls.
Thanks,
Thomas
------------
Enable GPEs before calling _WAK on resume
It seems it's required to enable GPEs before _WAK.
E.g. X60 triggers a LID related GPE instead of doing a Notify in WAK.
Now the GPE reaches the kernel and the Notify for LID status
change gets thrown from there.
Signed-off-by: Thomas Renninger <[EMAIL PROTECTED]>
Acked-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
---
drivers/acpi/hardware/hwsleep.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
Index: linux-2.6.22.1/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.22.1.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6.22.1/drivers/acpi/hardware/hwsleep.c
@@ -576,13 +576,10 @@ acpi_status acpi_leave_sleep_state(u8 sl
ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
}
- 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"));
- }
- /* TBD: _WAK "sometimes" returns stuff - do we want to look at it? */
-
/*
+ * GPEs must be enabled before _WAK is called as GPEs
+ * might get fired there
+ *
* Restore the GPEs:
* 1) Disable/Clear all GPEs
* 2) Enable all runtime GPEs
@@ -591,13 +588,19 @@ acpi_status acpi_leave_sleep_state(u8 sl
if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status);
}
- acpi_gbl_system_awake_and_running = TRUE;
-
status = acpi_hw_enable_all_runtime_gpes();
if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status);
}
+ 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"));
+ }
+ /* TBD: _WAK "sometimes" returns stuff - do we want to look at it? */
+
+ acpi_gbl_system_awake_and_running = TRUE;
+
/* Enable power button */
(void)
