On Tue, Jun 20, 2017 at 1:37 AM, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote: > Hi, Rafael > >> From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] On Behalf Of Rafael J. >> Wysocki >> Subject: [PATCH v2 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems >> >> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >> >> 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. In case the >> users would prefer not to do that after all, add a new kernel >> command line switch, acpi_sleep=no_ec_wakeup, to disable that new >> behavior. >> [cut] >> >> Index: linux-pm/drivers/acpi/sleep.c >> =================================================================== >> --- linux-pm.orig/drivers/acpi/sleep.c >> +++ linux-pm/drivers/acpi/sleep.c >> @@ -160,6 +160,23 @@ static int __init init_nvs_nosave(const >> return 0; >> } >> >> +/* If set, it is allowed to use the EC GPE to wake up the system. */ >> +static bool ec_gpe_wakeup_allowed __initdata = true; >> + >> +void __init acpi_disable_ec_gpe_wakeup(void) >> +{ >> + ec_gpe_wakeup_allowed = false; >> +} >> + >> +/* If set, the EC GPE will be configured to wake up the system. */ >> +static bool ec_gpe_wakeup; >> + >> +static int __init init_ec_gpe_wakeup(const struct dmi_system_id *d) >> +{ >> + ec_gpe_wakeup = ec_gpe_wakeup_allowed; >> + return 0; >> +} >> + >> static struct dmi_system_id acpisleep_dmi_table[] __initdata = { >> { >> .callback = init_old_suspend_ordering, >> @@ -343,6 +360,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"), >> + }, >> + }, >> {}, >> }; >> > > I have a concern here. > > ACPI spec has already defined a mechanism to statically > Mark GPEs as wake-capable and enable it, it is done via > _PRW. We may call it a "static wakeup GPE" mechanism. > > Now the problem might be on some platforms, _PRW cannot be > prepared unconditionally. And the platform designers wants > a "dynamic wakeup GPE" mechanism to dynamically > mark/enable GPEs as wakeup GPE after having done some > platform specific behaviors (ex., after/before > saving/restoring some firmware configurations). > > From this point of view, can we prepare several APIs in > sleep.c to allow dynamically mark/enable wakeup GPEs and > export EC information via a new API from ec.c, ex., > acpi_ec_get_attributes(), or just publish struct acpi_ec > and first_ec in acpi_ec.h to the other drivers. > So that all such kinds of platforms drivers can use both > interfaces to dynamically achieve this, which can help > to avoid introducing quirk tables here. I'm not sure how this is related to the patch. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
