Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1631738 > unrolled thread

[PATCH 0/5] PM / sleep: Support power button wakeup from S2I on recent Dell laptops

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] PM / sleep: Support power button wakeup from S2I on recent Dell laptops "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-26 23:40 +0200
    [PATCH 1/5] PM / wakeup: Integrate mechanism to abort transitions in progress "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-26 23:40 +0200
    [PATCH 2/5] ACPI / sleep: Ignore spurious SCI wakeups from suspend-to-idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-26 23:40 +0200
    [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-26 23:40 +0200
      RE: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on  Dell systems <Mario.Limonciello@dell.com> - 2017-04-27 16:50 +0200
        Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-28 00:40 +0200
          RE: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle  on Dell systems "Zheng, Lv" <lv.zheng@intel.com> - 2017-05-04 10:00 +0200
            Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-05-04 16:40 +0200
              Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-05-04 16:40 +0200
                RE: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle  on Dell systems "Zheng, Lv" <lv.zheng@intel.com> - 2017-05-05 02:40 +0200
    [PATCH 4/5] platform: x86: intel-vbtn: Wake up the system from suspend-to-idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-26 23:40 +0200
    [PATCH 5/5]  platform: x86: intel-hid: Wake up the system from suspend-to-idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-26 23:40 +0200
    Re: [PATCH 0/5] PM / sleep: Support power button wakeup from S2I on recent Dell laptops "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-05-04 17:00 +0200

#1631738 — [PATCH 0/5] PM / sleep: Support power button wakeup from S2I on recent Dell laptops

Hi All,

The underlying issue is that on some relatively new Dell laltops, including
Dell XPS13 9360 and 9365, pressing the power button is not sufficient to
wake up the system from suspend-to-idle (it has to be pressed and held
down for around 5 sec for the wakeup to happen) which is not expected
and does not match the Windows' behavior.

This turns out to be a consequence of the way power button events are signaled
on those systems, which is through the Embedded Controller (EC).  Namely,
button events go to the EC which then signals the event through its ACPI GPE
(General Purpose Event), which triggers an ACPI SCI (System Control Interrupt),
whose handler executes a specicif AML control method and triggers a Notify()
targetting a devie object associated with the power button.  The problem with
suspend-to-idle is that the EC GPE is disabled during suspend, because
otherwise all EC events would wake up the system from suspend-to-idle (and
there can be many of them).

The only way to address this is to allow the EC GPE to stay enabled while
suspended to idle, but that cannot be done unconditionally, because of the
possible spurious wakeup events from the EC.  Fortunately, on the Dell systems
in question it is possible to reduce the number of events coming from the EC
while suspended (see the changelog of patch [3/5] for details), but still some
spurious events may happen.  For this reason, when there is an SCI while
suspended, it is not practical to resume all the way up to user space.
Instead, it is better to resume partially up to the point at which the SCI can
be processed by its handler, which is after calling dpm_resume_noirq(), let
the SCI be processed and if no real wakeup events come out of that,
suspend again.  Actually, that can be done in general, because spurious
SCIs do happen while suspended to idle on other systems too, and that's
which it goes as patch [2/5] before the Dell-related changes.

Patch [1/5] is more of a cleanup, but makes the rest look slightly more
straightforward, and patches [4-5/5] update the drivers used for button
events processing on the affected systems so that they signal wakeup
as expected and avoid propagating the wakeup events as button events
to user space.

The series is available from a git branch at

 git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git s2idle-dell-test

and has been included into the testing branch thereof.

Please let me know if there are any concerns.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1631740 — [PATCH 1/5] PM / wakeup: Integrate mechanism to abort transitions in progress

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The system wakeup framework is not very consistent with respect to
the way it handles suspend-to-idle and generally wakeup events
occurring during transitions to system low-power states.

First off, system transitions in progress are aborted by the event
reporting helpers like pm_wakeup_event() only if the wakeup_count
sysfs attribute is in use (as documented), but there are cases in
which system-wide transitions should be aborted even if that is
not the case.  For example, a wakeup signal from a designated
wakeup device during system-wide PM transition, it should cause
the transition to be aborted right away.

Moreover, there is a freeze_wake() call in wakeup_source_activate(),
but that really is only effective after suspend_freeze_state has
been set to FREEZE_STATE_ENTER by freeze_enter().  However, it
is very unlikely that wakeup_source_activate() will ever be called
at that time, as it could only be triggered by a IRQF_NO_SUSPEND
interrupt handler, so wakeups from suspend-to-idle don't really
occur in wakeup_source_activate().

At the same time there is a way to abort a system suspend in
progress (or wake up the system from suspend-to-idle), which is by
calling pm_system_wakeup(), but in turn that doesn't cause any
wakeup source objects to be activated, so it will not be covered
by wakeup source statistics and will not prevent the system from
suspending again immediately (in case autosleep is used, for
example).  Consequently, if anyone wants to abort system transitions
in progress and allow the wakeup_count mechanism to work, they need
to use both pm_system_wakeup() and pm_wakeup_event(), say, at the
same time which is awkward.

For the above reasons, make it possible to trigger
pm_system_wakeup() from within wakeup_source_activate() and
provide a new pm_wakeup_hard_event() helper to do so within the
wakeup framework.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/wakeup.c |   36 ++++++++++++++++++------------------
 include/linux/pm_wakeup.h   |   25 +++++++++++++++++++++----
 2 files changed, 39 insertions(+), 22 deletions(-)

Index: linux-pm/include/linux/pm_wakeup.h
===================================================================
--- linux-pm.orig/include/linux/pm_wakeup.h
+++ linux-pm/include/linux/pm_wakeup.h
@@ -106,8 +106,8 @@ extern void __pm_stay_awake(struct wakeu
 extern void pm_stay_awake(struct device *dev);
 extern void __pm_relax(struct wakeup_source *ws);
 extern void pm_relax(struct device *dev);
-extern void __pm_wakeup_event(struct wakeup_source *ws, unsigned int msec);
-extern void pm_wakeup_event(struct device *dev, unsigned int msec);
+extern void pm_wakeup_ws_event(struct wakeup_source *ws, unsigned int msec, bool hard);
+extern void pm_wakeup_dev_event(struct device *dev, unsigned int msec, bool hard);
 
 #else /* !CONFIG_PM_SLEEP */
 
@@ -182,9 +182,11 @@ static inline void __pm_relax(struct wak
 
 static inline void pm_relax(struct device *dev) {}
 
-static inline void __pm_wakeup_event(struct wakeup_source *ws, unsigned int msec) {}
+static inline void pm_wakeup_ws_event(struct wakeup_source *ws,
+				      unsigned int msec, bool hard) {}
 
-static inline void pm_wakeup_event(struct device *dev, unsigned int msec) {}
+static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
+				       bool hard) {}
 
 #endif /* !CONFIG_PM_SLEEP */
 
@@ -201,4 +203,19 @@ static inline void wakeup_source_trash(s
 	wakeup_source_drop(ws);
 }
 
+static inline void __pm_wakeup_event(struct wakeup_source *ws, unsigned int msec)
+{
+	return pm_wakeup_ws_event(ws, msec, false);
+}
+
+static inline void pm_wakeup_event(struct device *dev, unsigned int msec)
+{
+	return pm_wakeup_dev_event(dev, msec, false);
+}
+
+static inline void pm_wakeup_hard_event(struct device *dev)
+{
+	return pm_wakeup_dev_event(dev, 0, true);
+}
+
 #endif /* _LINUX_PM_WAKEUP_H */
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -512,12 +512,13 @@ static bool wakeup_source_not_registered
 /**
  * wakup_source_activate - Mark given wakeup source as active.
  * @ws: Wakeup source to handle.
+ * @hard: If set, abort suspends in progress and wake up from suspend-to-idle.
  *
  * Update the @ws' statistics and, if @ws has just been activated, notify the PM
  * core of the event by incrementing the counter of of wakeup events being
  * processed.
  */
-static void wakeup_source_activate(struct wakeup_source *ws)
+static void wakeup_source_activate(struct wakeup_source *ws, bool hard)
 {
 	unsigned int cec;
 
@@ -525,11 +526,8 @@ static void wakeup_source_activate(struc
 			"unregistered wakeup source\n"))
 		return;
 
-	/*
-	 * active wakeup source should bring the system
-	 * out of PM_SUSPEND_FREEZE state
-	 */
-	freeze_wake();
+	if (hard)
+		pm_system_wakeup();
 
 	ws->active = true;
 	ws->active_count++;
@@ -546,8 +544,9 @@ static void wakeup_source_activate(struc
 /**
  * wakeup_source_report_event - Report wakeup event using the given source.
  * @ws: Wakeup source to report the event for.
+ * @hard: If set, abort suspends in progress and wake up from suspend-to-idle.
  */
-static void wakeup_source_report_event(struct wakeup_source *ws)
+static void wakeup_source_report_event(struct wakeup_source *ws, bool hard)
 {
 	ws->event_count++;
 	/* This is racy, but the counter is approximate anyway. */
@@ -555,7 +554,7 @@ static void wakeup_source_report_event(s
 		ws->wakeup_count++;
 
 	if (!ws->active)
-		wakeup_source_activate(ws);
+		wakeup_source_activate(ws, hard);
 }
 
 /**
@@ -573,7 +572,7 @@ void __pm_stay_awake(struct wakeup_sourc
 
 	spin_lock_irqsave(&ws->lock, flags);
 
-	wakeup_source_report_event(ws);
+	wakeup_source_report_event(ws, false);
 	del_timer(&ws->timer);
 	ws->timer_expires = 0;
 
@@ -739,9 +738,10 @@ static void pm_wakeup_timer_fn(unsigned
 }
 
 /**
- * __pm_wakeup_event - Notify the PM core of a wakeup event.
+ * pm_wakeup_ws_event - Notify the PM core of a wakeup event.
  * @ws: Wakeup source object associated with the event source.
  * @msec: Anticipated event processing time (in milliseconds).
+ * @hard: If set, abort suspends in progress and wake up from suspend-to-idle.
  *
  * Notify the PM core of a wakeup event whose source is @ws that will take
  * approximately @msec milliseconds to be processed by the kernel.  If @ws is
@@ -750,7 +750,7 @@ static void pm_wakeup_timer_fn(unsigned
  *
  * It is safe to call this function from interrupt context.
  */
-void __pm_wakeup_event(struct wakeup_source *ws, unsigned int msec)
+void pm_wakeup_ws_event(struct wakeup_source *ws, unsigned int msec, bool hard)
 {
 	unsigned long flags;
 	unsigned long expires;
@@ -760,7 +760,7 @@ void __pm_wakeup_event(struct wakeup_sou
 
 	spin_lock_irqsave(&ws->lock, flags);
 
-	wakeup_source_report_event(ws);
+	wakeup_source_report_event(ws, hard);
 
 	if (!msec) {
 		wakeup_source_deactivate(ws);
@@ -779,17 +779,17 @@ void __pm_wakeup_event(struct wakeup_sou
  unlock:
 	spin_unlock_irqrestore(&ws->lock, flags);
 }
-EXPORT_SYMBOL_GPL(__pm_wakeup_event);
-
+EXPORT_SYMBOL_GPL(pm_wakeup_ws_event);
 
 /**
  * pm_wakeup_event - Notify the PM core of a wakeup event.
  * @dev: Device the wakeup event is related to.
  * @msec: Anticipated event processing time (in milliseconds).
+ * @hard: If set, abort suspends in progress and wake up from suspend-to-idle.
  *
- * Call __pm_wakeup_event() for the @dev's wakeup source object.
+ * Call pm_wakeup_ws_event() for the @dev's wakeup source object.
  */
-void pm_wakeup_event(struct device *dev, unsigned int msec)
+void pm_wakeup_dev_event(struct device *dev, unsigned int msec, bool hard)
 {
 	unsigned long flags;
 
@@ -797,10 +797,10 @@ void pm_wakeup_event(struct device *dev,
 		return;
 
 	spin_lock_irqsave(&dev->power.lock, flags);
-	__pm_wakeup_event(dev->power.wakeup, msec);
+	pm_wakeup_ws_event(dev->power.wakeup, msec, hard);
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 }
-EXPORT_SYMBOL_GPL(pm_wakeup_event);
+EXPORT_SYMBOL_GPL(pm_wakeup_dev_event);
 
 void pm_print_active_wakeup_sources(void)
 {

[toc] | [prev] | [next] | [standalone]

#1631741 — [PATCH 2/5] ACPI / sleep: Ignore spurious SCI wakeups from suspend-to-idle

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The ACPI SCI (System Control Interrupt) is set up as a wakeup IRQ
during suspend-to-idle transitions and, consequently, any events
signaled through it wake up the system from that state.  However,
on some systems some of the events signaled via the ACPI SCI while
suspended to idle should not cause the system to wake up.  In fact,
quite often they should just be discarded.

Arguably, systems should not resume entirely on such events, but in
order to decide which events really should cause the system to resume
and which are spurious, it is necessary to resume up to the point
when ACPI SCIs are actually handled and processed, which is after
executing dpm_resume_noirq() in the system resume path.

For this reasons, add a loop around freeze_enter() in which the
platforms can process events signaled via multiplexed IRQ lines
like the ACPI SCI and add suspend-to-idle hooks that can be
used for this purpose to struct platform_freeze_ops.

In the ACPI case, the ->wake hook is used for checking if the SCI
has triggered while suspended and deferring the interrupt-induced
system wakeup until the events signaled through it are actually
processed sufficiently to decide whether or not the system should
resume.  In turn, the ->sync hook allows all of the relevant event
queues to be flushed so as to prevent events from being missed due
to race conditions.

In addition to that, some ACPI code processing wakeup events needs
to be modified to use the "hard" version of wakeup triggers, so that
it will cause a system resume to happen on device-induced wakeup
events even if the "soft" mechanism to prevent the system from
suspending is not enabled (that also helps to catch device-induced
wakeup events occurring during suspend transitions in progress).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/battery.c      |    2 +-
 drivers/acpi/button.c       |    5 +++--
 drivers/acpi/device_pm.c    |    3 ++-
 drivers/acpi/sleep.c        |   28 ++++++++++++++++++++++++++++
 drivers/base/power/main.c   |    5 -----
 drivers/base/power/wakeup.c |   18 ++++++++++++------
 include/linux/suspend.h     |    7 +++++--
 kernel/power/process.c      |    2 +-
 kernel/power/suspend.c      |   29 +++++++++++++++++++++++++----
 9 files changed, 77 insertions(+), 22 deletions(-)

Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -189,6 +189,8 @@ struct platform_suspend_ops {
 struct platform_freeze_ops {
 	int (*begin)(void);
 	int (*prepare)(void);
+	void (*wake)(void);
+	void (*sync)(void);
 	void (*restore)(void);
 	void (*end)(void);
 };
@@ -428,7 +430,8 @@ extern unsigned int pm_wakeup_irq;
 
 extern bool pm_wakeup_pending(void);
 extern void pm_system_wakeup(void);
-extern void pm_wakeup_clear(void);
+extern void pm_system_cancel_wakeup(void);
+extern void pm_wakeup_clear(bool reset);
 extern void pm_system_irq_wakeup(unsigned int irq_number);
 extern bool pm_get_wakeup_count(unsigned int *count, bool block);
 extern bool pm_save_wakeup_count(unsigned int count);
@@ -478,7 +481,7 @@ static inline int unregister_pm_notifier
 
 static inline bool pm_wakeup_pending(void) { return false; }
 static inline void pm_system_wakeup(void) {}
-static inline void pm_wakeup_clear(void) {}
+static inline void pm_wakeup_clear(bool reset) {}
 static inline void pm_system_irq_wakeup(unsigned int irq_number) {}
 
 static inline void lock_system_sleep(void) {}
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -72,6 +72,8 @@ static void freeze_begin(void)
 
 static void freeze_enter(void)
 {
+	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, true);
+
 	spin_lock_irq(&suspend_freeze_lock);
 	if (pm_wakeup_pending())
 		goto out;
@@ -98,6 +100,27 @@ static void freeze_enter(void)
  out:
 	suspend_freeze_state = FREEZE_STATE_NONE;
 	spin_unlock_irq(&suspend_freeze_lock);
+
+	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, false);
+}
+
+static void s2idle_loop(void)
+{
+	do {
+		freeze_enter();
+
+		if (freeze_ops && freeze_ops->wake)
+			freeze_ops->wake();
+
+		dpm_resume_noirq(PMSG_RESUME);
+		if (freeze_ops && freeze_ops->sync)
+			freeze_ops->sync();
+
+		if (pm_wakeup_pending())
+			break;
+
+		pm_wakeup_clear(false);
+	} while (!dpm_suspend_noirq(PMSG_SUSPEND));
 }
 
 void freeze_wake(void)
@@ -371,10 +394,8 @@ static int suspend_enter(suspend_state_t
 	 * all the devices are suspended.
 	 */
 	if (state == PM_SUSPEND_FREEZE) {
-		trace_suspend_resume(TPS("machine_suspend"), state, true);
-		freeze_enter();
-		trace_suspend_resume(TPS("machine_suspend"), state, false);
-		goto Platform_wake;
+		s2idle_loop();
+		goto Platform_early_resume;
 	}
 
 	error = disable_nonboot_cpus();
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -1091,11 +1091,6 @@ static int __device_suspend_noirq(struct
 	if (async_error)
 		goto Complete;
 
-	if (pm_wakeup_pending()) {
-		async_error = -EBUSY;
-		goto Complete;
-	}
-
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -28,8 +28,8 @@ bool events_check_enabled __read_mostly;
 /* First wakeup IRQ seen by the kernel in the last cycle. */
 unsigned int pm_wakeup_irq __read_mostly;
 
-/* If set and the system is suspending, terminate the suspend. */
-static bool pm_abort_suspend __read_mostly;
+/* If greater than 0 and the system is suspending, terminate the suspend. */
+static atomic_t pm_abort_suspend __read_mostly;
 
 /*
  * Combined counters of registered wakeup events and wakeup events in progress.
@@ -856,20 +856,26 @@ bool pm_wakeup_pending(void)
 		pm_print_active_wakeup_sources();
 	}
 
-	return ret || pm_abort_suspend;
+	return ret || atomic_read(&pm_abort_suspend) > 0;
 }
 
 void pm_system_wakeup(void)
 {
-	pm_abort_suspend = true;
+	atomic_inc(&pm_abort_suspend);
 	freeze_wake();
 }
 EXPORT_SYMBOL_GPL(pm_system_wakeup);
 
-void pm_wakeup_clear(void)
+void pm_system_cancel_wakeup(void)
+{
+	atomic_dec(&pm_abort_suspend);
+}
+
+void pm_wakeup_clear(bool reset)
 {
-	pm_abort_suspend = false;
 	pm_wakeup_irq = 0;
+	if (reset)
+		atomic_set(&pm_abort_suspend, 0);
 }
 
 void pm_system_irq_wakeup(unsigned int irq_number)
Index: linux-pm/kernel/power/process.c
===================================================================
--- linux-pm.orig/kernel/power/process.c
+++ linux-pm/kernel/power/process.c
@@ -132,7 +132,7 @@ int freeze_processes(void)
 	if (!pm_freezing)
 		atomic_inc(&system_freezing_cnt);
 
-	pm_wakeup_clear();
+	pm_wakeup_clear(true);
 	pr_info("Freezing user space processes ... ");
 	pm_freezing = true;
 	error = try_to_freeze_tasks(true);
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -662,14 +662,40 @@ static int acpi_freeze_prepare(void)
 	acpi_os_wait_events_complete();
 	if (acpi_sci_irq_valid())
 		enable_irq_wake(acpi_sci_irq);
+
 	return 0;
 }
 
+static void acpi_freeze_wake(void)
+{
+	/*
+	 * If IRQD_WAKEUP_ARMED is not set for the SCI at this point, it means
+	 * that the SCI has triggered while suspended, so cancel the wakeup in
+	 * case it has not been a wakeup event (the GPEs will be checked later).
+	 */
+	if (acpi_sci_irq_valid() &&
+	    !irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))
+		pm_system_cancel_wakeup();
+}
+
+static void acpi_freeze_sync(void)
+{
+	/*
+	 * Process all pending events in case there are any wakeup ones.
+	 *
+	 * The EC driver uses the system workqueue, so that one needs to be
+	 * flushed too.
+	 */
+	acpi_os_wait_events_complete();
+	flush_scheduled_work();
+}
+
 static void acpi_freeze_restore(void)
 {
 	acpi_disable_wakeup_devices(ACPI_STATE_S0);
 	if (acpi_sci_irq_valid())
 		disable_irq_wake(acpi_sci_irq);
+
 	acpi_enable_all_runtime_gpes();
 }
 
@@ -681,6 +707,8 @@ static void acpi_freeze_end(void)
 static const struct platform_freeze_ops acpi_freeze_ops = {
 	.begin = acpi_freeze_begin,
 	.prepare = acpi_freeze_prepare,
+	.wake = acpi_freeze_wake,
+	.sync = acpi_freeze_sync,
 	.restore = acpi_freeze_restore,
 	.end = acpi_freeze_end,
 };
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -24,6 +24,7 @@
 #include <linux/pm_qos.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/suspend.h>
 
 #include "internal.h"
 
@@ -399,7 +400,7 @@ static void acpi_pm_notify_handler(acpi_
 	mutex_lock(&acpi_pm_notifier_lock);
 
 	if (adev->wakeup.flags.notifier_present) {
-		__pm_wakeup_event(adev->wakeup.ws, 0);
+		pm_wakeup_ws_event(adev->wakeup.ws, 0, true);
 		if (adev->wakeup.context.work.func)
 			queue_pm_work(&adev->wakeup.context.work);
 	}
Index: linux-pm/drivers/acpi/button.c
===================================================================
--- linux-pm.orig/drivers/acpi/button.c
+++ linux-pm/drivers/acpi/button.c
@@ -216,7 +216,7 @@ static int acpi_lid_notify_state(struct
 	}
 
 	if (state)
-		pm_wakeup_event(&device->dev, 0);
+		pm_wakeup_hard_event(&device->dev);
 
 	ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device);
 	if (ret == NOTIFY_DONE)
@@ -398,7 +398,7 @@ static void acpi_button_notify(struct ac
 		} else {
 			int keycode;
 
-			pm_wakeup_event(&device->dev, 0);
+			pm_wakeup_hard_event(&device->dev);
 			if (button->suspended)
 				break;
 
@@ -530,6 +530,7 @@ static int acpi_button_add(struct acpi_d
 		lid_device = device;
 	}
 
+	device_init_wakeup(&device->dev, true);
 	printk(KERN_INFO PREFIX "%s [%s]\n", name, acpi_device_bid(device));
 	return 0;
 
Index: linux-pm/drivers/acpi/battery.c
===================================================================
--- linux-pm.orig/drivers/acpi/battery.c
+++ linux-pm/drivers/acpi/battery.c
@@ -782,7 +782,7 @@ static int acpi_battery_update(struct ac
 	if ((battery->state & ACPI_BATTERY_STATE_CRITICAL) ||
 	    (test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags) &&
             (battery->capacity_now <= battery->alarm)))
-		pm_wakeup_event(&battery->device->dev, 0);
+		pm_wakeup_hard_event(&battery->device->dev);
 
 	return result;
 }

[toc] | [prev] | [next] | [standalone]

#1631744 — [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Some recent Dell laptops, including the XPS13 model numbers 9360 and
9365, cannot be woken up from suspend-to-idle by pressing the power
button which is unexpected and makes that feature hardly usable on
those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
expected to be used at all (these systems ship with Windows 10 using
Modern Standby which never exercises the ACPI S3 path) and
suspend-to-idle is the only viable system suspend mechanism in there.

The reason why the power button wakeup from suspend-to-idle doesn't
work on those systems is because their power button events are
signaled by the EC (Embedded Controller), whose GPE (General Purpose
Event) line is disabled during suspend-to-idle transitions in Linux.
That is done on purpose, because in general the EC tends to generate
tons of events for various reasons (battery and thermal updates and
similar, for example) and all of them would kick the CPUs out of deep
idle states while in suspend-to-idle, which would not be desirable.

Of course, on the Dell systems in question the EC GPE must be enabled
during suspend-to-idle transitions for the button press events to
be signaled while suspended at all.  Fortunately, there is a way to
tell the EC to stop generating the non-wakeup events, which is by
using the _DSM object under the so called micro-PEP (uPEP) device
provided to support Modern Standby in Windows 10.

The expected way to use it is to invoke function 0 from it on system
initialization, functions 3 and 5 during suspend transitions and
functions 4 and 6 during resume transitions (to reverse the actions
carried out by the former).  In particular, function 5 from the uPEP
device _DSM causes the EC to become less verbose (so to speak) on the
affected systems and then its GPE can be enabled as a wakeup source
(then, on resume, function 6 switches it back to the "working state"
mode).

In support of the affected Dell systems, implement the uPEP device
handling as described and allow the EC to generate system wakeup
events if that device is present and behaves as expected.  Enable
that for Dell only, as there are other systems out there in which
the uPEP device is exposed in the ACPI tables and its _DSM appears
to be functional, but it actually isn't, whereas Dell is committed
to supporting it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/ec.c       |   19 +++++++-
 drivers/acpi/internal.h |    2 
 drivers/acpi/sleep.c    |  110 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -40,6 +40,7 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/suspend.h>
 #include <asm/io.h>
 
 #include "internal.h"
@@ -1493,6 +1494,16 @@ static int acpi_ec_setup(struct acpi_ec
 	acpi_handle_info(ec->handle,
 			 "GPE=0x%lx, EC_CMD/EC_SC=0x%lx, EC_DATA=0x%lx\n",
 			 ec->gpe, ec->command_addr, ec->data_addr);
+
+	/*
+	 * On some platforms the EC GPE is used for waking up the system from
+	 * suspend-to-idle, so mark it as a wakeup one.
+	 *
+	 * This can be done unconditionally, as the setting does not matter
+	 * until acpi_set_gpe_wake_mask() is called for the GPE.
+	 */
+	acpi_mark_gpe_for_wake(NULL, ec->gpe);
+
 	return ret;
 }
 
@@ -1835,8 +1846,11 @@ static int acpi_ec_suspend(struct device
 	struct acpi_ec *ec =
 		acpi_driver_data(to_acpi_device(dev));
 
-	if (ec_freeze_events)
+	if (!pm_suspend_via_firmware() && acpi_sleep_ec_gpe_may_wakeup())
+		acpi_set_gpe_wake_mask(NULL, ec->gpe, ACPI_GPE_ENABLE);
+	else if (ec_freeze_events)
 		acpi_ec_disable_event(ec);
+
 	return 0;
 }
 
@@ -1846,6 +1860,9 @@ static int acpi_ec_resume(struct device
 		acpi_driver_data(to_acpi_device(dev));
 
 	acpi_ec_enable_event(ec);
+	if (!pm_resume_via_firmware() && acpi_sleep_ec_gpe_may_wakeup())
+		acpi_set_gpe_wake_mask(NULL, ec->gpe, ACPI_GPE_DISABLE);
+
 	return 0;
 }
 #endif
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -199,8 +199,10 @@ void acpi_ec_remove_query_handler(struct
   -------------------------------------------------------------------------- */
 #ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT
 extern int acpi_sleep_init(void);
+bool acpi_sleep_ec_gpe_may_wakeup(void);
 #else
 static inline int acpi_sleep_init(void) { return -ENXIO; }
+static inline bool acpi_sleep_ec_gpe_may_wakeup(void) { return false; }
 #endif
 
 #ifdef CONFIG_ACPI_SLEEP
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -160,6 +160,10 @@ static int __init init_nvs_nosave(const
 	return 0;
 }
 
+#ifdef CONFIG_SUSPEND
+static int __init init_upep_device(const struct dmi_system_id *d);
+#endif
+
 static struct dmi_system_id acpisleep_dmi_table[] __initdata = {
 	{
 	.callback = init_old_suspend_ordering,
@@ -343,6 +347,15 @@ static struct dmi_system_id acpisleep_dm
 		DMI_MATCH(DMI_PRODUCT_NAME, "80E3"),
 		},
 	},
+#ifdef CONFIG_SUSPEND
+	{
+	 .callback = init_upep_device,
+	 .ident = "All Dell systems",
+	 .matches = {
+		      DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+		},
+	},
+#endif
 	{},
 };
 
@@ -649,6 +662,94 @@ static const struct platform_suspend_ops
 	.recover = acpi_pm_finish,
 };
 
+/*
+ * The micro-PEP (uPEP) device object is exposed in ACPI tables on systems
+ * supporting Windows 10 with Modern Standby.  The _DSM object under it, if
+ * present, can be used to indicate to the platform that the OS is transitioning
+ * into a low-power state in which certain types of activity are not desirable.
+ */
+static acpi_handle upep_device_handle;
+
+#define ACPI_S2IDLE_SCREEN_OFF	3
+#define ACPI_S2IDLE_SCREEN_ON	4
+#define ACPI_S2IDLE_IR_ENTRY	5
+#define ACPI_S2IDLE_IR_EXIT	6
+
+#define ACPI_S2IDLE_DSM_MASK	((1 << ACPI_S2IDLE_IR_ENTRY) | (1 << ACPI_S2IDLE_IR_EXIT))
+
+static char upep_dsm_func_mask;
+
+/* uPEP device _DSM UUID: c4eb40a0-6cd2-11e2-bcfd-0800200c9a66 */
+static const u8 upep_dsm_uuid[16] = {
+	0xa0, 0x40, 0xeb, 0xc4, 0xd2, 0x6c, 0xe2, 0x11,
+	0xbc, 0xfd, 0x08, 0x00, 0x20, 0x0c, 0x9a, 0x66
+};
+
+static void acpi_sleep_call_upep_dsm(unsigned int func)
+{
+	union acpi_object *out_obj;
+
+	if (!(upep_dsm_func_mask & (1 << func)))
+		return;
+
+	out_obj = acpi_evaluate_dsm(upep_device_handle, upep_dsm_uuid, 1, func,
+				    NULL);
+	ACPI_FREE(out_obj);
+
+	acpi_handle_debug(upep_device_handle, "_DSM function %u evaluation %s\n",
+			  func, out_obj ? "successful" : "failed");
+}
+
+static int upep_device_attach(struct acpi_device *adev,
+			      const struct acpi_device_id *not_used)
+{
+	union acpi_object *out_obj;
+
+	if (upep_device_handle)
+		return 0;
+
+	/* Check if the _DSM is present and as expected. */
+	out_obj = acpi_evaluate_dsm(adev->handle, upep_dsm_uuid, 1, 0, NULL);
+	if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
+		char bitmask = *(char *)out_obj->buffer.pointer;
+
+		if (bitmask & ACPI_S2IDLE_DSM_MASK) {
+			upep_dsm_func_mask = bitmask;
+			upep_device_handle = adev->handle;
+		}
+
+		acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
+				  bitmask);
+	} else {
+		acpi_handle_debug(adev->handle,
+				  "_DSM function 0 evaluation failed\n");
+	}
+	ACPI_FREE(out_obj);
+	return 0;
+}
+
+static const struct acpi_device_id upep_device_ids[] = {
+	{"INT33A1", },
+	{"PNP0D80", },
+	{"", },
+};
+
+static struct acpi_scan_handler upep_handler = {
+	.ids = upep_device_ids,
+	.attach = upep_device_attach,
+};
+
+static int __init init_upep_device(const struct dmi_system_id *d)
+{
+	acpi_scan_add_handler(&upep_handler);
+	return 0;
+}
+
+bool acpi_sleep_ec_gpe_may_wakeup(void)
+{
+	return !!upep_device_handle;
+}
+
 static int acpi_freeze_begin(void)
 {
 	acpi_scan_lock_acquire();
@@ -657,6 +758,8 @@ static int acpi_freeze_begin(void)
 
 static int acpi_freeze_prepare(void)
 {
+	acpi_sleep_call_upep_dsm(ACPI_S2IDLE_SCREEN_OFF);
+	acpi_sleep_call_upep_dsm(ACPI_S2IDLE_IR_ENTRY);
 	acpi_enable_wakeup_devices(ACPI_STATE_S0);
 	acpi_enable_all_wakeup_gpes();
 	acpi_enable_all_runtime_gpes();
@@ -698,6 +801,8 @@ static void acpi_freeze_restore(void)
 		disable_irq_wake(acpi_sci_irq);
 
 	acpi_enable_all_runtime_gpes();
+	acpi_sleep_call_upep_dsm(ACPI_S2IDLE_IR_EXIT);
+	acpi_sleep_call_upep_dsm(ACPI_S2IDLE_SCREEN_ON);
 }
 
 static void acpi_freeze_end(void)
@@ -729,6 +834,11 @@ static void acpi_sleep_suspend_setup(voi
 
 #else /* !CONFIG_SUSPEND */
 static inline void acpi_sleep_suspend_setup(void) {}
+
+bool acpi_sleep_ec_gpe_may_wakeup(void)
+{
+	return false;
+}
 #endif /* !CONFIG_SUSPEND */
 
 #ifdef CONFIG_PM_SLEEP

[toc] | [prev] | [next] | [standalone]

#1632152 — RE: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Wednesday, April 26, 2017 4:24 PM
> To: Linux PM <linux-pm@vger.kernel.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux ACPI <linux-
> acpi@vger.kernel.org>; Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>
> Subject: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell
> systems
> 
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> 9365, cannot be woken up from suspend-to-idle by pressing the power
> button which is unexpected and makes that feature hardly usable on
> those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
> expected to be used at all (these systems ship with Windows 10 using
> Modern Standby which never exercises the ACPI S3 path) and
> suspend-to-idle is the only viable system suspend mechanism in there.
> 
> The reason why the power button wakeup from suspend-to-idle doesn't
> work on those systems is because their power button events are
> signaled by the EC (Embedded Controller), whose GPE (General Purpose
> Event) line is disabled during suspend-to-idle transitions in Linux.
> That is done on purpose, because in general the EC tends to generate
> tons of events for various reasons (battery and thermal updates and
> similar, for example) and all of them would kick the CPUs out of deep
> idle states while in suspend-to-idle, which would not be desirable.
> 
> Of course, on the Dell systems in question the EC GPE must be enabled
> during suspend-to-idle transitions for the button press events to
> be signaled while suspended at all.  Fortunately, there is a way to
> tell the EC to stop generating the non-wakeup events, which is by
> using the _DSM object under the so called micro-PEP (uPEP) device
> provided to support Modern Standby in Windows 10.
> 
> The expected way to use it is to invoke function 0 from it on system
> initialization, functions 3 and 5 during suspend transitions and
> functions 4 and 6 during resume transitions (to reverse the actions
> carried out by the former).  In particular, function 5 from the uPEP
> device _DSM causes the EC to become less verbose (so to speak) on the
> affected systems and then its GPE can be enabled as a wakeup source
> (then, on resume, function 6 switches it back to the "working state"
> mode).
> 
> In support of the affected Dell systems, implement the uPEP device
> handling as described and allow the EC to generate system wakeup
> events if that device is present and behaves as expected.  Enable
> that for Dell only, as there are other systems out there in which
> the uPEP device is exposed in the ACPI tables and its _DSM appears
> to be functional, but it actually isn't, whereas Dell is committed
> to supporting it.
> 

I am of course biased in that my priority is for this to work for Dell.
Dell is definitely committed to supporting this on any system with
the low power idle bit in the FADT set.

So I'm fine with the current proposed solution, but have you
dug into what actually breaks on this other system?  Does it actually
work with Modern Standby + the uPEP device on Windows 10?

To my understanding I would think any OEM that is enabling this
uPEP device it should be getting called by the Windows kernel
identically when entering resiliency phases.

This makes me wonder if it should be inverted and a blacklist
of platforms that the uPEP device doesn't work.

[toc] | [prev] | [next] | [standalone]

#1632419 — Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

On Thursday, April 27, 2017 02:47:59 PM Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > Sent: Wednesday, April 26, 2017 4:24 PM
> > To: Linux PM <linux-pm@vger.kernel.org>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> > <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux ACPI <linux-
> > acpi@vger.kernel.org>; Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> > Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello, Mario
> > <Mario_Limonciello@Dell.com>
> > Subject: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell
> > systems
> > 
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Some recent Dell laptops, including the XPS13 model numbers 9360 and
> > 9365, cannot be woken up from suspend-to-idle by pressing the power
> > button which is unexpected and makes that feature hardly usable on
> > those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
> > expected to be used at all (these systems ship with Windows 10 using
> > Modern Standby which never exercises the ACPI S3 path) and
> > suspend-to-idle is the only viable system suspend mechanism in there.
> > 
> > The reason why the power button wakeup from suspend-to-idle doesn't
> > work on those systems is because their power button events are
> > signaled by the EC (Embedded Controller), whose GPE (General Purpose
> > Event) line is disabled during suspend-to-idle transitions in Linux.
> > That is done on purpose, because in general the EC tends to generate
> > tons of events for various reasons (battery and thermal updates and
> > similar, for example) and all of them would kick the CPUs out of deep
> > idle states while in suspend-to-idle, which would not be desirable.
> > 
> > Of course, on the Dell systems in question the EC GPE must be enabled
> > during suspend-to-idle transitions for the button press events to
> > be signaled while suspended at all.  Fortunately, there is a way to
> > tell the EC to stop generating the non-wakeup events, which is by
> > using the _DSM object under the so called micro-PEP (uPEP) device
> > provided to support Modern Standby in Windows 10.
> > 
> > The expected way to use it is to invoke function 0 from it on system
> > initialization, functions 3 and 5 during suspend transitions and
> > functions 4 and 6 during resume transitions (to reverse the actions
> > carried out by the former).  In particular, function 5 from the uPEP
> > device _DSM causes the EC to become less verbose (so to speak) on the
> > affected systems and then its GPE can be enabled as a wakeup source
> > (then, on resume, function 6 switches it back to the "working state"
> > mode).
> > 
> > In support of the affected Dell systems, implement the uPEP device
> > handling as described and allow the EC to generate system wakeup
> > events if that device is present and behaves as expected.  Enable
> > that for Dell only, as there are other systems out there in which
> > the uPEP device is exposed in the ACPI tables and its _DSM appears
> > to be functional, but it actually isn't, whereas Dell is committed
> > to supporting it.
> > 
> 
> I am of course biased in that my priority is for this to work for Dell.
> Dell is definitely committed to supporting this on any system with
> the low power idle bit in the FADT set.
> 
> So I'm fine with the current proposed solution, but have you
> dug into what actually breaks on this other system?  Does it actually
> work with Modern Standby + the uPEP device on Windows 10?
> 
> To my understanding I would think any OEM that is enabling this
> uPEP device it should be getting called by the Windows kernel
> identically when entering resiliency phases.
> 
> This makes me wonder if it should be inverted and a blacklist
> of platforms that the uPEP device doesn't work.

For now I'd prefer to only do it on platforms where the benefit is clear.

The next step may be to extend it to the other ones, but let's avoid making
what is problem mitigation really depend on things that may or may not
work elsewhere to start with.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1635458 — RE: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

Hi,

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J.
> Wysocki
> Sent: Friday, April 28, 2017 6:26 AM
> To: Mario.Limonciello@dell.com
> Cc: linux-pm@vger.kernel.org; andriy.shevchenko@linux.intel.com; dvhart@infradead.org; linux-
> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; srinivas.pandruvada@linux.intel.com;
> tglx@linutronix.de; mika.westerberg@linux.intel.com
> Subject: Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems
> 
> On Thursday, April 27, 2017 02:47:59 PM Mario.Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > Sent: Wednesday, April 26, 2017 4:24 PM
> > > To: Linux PM <linux-pm@vger.kernel.org>
> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> > > <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux ACPI <linux-
> > > acpi@vger.kernel.org>; Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> > > Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello, Mario
> > > <Mario_Limonciello@Dell.com>
> > > Subject: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell
> > > systems
> > >
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Some recent Dell laptops, including the XPS13 model numbers 9360 and
> > > 9365, cannot be woken up from suspend-to-idle by pressing the power
> > > button which is unexpected and makes that feature hardly usable on
> > > those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
> > > expected to be used at all (these systems ship with Windows 10 using
> > > Modern Standby which never exercises the ACPI S3 path) and
> > > suspend-to-idle is the only viable system suspend mechanism in there.
> > >
> > > The reason why the power button wakeup from suspend-to-idle doesn't
> > > work on those systems is because their power button events are
> > > signaled by the EC (Embedded Controller), whose GPE (General Purpose
> > > Event) line is disabled during suspend-to-idle transitions in Linux.
> > > That is done on purpose, because in general the EC tends to generate
> > > tons of events for various reasons (battery and thermal updates and
> > > similar, for example) and all of them would kick the CPUs out of deep
> > > idle states while in suspend-to-idle, which would not be desirable.
> > >
> > > Of course, on the Dell systems in question the EC GPE must be enabled
> > > during suspend-to-idle transitions for the button press events to
> > > be signaled while suspended at all.  Fortunately, there is a way to
> > > tell the EC to stop generating the non-wakeup events, which is by
> > > using the _DSM object under the so called micro-PEP (uPEP) device
> > > provided to support Modern Standby in Windows 10.
> > >
> > > The expected way to use it is to invoke function 0 from it on system
> > > initialization, functions 3 and 5 during suspend transitions and
> > > functions 4 and 6 during resume transitions (to reverse the actions
> > > carried out by the former).  In particular, function 5 from the uPEP
> > > device _DSM causes the EC to become less verbose (so to speak) on the
> > > affected systems and then its GPE can be enabled as a wakeup source
> > > (then, on resume, function 6 switches it back to the "working state"
> > > mode).
> > >
> > > In support of the affected Dell systems, implement the uPEP device
> > > handling as described and allow the EC to generate system wakeup
> > > events if that device is present and behaves as expected.  Enable
> > > that for Dell only, as there are other systems out there in which
> > > the uPEP device is exposed in the ACPI tables and its _DSM appears
> > > to be functional, but it actually isn't, whereas Dell is committed
> > > to supporting it.
> > >
> >
> > I am of course biased in that my priority is for this to work for Dell.
> > Dell is definitely committed to supporting this on any system with
> > the low power idle bit in the FADT set.
> >
> > So I'm fine with the current proposed solution, but have you
> > dug into what actually breaks on this other system?  Does it actually
> > work with Modern Standby + the uPEP device on Windows 10?
> >
> > To my understanding I would think any OEM that is enabling this
> > uPEP device it should be getting called by the Windows kernel
> > identically when entering resiliency phases.
> >
> > This makes me wonder if it should be inverted and a blacklist
> > of platforms that the uPEP device doesn't work.
> 
> For now I'd prefer to only do it on platforms where the benefit is clear.
> 
> The next step may be to extend it to the other ones, but let's avoid making
> what is problem mitigation really depend on things that may or may not
> work elsewhere to start with.

Then is it possible to invoke acpi_mark_gpe_for_wake() (and maybe also acpi_unmark_gpe_for_wake()) right after invoking uPEP functions?
So that such platform specific stuffs won't go into ec.c.

Thanks and best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1635797 — Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

On Thursday, May 04, 2017 07:58:53 AM Zheng, Lv wrote:
> Hi,
> 
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J.
> > Wysocki
> > Sent: Friday, April 28, 2017 6:26 AM
> > To: Mario.Limonciello@dell.com
> > Cc: linux-pm@vger.kernel.org; andriy.shevchenko@linux.intel.com; dvhart@infradead.org; linux-
> > kernel@vger.kernel.org; linux-acpi@vger.kernel.org; srinivas.pandruvada@linux.intel.com;
> > tglx@linutronix.de; mika.westerberg@linux.intel.com
> > Subject: Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems
> > 
> > On Thursday, April 27, 2017 02:47:59 PM Mario.Limonciello@dell.com wrote:
> > > > -----Original Message-----
> > > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > > Sent: Wednesday, April 26, 2017 4:24 PM
> > > > To: Linux PM <linux-pm@vger.kernel.org>
> > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> > > > <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux ACPI <linux-
> > > > acpi@vger.kernel.org>; Srinivas Pandruvada
> > > > <srinivas.pandruvada@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> > > > Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello, Mario
> > > > <Mario_Limonciello@Dell.com>
> > > > Subject: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell
> > > > systems
> > > >
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > >
> > > > Some recent Dell laptops, including the XPS13 model numbers 9360 and
> > > > 9365, cannot be woken up from suspend-to-idle by pressing the power
> > > > button which is unexpected and makes that feature hardly usable on
> > > > those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
> > > > expected to be used at all (these systems ship with Windows 10 using
> > > > Modern Standby which never exercises the ACPI S3 path) and
> > > > suspend-to-idle is the only viable system suspend mechanism in there.
> > > >
> > > > The reason why the power button wakeup from suspend-to-idle doesn't
> > > > work on those systems is because their power button events are
> > > > signaled by the EC (Embedded Controller), whose GPE (General Purpose
> > > > Event) line is disabled during suspend-to-idle transitions in Linux.
> > > > That is done on purpose, because in general the EC tends to generate
> > > > tons of events for various reasons (battery and thermal updates and
> > > > similar, for example) and all of them would kick the CPUs out of deep
> > > > idle states while in suspend-to-idle, which would not be desirable.
> > > >
> > > > Of course, on the Dell systems in question the EC GPE must be enabled
> > > > during suspend-to-idle transitions for the button press events to
> > > > be signaled while suspended at all.  Fortunately, there is a way to
> > > > tell the EC to stop generating the non-wakeup events, which is by
> > > > using the _DSM object under the so called micro-PEP (uPEP) device
> > > > provided to support Modern Standby in Windows 10.
> > > >
> > > > The expected way to use it is to invoke function 0 from it on system
> > > > initialization, functions 3 and 5 during suspend transitions and
> > > > functions 4 and 6 during resume transitions (to reverse the actions
> > > > carried out by the former).  In particular, function 5 from the uPEP
> > > > device _DSM causes the EC to become less verbose (so to speak) on the
> > > > affected systems and then its GPE can be enabled as a wakeup source
> > > > (then, on resume, function 6 switches it back to the "working state"
> > > > mode).
> > > >
> > > > In support of the affected Dell systems, implement the uPEP device
> > > > handling as described and allow the EC to generate system wakeup
> > > > events if that device is present and behaves as expected.  Enable
> > > > that for Dell only, as there are other systems out there in which
> > > > the uPEP device is exposed in the ACPI tables and its _DSM appears
> > > > to be functional, but it actually isn't, whereas Dell is committed
> > > > to supporting it.
> > > >
> > >
> > > I am of course biased in that my priority is for this to work for Dell.
> > > Dell is definitely committed to supporting this on any system with
> > > the low power idle bit in the FADT set.
> > >
> > > So I'm fine with the current proposed solution, but have you
> > > dug into what actually breaks on this other system?  Does it actually
> > > work with Modern Standby + the uPEP device on Windows 10?
> > >
> > > To my understanding I would think any OEM that is enabling this
> > > uPEP device it should be getting called by the Windows kernel
> > > identically when entering resiliency phases.
> > >
> > > This makes me wonder if it should be inverted and a blacklist
> > > of platforms that the uPEP device doesn't work.
> > 
> > For now I'd prefer to only do it on platforms where the benefit is clear.
> > 
> > The next step may be to extend it to the other ones, but let's avoid making
> > what is problem mitigation really depend on things that may or may not
> > work elsewhere to start with.
> 
> Then is it possible to invoke acpi_mark_gpe_for_wake() (and maybe also acpi_unmark_gpe_for_wake()) right after invoking uPEP functions?
> So that such platform specific stuffs won't go into ec.c.

I'm not sure ATM, but it should be doable in theory.

Let me try that approach, but it will need to be re-tested then, obviously.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1635799 — Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

On Thursday, May 04, 2017 04:23:30 PM Rafael J. Wysocki wrote:
> On Thursday, May 04, 2017 07:58:53 AM Zheng, Lv wrote:
> > Hi,
> > 
> > > -----Original Message-----
> > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J.
> > > Wysocki
> > > Sent: Friday, April 28, 2017 6:26 AM
> > > To: Mario.Limonciello@dell.com
> > > Cc: linux-pm@vger.kernel.org; andriy.shevchenko@linux.intel.com; dvhart@infradead.org; linux-
> > > kernel@vger.kernel.org; linux-acpi@vger.kernel.org; srinivas.pandruvada@linux.intel.com;
> > > tglx@linutronix.de; mika.westerberg@linux.intel.com
> > > Subject: Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems
> > > 
> > > On Thursday, April 27, 2017 02:47:59 PM Mario.Limonciello@dell.com wrote:
> > > > > -----Original Message-----
> > > > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > > > Sent: Wednesday, April 26, 2017 4:24 PM
> > > > > To: Linux PM <linux-pm@vger.kernel.org>
> > > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> > > > > <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux ACPI <linux-
> > > > > acpi@vger.kernel.org>; Srinivas Pandruvada
> > > > > <srinivas.pandruvada@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> > > > > Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello, Mario
> > > > > <Mario_Limonciello@Dell.com>
> > > > > Subject: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell
> > > > > systems
> > > > >
> > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > >
> > > > > Some recent Dell laptops, including the XPS13 model numbers 9360 and
> > > > > 9365, cannot be woken up from suspend-to-idle by pressing the power
> > > > > button which is unexpected and makes that feature hardly usable on
> > > > > those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
> > > > > expected to be used at all (these systems ship with Windows 10 using
> > > > > Modern Standby which never exercises the ACPI S3 path) and
> > > > > suspend-to-idle is the only viable system suspend mechanism in there.
> > > > >
> > > > > The reason why the power button wakeup from suspend-to-idle doesn't
> > > > > work on those systems is because their power button events are
> > > > > signaled by the EC (Embedded Controller), whose GPE (General Purpose
> > > > > Event) line is disabled during suspend-to-idle transitions in Linux.
> > > > > That is done on purpose, because in general the EC tends to generate
> > > > > tons of events for various reasons (battery and thermal updates and
> > > > > similar, for example) and all of them would kick the CPUs out of deep
> > > > > idle states while in suspend-to-idle, which would not be desirable.
> > > > >
> > > > > Of course, on the Dell systems in question the EC GPE must be enabled
> > > > > during suspend-to-idle transitions for the button press events to
> > > > > be signaled while suspended at all.  Fortunately, there is a way to
> > > > > tell the EC to stop generating the non-wakeup events, which is by
> > > > > using the _DSM object under the so called micro-PEP (uPEP) device
> > > > > provided to support Modern Standby in Windows 10.
> > > > >
> > > > > The expected way to use it is to invoke function 0 from it on system
> > > > > initialization, functions 3 and 5 during suspend transitions and
> > > > > functions 4 and 6 during resume transitions (to reverse the actions
> > > > > carried out by the former).  In particular, function 5 from the uPEP
> > > > > device _DSM causes the EC to become less verbose (so to speak) on the
> > > > > affected systems and then its GPE can be enabled as a wakeup source
> > > > > (then, on resume, function 6 switches it back to the "working state"
> > > > > mode).
> > > > >
> > > > > In support of the affected Dell systems, implement the uPEP device
> > > > > handling as described and allow the EC to generate system wakeup
> > > > > events if that device is present and behaves as expected.  Enable
> > > > > that for Dell only, as there are other systems out there in which
> > > > > the uPEP device is exposed in the ACPI tables and its _DSM appears
> > > > > to be functional, but it actually isn't, whereas Dell is committed
> > > > > to supporting it.
> > > > >
> > > >
> > > > I am of course biased in that my priority is for this to work for Dell.
> > > > Dell is definitely committed to supporting this on any system with
> > > > the low power idle bit in the FADT set.
> > > >
> > > > So I'm fine with the current proposed solution, but have you
> > > > dug into what actually breaks on this other system?  Does it actually
> > > > work with Modern Standby + the uPEP device on Windows 10?
> > > >
> > > > To my understanding I would think any OEM that is enabling this
> > > > uPEP device it should be getting called by the Windows kernel
> > > > identically when entering resiliency phases.
> > > >
> > > > This makes me wonder if it should be inverted and a blacklist
> > > > of platforms that the uPEP device doesn't work.
> > > 
> > > For now I'd prefer to only do it on platforms where the benefit is clear.
> > > 
> > > The next step may be to extend it to the other ones, but let's avoid making
> > > what is problem mitigation really depend on things that may or may not
> > > work elsewhere to start with.
> > 
> > Then is it possible to invoke acpi_mark_gpe_for_wake() (and maybe also acpi_unmark_gpe_for_wake()) right after invoking uPEP functions?
> > So that such platform specific stuffs won't go into ec.c.
> 
> I'm not sure ATM, but it should be doable in theory.

So the problem with that is that the EC GPE number is not known to the sleep.c
code, so it would need to be exported by the EC driver somehow or similar,
which would be uglier than the current patch IMO.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1636103 — RE: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems

Hi,

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Subject: Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems
> 
> On Thursday, May 04, 2017 04:23:30 PM Rafael J. Wysocki wrote:
> > On Thursday, May 04, 2017 07:58:53 AM Zheng, Lv wrote:
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of
> Rafael J.
> > > > Wysocki
> > > > Sent: Friday, April 28, 2017 6:26 AM
> > > > To: Mario.Limonciello@dell.com
> > > > Cc: linux-pm@vger.kernel.org; andriy.shevchenko@linux.intel.com; dvhart@infradead.org; linux-
> > > > kernel@vger.kernel.org; linux-acpi@vger.kernel.org; srinivas.pandruvada@linux.intel.com;
> > > > tglx@linutronix.de; mika.westerberg@linux.intel.com
> > > > Subject: Re: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell systems
> > > >
> > > > On Thursday, April 27, 2017 02:47:59 PM Mario.Limonciello@dell.com wrote:
> > > > > > -----Original Message-----
> > > > > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > > > > Sent: Wednesday, April 26, 2017 4:24 PM
> > > > > > To: Linux PM <linux-pm@vger.kernel.org>
> > > > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> > > > > > <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux ACPI <linux-
> > > > > > acpi@vger.kernel.org>; Srinivas Pandruvada
> > > > > > <srinivas.pandruvada@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> > > > > > Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello, Mario
> > > > > > <Mario_Limonciello@Dell.com>
> > > > > > Subject: [PATCH 3/5] ACPI / sleep: EC-based wakeup from suspend-to-idle on Dell
> > > > > > systems
> > > > > >
> > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > > >
> > > > > > Some recent Dell laptops, including the XPS13 model numbers 9360 and
> > > > > > 9365, cannot be woken up from suspend-to-idle by pressing the power
> > > > > > button which is unexpected and makes that feature hardly usable on
> > > > > > those systems.  However, on the 9365 ACPI S3 (suspend-to-RAM) is not
> > > > > > expected to be used at all (these systems ship with Windows 10 using
> > > > > > Modern Standby which never exercises the ACPI S3 path) and
> > > > > > suspend-to-idle is the only viable system suspend mechanism in there.
> > > > > >
> > > > > > The reason why the power button wakeup from suspend-to-idle doesn't
> > > > > > work on those systems is because their power button events are
> > > > > > signaled by the EC (Embedded Controller), whose GPE (General Purpose
> > > > > > Event) line is disabled during suspend-to-idle transitions in Linux.
> > > > > > That is done on purpose, because in general the EC tends to generate
> > > > > > tons of events for various reasons (battery and thermal updates and
> > > > > > similar, for example) and all of them would kick the CPUs out of deep
> > > > > > idle states while in suspend-to-idle, which would not be desirable.
> > > > > >
> > > > > > Of course, on the Dell systems in question the EC GPE must be enabled
> > > > > > during suspend-to-idle transitions for the button press events to
> > > > > > be signaled while suspended at all.  Fortunately, there is a way to
> > > > > > tell the EC to stop generating the non-wakeup events, which is by
> > > > > > using the _DSM object under the so called micro-PEP (uPEP) device
> > > > > > provided to support Modern Standby in Windows 10.
> > > > > >
> > > > > > The expected way to use it is to invoke function 0 from it on system
> > > > > > initialization, functions 3 and 5 during suspend transitions and
> > > > > > functions 4 and 6 during resume transitions (to reverse the actions
> > > > > > carried out by the former).  In particular, function 5 from the uPEP
> > > > > > device _DSM causes the EC to become less verbose (so to speak) on the
> > > > > > affected systems and then its GPE can be enabled as a wakeup source
> > > > > > (then, on resume, function 6 switches it back to the "working state"
> > > > > > mode).
> > > > > >
> > > > > > In support of the affected Dell systems, implement the uPEP device
> > > > > > handling as described and allow the EC to generate system wakeup
> > > > > > events if that device is present and behaves as expected.  Enable
> > > > > > that for Dell only, as there are other systems out there in which
> > > > > > the uPEP device is exposed in the ACPI tables and its _DSM appears
> > > > > > to be functional, but it actually isn't, whereas Dell is committed
> > > > > > to supporting it.
> > > > > >
> > > > >
> > > > > I am of course biased in that my priority is for this to work for Dell.
> > > > > Dell is definitely committed to supporting this on any system with
> > > > > the low power idle bit in the FADT set.
> > > > >
> > > > > So I'm fine with the current proposed solution, but have you
> > > > > dug into what actually breaks on this other system?  Does it actually
> > > > > work with Modern Standby + the uPEP device on Windows 10?
> > > > >
> > > > > To my understanding I would think any OEM that is enabling this
> > > > > uPEP device it should be getting called by the Windows kernel
> > > > > identically when entering resiliency phases.
> > > > >
> > > > > This makes me wonder if it should be inverted and a blacklist
> > > > > of platforms that the uPEP device doesn't work.
> > > >
> > > > For now I'd prefer to only do it on platforms where the benefit is clear.
> > > >
> > > > The next step may be to extend it to the other ones, but let's avoid making
> > > > what is problem mitigation really depend on things that may or may not
> > > > work elsewhere to start with.
> > >
> > > Then is it possible to invoke acpi_mark_gpe_for_wake() (and maybe also acpi_unmark_gpe_for_wake())
> right after invoking uPEP functions?
> > > So that such platform specific stuffs won't go into ec.c.
> >
> > I'm not sure ATM, but it should be doable in theory.
> 
> So the problem with that is that the EC GPE number is not known to the sleep.c
> code, so it would need to be exported by the EC driver somehow or similar,
> which would be uglier than the current patch IMO.

Ah, I see.
Anyway, this is not urgent.
We can just focus on user issue now.

Thanks and best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1631745 — [PATCH 4/5] platform: x86: intel-vbtn: Wake up the system from suspend-to-idle

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Allow the intel-vbtn driver to wake up the system from suspend-to-idle
by configuring its platform device as a wakeup one by default and
switching it over to a system wakeup events triggering mode during
system suspend transitions.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/intel-vbtn.c |   38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/platform/x86/intel-vbtn.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/intel-vbtn.c
+++ linux-pm/drivers/platform/x86/intel-vbtn.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/input/sparse-keymap.h>
 #include <linux/acpi.h>
+#include <linux/suspend.h>
 #include <acpi/acpi_bus.h>
 
 MODULE_LICENSE("GPL");
@@ -42,6 +43,7 @@ static const struct key_entry intel_vbtn
 
 struct intel_vbtn_priv {
 	struct input_dev *input_dev;
+	bool wakeup_mode;
 };
 
 static int intel_vbtn_input_setup(struct platform_device *device)
@@ -69,9 +71,15 @@ static void notify_handler(acpi_handle h
 	struct platform_device *device = context;
 	struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
 
-	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
-		dev_info(&device->dev, "unknown event index 0x%x\n",
-			 event);
+	if (priv->wakeup_mode) {
+		if (sparse_keymap_entry_from_scancode(priv->input_dev, event)) {
+			pm_wakeup_hard_event(&device->dev);
+			return;
+		}
+	} else if (sparse_keymap_report_event(priv->input_dev, event, 1, true)) {
+		return;
+	}
+	dev_info(&device->dev, "unknown event index 0x%x\n", event);
 }
 
 static int intel_vbtn_probe(struct platform_device *device)
@@ -105,6 +113,7 @@ static int intel_vbtn_probe(struct platf
 	if (ACPI_FAILURE(status))
 		return -EBUSY;
 
+	device_init_wakeup(&device->dev, true);
 	return 0;
 }
 
@@ -121,10 +130,33 @@ static int intel_vbtn_remove(struct plat
 	return 0;
 }
 
+static int intel_vbtn_pm_prepare(struct device *dev)
+{
+	struct platform_device *device = to_platform_device(dev);
+	struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
+
+	priv->wakeup_mode = true;
+	return 0;
+}
+
+static void intel_vbtn_pm_complete(struct device *dev)
+{
+	struct platform_device *device = to_platform_device(dev);
+	struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
+
+	priv->wakeup_mode = false;
+}
+
+static const struct dev_pm_ops intel_vbtn_pm_ops = {
+	.prepare = intel_vbtn_pm_prepare,
+	.complete = intel_vbtn_pm_complete,
+};
+
 static struct platform_driver intel_vbtn_pl_driver = {
 	.driver = {
 		.name = "intel-vbtn",
 		.acpi_match_table = intel_vbtn_ids,
+		.pm = &intel_vbtn_pm_ops,
 	},
 	.probe = intel_vbtn_probe,
 	.remove = intel_vbtn_remove,

[toc] | [prev] | [next] | [standalone]

#1631746 — [PATCH 5/5] platform: x86: intel-hid: Wake up the system from suspend-to-idle

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Allow the intel-hid driver to wake up the system from suspend-to-idle
by configuring its platform device as a wakeup one by default and
switching it over to a system wakeup events triggering mode during
system suspend transitions.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This depends on https://patchwork.kernel.org/patch/9685529/

---
 drivers/platform/x86/intel-hid.c |   49 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/platform/x86/intel-hid.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/intel-hid.c
+++ linux-pm/drivers/platform/x86/intel-hid.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/input/sparse-keymap.h>
 #include <linux/acpi.h>
+#include <linux/suspend.h>
 #include <acpi/acpi_bus.h>
 
 MODULE_LICENSE("GPL");
@@ -75,6 +76,7 @@ static const struct key_entry intel_arra
 struct intel_hid_priv {
 	struct input_dev *input_dev;
 	struct input_dev *array;
+	bool wakeup_mode;
 };
 
 static int intel_hid_set_enable(struct device *device, int enable)
@@ -118,23 +120,44 @@ static void intel_button_array_enable(st
 		dev_warn(device, "failed to set button capability\n");
 }
 
-static int intel_hid_pl_suspend_handler(struct device *device)
+static int intel_hid_pm_prepare(struct device *dev)
 {
-	intel_hid_set_enable(device, 0);
-	intel_button_array_enable(device, false);
+	struct platform_device *device = to_platform_device(dev);
+	struct intel_hid_priv *priv = dev_get_drvdata(&device->dev);
 
+	priv->wakeup_mode = true;
 	return 0;
 }
 
-static int intel_hid_pl_resume_handler(struct device *device)
+static void intel_hid_pm_complete(struct device *dev)
+{
+	struct platform_device *device = to_platform_device(dev);
+	struct intel_hid_priv *priv = dev_get_drvdata(&device->dev);
+
+	priv->wakeup_mode = false;
+}
+
+static int intel_hid_pl_suspend_handler(struct device *device)
 {
-	intel_hid_set_enable(device, 1);
-	intel_button_array_enable(device, true);
+	if (pm_suspend_via_firmware()) {
+		intel_hid_set_enable(device, 0);
+		intel_button_array_enable(device, false);
+	}
+	return 0;
+}
 
+static int intel_hid_pl_resume_handler(struct device *device)
+{
+	if (pm_resume_via_firmware()) {
+		intel_hid_set_enable(device, 1);
+		intel_button_array_enable(device, true);
+	}
 	return 0;
 }
 
 static const struct dev_pm_ops intel_hid_pl_pm_ops = {
+	.prepare = intel_hid_pm_prepare,
+	.complete = intel_hid_pm_complete,
 	.freeze  = intel_hid_pl_suspend_handler,
 	.thaw  = intel_hid_pl_resume_handler,
 	.restore  = intel_hid_pl_resume_handler,
@@ -206,6 +229,19 @@ static void notify_handler(acpi_handle h
 	unsigned long long ev_index;
 	acpi_status status;
 
+	if (priv->wakeup_mode) {
+		/* Wake up on 5-button array events only. */
+		if (event == 0xc0 || !priv->array)
+			return;
+
+		if (sparse_keymap_entry_from_scancode(priv->array, event))
+			pm_wakeup_hard_event(&device->dev);
+		else
+			dev_info(&device->dev, "unknown event 0x%x\n", event);
+
+		return;
+	}
+
 	/* 0xC0 is for HID events, other values are for 5 button array */
 	if (event != 0xc0) {
 		if (!priv->array ||
@@ -292,6 +328,7 @@ static int intel_hid_probe(struct platfo
 				 "failed to enable HID power button\n");
 	}
 
+	device_init_wakeup(&device->dev, true);
 	return 0;
 
 err_remove_notify:

[toc] | [prev] | [next] | [standalone]

#1635815

On Wednesday, April 26, 2017 11:21:11 PM Rafael J. Wysocki wrote:
> Hi All,
> 
> The underlying issue is that on some relatively new Dell laltops, including
> Dell XPS13 9360 and 9365, pressing the power button is not sufficient to
> wake up the system from suspend-to-idle (it has to be pressed and held
> down for around 5 sec for the wakeup to happen) which is not expected
> and does not match the Windows' behavior.
> 
> This turns out to be a consequence of the way power button events are signaled
> on those systems, which is through the Embedded Controller (EC).  Namely,
> button events go to the EC which then signals the event through its ACPI GPE
> (General Purpose Event), which triggers an ACPI SCI (System Control Interrupt),
> whose handler executes a specicif AML control method and triggers a Notify()
> targetting a devie object associated with the power button.  The problem with
> suspend-to-idle is that the EC GPE is disabled during suspend, because
> otherwise all EC events would wake up the system from suspend-to-idle (and
> there can be many of them).
> 
> The only way to address this is to allow the EC GPE to stay enabled while
> suspended to idle, but that cannot be done unconditionally, because of the
> possible spurious wakeup events from the EC.  Fortunately, on the Dell systems
> in question it is possible to reduce the number of events coming from the EC
> while suspended (see the changelog of patch [3/5] for details), but still some
> spurious events may happen.  For this reason, when there is an SCI while
> suspended, it is not practical to resume all the way up to user space.
> Instead, it is better to resume partially up to the point at which the SCI can
> be processed by its handler, which is after calling dpm_resume_noirq(), let
> the SCI be processed and if no real wakeup events come out of that,
> suspend again.  Actually, that can be done in general, because spurious
> SCIs do happen while suspended to idle on other systems too, and that's
> which it goes as patch [2/5] before the Dell-related changes.
> 
> Patch [1/5] is more of a cleanup, but makes the rest look slightly more
> straightforward, and patches [4-5/5] update the drivers used for button
> events processing on the affected systems so that they signal wakeup
> as expected and avoid propagating the wakeup events as button events
> to user space.
> 
> The series is available from a git branch at
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git s2idle-dell-test
> 
> and has been included into the testing branch thereof.
> 
> Please let me know if there are any concerns.

Patches [1-2/5] from this series don't seem to be controversial, so I'd like to
go ahead with them, as they are general improvements independent of the
rest of the series.

If anyone has any concerns about that, please let me know.

Thanks,
Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web