[PATCH v3] ACPI: s2idle: Handle shared _Ixx GPEs without an EC

[PATCH v3] ACPI: s2idle: Handle shared _Ixx GPEs without an EC

[Saranya Gopal]

When first_ec is NULL, acpi_ec_dispatch_gpe() previously treated any
active GPE as a wake source. On platforms where an _Ixx GPE is shared
by user-wakeup and background events, that can cause unnecessary
wakeups.

Make acpi_ec_dispatch_gpe() return an int and return -ENODEV when no
EC instance is available so that acpi_s2idle_wake() can handle that
case: on x86, treat the first GPE armed for wake via an _Ixx method
like the EC GPE, mirroring acpi_ec_dispatch_gpe(). That is, report a
wakeup if the status bit is set for any enabled GPE other than the
_Ixx one. Otherwise, cancel the SCI-related wakeup and dispatch the
_Ixx GPE if its status bit is set, so the events behind it can be
processed. Drivers handling genuine wake events are expected to
report them via pm_wakeup_event(), which the common s2idle path will
catch after synchronizing event processing.

If no _Ixx GPEs have been armed, and in the non-x86 fallback, the
previous any-GPE status check is retained. If GPEs are armed via more
than one _Ixx method, the first one is used as the wake indicator and
the remaining ones are treated as regular wake sources.

Fixes: 46767b17fda3 ("ACPI: PM: s2idle: Add _Ixx GPE wake indicator support")
Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
---
Changes from v2:
 - Restructured ret checks in sleep.c as per Rafael's suggestion to
   cover !CONFIG_ACPI_EC case that returns -EOPNOTSUPP (Sashiko report)
 - Drop the open-coded scan over acpi_current_gpe_count that conflated
   sequential GPE indices with raw GPE numbers (Sashiko report)
 - Mirror acpi_ec_dispatch_gpe() with the first GPE armed via an _Ixx
   method playing the role of the EC GPE, so the existing single-skip
   acpi_any_gpe_status_set() is sufficient and no ACPICA changes are
   needed.
 - Follow acpi_ec_dispatch_gpe() for the status check and dispatch of
   the _Ixx GPE as well, which also drops the error paths that
   requested a system wakeup in v2 (questioned by Rafael).
 - Add a comment describing acpi_x86_s2idle_check_ixx_gpes(), including
   the role of pm_system_cancel_wakeup().
 - Preserve the previous any-GPE behavior when no _Ixx GPEs have been
   armed for wakeup.
Changes from v1:
 - -ENOTSUP replaced with the correct -EOPNOTSUPP for !CONFIG_PM_SLEEP
   case

 drivers/acpi/ec.c         | 10 +++----
 drivers/acpi/internal.h   |  6 ++--
 drivers/acpi/sleep.c      | 11 +++++--
 drivers/acpi/sleep.h      |  8 +++++
 drivers/acpi/x86/s2idle.c | 61 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 64ad4cfa6208..8be572af4115 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -2155,19 +2155,19 @@ static bool acpi_ec_work_in_progress(struct acpi_ec *ec)
 	return ec->events_in_progress + ec->queries_in_progress > 0;
 }
 
-bool acpi_ec_dispatch_gpe(void)
+int acpi_ec_dispatch_gpe(void)
 {
 	bool work_in_progress = false;
 
 	if (!first_ec)
-		return acpi_any_gpe_status_set(U32_MAX);
+		return -ENODEV;
 
 	/*
 	 * Report wakeup if the status bit is set for any enabled GPE other
 	 * than the EC one.
 	 */
 	if (acpi_any_gpe_status_set(first_ec->gpe))
-		return true;
+		return 1;
 
 	/*
 	 * Cancel the SCI wakeup and process all pending events in case there
@@ -2195,7 +2195,7 @@ bool acpi_ec_dispatch_gpe(void)
 	spin_unlock_irq(&first_ec->lock);
 
 	if (!work_in_progress)
-		return false;
+		return 0;
 
 	pm_pr_dbg("ACPI EC GPE dispatched\n");
 
@@ -2212,7 +2212,7 @@ bool acpi_ec_dispatch_gpe(void)
 		spin_unlock_irq(&first_ec->lock);
 	} while (work_in_progress && !pm_wakeup_pending());
 
-	return false;
+	return 0;
 }
 #endif /* CONFIG_PM_SLEEP */
 
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 40f875b265a9..d5435338d6ca 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -236,7 +236,7 @@ void acpi_ec_register_opregions(struct acpi_device *adev);
 
 #ifdef CONFIG_PM_SLEEP
 void acpi_ec_flush_work(void);
-bool acpi_ec_dispatch_gpe(void);
+int acpi_ec_dispatch_gpe(void);
 #endif
 
 #else
@@ -256,9 +256,9 @@ static inline void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit
 static inline void acpi_ec_register_opregions(struct acpi_device *adev) {}
 
 static inline void acpi_ec_flush_work(void) {}
-static inline bool acpi_ec_dispatch_gpe(void)
+static inline int acpi_ec_dispatch_gpe(void)
 {
-	return false;
+	return -EOPNOTSUPP;
 }
 
 #endif
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 132a9df98471..09bd661187b6 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -761,6 +761,8 @@ bool acpi_s2idle_wake(void)
 		return pm_wakeup_pending();
 
 	while (pm_wakeup_pending()) {
+		int ret;
+
 		/*
 		 * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the
 		 * SCI has not triggered while suspended, so bail out (the
@@ -789,11 +791,16 @@ bool acpi_s2idle_wake(void)
 
 		/*
 		 * Check non-EC GPE wakeups and if there are none, cancel the
-		 * SCI-related wakeup and dispatch the EC GPE.
+		 * SCI-related wakeup and dispatch the EC GPE.  If there is no
+		 * EC to consult, check the GPEs armed for wakeup via _Ixx
+		 * methods instead.
 		 */
-		if (acpi_ec_dispatch_gpe()) {
+		ret = acpi_ec_dispatch_gpe();
+		if (ret > 0) {
 			pm_pr_dbg("ACPI non-EC GPE wakeup\n");
 			return true;
+		} else if (ret < 0 && acpi_x86_s2idle_check_ixx_gpes()) {
+			return true;
 		}
 
 		acpi_os_wait_events_complete();
diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h
index 9c3cb109c5d2..955e9c22bf0d 100644
--- a/drivers/acpi/sleep.h
+++ b/drivers/acpi/sleep.h
@@ -20,6 +20,14 @@ extern int acpi_s2idle_prepare(void);
 extern bool acpi_s2idle_wake(void);
 extern void acpi_s2idle_restore(void);
 extern void acpi_s2idle_end(void);
+#if defined(CONFIG_X86) && defined(CONFIG_SUSPEND)
+extern bool acpi_x86_s2idle_check_ixx_gpes(void);
+#else
+static inline bool acpi_x86_s2idle_check_ixx_gpes(void)
+{
+	return acpi_any_gpe_status_set(U32_MAX);
+}
+#endif
 
 extern void acpi_s2idle_setup(void);
 
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index c278ea14c0b3..b79bf0fa9a7f 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -24,6 +24,13 @@
 
 #ifdef CONFIG_SUSPEND
 
+/*
+ * First GPE armed for wake via an _Ixx method, U32_MAX if none.  On
+ * platforms without an ACPI-enumerated EC, this GPE plays the role of
+ * the EC GPE in acpi_ec_dispatch_gpe().
+ */
+static u32 acpi_s2idle_ixx_gpe = U32_MAX;
+
 static void acpi_setup_ixx_gpes(void)
 {
 	acpi_handle gpe_root;
@@ -47,11 +54,65 @@ static void acpi_setup_ixx_gpes(void)
 
 		acpi_mark_gpe_for_wake(NULL, i);
 		acpi_set_gpe_wake_mask(NULL, i, ACPI_GPE_ENABLE);
+		if (acpi_s2idle_ixx_gpe == U32_MAX)
+			acpi_s2idle_ixx_gpe = i;
 
 		pm_pr_dbg("ACPI: GPE0x%02x armed for wake via %s\n", i, gpe_nr_str);
 	}
 }
 
+/*
+ * acpi_x86_s2idle_check_ixx_gpes - Handle an SCI wakeup without an EC
+ *
+ * Called to decide whether or not to wake up the system when an SCI has
+ * triggered during suspend-to-idle on a platform without an EC instance,
+ * mirroring acpi_ec_dispatch_gpe() with the first GPE armed for wake via
+ * an _Ixx method playing the role of the EC GPE.
+ *
+ * Report a wakeup if the status bit is set for any enabled GPE other than
+ * the _Ixx one.  If no _Ixx GPE has been armed, acpi_s2idle_ixx_gpe is
+ * U32_MAX and this amounts to checking all of the GPEs, which is the
+ * previous behavior.
+ *
+ * Otherwise, cancel the pending SCI-related wakeup, so it does not cause
+ * the pm_wakeup_pending() check in the caller to succeed by itself, and
+ * dispatch the _Ixx GPE if it is active, so the events behind it can be
+ * processed.  If any of those events turn out to be genuine wake events,
+ * the drivers handling them are expected to call pm_wakeup_event() and
+ * the caller will catch that via pm_wakeup_pending() after synchronizing
+ * the event processing.
+ */
+bool acpi_x86_s2idle_check_ixx_gpes(void)
+{
+	acpi_event_status status = 0;
+
+	/*
+	 * Report wakeup if the status bit is set for any enabled GPE other
+	 * than the _Ixx one.
+	 */
+	if (acpi_any_gpe_status_set(acpi_s2idle_ixx_gpe))
+		return true;
+
+	if (acpi_s2idle_ixx_gpe == U32_MAX)
+		return false;
+
+	/*
+	 * Cancel the SCI wakeup and dispatch the _Ixx GPE in case there are
+	 * any wakeup events behind it.
+	 */
+	pm_system_cancel_wakeup();
+
+	acpi_get_gpe_status(NULL, acpi_s2idle_ixx_gpe, &status);
+	if (status & ACPI_EVENT_FLAG_STATUS_SET) {
+		pm_pr_dbg("ACPI _Ixx GPE 0x%02x dispatched\n",
+			  acpi_s2idle_ixx_gpe);
+
+		acpi_dispatch_gpe(NULL, acpi_s2idle_ixx_gpe);
+	}
+
+	return false;
+}
+
 static bool sleep_no_lps0 __read_mostly;
 module_param(sleep_no_lps0, bool, 0644);
 MODULE_PARM_DESC(sleep_no_lps0, "Do not use the special LPS0 device interface");
-- 
2.34.1