[PATCH 0/4] PM / ACPI: sleep: Additional changes related to suspend-to-idle

[PATCH 0/4] PM / ACPI: sleep: Additional changes related to suspend-to-idle

[Rafael J. Wysocki]

Hi All,

On top of the "Simplify the suspend-to-idle control flow" patch series posted previously:

https://lore.kernel.org/lkml/71085220.z6FKkvYQPX@kreacher/

sanitize the suspend-to-idle flow even further.

First off, decouple EC wakeup from the LPS0 _DSM processing (patch 1).

Next, reorder the code to invoke LPS0 _DSM Functions 5 and 6 in the
specification-compliant order with respect to suspending and resuming
devices (patch 2).

Finally, rearrange lps0_device_attach() (patch 3) and add a command line switch
to prevent the LPS0 _DSM from being used.

Please refer to the changelogs for details.

Thanks,
Rafael

[PATCH 1/4] ACPI: PM: Set up EC GPE for system wakeup from drivers that need it

[Rafael J. Wysocki]

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

The EC GPE needs to be set up for system wakeup only if there is a
driver depending on it, either intel-hid or intel-vbtn, bound to a
button device that is expected to wake up the system from sleep (such
as the power button on some Dell systems, like the XPS13 9360).  It
doesn't need to be set up for waking up the system from sleep in any
other cases and whether or not it is expected to wake up the system
from sleep doesn't depend on whether or not the LPS0 device is
present in the ACPI namespace.

For this reason, rearrange the ACPI suspend-to-idle code to make the
drivers depending on the EC GPE wakeup take care of setting it up and
decouple that from the LPS0 device handling.

While at it, make intel-hid and intel-vbtn prepare for system wakeup
only if they are allowed to wake up the system from sleep by user
space (via sysfs).

[Note that acpi_ec_mark_gpe_for_wake() and acpi_ec_set_gpe_wake_mask()
 are there to prevent the EC GPE from being disabled by the
 acpi_enable_all_wakeup_gpes() call in acpi_s2idle_prepare(), so on
 systems with either intel-hid or intel-vbtn this change doesn't
 affect any interactions with the hardware or platform firmware.]

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/ec.c                 |    5 ++++-
 drivers/acpi/internal.h           |    2 --
 drivers/acpi/sleep.c              |   11 +----------
 drivers/platform/x86/intel-hid.c  |   20 ++++++++++++++++----
 drivers/platform/x86/intel-vbtn.c |   20 ++++++++++++++++----
 include/linux/acpi.h              |    4 ++++
 6 files changed, 41 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -194,8 +194,6 @@ void acpi_ec_ecdt_probe(void);
 void acpi_ec_dsdt_probe(void);
 void acpi_ec_block_transactions(void);
 void acpi_ec_unblock_transactions(void);
-void acpi_ec_mark_gpe_for_wake(void);
-void acpi_ec_set_gpe_wake_mask(u8 action);
 bool acpi_ec_dispatch_gpe(void);
 int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
 			      acpi_handle handle, acpi_ec_query_func func,
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
@@ -253,9 +253,12 @@ static void intel_button_array_enable(st
 
 static int intel_hid_pm_prepare(struct device *device)
 {
-	struct intel_hid_priv *priv = dev_get_drvdata(device);
+	if (device_may_wakeup(device)) {
+		struct intel_hid_priv *priv = dev_get_drvdata(device);
 
-	priv->wakeup_mode = true;
+		priv->wakeup_mode = true;
+		acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE);
+	}
 	return 0;
 }
 
@@ -270,9 +273,12 @@ static int intel_hid_pl_suspend_handler(
 
 static int intel_hid_pl_resume_handler(struct device *device)
 {
-	struct intel_hid_priv *priv = dev_get_drvdata(device);
+	if (device_may_wakeup(device)) {
+		struct intel_hid_priv *priv = dev_get_drvdata(device);
 
-	priv->wakeup_mode = false;
+		acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE);
+		priv->wakeup_mode = false;
+	}
 	if (pm_resume_via_firmware()) {
 		intel_hid_set_enable(device, true);
 		intel_button_array_enable(device, true);
@@ -491,6 +497,12 @@ static int intel_hid_probe(struct platfo
 	}
 
 	device_init_wakeup(&device->dev, true);
+	/*
+	 * In order for system wakeup to work, the EC GPE has to be marked as
+	 * a wakeup one, so do that here (this setting will persist, but it has
+	 * no effect until the wakeup mask is set for the EC GPE).
+	 */
+	acpi_ec_mark_gpe_for_wake();
 	return 0;
 
 err_remove_notify:
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
@@ -176,6 +176,12 @@ static int intel_vbtn_probe(struct platf
 		return -EBUSY;
 
 	device_init_wakeup(&device->dev, true);
+	/*
+	 * In order for system wakeup to work, the EC GPE has to be marked as
+	 * a wakeup one, so do that here (this setting will persist, but it has
+	 * no effect until the wakeup mask is set for the EC GPE).
+	 */
+	acpi_ec_mark_gpe_for_wake();
 	return 0;
 }
 
@@ -195,17 +201,23 @@ static int intel_vbtn_remove(struct plat
 
 static int intel_vbtn_pm_prepare(struct device *dev)
 {
-	struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
+	if (device_may_wakeup(dev)) {
+		struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
 
-	priv->wakeup_mode = true;
+		priv->wakeup_mode = true;
+		acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE);
+	}
 	return 0;
 }
 
 static int intel_vbtn_pm_resume(struct device *dev)
 {
-	struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
+	if (device_may_wakeup(dev)) {
+		struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
 
-	priv->wakeup_mode = false;
+		acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE);
+		priv->wakeup_mode = false;
+	}
 	return 0;
 }
 
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -931,6 +931,8 @@ int acpi_subsys_suspend_noirq(struct dev
 int acpi_subsys_suspend(struct device *dev);
 int acpi_subsys_freeze(struct device *dev);
 int acpi_subsys_poweroff(struct device *dev);
+void acpi_ec_mark_gpe_for_wake(void);
+void acpi_ec_set_gpe_wake_mask(u8 action);
 #else
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
 static inline void acpi_subsys_complete(struct device *dev) {}
@@ -939,6 +941,8 @@ static inline int acpi_subsys_suspend_no
 static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
 static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
 static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
+static inline void acpi_ec_mark_gpe_for_wake(void) {}
+static inline void acpi_ec_set_gpe_wake_mask(u8 action) {}
 #endif
 
 #ifdef CONFIG_ACPI
Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -25,6 +25,7 @@
 #include <linux/list.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
+#include <linux/suspend.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
 #include <asm/io.h>
@@ -1053,12 +1054,14 @@ void acpi_ec_mark_gpe_for_wake(void)
 	if (first_ec && !ec_no_wakeup)
 		acpi_mark_gpe_for_wake(NULL, first_ec->gpe);
 }
+EXPORT_SYMBOL_GPL(acpi_ec_mark_gpe_for_wake);
 
 void acpi_ec_set_gpe_wake_mask(u8 action)
 {
-	if (first_ec && !ec_no_wakeup)
+	if (pm_suspend_no_platform() && first_ec && !ec_no_wakeup)
 		acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action);
 }
+EXPORT_SYMBOL_GPL(acpi_ec_set_gpe_wake_mask);
 
 bool acpi_ec_dispatch_gpe(void)
 {
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -930,8 +930,6 @@ static int lps0_device_attach(struct acp
 
 		acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
 				  bitmask);
-
-		acpi_ec_mark_gpe_for_wake();
 	} else {
 		acpi_handle_debug(adev->handle,
 				  "_DSM function 0 evaluation failed\n");
@@ -960,8 +958,6 @@ static int acpi_s2idle_prepare(void)
 	if (lps0_device_handle) {
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
-
-		acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE);
 	}
 
 	if (acpi_sci_irq_valid())
@@ -979,10 +975,7 @@ static int acpi_s2idle_prepare(void)
 
 static void acpi_s2idle_wake(void)
 {
-	if (!lps0_device_handle)
-		return;
-
-	if (pm_debug_messages_on)
+	if (lps0_device_handle && pm_debug_messages_on)
 		lpi_check_constraints();
 
 	/*
@@ -1031,8 +1024,6 @@ static void acpi_s2idle_restore(void)
 		disable_irq_wake(acpi_sci_irq);
 
 	if (lps0_device_handle) {
-		acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE);
-
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
 	}

Re: [PATCH 1/4] ACPI: PM: Set up EC GPE for system wakeup from drivers that need it

[Andy Shevchenko]

On Fri, Jul 26, 2019 at 1:57 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The EC GPE needs to be set up for system wakeup only if there is a
> driver depending on it, either intel-hid or intel-vbtn, bound to a
> button device that is expected to wake up the system from sleep (such
> as the power button on some Dell systems, like the XPS13 9360).  It
> doesn't need to be set up for waking up the system from sleep in any
> other cases and whether or not it is expected to wake up the system
> from sleep doesn't depend on whether or not the LPS0 device is
> present in the ACPI namespace.
>
> For this reason, rearrange the ACPI suspend-to-idle code to make the
> drivers depending on the EC GPE wakeup take care of setting it up and
> decouple that from the LPS0 device handling.
>
> While at it, make intel-hid and intel-vbtn prepare for system wakeup
> only if they are allowed to wake up the system from sleep by user
> space (via sysfs).
>
> [Note that acpi_ec_mark_gpe_for_wake() and acpi_ec_set_gpe_wake_mask()
>  are there to prevent the EC GPE from being disabled by the
>  acpi_enable_all_wakeup_gpes() call in acpi_s2idle_prepare(), so on
>  systems with either intel-hid or intel-vbtn this change doesn't
>  affect any interactions with the hardware or platform firmware.]
>

Thank you!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/ec.c                 |    5 ++++-
>  drivers/acpi/internal.h           |    2 --
>  drivers/acpi/sleep.c              |   11 +----------
>  drivers/platform/x86/intel-hid.c  |   20 ++++++++++++++++----
>  drivers/platform/x86/intel-vbtn.c |   20 ++++++++++++++++----
>  include/linux/acpi.h              |    4 ++++
>  6 files changed, 41 insertions(+), 21 deletions(-)
>
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -194,8 +194,6 @@ void acpi_ec_ecdt_probe(void);
>  void acpi_ec_dsdt_probe(void);
>  void acpi_ec_block_transactions(void);
>  void acpi_ec_unblock_transactions(void);
> -void acpi_ec_mark_gpe_for_wake(void);
> -void acpi_ec_set_gpe_wake_mask(u8 action);
>  bool acpi_ec_dispatch_gpe(void);
>  int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
>                               acpi_handle handle, acpi_ec_query_func func,
> 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
> @@ -253,9 +253,12 @@ static void intel_button_array_enable(st
>
>  static int intel_hid_pm_prepare(struct device *device)
>  {
> -       struct intel_hid_priv *priv = dev_get_drvdata(device);
> +       if (device_may_wakeup(device)) {
> +               struct intel_hid_priv *priv = dev_get_drvdata(device);
>
> -       priv->wakeup_mode = true;
> +               priv->wakeup_mode = true;
> +               acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE);
> +       }
>         return 0;
>  }
>
> @@ -270,9 +273,12 @@ static int intel_hid_pl_suspend_handler(
>
>  static int intel_hid_pl_resume_handler(struct device *device)
>  {
> -       struct intel_hid_priv *priv = dev_get_drvdata(device);
> +       if (device_may_wakeup(device)) {
> +               struct intel_hid_priv *priv = dev_get_drvdata(device);
>
> -       priv->wakeup_mode = false;
> +               acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE);
> +               priv->wakeup_mode = false;
> +       }
>         if (pm_resume_via_firmware()) {
>                 intel_hid_set_enable(device, true);
>                 intel_button_array_enable(device, true);
> @@ -491,6 +497,12 @@ static int intel_hid_probe(struct platfo
>         }
>
>         device_init_wakeup(&device->dev, true);
> +       /*
> +        * In order for system wakeup to work, the EC GPE has to be marked as
> +        * a wakeup one, so do that here (this setting will persist, but it has
> +        * no effect until the wakeup mask is set for the EC GPE).
> +        */
> +       acpi_ec_mark_gpe_for_wake();
>         return 0;
>
>  err_remove_notify:
> 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
> @@ -176,6 +176,12 @@ static int intel_vbtn_probe(struct platf
>                 return -EBUSY;
>
>         device_init_wakeup(&device->dev, true);
> +       /*
> +        * In order for system wakeup to work, the EC GPE has to be marked as
> +        * a wakeup one, so do that here (this setting will persist, but it has
> +        * no effect until the wakeup mask is set for the EC GPE).
> +        */
> +       acpi_ec_mark_gpe_for_wake();
>         return 0;
>  }
>
> @@ -195,17 +201,23 @@ static int intel_vbtn_remove(struct plat
>
>  static int intel_vbtn_pm_prepare(struct device *dev)
>  {
> -       struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
> +       if (device_may_wakeup(dev)) {
> +               struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
>
> -       priv->wakeup_mode = true;
> +               priv->wakeup_mode = true;
> +               acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE);
> +       }
>         return 0;
>  }
>
>  static int intel_vbtn_pm_resume(struct device *dev)
>  {
> -       struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
> +       if (device_may_wakeup(dev)) {
> +               struct intel_vbtn_priv *priv = dev_get_drvdata(dev);
>
> -       priv->wakeup_mode = false;
> +               acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE);
> +               priv->wakeup_mode = false;
> +       }
>         return 0;
>  }
>
> Index: linux-pm/include/linux/acpi.h
> ===================================================================
> --- linux-pm.orig/include/linux/acpi.h
> +++ linux-pm/include/linux/acpi.h
> @@ -931,6 +931,8 @@ int acpi_subsys_suspend_noirq(struct dev
>  int acpi_subsys_suspend(struct device *dev);
>  int acpi_subsys_freeze(struct device *dev);
>  int acpi_subsys_poweroff(struct device *dev);
> +void acpi_ec_mark_gpe_for_wake(void);
> +void acpi_ec_set_gpe_wake_mask(u8 action);
>  #else
>  static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
>  static inline void acpi_subsys_complete(struct device *dev) {}
> @@ -939,6 +941,8 @@ static inline int acpi_subsys_suspend_no
>  static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
>  static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>  static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
> +static inline void acpi_ec_mark_gpe_for_wake(void) {}
> +static inline void acpi_ec_set_gpe_wake_mask(u8 action) {}
>  #endif
>
>  #ifdef CONFIG_ACPI
> Index: linux-pm/drivers/acpi/ec.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/ec.c
> +++ linux-pm/drivers/acpi/ec.c
> @@ -25,6 +25,7 @@
>  #include <linux/list.h>
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
> +#include <linux/suspend.h>
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
>  #include <asm/io.h>
> @@ -1053,12 +1054,14 @@ void acpi_ec_mark_gpe_for_wake(void)
>         if (first_ec && !ec_no_wakeup)
>                 acpi_mark_gpe_for_wake(NULL, first_ec->gpe);
>  }
> +EXPORT_SYMBOL_GPL(acpi_ec_mark_gpe_for_wake);
>
>  void acpi_ec_set_gpe_wake_mask(u8 action)
>  {
> -       if (first_ec && !ec_no_wakeup)
> +       if (pm_suspend_no_platform() && first_ec && !ec_no_wakeup)
>                 acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action);
>  }
> +EXPORT_SYMBOL_GPL(acpi_ec_set_gpe_wake_mask);
>
>  bool acpi_ec_dispatch_gpe(void)
>  {
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -930,8 +930,6 @@ static int lps0_device_attach(struct acp
>
>                 acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
>                                   bitmask);
> -
> -               acpi_ec_mark_gpe_for_wake();
>         } else {
>                 acpi_handle_debug(adev->handle,
>                                   "_DSM function 0 evaluation failed\n");
> @@ -960,8 +958,6 @@ static int acpi_s2idle_prepare(void)
>         if (lps0_device_handle) {
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
> -
> -               acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE);
>         }
>
>         if (acpi_sci_irq_valid())
> @@ -979,10 +975,7 @@ static int acpi_s2idle_prepare(void)
>
>  static void acpi_s2idle_wake(void)
>  {
> -       if (!lps0_device_handle)
> -               return;
> -
> -       if (pm_debug_messages_on)
> +       if (lps0_device_handle && pm_debug_messages_on)
>                 lpi_check_constraints();
>
>         /*
> @@ -1031,8 +1024,6 @@ static void acpi_s2idle_restore(void)
>                 disable_irq_wake(acpi_sci_irq);
>
>         if (lps0_device_handle) {
> -               acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE);
> -
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
>         }
>
>
>


-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/4] ACPI: PM: s2idle: Execute LPS0 _DSM functions with suspended devices

[Rafael J. Wysocki]

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

According to Section 3.5 of the "Intel Low Power S0 Idle" document [1],
Function 5 of the LPS0 _DSM is expected to be invoked when the system
configuration matches the criteria for entering the target low-power
state of the platform.  In particular, this means that all devices
should be suspended and in low-power states already when that function
is invoked.

This is not the case currently, however, because Function 5 of the
LPS0 _DSM is invoked by it before the "noirq" phase of device suspend,
which means that some devices may not have been put into low-power
states yet at that point.  That is a consequence of the previous
design of the suspend-to-idle flow that allowed the "noirq" phase of
device suspend and the "noirq" phase of device resume to be carried
out for multiple times while "suspended" (if any spurious wakeup
events were detected) and the point of the LPS0 _DSM Function 5
invocation was chosen so as to call it (and LPS0 _DSM Function 6
analogously) once per suspend-resume cycle (regardless of how many
times the "noirq" phases of device suspend and resume were carried
out while "suspended").

Now that the suspend-to-idle flow has been redesigned to carry out
the "noirq" phases of device suspend and resume once in each cycle,
the code can be reordered to follow the specification that it is
based on more closely.

For this purpose, add ->prepare_late and ->restore_early platform
callbacks for suspend-to-idle, to be executed, respectively, after
the "noirq" phase of suspending devices and before the "noirq"
phase of resuming them and make ACPI use them for the invocation
of LPS0 _DSM functions as appropriate.

While at it, move the LPS0 entry requirements check to be made
before invoking Functions 3 and 5 of the LPS0 _DSM (also once
per cycle) as follows from the specification [1].

Link: https://uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf # [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/sleep.c    |   36 ++++++++++++++++++++++++------------
 include/linux/suspend.h |    2 ++
 kernel/power/suspend.c  |   12 +++++++++---
 3 files changed, 35 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -955,11 +955,6 @@ static int acpi_s2idle_begin(void)
 
 static int acpi_s2idle_prepare(void)
 {
-	if (lps0_device_handle) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
-	}
-
 	if (acpi_sci_irq_valid())
 		enable_irq_wake(acpi_sci_irq);
 
@@ -973,11 +968,22 @@ static int acpi_s2idle_prepare(void)
 	return 0;
 }
 
-static void acpi_s2idle_wake(void)
+static int acpi_s2idle_prepare_late(void)
 {
-	if (lps0_device_handle && pm_debug_messages_on)
+	if (!lps0_device_handle)
+		return 0;
+
+	if (pm_debug_messages_on)
 		lpi_check_constraints();
 
+	acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
+	acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
+
+	return 0;
+}
+
+static void acpi_s2idle_wake(void)
+{
 	/*
 	 * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the SCI has
 	 * not triggered while suspended, so bail out.
@@ -1012,6 +1018,15 @@ static void acpi_s2idle_wake(void)
 	rearm_wake_irq(acpi_sci_irq);
 }
 
+static void acpi_s2idle_restore_early(void)
+{
+	if (!lps0_device_handle)
+		return;
+
+	acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
+	acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
+}
+
 static void acpi_s2idle_restore(void)
 {
 	s2idle_wakeup = false;
@@ -1022,11 +1037,6 @@ static void acpi_s2idle_restore(void)
 
 	if (acpi_sci_irq_valid())
 		disable_irq_wake(acpi_sci_irq);
-
-	if (lps0_device_handle) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
-	}
 }
 
 static void acpi_s2idle_end(void)
@@ -1038,7 +1048,9 @@ static void acpi_s2idle_end(void)
 static const struct platform_s2idle_ops acpi_s2idle_ops = {
 	.begin = acpi_s2idle_begin,
 	.prepare = acpi_s2idle_prepare,
+	.prepare_late = acpi_s2idle_prepare_late,
 	.wake = acpi_s2idle_wake,
+	.restore_early = acpi_s2idle_restore_early,
 	.restore = acpi_s2idle_restore,
 	.end = acpi_s2idle_end,
 };
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -267,13 +267,19 @@ static int platform_suspend_prepare_late
 
 static int platform_suspend_prepare_noirq(suspend_state_t state)
 {
-	return state != PM_SUSPEND_TO_IDLE && suspend_ops->prepare_late ?
-		suspend_ops->prepare_late() : 0;
+	if (state == PM_SUSPEND_TO_IDLE) {
+		if (s2idle_ops && s2idle_ops->prepare_late)
+			return s2idle_ops->prepare_late();
+	}
+	return suspend_ops->prepare_late ? suspend_ops->prepare_late() : 0;
 }
 
 static void platform_resume_noirq(suspend_state_t state)
 {
-	if (state != PM_SUSPEND_TO_IDLE && suspend_ops->wake)
+	if (state == PM_SUSPEND_TO_IDLE) {
+		if (s2idle_ops && s2idle_ops->restore_early)
+			s2idle_ops->restore_early();
+	} else if (suspend_ops->wake)
 		suspend_ops->wake();
 }
 
Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -190,7 +190,9 @@ struct platform_suspend_ops {
 struct platform_s2idle_ops {
 	int (*begin)(void);
 	int (*prepare)(void);
+	int (*prepare_late)(void);
 	void (*wake)(void);
+	void (*restore_early)(void);
 	void (*restore)(void);
 	void (*end)(void);
 };

[PATCH 3/4] ACPI: PM: s2idle: Rearrange lps0_device_attach()

[Rafael J. Wysocki]

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

To allow a subsequent change to be simpler, rearrange the code in
lps0_device_attach() to reduce the indentation level and (while
at it) make it avoid calling lpi_device_get_constraints() when
lps0_device_handle is not going to be set.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/sleep.c |   32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -916,28 +916,30 @@ static int lps0_device_attach(struct acp
 	guid_parse(ACPI_LPS0_DSM_UUID, &lps0_dsm_guid);
 	/* Check if the _DSM is present and as expected. */
 	out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 1, 0, NULL);
-	if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
-		char bitmask = *(char *)out_obj->buffer.pointer;
-
-		lps0_dsm_func_mask = bitmask;
-		lps0_device_handle = adev->handle;
-		/*
-		 * Use suspend-to-idle by default if the default
-		 * suspend mode was not set from the command line.
-		 */
-		if (mem_sleep_default > PM_SUSPEND_MEM)
-			mem_sleep_current = PM_SUSPEND_TO_IDLE;
-
-		acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
-				  bitmask);
-	} else {
+	if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER) {
 		acpi_handle_debug(adev->handle,
 				  "_DSM function 0 evaluation failed\n");
+		return 0;
 	}
+
+	lps0_dsm_func_mask = *(char *)out_obj->buffer.pointer;
+
 	ACPI_FREE(out_obj);
 
+	acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
+			  lps0_dsm_func_mask);
+
+	lps0_device_handle = adev->handle;
+
 	lpi_device_get_constraints();
 
+	/*
+	 * Use suspend-to-idle by default if the default suspend mode was not
+	 * set from the command line.
+	 */
+	if (mem_sleep_default > PM_SUSPEND_MEM)
+		mem_sleep_current = PM_SUSPEND_TO_IDLE;
+
 	return 0;
 }
 

[PATCH 4/4] ACPI: PM: s2idle: Add acpi_sleep=no_lps0 command line switch

[Rafael J. Wysocki]

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

Add a kernel command line switch to prevent the LPS0 _DSM funtions
from being invoked (if need be) and rework the suspend-to-idle
blacklist entries in acpisleep_dmi_table[] to make them simply
prevent suspend-to-idle from being used by default on the systems
in question (which really is the original purpose of those entries).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |    5 ++-
 arch/x86/kernel/acpi/sleep.c                    |    2 +
 drivers/acpi/sleep.c                            |   34 +++++++++++++++---------
 include/linux/acpi.h                            |    1 
 4 files changed, 29 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -158,11 +158,19 @@ static int __init init_nvs_nosave(const
 	return 0;
 }
 
-static bool acpi_sleep_no_lps0;
+#define ACPI_SLEEP_AVOID_LPS0_DSM	BIT(0)
+#define ACPI_SLEEP_DEFAULT_S3		BIT(1)
 
-static int __init init_no_lps0(const struct dmi_system_id *d)
+static u8 acpi_sleep_lps0_flags;
+
+void __init acpi_sleep_init_no_lps0(void)
+{
+	acpi_sleep_lps0_flags |= ACPI_SLEEP_AVOID_LPS0_DSM;
+}
+
+static int __init init_default_s3(const struct dmi_system_id *d)
 {
-	acpi_sleep_no_lps0 = true;
+	acpi_sleep_lps0_flags |= ACPI_SLEEP_DEFAULT_S3;
 	return 0;
 }
 
@@ -363,7 +371,7 @@ static const struct dmi_system_id acpisl
 	 * S0 Idle firmware interface.
 	 */
 	{
-	.callback = init_no_lps0,
+	.callback = init_default_s3,
 	.ident = "Dell XPS13 9360",
 	.matches = {
 		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
@@ -376,7 +384,7 @@ static const struct dmi_system_id acpisl
 	 * https://bugzilla.kernel.org/show_bug.cgi?id=199057).
 	 */
 	{
-	.callback = init_no_lps0,
+	.callback = init_default_s3,
 	.ident = "ThinkPad X1 Tablet(2016)",
 	.matches = {
 		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
@@ -525,7 +533,7 @@ static void acpi_pm_end(void)
 }
 #else /* !CONFIG_ACPI_SLEEP */
 #define acpi_target_sleep_state	ACPI_STATE_S0
-#define acpi_sleep_no_lps0	(false)
+#define acpi_sleep_lps0_flags	(0)
 static inline void acpi_sleep_dmi_check(void) {}
 #endif /* CONFIG_ACPI_SLEEP */
 
@@ -904,15 +912,15 @@ static int lps0_device_attach(struct acp
 	if (lps0_device_handle)
 		return 0;
 
-	if (acpi_sleep_no_lps0) {
+	if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+		return 0;
+
+	if (acpi_sleep_lps0_flags & ACPI_SLEEP_AVOID_LPS0_DSM) {
 		acpi_handle_info(adev->handle,
 				 "Low Power S0 Idle interface disabled\n");
-		return 0;
+		goto default_sleep;
 	}
 
-	if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
-		return 0;
-
 	guid_parse(ACPI_LPS0_DSM_UUID, &lps0_dsm_guid);
 	/* Check if the _DSM is present and as expected. */
 	out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 1, 0, NULL);
@@ -933,11 +941,13 @@ static int lps0_device_attach(struct acp
 
 	lpi_device_get_constraints();
 
+default_sleep:
 	/*
 	 * Use suspend-to-idle by default if the default suspend mode was not
 	 * set from the command line.
 	 */
-	if (mem_sleep_default > PM_SUSPEND_MEM)
+	if (mem_sleep_default > PM_SUSPEND_MEM &&
+	    !(acpi_sleep_lps0_flags & ACPI_SLEEP_DEFAULT_S3))
 		mem_sleep_current = PM_SUSPEND_TO_IDLE;
 
 	return 0;
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -471,6 +471,7 @@ void __init acpi_old_suspend_ordering(vo
 void __init acpi_nvs_nosave(void);
 void __init acpi_nvs_nosave_s3(void);
 void __init acpi_sleep_no_blacklist(void);
+void __init acpi_sleep_init_no_lps0(void);
 #endif /* CONFIG_PM_SLEEP */
 
 struct acpi_osc_context {
Index: linux-pm/arch/x86/kernel/acpi/sleep.c
===================================================================
--- linux-pm.orig/arch/x86/kernel/acpi/sleep.c
+++ linux-pm/arch/x86/kernel/acpi/sleep.c
@@ -137,6 +137,8 @@ static int __init acpi_sleep_setup(char
 			acpi_nvs_nosave_s3();
 		if (strncmp(str, "old_ordering", 12) == 0)
 			acpi_old_suspend_ordering();
+		if (strncmp(str, "no_lps0", 7) == 0)
+			acpi_sleep_init_no_lps0();
 		if (strncmp(str, "nobl", 4) == 0)
 			acpi_sleep_no_blacklist();
 		str = strchr(str, ',');
Index: linux-pm/Documentation/admin-guide/kernel-parameters.txt
===================================================================
--- linux-pm.orig/Documentation/admin-guide/kernel-parameters.txt
+++ linux-pm/Documentation/admin-guide/kernel-parameters.txt
@@ -222,7 +222,8 @@
 
 	acpi_sleep=	[HW,ACPI] Sleep options
 			Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig,
-				  old_ordering, nonvs, sci_force_enable, nobl }
+				  old_ordering, nonvs, sci_force_enable,
+				  no_lps0, nobl }
 			See Documentation/power/video.rst for information on
 			s3_bios and s3_mode.
 			s3_beep is for debugging; it makes the PC's speaker beep
@@ -238,6 +239,8 @@
 			sci_force_enable causes the kernel to set SCI_EN directly
 			on resume from S1/S3 (which is against the ACPI spec,
 			but some broken systems don't work without it).
+			no_lps0 prevents the kernel from using the special
+			LPS0 device interface during suspend-to-idle.
 			nobl causes the internal blacklist of systems known to
 			behave incorrectly in some ways with respect to system
 			suspend and resume to be ignored (use wisely).