From: Chris Chiu <chiu@endlessm.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Linux ACPI <linux-acpi@vger.kernel.org>,
Linux PM <linux-pm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH[RFT]] ACPI: EC: s2idle: Avoid flushing EC work when EC GPE is inactive
Date: Mon, 18 May 2020 19:11:41 +0800 [thread overview]
Message-ID: <CAB4CAwc4y7xitv9L9w61GKfBsbhHXuk+iM+QWKc2=0mks_fNFg@mail.gmail.com> (raw)
In-Reply-To: <CAJZ5v0j6S+we7tHeV9TM30LS+TO3zWigACe0ZUFfWphg2FBBZQ@mail.gmail.com>
On Mon, May 18, 2020 at 4:59 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, May 14, 2020 at 12:10 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Flushing the EC work while suspended to idle when the EC GPE status
> > is not set causes some EC wakeup events (notably power button and
> > lid ones) to be missed after a series of spurious wakeups on the Dell
> > XPS13 9360 in my office.
> >
> > If that happens, the machine cannot be woken up from suspend-to-idle
> > by a power button press or lid status change and it needs to be woken
> > up in some other way (eg. by a key press).
> >
> > Flushing the EC work only after successful dispatching the EC GPE,
> > which means that its status has been set, avoids the issue, so change
> > the code in question accordingly.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > Hi Chris,
> >
> > Please check if the key press wakeup still works on your system with this patch
> > applied (on top of https://patchwork.kernel.org/patch/11538065/).
>
> Hi Chris,
>
> Since I haven't heard back from you and the problem at hand is a
> regression on the machine where it happens, I'm going to push this
> patch for merging.
>
> If it causes the key press wakeup issue to reappear on your machine,
> I'm afraid that we'll need to quirk it in the EC driver.
>
> Thanks!
Hi Rafael,
My laptop works w/o problem waking up from a keystroke with this patch
on top of https://patchwork.kernel.org/patch/11538065/).
Chris
>
> > ---
> > drivers/acpi/ec.c | 6 +++++-
> > drivers/acpi/sleep.c | 15 ++++-----------
> > 2 files changed, 9 insertions(+), 12 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/ec.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/ec.c
> > +++ linux-pm/drivers/acpi/ec.c
> > @@ -2020,9 +2020,13 @@ bool acpi_ec_dispatch_gpe(void)
> > * to allow the caller to process events properly after that.
> > */
> > ret = acpi_dispatch_gpe(NULL, first_ec->gpe);
> > - if (ret == ACPI_INTERRUPT_HANDLED)
> > + if (ret == ACPI_INTERRUPT_HANDLED) {
> > pm_pr_dbg("EC GPE dispatched\n");
> >
> > + /* Flush the event and query workqueues. */
> > + acpi_ec_flush_work();
> > + }
> > +
> > return false;
> > }
> > #endif /* CONFIG_PM_SLEEP */
> > Index: linux-pm/drivers/acpi/sleep.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/sleep.c
> > +++ linux-pm/drivers/acpi/sleep.c
> > @@ -980,13 +980,6 @@ static int acpi_s2idle_prepare_late(void
> > return 0;
> > }
> >
> > -static void acpi_s2idle_sync(void)
> > -{
> > - /* The EC driver uses special workqueues that need to be flushed. */
> > - acpi_ec_flush_work();
> > - acpi_os_wait_events_complete(); /* synchronize Notify handling */
> > -}
> > -
> > static bool acpi_s2idle_wake(void)
> > {
> > if (!acpi_sci_irq_valid())
> > @@ -1018,7 +1011,7 @@ static bool acpi_s2idle_wake(void)
> > return true;
> >
> > /*
> > - * Cancel the wakeup and process all pending events in case
> > + * Cancel the SCI wakeup and process all pending events in case
> > * there are any wakeup ones in there.
> > *
> > * Note that if any non-EC GPEs are active at this point, the
> > @@ -1026,8 +1019,7 @@ static bool acpi_s2idle_wake(void)
> > * should be missed by canceling the wakeup here.
> > */
> > pm_system_cancel_wakeup();
> > -
> > - acpi_s2idle_sync();
> > + acpi_os_wait_events_complete();
> >
> > /*
> > * The SCI is in the "suspended" state now and it cannot produce
> > @@ -1060,7 +1052,8 @@ static void acpi_s2idle_restore(void)
> > * of GPEs.
> > */
> > acpi_os_wait_events_complete(); /* synchronize GPE processing */
> > - acpi_s2idle_sync();
> > + acpi_ec_flush_work(); /* flush the EC driver's workqueues */
> > + acpi_os_wait_events_complete(); /* synchronize Notify handling */
> >
> > s2idle_wakeup = false;
> >
> >
> >
> >
next prev parent reply other threads:[~2020-05-18 11:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-14 10:10 [PATCH[RFT]] ACPI: EC: s2idle: Avoid flushing EC work when EC GPE is inactive Rafael J. Wysocki
2020-05-18 8:59 ` Rafael J. Wysocki
2020-05-18 9:32 ` Chris Chiu
2020-05-18 11:11 ` Chris Chiu [this message]
2020-05-18 11:14 ` Rafael J. Wysocki
Reply instructions:
You may reply publicly 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='CAB4CAwc4y7xitv9L9w61GKfBsbhHXuk+iM+QWKc2=0mks_fNFg@mail.gmail.com' \
--to=chiu@endlessm.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).