Hi, > From: rjwysocki@xxxxxxxxx [mailto:rjwysocki@xxxxxxxxx] On Behalf Of Rafael J. Wysocki > Subject: Re: [PATCH v2 3/3] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent Dell systems > > 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. Sorry, I was thinking this is still related to uPEP. Best regards Lv ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
