RE: [PATCH v2 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems

[Zheng, Lv]

Hi, Rafael

> From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] On Behalf Of Rafael J.
> Wysocki
> Subject: Re: [PATCH v2 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems
> 
> On Tue, Jun 20, 2017 at 2:07 AM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, Jun 20, 2017 at 5:53 AM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> >>
> >> -> v2: Added acpi_sleep=no_ec_wakeup to prevent EC events from waking up
> >>           the system from s2idle on systems where they do that by default.
> >
> > This seems a big hacky.
> >
> > Is there no way to simply make acpi_ec_suspend() smarter while going
> > to sleep? Instead of just unconditionally disabling every EC GPE, can
> > we see that "this gpe is the power botton" somehow?
> 
> Unfortunately, the connection between the GPE and the power button is
> not direct.
> 
> The EC GPE handler has no idea that it will generate power button
> events.  It simply executes an AML method doing that.
> 
> The AML method, in turn, executes Notify(power button device) and the
> "power button device" driver has to register a notify handler that
> will recognize and process the events.  It doesn't know in principle
> where the events will come from, though.  They may come from the EC or
> from a different GPE etc.
> 
> Neither the EC driver, nor the "power button device" driver can figure
> out that the connection is there.

The EC driver can only get an event number after querying the firmware.
And it has no idea whether handling this event by executing _Exx where
Xx is the number of the event can result in Notify(power button device).

Traditional ACPI power button events are ACPI fixed events, not EC GPE:

Power button signal
A power button can be supplied in two ways.
 One way is to simply use the fixed status bit, and
 The other uses the declaration of an ACPI power device and AML code to
  determine the event.
For more information about the alternate-device based power button, see
Section 4.8.2.2.1.2, Control Method Power Button.â

If it is not designed as fixed event, OS has no idea what GPE, or
EC event number is related to the power button.

> 
> > Disabling the power button event sounds fundamentally broken, and it
> > sounds like Windows doesn't do that. I doubt Windows has some hacky
> > whitelist. So I'd rather fix a deeper issue than have these kinds of
> > hacks, if at all possible.
> 
> My understanding is that Windows uses the ACPI_FADT_LOW_POWER_S0 flag.
> It generally enables non-S3 suspend/resume when this flag is set and
> it doesn't touch S3 then.  Keeping the EC GPE (and other GPEs for that
> matter) enabled over suspend/resume is part of that if my
> understanding is correct.

This sounds reasonable, but I have a question.

On Surface notebooks, an EC GPE wake capable setting is prepared:
                    Device (EC0)
                    {
                        Name (_HID, EisaId ("PNP0C09"))  // _HID: Hardware ID
                        ...
                        Method (_STA, 0, NotSerialized)  // _STA: Status
                        {
                            ...
                            Return (0x0F)
                        }
                        Name (_GPE, 0x38)  // _GPE: General Purpose Events
                        Name (_PRW, Package (0x02)  // _PRW: Power Resources for Wake
                        {
                            0x38, 
                            0x03
                        })

The _PRW means GPE 0x38 (EC GPE) can wake-up the system from S3-S0.
And the platform only supports s2idle.
Decoding its FADT, we can see the flag is set:
[070h 0112   4]        Flags (decoded below) : 002384B5
...
                      Low Power S0 Idle (V5) : 1

If EC GPE should always be enabled when the flag is set, why MS
(surface pros are manufactured by MS) prepares _PRW for its EC?

Thanks,
Lv

> 
> During suspend we generally disable all GPEs that are not expected to
> generate wakeup events in order to avoid spurious wakeups, but we can
> try to keep them enabled if ACPI_FADT_LOW_POWER_S0 is set.  That will
> reduce the ugliness, but the cost may be more energy used while
> suspended on some systems.
> 
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html