Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-acpi <linux-acpi@vger.kernel.org>
Subject: Problems with s2idle and _AEI handlers which are marked as wakeup
Date: Thu, 21 Nov 2019 20:05:23 +0100
Message-ID: <61450f9b-cbc6-0c09-8b3a-aff6bf9a0b3c@redhat.com> (raw)

Hi Rafael,

I received an email from a user about his HP X2 10-x2-10-p018wm 2-in-1
not suspending. I checked which devices I have access to and found the
very similar HP X2 10-x2-10-p002nd model and I've reproduced this on
this model and I have spend 1.5 days debugging this.

There are 2 causes of spurious wakeups, 1 cause is the INT0002 vgpio
device. I have a local fix (dmi quirk) for the driver for that, but
that otherwise is out of scope for this discussion.

The other, more troublesome, cause of spurious wakeups is the way the
embedded-controller is hooked up. AFAICT normally Cherry Trail tablet
designs follow the so called "Hardware-reduced ACPI model" and they
do not have a classic external (to the SoC) EC.

But HP seems to have decided they wanted an external EC anyways so
they have added one, but since the SoC lacks the regular interface
bits for this it is bolted on and it does not use the standard ACPI EC
interface :|

Instead it uses I2C accesses and to report events it uses a _AEI handler
connected to one of the SoC's GPIOs. This handler is marked
"ExclusiveAndWake" which is not surprising for an EC event hander,
but since this is not using the standard ACPI EC mechanism,
this _AEI handler avoids the workaround for spurious EC wakeups from
acpi_s2idle_wake(), and thus is causing spurious wakeups.
Here is an acpidump + ready to use dsdt.dsl:
https://fedorapeople.org/~jwrdegoede/hp-x2-10-cht-p002nd/

To be precise, the _AEI handler calls a method called INTC() (defined at
line 24836 of dsdt.dsl) which does:

                 Local0 = GEVT ()
                 Switch (Local0)
                 {
                     Case (0x09)
                         do-stuff...
                     Case (0x0A)
                         do-other-stuff...

Etc. GEVT reads an event register over i2c and then the event gets
dispatched. While suspended 2 events trigger:

0x33 calls SYNT(), which reads time from RTC, writes it to EC
  This happens say every half hour or so
0x85 calls STMP(), which reads something through SMI, writes result to EC register CM17
  When suspended through the power-button this does not get called.
  When suspended by closing the LID, this gets called once a minute.

These really should just be handled and then we should go back to sleep,
but with the way we currently handle s2idle these events cause a full
wakeup.

I've checked what Windows does and for s2idle Windows seems to just power
down as much as possible, without really "freezing" as we do. Since it is
hard to detect a spurious wakeup with the LID closed I used ping to detect
wake-ups, with Windows this does not work, it simply keeps on pinging when
the LID is closed, so it seems to stay somewhat more "alive" then we do
and likely does not have issues with HP's approach here because of that.

I have the feeling that supporting HP's approach here is not going to be
easy. My main reason for sending out this email is to get my findings out
here and to start a discussion on how we can possibly fix this.

Since I do not expect this to get fixed anytime soon, for now I'm going
to submit a workaround where I disable the wakeup flag on the _AEI handler
with a quirk. This should be safe to do in this case, the event 0x33 thing
seems ok to ignore and since when the LID is not closed no 0x85 events happen
I assume we can safely ignore those while suspended too. This does mean that
when opening the LID the device will not wakeup without pressing the
power-button as that is handled by the INTC() method too, in that case
it also does:

                            Notify (PWRB, 0x02) // Device Wake

So it seems that any solution for this would involve ignoring _AEI
handlers as wakeup sources (like we do for EC events) except when they
do a Notify with a parameter of 0x02 on some device.

Regards,

Hans


                 reply index

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=61450f9b-cbc6-0c09-8b3a-aff6bf9a0b3c@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git