> -----Original Message-----
> From: Rafael J. Wysocki [mailto:r...@rjwysocki.net]
> Sent: Monday, July 31, 2017 4:43 PM
> To: Linux ACPI <linux-a...@vger.kernel.org>
> Cc: LKML <linux-kernel@vger.kernel.org>; Len Brown <len.br...@intel.com>;
> Linux PM <linux...@vger.kernel.org>; Srinivas Pandruvada
> <srinivas.pandruv...@linux.intel.com>; Limonciello, Mario
> <mario_limoncie...@dell.com>
> Subject: [PATCH] ACPI / PM: Prefer suspend-to-idle over S3 on some systems
> 
> From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> 
> Modify the ACPI system sleep support setup code to select
> suspend-to-idle as the default system sleep state if
> (1) the ACPI_FADT_LOW_POWER_S0 flag is set in the FADT and
> (2) the Low Power Idle S0 _DSM interface has been discovered and
> (3) the default sleep state was not selected from the kernel command
> line.
> 
> The main motivation for this change is that systems where the (1) and
> (2) conditions are met typically ship with OSes that don't exercise
> the S3 path in the platform firmware which remains untested and turns
> out to be non-functional at least in some cases.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> ---
> 
> This patch depends on the intel-hid change at
> 
> https://patchwork.kernel.org/patch/9867703/
> 
> ---
>  Documentation/power/states.txt |    4 +++-
>  drivers/acpi/sleep.c           |    6 ++++++
>  include/linux/suspend.h        |    3 +++
>  kernel/power/power.h           |    1 -
>  kernel/power/suspend.c         |    4 ++--
>  5 files changed, 14 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/Documentation/power/states.txt
> ===================================================================
> --- linux-pm.orig/Documentation/power/states.txt
> +++ linux-pm/Documentation/power/states.txt
> @@ -35,7 +35,9 @@ only one way to cause the system to go i
>  The default suspend mode (ie. the one to be used without writing anything 
> into
>  /sys/power/mem_sleep) is either "deep" (if Suspend-To-RAM is supported) or
>  "s2idle", but it can be overridden by the value of the "mem_sleep_default"
> -parameter in the kernel command line.
> +parameter in the kernel command line.  On some ACPI-based systems, depending
> on
> +the information in the FADT, the default may be "s2idle" even if 
> Suspend-To-RAM
> +is supported.
> 
>  The properties of all of the sleep states are described below.
> 
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -714,6 +714,12 @@ static int lps0_device_attach(struct acp
>               if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
> ACPI_S2IDLE_FUNC_MASK) {
>                       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_FREEZE;
>               }
> 
>               acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
> Index: linux-pm/include/linux/suspend.h
> ===================================================================
> --- linux-pm.orig/include/linux/suspend.h
> +++ linux-pm/include/linux/suspend.h
> @@ -196,6 +196,9 @@ struct platform_freeze_ops {
>  };
> 
>  #ifdef CONFIG_SUSPEND
> +extern suspend_state_t mem_sleep_current;
> +extern suspend_state_t mem_sleep_default;
> +
>  /**
>   * suspend_set_ops - set platform dependent suspend operations
>   * @ops: The new suspend operations to set.
> Index: linux-pm/kernel/power/suspend.c
> ===================================================================
> --- linux-pm.orig/kernel/power/suspend.c
> +++ linux-pm/kernel/power/suspend.c
> @@ -48,7 +48,7 @@ static const char * const mem_sleep_labe
>  const char *mem_sleep_states[PM_SUSPEND_MAX];
> 
>  suspend_state_t mem_sleep_current = PM_SUSPEND_FREEZE;
> -static suspend_state_t mem_sleep_default = PM_SUSPEND_MEM;
> +suspend_state_t mem_sleep_default = PM_SUSPEND_MAX;
>  suspend_state_t pm_suspend_target_state;
>  EXPORT_SYMBOL_GPL(pm_suspend_target_state);
> 
> @@ -216,7 +216,7 @@ void suspend_set_ops(const struct platfo
>       }
>       if (valid_state(PM_SUSPEND_MEM)) {
>               mem_sleep_states[PM_SUSPEND_MEM] =
> mem_sleep_labels[PM_SUSPEND_MEM];
> -             if (mem_sleep_default == PM_SUSPEND_MEM)
> +             if (mem_sleep_default >= PM_SUSPEND_MEM)
>                       mem_sleep_current = PM_SUSPEND_MEM;
>       }
> 
> Index: linux-pm/kernel/power/power.h
> ===================================================================
> --- linux-pm.orig/kernel/power/power.h
> +++ linux-pm/kernel/power/power.h
> @@ -192,7 +192,6 @@ extern void swsusp_show_speed(ktime_t, k
>  extern const char * const pm_labels[];
>  extern const char *pm_states[];
>  extern const char *mem_sleep_states[];
> -extern suspend_state_t mem_sleep_current;
> 
>  extern int suspend_devices_and_enter(suspend_state_t state);
>  #else /* !CONFIG_SUSPEND */

Rafael,

This patch works for me on XPS 9365.

I ran into some problems with linux-pm.git/linux-next including
this patch though.  I spent a little time debugging and sent a follow 
up patch to intel-vbtn.  It's not strictly caused by this patch, but I 
just noticed it now with this default in place.

Tested-by: Mario Limonciello <mario.limoncie...@dell.com>

Thanks,

Reply via email to