Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1654722 > unrolled thread
| Started by | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| First post | 2017-06-01 01:40 +0200 |
| Last post | 2017-06-01 17:10 +0200 |
| Articles | 10 — 5 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-01 01:40 +0200
[PATCH 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-01 01:40 +0200
RE: [PATCH 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems <Mario.Limonciello@dell.com> - 2017-06-05 17:20 +0200
Re: [PATCH 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems "Rafael J. Wysocki" <rafael@kernel.org> - 2017-06-05 23:00 +0200
Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-06-01 12:50 +0200
Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops Tom Lanyon <tom@oneshoeco.com> - 2017-06-01 14:00 +0200
Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops "Rafael J. Wysocki" <rafael@kernel.org> - 2017-06-01 17:10 +0200
Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops Tom Lanyon <tom@oneshoeco.com> - 2017-06-02 03:10 +0200
Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-03 01:30 +0200
Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops "Rafael J. Wysocki" <rafael@kernel.org> - 2017-06-01 17:10 +0200
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2017-06-01 01:40 +0200 |
| Subject | [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops |
| Message-ID | <tNlnz-3cu-3@gated-at.bofh.it> |
Hi All, This is a follow-up for a patch series posted some time ago: http://marc.info/?l=linux-kernel&m=149324246701378&w=2 The first two patches from that series are in 4.12-rc already and the rest have been rearranged. The issue at hand is still the same as before: On Wednesday, April 26, 2017 11:21:11 PM Rafael J. Wysocki wrote: > > 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 first two patches in the current series 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 third patch allows the EC GPE to become a wakeup GPE on the affected Dell laptops and finally makes power button events wake up those systems from suspend-to-idle. After this series there still is a concern regarding the possible increase of power draw that may result from the processing of non-wakeup EC events while suspended which is why the change only affects Dell XPS13 9360 and 9365 for now. There is no code dependency between patches [1-2/3] and patch [3/3], but all of them together are necessary for the feature in question to work on both the affected systems, so IMO they should be applied together. 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. If there are any concerns regarding this series, please let me know. Thanks, Rafael
[toc] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2017-06-01 01:40 +0200 |
| Subject | [PATCH 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems |
| Message-ID | <tNlnz-3cu-13@gated-at.bofh.it> |
| In reply to | #1654722 |
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 less usable on
those systems. Moreover, on the 9365 ACPI S3 (suspend-to-RAM) is
not expected to be used at all (the OS these systems ship with 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 effectively would defeat
its purpose.
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. For this reason, add a DMI
switch to the ACPI system suspend infrastructure to treat the EC
GPE as a wakeup one on the affected Dell systems.
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 | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 54 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,14 @@ static int __init init_nvs_nosave(const
return 0;
}
+static bool ec_gpe_wakeup;
+
+static int __init init_ec_gpe_wakeup(const struct dmi_system_id *d)
+{
+ ec_gpe_wakeup = true;
+ return 0;
+}
+
static struct dmi_system_id acpisleep_dmi_table[] __initdata = {
{
.callback = init_old_suspend_ordering,
@@ -343,6 +351,26 @@ static struct dmi_system_id acpisleep_dm
DMI_MATCH(DMI_PRODUCT_NAME, "80E3"),
},
},
+ /*
+ * Enable the EC to wake up the system from suspend-to-idle to allow
+ * power button events to it wake up.
+ */
+ {
+ .callback = init_ec_gpe_wakeup,
+ .ident = "Dell XPS 13 9360",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9360"),
+ },
+ },
+ {
+ .callback = init_ec_gpe_wakeup,
+ .ident = "Dell XPS 13 9365",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9365"),
+ },
+ },
{},
};
@@ -485,6 +513,7 @@ static void acpi_pm_end(void)
}
#else /* !CONFIG_ACPI_SLEEP */
#define acpi_target_sleep_state ACPI_STATE_S0
+#define ec_gpe_wakeup false
static inline void acpi_sleep_dmi_check(void) {}
#endif /* CONFIG_ACPI_SLEEP */
@@ -731,6 +760,11 @@ static void acpi_sleep_suspend_setup(voi
static inline void acpi_sleep_suspend_setup(void) {}
#endif /* !CONFIG_SUSPEND */
+bool acpi_sleep_ec_gpe_may_wakeup(void)
+{
+ return ec_gpe_wakeup;
+}
+
#ifdef CONFIG_PM_SLEEP
static u32 saved_bm_rld;
[toc] | [prev] | [next] | [standalone]
| From | <Mario.Limonciello@dell.com> |
|---|---|
| Date | 2017-06-05 17:20 +0200 |
| Subject | RE: [PATCH 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems |
| Message-ID | <tP1Xr-4zX-5@gated-at.bofh.it> |
| In reply to | #1654724 |
> + /*
> + * Enable the EC to wake up the system from suspend-to-idle to allow
> + * power button events to it wake up.
> + */
> + {
> + .callback = init_ec_gpe_wakeup,
> + .ident = "Dell XPS 13 9360",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9360"),
> + },
> + },
> + {
> + .callback = init_ec_gpe_wakeup,
> + .ident = "Dell XPS 13 9365",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9365"),
> + },
> + },
> {},
> };
>
Although the XPS 9360 and XPS 9365 are currently the only Dell products on
the market that do the wakeup this way, this is part of Dell's BIOS and EC
codebase. As other Dell products start to ship Windows 10 with Modern Standby
and (eventually) Linux with Suspend-to-Idle I'd expect this list to be growing.
I know that we talked about this previously in relation to the EC notification
of going in to suspend-to-idle and you had instead marked it across all Dell Inc systems.
Rather than hardcode a whitelist of platform that can do this, could you do the
same thing here and allow EC wakeups across Dell Inc. systems?
Thanks,
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2017-06-05 23:00 +0200 |
| Subject | Re: [PATCH 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems |
| Message-ID | <tP7gt-7Lh-3@gated-at.bofh.it> |
| In reply to | #1657655 |
On Mon, Jun 5, 2017 at 5:18 PM, <Mario.Limonciello@dell.com> wrote:
>> + /*
>> + * Enable the EC to wake up the system from suspend-to-idle to allow
>> + * power button events to it wake up.
>> + */
>> + {
>> + .callback = init_ec_gpe_wakeup,
>> + .ident = "Dell XPS 13 9360",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9360"),
>> + },
>> + },
>> + {
>> + .callback = init_ec_gpe_wakeup,
>> + .ident = "Dell XPS 13 9365",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9365"),
>> + },
>> + },
>> {},
>> };
>>
>
> Although the XPS 9360 and XPS 9365 are currently the only Dell products on
> the market that do the wakeup this way, this is part of Dell's BIOS and EC
> codebase. As other Dell products start to ship Windows 10 with Modern Standby
> and (eventually) Linux with Suspend-to-Idle I'd expect this list to be growing.
Well, if init_ec_gpe_wakeup() was enabled on all Dell systems at this
point, it would affect all previous generations of them too and that
might not be desirable.
Thanks,
Rafael
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andriy.shevchenko@linux.intel.com> |
|---|---|
| Date | 2017-06-01 12:50 +0200 |
| Subject | Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops |
| Message-ID | <tNvPX-1sb-5@gated-at.bofh.it> |
| In reply to | #1654722 |
On Thu, 2017-06-01 at 01:23 +0200, Rafael J. Wysocki wrote: > Hi All, > > This is a follow-up for a patch series posted some time ago: > > http://marc.info/?l=linux-kernel&m=149324246701378&w=2 > > The first two patches from that series are in 4.12-rc already and the > rest > have been rearranged. > ... > The first two patches in the current series 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. ... > There is no code dependency between patches [1-2/3] and patch [3/3], > but all > of them together are necessary for the feature in question to work on > both the > affected systems, so IMO they should be applied together. > > 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. > > If there are any concerns regarding this series, please let me know. Is this supposed to go via linux PM tree? I'm fine with the first two: Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
[toc] | [prev] | [next] | [standalone]
| From | Tom Lanyon <tom@oneshoeco.com> |
|---|---|
| Date | 2017-06-01 14:00 +0200 |
| Subject | Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops |
| Message-ID | <tNwVI-24Q-19@gated-at.bofh.it> |
| In reply to | #1654998 |
[resend as text/plain] On Thu, 2017-06-01 at 01:23 +0200, Rafael J. Wysocki wrote: > Hi All, > > This is a follow-up for a patch series posted some time ago: > > http://marc.info/?l=linux-kernel&m=149324246701378&w=2 I've applied Rafael's s2idle-dell-test branch to 4.12.0-rc3 and tested on a Dell 9365 and, whilst it's significantly improved, it's not yet working correctly. Previously I could suspend (s2idle and deep), but it took an awkward ~8 second press of the power button to get it to resume, and I could never get resume to work when triggering suspend/resume via close/open of the lid. With this patchset applied, I can suspend (s2idle) and a momentary press of the power button resumes successfully. I can also use the lid switch to both suspend and resume successfully. However, the EC events appear to trigger the machine to wake very frequently whilst it's supposed to be suspended. This is visible via the kernel messages at the end of this mail (leaving it in a suspended state for a few hours resulted in many thousands of these messages), and the high power draw witnessed. Let me know if there's anything I can do to help debug further. Tom [ 43.669798] PM: Syncing filesystems ... done. [ 43.675412] PM: Preparing system for sleep (freeze) [ 43.695469] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 43.696769] OOM killer disabled. [ 43.696770] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 43.697914] PM: Suspending system (freeze) [ 43.910325] wlan0: deauthenticating from 9c:1c:12:c7:c2:f1 by local choice (Reason: 3=DEAUTH_LEAVING) [ 44.112944] psmouse serio1: Failed to disable mouse on isa0060/serio1 [ 45.721790] PM: suspend of devices complete after 1811.701 msecs [ 45.739769] PM: late suspend of devices complete after 17.956 msecs [ 45.742599] ACPI : EC: interrupt blocked [ 45.744774] xhci_hcd 0000:00:14.0: System wakeup enabled by ACPI [ 45.776254] PM: noirq suspend of devices complete after 34.702 msecs [ 45.776256] PM: suspend-to-idle [ 47.029237] Suspended for 0.327 seconds [ 47.029335] PM: resume from suspend-to-idle [ 47.029587] ACPI : EC: interrupt unblocked [ 47.064470] xhci_hcd 0000:00:14.0: System wakeup disabled by ACPI [ 47.065268] PM: noirq resume of devices complete after 35.894 msecs [ 47.066264] ACPI : EC: interrupt blocked [ 47.079210] xhci_hcd 0000:00:14.0: System wakeup enabled by ACPI [ 47.112461] PM: noirq suspend of devices complete after 47.019 msecs [ 47.112538] ACPI : EC: interrupt unblocked [ 47.147672] xhci_hcd 0000:00:14.0: System wakeup disabled by ACPI [ 47.148516] PM: noirq resume of devices complete after 36.051 msecs [ 47.149610] ACPI : EC: interrupt blocked [ 47.161002] xhci_hcd 0000:00:14.0: System wakeup enabled by ACPI [ 47.192546] PM: noirq suspend of devices complete after 43.955 msecs [ 47.192551] PM: suspend-to-idle [ 48.374756] Suspended for 1.836 seconds [ 48.374870] PM: resume from suspend-to-idle [ 48.375284] ACPI : EC: interrupt unblocked
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2017-06-01 17:10 +0200 |
| Subject | Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops |
| Message-ID | <tNzTz-491-9@gated-at.bofh.it> |
| In reply to | #1655069 |
Hi, On Thu, Jun 1, 2017 at 1:50 PM, Tom Lanyon <tom@oneshoeco.com> wrote: > [resend as text/plain] > > On Thu, 2017-06-01 at 01:23 +0200, Rafael J. Wysocki wrote: >> Hi All, >> >> This is a follow-up for a patch series posted some time ago: >> >> http://marc.info/?l=linux-kernel&m=149324246701378&w=2 > > I've applied Rafael's s2idle-dell-test branch to 4.12.0-rc3 and tested > on a Dell 9365 and, whilst it's significantly improved, it's not yet > working correctly. > > Previously I could suspend (s2idle and deep), but it took an awkward > ~8 second press of the power button to get it to resume, and I could > never get resume to work when triggering suspend/resume via close/open > of the lid. > > With this patchset applied, I can suspend (s2idle) and a momentary > press of the power button resumes successfully. I can also use the > lid switch to both suspend and resume successfully. > > However, the EC events appear to trigger the machine to wake very > frequently whilst it's supposed to be suspended. This is visible via > the kernel messages at the end of this mail (leaving it in a suspended > state for a few hours resulted in many thousands of these messages), > and the high power draw witnessed. > > Let me know if there's anything I can do to help debug further. Quoting from my cover letter: "After this series there still is a concern regarding the possible increase of power draw that may result from the processing of non-wakeup EC events while suspended which is why the change only affects Dell XPS13 9360 and 9365 for now." So that is what happens, unfortunately, and we can't do much about it at the moment. The only way to avoid that would be to reconfigure the EC during suspend to stop generating non-wakeup events, but today we have no reliable way to do that. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | Tom Lanyon <tom@oneshoeco.com> |
|---|---|
| Date | 2017-06-02 03:10 +0200 |
| Subject | Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops |
| Message-ID | <tNJgd-26p-1@gated-at.bofh.it> |
| In reply to | #1655231 |
On 2 June 2017 at 00:59, Rafael J. Wysocki <rafael@kernel.org> wrote: > > Quoting from my cover letter: > > "After this series there still is a concern regarding the possible increase of > power draw that may result from the processing of non-wakeup EC events while > suspended which is why the change only affects Dell XPS13 9360 and 9365 > for now." > > So that is what happens, unfortunately, and we can't do much about it > at the moment. OK, but at the moment this is a regression in functionality on those platforms. Without this patchset, I can successfully s2idle suspend/resume on an XPS 9365 (albeit with a little bit of awkward fiddling of the power button to resume). After the patchset, I can't realistically go into s2idle at all. > The only way to avoid that would be to reconfigure the EC during > suspend to stop generating non-wakeup events, but today we have no > reliable way to do that. I thought I had read one one of the threads that this was possible in the same way that it is for Windows on these laptops. What's missing to make this possible? Tom
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2017-06-03 01:30 +0200 |
| Message-ID | <tO4aZ-7FZ-3@gated-at.bofh.it> |
| In reply to | #1655827 |
On Friday, June 02, 2017 11:06:49 AM Tom Lanyon wrote: > On 2 June 2017 at 00:59, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > Quoting from my cover letter: > > > > "After this series there still is a concern regarding the possible increase of > > power draw that may result from the processing of non-wakeup EC events while > > suspended which is why the change only affects Dell XPS13 9360 and 9365 > > for now." > > > > So that is what happens, unfortunately, and we can't do much about it > > at the moment. > > OK, but at the moment this is a regression in functionality on those > platforms. Without this patchset, I can successfully s2idle > suspend/resume on an XPS 9365 (albeit with a little bit of awkward > fiddling of the power button to resume). After the patchset, I can't > realistically go into s2idle at all. Well, if you think about s2idle as a state in which there's no activity in the system at all, this is not how it is defined. In fact, if there is a non-wakeup edge-triggered interrupt occurring while suspended, for example, it will cause the IRQ core to run a low-level handler for it even though the action handler(s) won't be run, which is far from "no activity". s2idle basically is a state in which all system components are (or at least should be) in low-power states most of the time and user space has been frozen and patch [3/3] in this series doesn't change that really, so I really wouldn't call it a "regression in functionality". [I guess the messages in the log are somewhat confusing, but see below.] Of course, it does increase the amount of activity in the system while suspended which in turn causes more energy to be used and battery life to shorten, so it may be regarded as a power regression. Still, it really is a tradeoff between functionality (power button wakeups working as expected) and energy-efficiency and I know about a few people who actually prefer the functionality to be there. On top of that, there are a few things that can be optimized slightly. For instance, we generally run too much code when those EC wakeups happen, we print too much to the log (even without debug enabled) etc, which also should reduce the amout of energy that goes away and I have some patches going in that direction. Moreover, the messages printed to the logs are not quite as accurate as they should be which needs to be fixed too. What really matters is how much more energy the system uses after that patch (or how much less time it will stay on battery) while suspended. Can you please try to estimate that for your system? In any case, you have a point that there may be users wanting the systems to use less energy while suspended to idle at the cost of semi-functional power button wakeups, so I guess I'll add an acpi_sleep= kernel command line switch to force-disable EC wakeups even on systems where they would be enabled by default. > > The only way to avoid that would be to reconfigure the EC during > > suspend to stop generating non-wakeup events, but today we have no > > reliable way to do that. > > I thought I had read one one of the threads that this was possible in > the same way that it is for Windows on these laptops. What's missing > to make this possible? Let's say there is work in progress to make it possible to use that interface in Linux, but it may not actually do everything we want it to do, so it may not help much here on this particular laptop model. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2017-06-01 17:10 +0200 |
| Subject | Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on recent Dell laptops |
| Message-ID | <tNzTz-491-11@gated-at.bofh.it> |
| In reply to | #1654998 |
On Thu, Jun 1, 2017 at 12:43 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Thu, 2017-06-01 at 01:23 +0200, Rafael J. Wysocki wrote: >> Hi All, >> >> This is a follow-up for a patch series posted some time ago: >> >> http://marc.info/?l=linux-kernel&m=149324246701378&w=2 >> >> The first two patches from that series are in 4.12-rc already and the >> rest >> have been rearranged. >> > > ... > >> The first two patches in the current series 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. > > ... > >> There is no code dependency between patches [1-2/3] and patch [3/3], >> but all >> of them together are necessary for the feature in question to work on >> both the >> affected systems, so IMO they should be applied together. >> >> 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. >> >> If there are any concerns regarding this series, please let me know. > > Is this supposed to go via linux PM tree? Yes, it is. > I'm fine with the first two: > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks!
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web