From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753042AbdHDRD5 (ORCPT ); Fri, 4 Aug 2017 13:03:57 -0400 Received: from esa5.dell-outbound.iphmx.com ([68.232.153.95]:54440 "EHLO esa5.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752558AbdHDRDz (ORCPT ); Fri, 4 Aug 2017 13:03:55 -0400 From: X-LoopCount0: from 10.175.216.249 X-IronPort-AV: E=Sophos;i="5.41,321,1498539600"; d="scan'208";a="133921482" X-DLP: DLP_GlobalPCIDSS To: , CC: , , , Subject: RE: [PATCH] ACPI / PM: Prefer suspend-to-idle over S3 on some systems Thread-Topic: [PATCH] ACPI / PM: Prefer suspend-to-idle over S3 on some systems Thread-Index: AQHTCkcsG+JZAgCjsUuF8YiM9yPaEKJ0ZwjA Date: Fri, 4 Aug 2017 17:02:56 +0000 Message-ID: References: <3043895.8nuHiCWbDe@aspire.rjw.lan> In-Reply-To: <3043895.8nuHiCWbDe@aspire.rjw.lan> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.143.18.86] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v74H42jM021978 > -----Original Message----- > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] > Sent: Monday, July 31, 2017 4:43 PM > To: Linux ACPI > Cc: LKML ; Len Brown ; > Linux PM ; Srinivas Pandruvada > ; Limonciello, Mario > > Subject: [PATCH] ACPI / PM: Prefer suspend-to-idle over S3 on some systems > > From: Rafael J. Wysocki > > 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 > --- > > 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 Thanks,