From 3a79bc63d90750f737ab9d7219bd3091d2fd6d84 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 13:03:20 +0200 Subject: [PATCH 01/21] PCI: irq: Introduce rearm_wake_irq() Introduce a new function, rearm_wake_irq(), allowing a wakeup IRQ to be armed for systen wakeup detection again without running any action handlers associated with it after it has been armed for wakeup detection and triggered. That is useful for IRQs, like ACPI SCI, that may deliver wakeup as well as non-wakeup interrupts when armed for systen wakeup detection. In those cases, it may be possible to determine whether or not the delivered interrupt is a systen wakeup one without running the entire action handler (or handlers, if the IRQ is shared) for the IRQ, and if the interrupt turns out to be a non-wakeup one, the IRQ can be rearmed with the help of the new function. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- include/linux/interrupt.h | 1 + kernel/irq/pm.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index 5b8328a99b2a..0e9cdb3efda7 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -238,6 +238,7 @@ extern void teardown_percpu_nmi(unsigned int irq); /* The following three functions are for the core kernel use only. */ extern void suspend_device_irqs(void); extern void resume_device_irqs(void); +extern void rearm_wake_irq(unsigned int irq); /** * struct irq_affinity_notify - context for notification of IRQ affinity changes diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c index d6961d3c6f9e..8f557fa1f4fe 100644 --- a/kernel/irq/pm.c +++ b/kernel/irq/pm.c @@ -176,6 +176,26 @@ static void resume_irqs(bool want_early) } } +/** + * rearm_wake_irq - rearm a wakeup interrupt line after signaling wakeup + * @irq: Interrupt to rearm + */ +void rearm_wake_irq(unsigned int irq) +{ + unsigned long flags; + struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL); + + if (!desc || !(desc->istate & IRQS_SUSPENDED) || + !irqd_is_wakeup_set(&desc->irq_data)) + return; + + desc->istate &= ~IRQS_SUSPENDED; + irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED); + __enable_irq(desc); + + irq_put_desc_busunlock(desc, flags); +} + /** * irq_pm_syscore_ops - enable interrupt lines early * From 6921de898ba8f2ec91cfea70e7160b89c477382e Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 13:03:28 +0200 Subject: [PATCH 02/21] ACPICA: Return u32 from acpi_dispatch_gpe() In some cases it is useful to know whether or not the acpi_ev_detect_gpe() called by acpi_dispatch_gpe() has found the GPE to be active, so return the return value of it (whose data type is u32) from latter. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- drivers/acpi/acpica/evxfgpe.c | 6 +++--- include/acpi/acpixf.h | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c index 710488ec59e9..04a40d563dd6 100644 --- a/drivers/acpi/acpica/evxfgpe.c +++ b/drivers/acpi/acpica/evxfgpe.c @@ -644,17 +644,17 @@ ACPI_EXPORT_SYMBOL(acpi_get_gpe_status) * PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1 * gpe_number - GPE level within the GPE block * - * RETURN: None + * RETURN: INTERRUPT_HANDLED or INTERRUPT_NOT_HANDLED * * DESCRIPTION: Detect and dispatch a General Purpose Event to either a function * (e.g. EC) or method (e.g. _Lxx/_Exx) handler. * ******************************************************************************/ -void acpi_dispatch_gpe(acpi_handle gpe_device, u32 gpe_number) +u32 acpi_dispatch_gpe(acpi_handle gpe_device, u32 gpe_number) { ACPI_FUNCTION_TRACE(acpi_dispatch_gpe); - acpi_ev_detect_gpe(gpe_device, NULL, gpe_number); + return acpi_ev_detect_gpe(gpe_device, NULL, gpe_number); } ACPI_EXPORT_SYMBOL(acpi_dispatch_gpe) diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 3845c8fcc94e..4ed603a3b448 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -297,6 +297,9 @@ ACPI_GLOBAL(u8, acpi_gbl_system_awake_and_running); #define ACPI_HW_DEPENDENT_RETURN_OK(prototype) \ ACPI_EXTERNAL_RETURN_OK(prototype) +#define ACPI_HW_DEPENDENT_RETURN_UINT32(prototype) \ + ACPI_EXTERNAL_RETURN_UINT32(prototype) + #define ACPI_HW_DEPENDENT_RETURN_VOID(prototype) \ ACPI_EXTERNAL_RETURN_VOID(prototype) @@ -307,6 +310,9 @@ ACPI_GLOBAL(u8, acpi_gbl_system_awake_and_running); #define ACPI_HW_DEPENDENT_RETURN_OK(prototype) \ static ACPI_INLINE prototype {return(AE_OK);} +#define ACPI_HW_DEPENDENT_RETURN_UINT32(prototype) \ + static ACPI_INLINE prototype {return(0);} + #define ACPI_HW_DEPENDENT_RETURN_VOID(prototype) \ static ACPI_INLINE prototype {return;} @@ -738,7 +744,7 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status u32 gpe_number, acpi_event_status *event_status)) -ACPI_HW_DEPENDENT_RETURN_VOID(void acpi_dispatch_gpe(acpi_handle gpe_device, u32 gpe_number)) +ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi_dispatch_gpe(acpi_handle gpe_device, u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_disable_all_gpes(void)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_enable_all_runtime_gpes(void)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_enable_all_wakeup_gpes(void)) From 9089f16e053afc5e18feaeb9f64cc7c90d6bd687 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 13:03:39 +0200 Subject: [PATCH 03/21] ACPI: EC: Return bool from acpi_ec_dispatch_gpe() On some systems, if suspend-to-idle is used, the EC may signal system wakeup events (power button events, for example) as well as events that should not cause the system to resume and acpi_ec_dispatch_gpe() needs to be called to determine whether or not the system should resume then. In particular, if acpi_ec_dispatch_gpe() doesn't detect any EC events at all, the system should remain suspended, so it is useful to know when that is the case. For this reason, make acpi_ec_dispatch_gpe() return a bool value indicating whether or not any EC events have been detected by it. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- drivers/acpi/ec.c | 11 ++++++++--- drivers/acpi/internal.h | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index c33756ed3304..58c7ad402d8d 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1060,10 +1060,15 @@ void acpi_ec_set_gpe_wake_mask(u8 action) acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action); } -void acpi_ec_dispatch_gpe(void) +bool acpi_ec_dispatch_gpe(void) { - if (first_ec) - acpi_dispatch_gpe(NULL, first_ec->gpe); + u32 ret; + + if (!first_ec) + return false; + + ret = acpi_dispatch_gpe(NULL, first_ec->gpe); + return ret == ACPI_INTERRUPT_HANDLED; } /* -------------------------------------------------------------------------- diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index f4c2fe6be4f2..1b5f9ac06ea8 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -196,7 +196,7 @@ void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void); void acpi_ec_mark_gpe_for_wake(void); void acpi_ec_set_gpe_wake_mask(u8 action); -void acpi_ec_dispatch_gpe(void); +bool acpi_ec_dispatch_gpe(void); int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle, acpi_ec_query_func func, void *data); From 2933954b71f10d392764f95eec0f0aa2d103054b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 13:03:48 +0200 Subject: [PATCH 04/21] PM: sleep: Fix possible overflow in pm_system_cancel_wakeup() It is not actually guaranteed that pm_abort_suspend will be nonzero when pm_system_cancel_wakeup() is called which may lead to subtle issues, so make it use atomic_dec_if_positive() instead of atomic_dec() for the safety sake. Fixes: 33e4f80ee69b ("ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle") Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- drivers/base/power/wakeup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index ee31d4f8d856..b30c45aad10f 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -859,7 +859,7 @@ EXPORT_SYMBOL_GPL(pm_system_wakeup); void pm_system_cancel_wakeup(void) { - atomic_dec(&pm_abort_suspend); + atomic_dec_if_positive(&pm_abort_suspend); } void pm_wakeup_clear(bool reset) From 41275eb5c7181febdfaa63c3a0ad9b7acdadcd52 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 23:51:19 +0200 Subject: [PATCH 05/21] ACPI: PM: Set s2idle_wakeup earlier and clear it later The role of the s2idle_wakeup variable is to cause acpi_pm_wakeup_event() and acpi_pm_notify_handler() to increment pm_abort_suspend and trigger a wakeup from suspend-to-idle in case the ACPI SCI wakeup was canceled by acpi_s2idle_wake(). However, for this purpose it need not be set in acpi_s2idle_wake() and cleared in acpi_s2idle_sync(), respectively. In fact, it may be set as early as in acpi_s2idle_prepare() and cleared as late as in acpi_s2idle_restore(), so do that to allow subsequent changes to be simpler. This change is not expected to alter functionality. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- drivers/acpi/sleep.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index f0fe7c15d657..3debe1a42655 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -972,6 +972,8 @@ static int acpi_s2idle_prepare(void) /* Change the configuration of GPEs to avoid spurious wakeup. */ acpi_enable_all_wakeup_gpes(); acpi_os_wait_events_complete(); + + s2idle_wakeup = true; return 0; } @@ -991,7 +993,6 @@ static void acpi_s2idle_wake(void) if (acpi_sci_irq_valid() && !irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) { pm_system_cancel_wakeup(); - s2idle_wakeup = true; /* * On some platforms with the LPS0 _DSM device noirq resume * takes too much time for EC wakeup events to survive, so look @@ -1012,11 +1013,12 @@ static void acpi_s2idle_sync(void) acpi_os_wait_events_complete(); /* synchronize SCI IRQ handling */ acpi_ec_flush_work(); acpi_os_wait_events_complete(); /* synchronize Notify handling */ - s2idle_wakeup = false; } static void acpi_s2idle_restore(void) { + s2idle_wakeup = false; + acpi_enable_all_runtime_gpes(); acpi_disable_wakeup_devices(ACPI_STATE_S0); From 56b991849009f5def0443bfb2f48c8321d888e15 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 23:52:03 +0200 Subject: [PATCH 06/21] PM: sleep: Simplify suspend-to-idle control flow After commit 33e4f80ee69b ("ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle") the "noirq" phases of device suspend and resume may run for multiple times during suspend-to-idle, if there are spurious system wakeup events while suspended. However, this is complicated and fragile and actually unnecessary. The main reason for doing this is that on some systems the EC may signal system wakeup events (power button events, for example) as well as events that should not cause the system to resume (spurious system wakeup events). Thus, in order to determine whether or not a given event signaled by the EC while suspended is a proper system wakeup one, the EC GPE needs to be dispatched and to start with that was achieved by allowing the ACPI SCI action handler to run, which was only possible after calling resume_device_irqs(). However, dispatching the EC GPE this way turned out to take too much time in some cases and some EC events might be missed due to that, so commit 68e22011856f ("ACPI: EC: Dispatch the EC GPE directly on s2idle wake") started to dispatch the EC GPE right after a wakeup event has been detected, so in fact the full ACPI SCI action handler doesn't need to run any more to deal with the wakeups coming from the EC. Use this observation to simplify the suspend-to-idle control flow so that the "noirq" phases of device suspend and resume are each run only once in every suspend-to-idle cycle, which is reported to significantly reduce power drawn by some systems when suspended to idle (by allowing them to reach a deep platform-wide low-power state through the suspend-to-idle flow). [What appears to happen is that the "noirq" resume of devices after a spurious EC wakeup brings some devices into a state in which they prevent the platform from reaching the deep low-power state going forward, even after a subsequent "noirq" suspend phase, and on some systems the EC triggers such wakeups already when the "noirq" suspend of devices is running for the first time in the given suspend/resume cycle, so the platform cannot reach the deep low-power state at all.] First, make acpi_s2idle_wake() use the acpi_ec_dispatch_gpe() return value to determine whether or not the wakeup may have been triggered by the EC (in which case the system wakeup is canceled and ACPI events are processed in order to determine whether or not the event is a proper system wakeup one) and use rearm_wake_irq() (introduced by a previous change) in it to rearm the ACPI SCI for system wakeup detection in case the system will remain suspended. Second, drop acpi_s2idle_sync(), which is not needed any more, and the corresponding global platform suspend-to-idle callback. Next, drop the pm_wakeup_pending() check (which is an optimization only) from __device_suspend_noirq() to prevent it from returning errors on system wakeups occurring before the "noirq" phase of device suspend is complete (as in the case of suspend-to-idle it is not known whether or not these wakeups are suprious at that point), in order to avoid having to carry out a "noirq" resume of devices on a spurious system wakeup. Finally, change the code flow in s2idle_loop() to (1) run the "noirq" suspend of devices once before starting the loop, (2) check for spurious EC wakeups (via the platform ->wake callback) for the first time before calling s2idle_enter(), and (3) run the "noirq" resume of devices once after leaving the loop. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- drivers/acpi/sleep.c | 47 ++++++++++++++++++---------------- drivers/base/power/main.c | 5 ---- include/linux/suspend.h | 1 - kernel/power/suspend.c | 53 +++++++++++++++++---------------------- 4 files changed, 48 insertions(+), 58 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 3debe1a42655..970ae7c7a3f7 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -986,33 +986,37 @@ static void acpi_s2idle_wake(void) lpi_check_constraints(); /* - * If IRQD_WAKEUP_ARMED is not set for the SCI at this point, it means - * that the SCI has triggered while suspended, so cancel the wakeup in - * case it has not been a wakeup event (the GPEs will be checked later). + * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the SCI has + * not triggered while suspended, so bail out. */ - if (acpi_sci_irq_valid() && - !irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) { + if (!acpi_sci_irq_valid() || + irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) + return; + + /* + * If there are EC events to process, the wakeup may be a spurious one + * coming from the EC. + */ + if (acpi_ec_dispatch_gpe()) { + /* + * Cancel the wakeup and process all pending events in case + * there are any wakeup ones in there. + * + * Note that if any non-EC GPEs are active at this point, the + * SCI will retrigger after the rearming below, so no events + * should be missed by canceling the wakeup here. + */ pm_system_cancel_wakeup(); /* - * On some platforms with the LPS0 _DSM device noirq resume - * takes too much time for EC wakeup events to survive, so look - * for them now. + * The EC driver uses the system workqueue and an additional + * special one, so those need to be flushed too. */ - acpi_ec_dispatch_gpe(); + acpi_os_wait_events_complete(); /* synchronize EC GPE processing */ + acpi_ec_flush_work(); + acpi_os_wait_events_complete(); /* synchronize Notify handling */ } -} -static void acpi_s2idle_sync(void) -{ - /* - * Process all pending events in case there are any wakeup ones. - * - * The EC driver uses the system workqueue and an additional special - * one, so those need to be flushed too. - */ - acpi_os_wait_events_complete(); /* synchronize SCI IRQ handling */ - acpi_ec_flush_work(); - acpi_os_wait_events_complete(); /* synchronize Notify handling */ + rearm_wake_irq(acpi_sci_irq); } static void acpi_s2idle_restore(void) @@ -1044,7 +1048,6 @@ static const struct platform_s2idle_ops acpi_s2idle_ops = { .begin = acpi_s2idle_begin, .prepare = acpi_s2idle_prepare, .wake = acpi_s2idle_wake, - .sync = acpi_s2idle_sync, .restore = acpi_s2idle_restore, .end = acpi_s2idle_end, }; diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 7fb2c39bc725..f08332fab531 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -1291,11 +1291,6 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a if (async_error) goto Complete; - if (pm_wakeup_pending()) { - async_error = -EBUSY; - goto Complete; - } - if (dev->power.syscore || dev->power.direct_complete) goto Complete; diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 9c0ad1a3a727..66ce3871ed61 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -191,7 +191,6 @@ struct platform_s2idle_ops { int (*begin)(void); int (*prepare)(void); void (*wake)(void); - void (*sync)(void); void (*restore)(void); void (*end)(void); }; diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index c874a7026e24..907b2be0372f 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -119,48 +119,41 @@ static void s2idle_enter(void) static void s2idle_loop(void) { + int error; + + dpm_noirq_begin(); + error = dpm_noirq_suspend_devices(PMSG_SUSPEND); + if (error) + goto resume; + pm_pr_dbg("suspend-to-idle\n"); + /* + * Suspend-to-idle equals: + * frozen processes + suspended devices + idle processors. + * Thus s2idle_enter() should be called right after all devices have + * been suspended. + * + * Wakeups during the noirq suspend of devices may be spurious, so try + * to avoid them upfront. + */ for (;;) { - int error; - - dpm_noirq_begin(); - - /* - * Suspend-to-idle equals - * frozen processes + suspended devices + idle processors. - * Thus s2idle_enter() should be called right after - * all devices have been suspended. - * - * Wakeups during the noirq suspend of devices may be spurious, - * so prevent them from terminating the loop right away. - */ - error = dpm_noirq_suspend_devices(PMSG_SUSPEND); - if (!error) - s2idle_enter(); - else if (error == -EBUSY && pm_wakeup_pending()) - error = 0; - - if (!error && s2idle_ops && s2idle_ops->wake) + if (s2idle_ops && s2idle_ops->wake) s2idle_ops->wake(); - dpm_noirq_resume_devices(PMSG_RESUME); - - dpm_noirq_end(); - - if (error) - break; - - if (s2idle_ops && s2idle_ops->sync) - s2idle_ops->sync(); - if (pm_wakeup_pending()) break; pm_wakeup_clear(false); + + s2idle_enter(); } pm_pr_dbg("resume from suspend-to-idle\n"); + +resume: + dpm_noirq_resume_devices(PMSG_RESUME); + dpm_noirq_end(); } void s2idle_wake(void) From 8eb0fd3b55f084320ae511cd5a64d356cf497c83 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 23:52:12 +0200 Subject: [PATCH 07/21] PM: sleep: Integrate suspend-to-idle with generig suspend flow After previous changes the suspend-to-idle code flow can be integrated more tightly with the generic system suspend code flow by making suspend_enter() call s2idle_loop() later and removing the direct invocations of dpm_noirq_begin(), dpm_noirq_suspend_devices(), dpm_noirq_end(), and dpm_noirq_resume_devices() from the latter, so do that. This change is not expected to alter functionality. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- kernel/power/suspend.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index 907b2be0372f..2b6057853b33 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -119,13 +119,6 @@ static void s2idle_enter(void) static void s2idle_loop(void) { - int error; - - dpm_noirq_begin(); - error = dpm_noirq_suspend_devices(PMSG_SUSPEND); - if (error) - goto resume; - pm_pr_dbg("suspend-to-idle\n"); /* @@ -150,10 +143,6 @@ static void s2idle_loop(void) } pm_pr_dbg("resume from suspend-to-idle\n"); - -resume: - dpm_noirq_resume_devices(PMSG_RESUME); - dpm_noirq_end(); } void s2idle_wake(void) @@ -408,11 +397,6 @@ static int suspend_enter(suspend_state_t state, bool *wakeup) if (error) goto Devices_early_resume; - if (state == PM_SUSPEND_TO_IDLE && pm_test_level != TEST_PLATFORM) { - s2idle_loop(); - goto Platform_early_resume; - } - error = dpm_suspend_noirq(PMSG_SUSPEND); if (error) { pr_err("noirq suspend of devices failed\n"); @@ -425,6 +409,11 @@ static int suspend_enter(suspend_state_t state, bool *wakeup) if (suspend_test(TEST_PLATFORM)) goto Platform_wake; + if (state == PM_SUSPEND_TO_IDLE) { + s2idle_loop(); + goto Platform_wake; + } + error = suspend_disable_secondary_cpus(); if (error || suspend_test(TEST_CPUS)) goto Enable_cpus; From b605c44c30b59990e806f930c37bd288b9d901a5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 15 Jul 2019 23:52:18 +0200 Subject: [PATCH 08/21] PM: sleep: Drop dpm_noirq_begin() and dpm_noirq_end() Note that after previous changes dpm_noirq_begin() and dpm_noirq_end() each have only one caller, so move the code from them to their respective callers and drop them. Also note that dpm_noirq_resume_devices() and dpm_noirq_suspend_devices() need not be exported any more, so make them both static. This change is not expected to alter functionality. Signed-off-by: Rafael J. Wysocki Acked-by: Thomas Gleixner --- drivers/base/power/main.c | 30 ++++++++++++------------------ include/linux/pm.h | 4 ---- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index f08332fab531..134a8af51511 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -716,7 +716,7 @@ static void async_resume_noirq(void *data, async_cookie_t cookie) put_device(dev); } -void dpm_noirq_resume_devices(pm_message_t state) +static void dpm_noirq_resume_devices(pm_message_t state) { struct device *dev; ktime_t starttime = ktime_get(); @@ -760,13 +760,6 @@ void dpm_noirq_resume_devices(pm_message_t state) trace_suspend_resume(TPS("dpm_resume_noirq"), state.event, false); } -void dpm_noirq_end(void) -{ - resume_device_irqs(); - device_wakeup_disarm_wake_irqs(); - cpuidle_resume(); -} - /** * dpm_resume_noirq - Execute "noirq resume" callbacks for all devices. * @state: PM transition of the system being carried out. @@ -777,7 +770,11 @@ void dpm_noirq_end(void) void dpm_resume_noirq(pm_message_t state) { dpm_noirq_resume_devices(state); - dpm_noirq_end(); + + resume_device_irqs(); + device_wakeup_disarm_wake_irqs(); + + cpuidle_resume(); } static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev, @@ -1357,14 +1354,7 @@ static int device_suspend_noirq(struct device *dev) return __device_suspend_noirq(dev, pm_transition, false); } -void dpm_noirq_begin(void) -{ - cpuidle_pause(); - device_wakeup_arm_wake_irqs(); - suspend_device_irqs(); -} - -int dpm_noirq_suspend_devices(pm_message_t state) +static int dpm_noirq_suspend_devices(pm_message_t state) { ktime_t starttime = ktime_get(); int error = 0; @@ -1421,7 +1411,11 @@ int dpm_suspend_noirq(pm_message_t state) { int ret; - dpm_noirq_begin(); + cpuidle_pause(); + + device_wakeup_arm_wake_irqs(); + suspend_device_irqs(); + ret = dpm_noirq_suspend_devices(state); if (ret) dpm_resume_noirq(resume_event(state)); diff --git a/include/linux/pm.h b/include/linux/pm.h index 3619a870eaa4..4c441be03079 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -712,8 +712,6 @@ struct dev_pm_domain { extern void device_pm_lock(void); extern void dpm_resume_start(pm_message_t state); extern void dpm_resume_end(pm_message_t state); -extern void dpm_noirq_resume_devices(pm_message_t state); -extern void dpm_noirq_end(void); extern void dpm_resume_noirq(pm_message_t state); extern void dpm_resume_early(pm_message_t state); extern void dpm_resume(pm_message_t state); @@ -722,8 +720,6 @@ extern void dpm_complete(pm_message_t state); extern void device_pm_unlock(void); extern int dpm_suspend_end(pm_message_t state); extern int dpm_suspend_start(pm_message_t state); -extern void dpm_noirq_begin(void); -extern int dpm_noirq_suspend_devices(pm_message_t state); extern int dpm_suspend_noirq(pm_message_t state); extern int dpm_suspend_late(pm_message_t state); extern int dpm_suspend(pm_message_t state); From 10a08fd65ec1a68ccd86b19ec822ed5f2e50113f Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 30 Jul 2019 11:55:59 +0200 Subject: [PATCH 09/21] ACPI: PM: Set up EC GPE for system wakeup from drivers that need it The EC GPE needs to be set up for system wakeup only if there is a driver depending on it, either intel-hid or intel-vbtn, bound to a button device that is expected to wake up the system from sleep (such as the power button on some Dell systems, like the XPS13 9360). It doesn't need to be set up for waking up the system from sleep in any other cases and whether or not it is expected to wake up the system from sleep doesn't depend on whether or not the LPS0 device is present in the ACPI namespace. For this reason, rearrange the ACPI suspend-to-idle code to make the drivers depending on the EC GPE wakeup take care of setting it up and decouple that from the LPS0 device handling. While at it, make intel-hid and intel-vbtn prepare for system wakeup only if they are allowed to wake up the system from sleep by user space (via sysfs). [Note that acpi_ec_mark_gpe_for_wake() and acpi_ec_set_gpe_wake_mask() are there to prevent the EC GPE from being disabled by the acpi_enable_all_wakeup_gpes() call in acpi_s2idle_prepare(), so on systems with either intel-hid or intel-vbtn this change doesn't affect any interactions with the hardware or platform firmware.] Signed-off-by: Rafael J. Wysocki Reviewed-by: Andy Shevchenko --- drivers/acpi/ec.c | 7 ++++++- drivers/acpi/internal.h | 2 -- drivers/acpi/sleep.c | 13 ++----------- drivers/platform/x86/intel-hid.c | 20 ++++++++++++++++---- drivers/platform/x86/intel-vbtn.c | 20 ++++++++++++++++---- include/linux/acpi.h | 4 ++++ include/linux/suspend.h | 1 + 7 files changed, 45 insertions(+), 22 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 58c7ad402d8d..b996ca5f253f 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -1048,17 +1049,21 @@ void acpi_ec_unblock_transactions(void) acpi_ec_start(first_ec, true); } +#ifdef CONFIG_PM_SLEEP void acpi_ec_mark_gpe_for_wake(void) { if (first_ec && !ec_no_wakeup) acpi_mark_gpe_for_wake(NULL, first_ec->gpe); } +EXPORT_SYMBOL_GPL(acpi_ec_mark_gpe_for_wake); void acpi_ec_set_gpe_wake_mask(u8 action) { - if (first_ec && !ec_no_wakeup) + if (pm_suspend_no_platform() && first_ec && !ec_no_wakeup) acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action); } +EXPORT_SYMBOL_GPL(acpi_ec_set_gpe_wake_mask); +#endif bool acpi_ec_dispatch_gpe(void) { diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 1b5f9ac06ea8..bcc080511197 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -194,8 +194,6 @@ void acpi_ec_ecdt_probe(void); void acpi_ec_dsdt_probe(void); void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void); -void acpi_ec_mark_gpe_for_wake(void); -void acpi_ec_set_gpe_wake_mask(u8 action); bool acpi_ec_dispatch_gpe(void); int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle, acpi_ec_query_func func, diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 970ae7c7a3f7..9cb0532f7471 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -930,8 +930,6 @@ static int lps0_device_attach(struct acpi_device *adev, acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n", bitmask); - - acpi_ec_mark_gpe_for_wake(); } else { acpi_handle_debug(adev->handle, "_DSM function 0 evaluation failed\n"); @@ -960,8 +958,6 @@ static int acpi_s2idle_prepare(void) if (lps0_device_handle) { acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF); acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY); - - acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE); } if (acpi_sci_irq_valid()) @@ -979,10 +975,7 @@ static int acpi_s2idle_prepare(void) static void acpi_s2idle_wake(void) { - if (!lps0_device_handle) - return; - - if (pm_debug_messages_on) + if (lps0_device_handle && pm_debug_messages_on) lpi_check_constraints(); /* @@ -1031,8 +1024,6 @@ static void acpi_s2idle_restore(void) disable_irq_wake(acpi_sci_irq); if (lps0_device_handle) { - acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); - acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT); acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON); } @@ -1081,7 +1072,7 @@ bool acpi_s2idle_wakeup(void) bool acpi_sleep_no_ec_events(void) { - return !s2idle_in_progress || !lps0_device_handle; + return !s2idle_in_progress; } #ifdef CONFIG_PM_SLEEP diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index bc0d55a59015..e51c72b92cbd 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -253,9 +253,12 @@ static void intel_button_array_enable(struct device *device, bool enable) static int intel_hid_pm_prepare(struct device *device) { - struct intel_hid_priv *priv = dev_get_drvdata(device); + if (device_may_wakeup(device)) { + struct intel_hid_priv *priv = dev_get_drvdata(device); - priv->wakeup_mode = true; + priv->wakeup_mode = true; + acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE); + } return 0; } @@ -270,9 +273,12 @@ static int intel_hid_pl_suspend_handler(struct device *device) static int intel_hid_pl_resume_handler(struct device *device) { - struct intel_hid_priv *priv = dev_get_drvdata(device); + if (device_may_wakeup(device)) { + struct intel_hid_priv *priv = dev_get_drvdata(device); - priv->wakeup_mode = false; + acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); + priv->wakeup_mode = false; + } if (pm_resume_via_firmware()) { intel_hid_set_enable(device, true); intel_button_array_enable(device, true); @@ -491,6 +497,12 @@ static int intel_hid_probe(struct platform_device *device) } device_init_wakeup(&device->dev, true); + /* + * In order for system wakeup to work, the EC GPE has to be marked as + * a wakeup one, so do that here (this setting will persist, but it has + * no effect until the wakeup mask is set for the EC GPE). + */ + acpi_ec_mark_gpe_for_wake(); return 0; err_remove_notify: diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c index a0d0cecff55f..ab84e1bbdedd 100644 --- a/drivers/platform/x86/intel-vbtn.c +++ b/drivers/platform/x86/intel-vbtn.c @@ -176,6 +176,12 @@ static int intel_vbtn_probe(struct platform_device *device) return -EBUSY; device_init_wakeup(&device->dev, true); + /* + * In order for system wakeup to work, the EC GPE has to be marked as + * a wakeup one, so do that here (this setting will persist, but it has + * no effect until the wakeup mask is set for the EC GPE). + */ + acpi_ec_mark_gpe_for_wake(); return 0; } @@ -195,17 +201,23 @@ static int intel_vbtn_remove(struct platform_device *device) static int intel_vbtn_pm_prepare(struct device *dev) { - struct intel_vbtn_priv *priv = dev_get_drvdata(dev); + if (device_may_wakeup(dev)) { + struct intel_vbtn_priv *priv = dev_get_drvdata(dev); - priv->wakeup_mode = true; + priv->wakeup_mode = true; + acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE); + } return 0; } static int intel_vbtn_pm_resume(struct device *dev) { - struct intel_vbtn_priv *priv = dev_get_drvdata(dev); + if (device_may_wakeup(dev)) { + struct intel_vbtn_priv *priv = dev_get_drvdata(dev); - priv->wakeup_mode = false; + acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); + priv->wakeup_mode = false; + } return 0; } diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 9426b9aaed86..e65a4c5bbeae 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -931,6 +931,8 @@ int acpi_subsys_suspend_noirq(struct device *dev); int acpi_subsys_suspend(struct device *dev); int acpi_subsys_freeze(struct device *dev); int acpi_subsys_poweroff(struct device *dev); +void acpi_ec_mark_gpe_for_wake(void); +void acpi_ec_set_gpe_wake_mask(u8 action); #else static inline int acpi_subsys_prepare(struct device *dev) { return 0; } static inline void acpi_subsys_complete(struct device *dev) {} @@ -939,6 +941,8 @@ static inline int acpi_subsys_suspend_noirq(struct device *dev) { return 0; } static inline int acpi_subsys_suspend(struct device *dev) { return 0; } static inline int acpi_subsys_freeze(struct device *dev) { return 0; } static inline int acpi_subsys_poweroff(struct device *dev) { return 0; } +static inline void acpi_ec_mark_gpe_for_wake(void) {} +static inline void acpi_ec_set_gpe_wake_mask(u8 action) {} #endif #ifdef CONFIG_ACPI diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 66ce3871ed61..f0c4a8445140 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -335,6 +335,7 @@ static inline void pm_set_suspend_via_firmware(void) {} static inline void pm_set_resume_via_firmware(void) {} static inline bool pm_suspend_via_firmware(void) { return false; } static inline bool pm_resume_via_firmware(void) { return false; } +static inline bool pm_suspend_no_platform(void) { return false; } static inline bool pm_suspend_default_s2idle(void) { return false; } static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {} From 2e2c2fdc53437beffd2cf26aaf6187e602d565bc Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 31 Jul 2019 11:05:15 +0200 Subject: [PATCH 10/21] ACPI: PM: s2idle: Rearrange lps0_device_attach() To allow a subsequent change to be simpler, rearrange the code in lps0_device_attach() to reduce the indentation level and (while at it) make it avoid calling lpi_device_get_constraints() when lps0_device_handle is not going to be set. Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/sleep.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 9cb0532f7471..3d706938980a 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -916,28 +916,30 @@ static int lps0_device_attach(struct acpi_device *adev, guid_parse(ACPI_LPS0_DSM_UUID, &lps0_dsm_guid); /* Check if the _DSM is present and as expected. */ out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 1, 0, NULL); - if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) { - char bitmask = *(char *)out_obj->buffer.pointer; - - 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_TO_IDLE; - - acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n", - bitmask); - } else { + if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER) { acpi_handle_debug(adev->handle, "_DSM function 0 evaluation failed\n"); + return 0; } + + lps0_dsm_func_mask = *(char *)out_obj->buffer.pointer; + ACPI_FREE(out_obj); + acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n", + lps0_dsm_func_mask); + + lps0_device_handle = adev->handle; + lpi_device_get_constraints(); + /* + * 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_TO_IDLE; + return 0; } From 068b47d0984b8756ae71702a1a87aa226cb72fe8 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 31 Jul 2019 11:05:25 +0200 Subject: [PATCH 11/21] ACPI: PM: s2idle: Add acpi.sleep_no_lps0 module parameter Add a module parameter to prevent the ACPI LPS0 _DSM functions from being invoked (if need be) and rework the suspend-to-idle blacklist entries in acpisleep_dmi_table[] to make them simply prevent suspend-to-idle from being used by default on the systems in question (which really is the original purpose of those entries). Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/sleep.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 3d706938980a..4a94600fea39 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -89,6 +89,10 @@ bool acpi_sleep_state_supported(u8 sleep_state) } #ifdef CONFIG_ACPI_SLEEP +static bool sleep_no_lps0 __read_mostly; +module_param(sleep_no_lps0, bool, 0644); +MODULE_PARM_DESC(sleep_no_lps0, "Do not use the special LPS0 device interface"); + static u32 acpi_target_sleep_state = ACPI_STATE_S0; u32 acpi_target_system_state(void) @@ -158,11 +162,11 @@ static int __init init_nvs_nosave(const struct dmi_system_id *d) return 0; } -static bool acpi_sleep_no_lps0; +static bool acpi_sleep_default_s3; -static int __init init_no_lps0(const struct dmi_system_id *d) +static int __init init_default_s3(const struct dmi_system_id *d) { - acpi_sleep_no_lps0 = true; + acpi_sleep_default_s3 = true; return 0; } @@ -363,7 +367,7 @@ static const struct dmi_system_id acpisleep_dmi_table[] __initconst = { * S0 Idle firmware interface. */ { - .callback = init_no_lps0, + .callback = init_default_s3, .ident = "Dell XPS13 9360", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), @@ -376,7 +380,7 @@ static const struct dmi_system_id acpisleep_dmi_table[] __initconst = { * https://bugzilla.kernel.org/show_bug.cgi?id=199057). */ { - .callback = init_no_lps0, + .callback = init_default_s3, .ident = "ThinkPad X1 Tablet(2016)", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), @@ -524,8 +528,9 @@ static void acpi_pm_end(void) acpi_sleep_tts_switch(acpi_target_sleep_state); } #else /* !CONFIG_ACPI_SLEEP */ +#define sleep_no_lps0 (1) #define acpi_target_sleep_state ACPI_STATE_S0 -#define acpi_sleep_no_lps0 (false) +#define acpi_sleep_default_s3 (1) static inline void acpi_sleep_dmi_check(void) {} #endif /* CONFIG_ACPI_SLEEP */ @@ -904,12 +909,6 @@ static int lps0_device_attach(struct acpi_device *adev, if (lps0_device_handle) return 0; - if (acpi_sleep_no_lps0) { - acpi_handle_info(adev->handle, - "Low Power S0 Idle interface disabled\n"); - return 0; - } - if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) return 0; @@ -937,7 +936,7 @@ static int lps0_device_attach(struct acpi_device *adev, * 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) + if (mem_sleep_default > PM_SUSPEND_MEM && !acpi_sleep_default_s3) mem_sleep_current = PM_SUSPEND_TO_IDLE; return 0; @@ -957,7 +956,7 @@ static int acpi_s2idle_begin(void) static int acpi_s2idle_prepare(void) { - if (lps0_device_handle) { + if (lps0_device_handle && !sleep_no_lps0) { acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF); acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY); } @@ -977,7 +976,7 @@ static int acpi_s2idle_prepare(void) static void acpi_s2idle_wake(void) { - if (lps0_device_handle && pm_debug_messages_on) + if (lps0_device_handle && !sleep_no_lps0 && pm_debug_messages_on) lpi_check_constraints(); /* @@ -1025,7 +1024,7 @@ static void acpi_s2idle_restore(void) if (acpi_sci_irq_valid()) disable_irq_wake(acpi_sci_irq); - if (lps0_device_handle) { + if (lps0_device_handle && !sleep_no_lps0) { acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT); acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON); } From fcd0a04267ac7c5d5f9a27d2af824270f2091760 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 31 Jul 2019 11:05:33 +0200 Subject: [PATCH 12/21] ACPI: PM: s2idle: Switch EC over to polling during "noirq" suspend Since the ACPI SCI is set up for system wakeup before the "noirq" suspend of devices, it is better to make suspend-to-idle follow suspend-to-RAM (S3) and switch over the EC to polling during "noirq" suspend (and back to interrupt-based flow during "noirq" resume). The frequency of spurious wakeup interrupts from the EC may be reduced this way. Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/ec.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index b996ca5f253f..5a38409114d8 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1958,8 +1958,7 @@ static int acpi_ec_suspend_noirq(struct device *dev) ec->reference_count >= 1) acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE); - if (acpi_sleep_no_ec_events()) - acpi_ec_enter_noirq(ec); + acpi_ec_enter_noirq(ec); return 0; } @@ -1968,8 +1967,7 @@ static int acpi_ec_resume_noirq(struct device *dev) { struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev)); - if (acpi_sleep_no_ec_events()) - acpi_ec_leave_noirq(ec); + acpi_ec_leave_noirq(ec); if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) && ec->reference_count >= 1) From 6e86633a791fdf631617ef3a9af3263141d34bc9 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 31 Jul 2019 11:05:42 +0200 Subject: [PATCH 13/21] ACPI: PM: s2idle: Eliminate acpi_sleep_no_ec_events() Change acpi_ec_suspend() to use pm_suspend_no_platform() instead of acpi_sleep_no_ec_events(), which allows the latter to be eliminated along with the s2idle_in_progress variable which is only used by it. Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/ec.c | 2 +- drivers/acpi/internal.h | 2 -- drivers/acpi/sleep.c | 9 --------- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 5a38409114d8..be29b9919e2d 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1941,7 +1941,7 @@ static int acpi_ec_suspend(struct device *dev) struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev)); - if (acpi_sleep_no_ec_events() && ec_freeze_events) + if (!pm_suspend_no_platform() && ec_freeze_events) acpi_ec_disable_event(ec); return 0; } diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index bcc080511197..8c9cd3733f07 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -210,11 +210,9 @@ void acpi_ec_flush_work(void); -------------------------------------------------------------------------- */ #ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT extern bool acpi_s2idle_wakeup(void); -extern bool acpi_sleep_no_ec_events(void); extern int acpi_sleep_init(void); #else static inline bool acpi_s2idle_wakeup(void) { return false; } -static inline bool acpi_sleep_no_ec_events(void) { return true; } static inline int acpi_sleep_init(void) { return -ENXIO; } #endif diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 4a94600fea39..864bb18d3a5d 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -696,7 +696,6 @@ static const struct platform_suspend_ops acpi_suspend_ops_old = { .recover = acpi_pm_finish, }; -static bool s2idle_in_progress; static bool s2idle_wakeup; /* @@ -950,7 +949,6 @@ static struct acpi_scan_handler lps0_handler = { static int acpi_s2idle_begin(void) { acpi_scan_lock_acquire(); - s2idle_in_progress = true; return 0; } @@ -1032,7 +1030,6 @@ static void acpi_s2idle_restore(void) static void acpi_s2idle_end(void) { - s2idle_in_progress = false; acpi_scan_lock_release(); } @@ -1060,7 +1057,6 @@ static void acpi_sleep_suspend_setup(void) } #else /* !CONFIG_SUSPEND */ -#define s2idle_in_progress (false) #define s2idle_wakeup (false) #define lps0_device_handle (NULL) static inline void acpi_sleep_suspend_setup(void) {} @@ -1071,11 +1067,6 @@ bool acpi_s2idle_wakeup(void) return s2idle_wakeup; } -bool acpi_sleep_no_ec_events(void) -{ - return !s2idle_in_progress; -} - #ifdef CONFIG_PM_SLEEP static u32 saved_bm_rld; From d7589404932be148fabe696b56b7c391bad6bdb1 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 31 Jul 2019 11:05:52 +0200 Subject: [PATCH 14/21] ACPI: EC: PM: Consolidate some code depending on PM_SLEEP Move some routines, including acpi_ec_dispatch_gpe(), that are only used if CONFIG_PM_SLEEP is set to the #ifdef block containing the EC suspend and resume callbacks, to make the "full EC PM picture" easier to follow. While at it, move the header of acpi_ec_dispatch_gpe() in the header file to a CONFIG_PM_SLEEP #ifdef block. Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/ec.c | 54 ++++++++++++++++++++--------------------- drivers/acpi/internal.h | 2 +- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index be29b9919e2d..8d7247b4441f 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1049,33 +1049,6 @@ void acpi_ec_unblock_transactions(void) acpi_ec_start(first_ec, true); } -#ifdef CONFIG_PM_SLEEP -void acpi_ec_mark_gpe_for_wake(void) -{ - if (first_ec && !ec_no_wakeup) - acpi_mark_gpe_for_wake(NULL, first_ec->gpe); -} -EXPORT_SYMBOL_GPL(acpi_ec_mark_gpe_for_wake); - -void acpi_ec_set_gpe_wake_mask(u8 action) -{ - if (pm_suspend_no_platform() && first_ec && !ec_no_wakeup) - acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action); -} -EXPORT_SYMBOL_GPL(acpi_ec_set_gpe_wake_mask); -#endif - -bool acpi_ec_dispatch_gpe(void) -{ - u32 ret; - - if (!first_ec) - return false; - - ret = acpi_dispatch_gpe(NULL, first_ec->gpe); - return ret == ACPI_INTERRUPT_HANDLED; -} - /* -------------------------------------------------------------------------- Event Management -------------------------------------------------------------------------- */ @@ -1984,7 +1957,32 @@ static int acpi_ec_resume(struct device *dev) acpi_ec_enable_event(ec); return 0; } -#endif + +void acpi_ec_mark_gpe_for_wake(void) +{ + if (first_ec && !ec_no_wakeup) + acpi_mark_gpe_for_wake(NULL, first_ec->gpe); +} +EXPORT_SYMBOL_GPL(acpi_ec_mark_gpe_for_wake); + +void acpi_ec_set_gpe_wake_mask(u8 action) +{ + if (pm_suspend_no_platform() && first_ec && !ec_no_wakeup) + acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action); +} +EXPORT_SYMBOL_GPL(acpi_ec_set_gpe_wake_mask); + +bool acpi_ec_dispatch_gpe(void) +{ + u32 ret; + + if (!first_ec) + return false; + + ret = acpi_dispatch_gpe(NULL, first_ec->gpe); + return ret == ACPI_INTERRUPT_HANDLED; +} +#endif /* CONFIG_PM_SLEEP */ static const struct dev_pm_ops acpi_ec_pm = { SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend_noirq, acpi_ec_resume_noirq) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 8c9cd3733f07..afe6636f9ad3 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -194,7 +194,6 @@ void acpi_ec_ecdt_probe(void); void acpi_ec_dsdt_probe(void); void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void); -bool acpi_ec_dispatch_gpe(void); int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle, acpi_ec_query_func func, void *data); @@ -202,6 +201,7 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit); #ifdef CONFIG_PM_SLEEP void acpi_ec_flush_work(void); +bool acpi_ec_dispatch_gpe(void); #endif From 29113f2f0a7d8d5332bfdfdfca995c06d0896e83 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 31 Jul 2019 11:06:00 +0200 Subject: [PATCH 15/21] ACPI: EC: PM: Make acpi_ec_dispatch_gpe() print debug message Add a pm_pr_dbg() debug statement to acpi_ec_dispatch_gpe() to print a message when the EC GPE has been dispatched (because its status was set). Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/ec.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 8d7247b4441f..58597ec813eb 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1980,7 +1980,11 @@ bool acpi_ec_dispatch_gpe(void) return false; ret = acpi_dispatch_gpe(NULL, first_ec->gpe); - return ret == ACPI_INTERRUPT_HANDLED; + if (ret == ACPI_INTERRUPT_HANDLED) { + pm_pr_dbg("EC GPE dispatched\n"); + return true; + } + return false; } #endif /* CONFIG_PM_SLEEP */ From ac9eafbe930abb589e9289842a99cc575cadb854 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 1 Aug 2019 19:31:10 +0200 Subject: [PATCH 16/21] ACPI: PM: s2idle: Execute LPS0 _DSM functions with suspended devices According to Section 3.5 of the "Intel Low Power S0 Idle" document [1], Function 5 of the LPS0 _DSM is expected to be invoked when the system configuration matches the criteria for entering the target low-power state of the platform. In particular, this means that all devices should be suspended and in low-power states already when that function is invoked. This is not the case currently, however, because Function 5 of the LPS0 _DSM is invoked by it before the "noirq" phase of device suspend, which means that some devices may not have been put into low-power states yet at that point. That is a consequence of the previous design of the suspend-to-idle flow that allowed the "noirq" phase of device suspend and the "noirq" phase of device resume to be carried out for multiple times while "suspended" (if any spurious wakeup events were detected) and the point of the LPS0 _DSM Function 5 invocation was chosen so as to call it (and LPS0 _DSM Function 6 analogously) once per suspend-resume cycle (regardless of how many times the "noirq" phases of device suspend and resume were carried out while "suspended"). Now that the suspend-to-idle flow has been redesigned to carry out the "noirq" phases of device suspend and resume once in each cycle, the code can be reordered to follow the specification that it is based on more closely. For this purpose, add ->prepare_late and ->restore_early platform callbacks for suspend-to-idle, to be executed, respectively, after the "noirq" phase of suspending devices and before the "noirq" phase of resuming them and make ACPI use them for the invocation of LPS0 _DSM functions as appropriate. While at it, move the LPS0 entry requirements check to be made before invoking Functions 3 and 5 of the LPS0 _DSM (also once per cycle) as follows from the specification [1]. Link: https://uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf # [1] Signed-off-by: Rafael J. Wysocki Tested-by: Kai-Heng Feng --- drivers/acpi/sleep.c | 36 ++++++++++++++++++++++++------------ include/linux/suspend.h | 2 ++ kernel/power/suspend.c | 12 +++++++++--- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 864bb18d3a5d..8f7e95f97e1f 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -954,11 +954,6 @@ static int acpi_s2idle_begin(void) static int acpi_s2idle_prepare(void) { - if (lps0_device_handle && !sleep_no_lps0) { - acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF); - acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY); - } - if (acpi_sci_irq_valid()) enable_irq_wake(acpi_sci_irq); @@ -972,11 +967,22 @@ static int acpi_s2idle_prepare(void) return 0; } -static void acpi_s2idle_wake(void) +static int acpi_s2idle_prepare_late(void) { - if (lps0_device_handle && !sleep_no_lps0 && pm_debug_messages_on) + if (!lps0_device_handle || sleep_no_lps0) + return 0; + + if (pm_debug_messages_on) lpi_check_constraints(); + acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF); + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY); + + return 0; +} + +static void acpi_s2idle_wake(void) +{ /* * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the SCI has * not triggered while suspended, so bail out. @@ -1011,6 +1017,15 @@ static void acpi_s2idle_wake(void) rearm_wake_irq(acpi_sci_irq); } +static void acpi_s2idle_restore_early(void) +{ + if (!lps0_device_handle || sleep_no_lps0) + return; + + acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT); + acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON); +} + static void acpi_s2idle_restore(void) { s2idle_wakeup = false; @@ -1021,11 +1036,6 @@ static void acpi_s2idle_restore(void) if (acpi_sci_irq_valid()) disable_irq_wake(acpi_sci_irq); - - if (lps0_device_handle && !sleep_no_lps0) { - acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT); - acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON); - } } static void acpi_s2idle_end(void) @@ -1036,7 +1046,9 @@ static void acpi_s2idle_end(void) static const struct platform_s2idle_ops acpi_s2idle_ops = { .begin = acpi_s2idle_begin, .prepare = acpi_s2idle_prepare, + .prepare_late = acpi_s2idle_prepare_late, .wake = acpi_s2idle_wake, + .restore_early = acpi_s2idle_restore_early, .restore = acpi_s2idle_restore, .end = acpi_s2idle_end, }; diff --git a/include/linux/suspend.h b/include/linux/suspend.h index f0c4a8445140..6fc8843f1c9e 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -190,7 +190,9 @@ struct platform_suspend_ops { struct platform_s2idle_ops { int (*begin)(void); int (*prepare)(void); + int (*prepare_late)(void); void (*wake)(void); + void (*restore_early)(void); void (*restore)(void); void (*end)(void); }; diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index 2b6057853b33..ed9ddef12b13 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -253,13 +253,19 @@ static int platform_suspend_prepare_late(suspend_state_t state) static int platform_suspend_prepare_noirq(suspend_state_t state) { - return state != PM_SUSPEND_TO_IDLE && suspend_ops->prepare_late ? - suspend_ops->prepare_late() : 0; + if (state == PM_SUSPEND_TO_IDLE) { + if (s2idle_ops && s2idle_ops->prepare_late) + return s2idle_ops->prepare_late(); + } + return suspend_ops->prepare_late ? suspend_ops->prepare_late() : 0; } static void platform_resume_noirq(suspend_state_t state) { - if (state != PM_SUSPEND_TO_IDLE && suspend_ops->wake) + if (state == PM_SUSPEND_TO_IDLE) { + if (s2idle_ops && s2idle_ops->restore_early) + s2idle_ops->restore_early(); + } else if (suspend_ops->wake) suspend_ops->wake(); } From 31eb845718398f9bc9f6fbe1aca675f4e6284392 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 8 Aug 2019 11:39:17 +0200 Subject: [PATCH 17/21] intel-hid: intel-vbtn: Avoid leaking wakeup_mode set Both intel-hid and intel-vbtn set a wakeup_mode flag causing them to behave in a special way during system suspend and while suspended in their "prepare" PM callbacks and clear it in their "resume" PM callbacks. That may cause the wakeup_mode flag to remain set after a system suspend failure (if some other driver's "suspend" callback returns an error before the "suspend" callback of either intel-hid or intel-vbtn is invoked). After commit 10a08fd65ec1 ("ACPI: PM: Set up EC GPE for system wakeup from drivers that need it") that also affects the "wakeup mask" bit of the EC GPE, which may not be cleared after a failing system suspend. Fix this issue by adding "complete" PM callbacks to intel-hid and intel-vbtn to clear the wakeup_mode flag and the "wakeup mask" bit of the EC GPE if they have not been cleared earlier. Fixes: 10a08fd65ec1 ("ACPI: PM: Set up EC GPE for system wakeup from drivers that need it") Signed-off-by: Rafael J. Wysocki Acked-by: Andy Shevchenko --- drivers/platform/x86/intel-hid.c | 17 ++++++++++++----- drivers/platform/x86/intel-vbtn.c | 12 +++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index e51c72b92cbd..6feda887df9d 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -262,6 +262,16 @@ static int intel_hid_pm_prepare(struct device *device) return 0; } +static void intel_hid_pm_complete(struct device *device) +{ + struct intel_hid_priv *priv = dev_get_drvdata(device); + + if (priv->wakeup_mode) { + acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); + priv->wakeup_mode = false; + } +} + static int intel_hid_pl_suspend_handler(struct device *device) { if (pm_suspend_via_firmware()) { @@ -273,12 +283,8 @@ static int intel_hid_pl_suspend_handler(struct device *device) static int intel_hid_pl_resume_handler(struct device *device) { - if (device_may_wakeup(device)) { - struct intel_hid_priv *priv = dev_get_drvdata(device); + intel_hid_pm_complete(device); - acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); - priv->wakeup_mode = false; - } if (pm_resume_via_firmware()) { intel_hid_set_enable(device, true); intel_button_array_enable(device, true); @@ -288,6 +294,7 @@ static int intel_hid_pl_resume_handler(struct device *device) static const struct dev_pm_ops intel_hid_pl_pm_ops = { .prepare = intel_hid_pm_prepare, + .complete = intel_hid_pm_complete, .freeze = intel_hid_pl_suspend_handler, .thaw = intel_hid_pl_resume_handler, .restore = intel_hid_pl_resume_handler, diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c index ab84e1bbdedd..b28e5519337e 100644 --- a/drivers/platform/x86/intel-vbtn.c +++ b/drivers/platform/x86/intel-vbtn.c @@ -210,19 +210,25 @@ static int intel_vbtn_pm_prepare(struct device *dev) return 0; } -static int intel_vbtn_pm_resume(struct device *dev) +static void intel_vbtn_pm_complete(struct device *dev) { - if (device_may_wakeup(dev)) { - struct intel_vbtn_priv *priv = dev_get_drvdata(dev); + struct intel_vbtn_priv *priv = dev_get_drvdata(dev); + if (priv->wakeup_mode) { acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); priv->wakeup_mode = false; } +} + +static int intel_vbtn_pm_resume(struct device *dev) +{ + intel_vbtn_pm_complete(dev); return 0; } static const struct dev_pm_ops intel_vbtn_pm_ops = { .prepare = intel_vbtn_pm_prepare, + .complete = intel_vbtn_pm_complete, .resume = intel_vbtn_pm_resume, .restore = intel_vbtn_pm_resume, .thaw = intel_vbtn_pm_resume, From d19bdb876bece27187d4ffbc272672e1239cd313 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 8 Aug 2019 11:39:23 +0200 Subject: [PATCH 18/21] intel-hid: Disable button array during suspend-to-idle Notice that intel_button_array_enable() never disables the power button which is the only one needed to wake up the system from suspend-to-idle, so it can be safely called during suspend-to-idle as well as during "regular" system suspend, and rearrange the code in the driver's "suspend" and "resume" callbacks accordingly. While at it, use pm_suspend_no_platform() to check if the current suspend-resume cycle is suspend-to-idle, as that is the only case when the device should be enabled while suspended. Signed-off-by: Rafael J. Wysocki Acked-by: Andy Shevchenko --- drivers/platform/x86/intel-hid.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index 6feda887df9d..18ac237114ff 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -274,10 +274,11 @@ static void intel_hid_pm_complete(struct device *device) static int intel_hid_pl_suspend_handler(struct device *device) { - if (pm_suspend_via_firmware()) { + intel_button_array_enable(device, false); + + if (!pm_suspend_no_platform()) intel_hid_set_enable(device, false); - intel_button_array_enable(device, false); - } + return 0; } @@ -285,10 +286,10 @@ static int intel_hid_pl_resume_handler(struct device *device) { intel_hid_pm_complete(device); - if (pm_resume_via_firmware()) { + if (!pm_suspend_no_platform()) intel_hid_set_enable(device, true); - intel_button_array_enable(device, true); - } + + intel_button_array_enable(device, true); return 0; } From 11f26633cccb7243217370837cbb066a73f678a5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sat, 10 Aug 2019 13:18:06 +0200 Subject: [PATCH 19/21] PM: suspend: Fix platform_suspend_prepare_noirq() After commit ac9eafbe930a ("ACPI: PM: s2idle: Execute LPS0 _DSM functions with suspended devices"), a NULL pointer may be dereferenced if suspend-to-idle is attempted on a platform without "traditional" suspend support due to invalid fall-through in platform_suspend_prepare_noirq(). Fix that and while at it add missing braces in platform_resume_noirq(). Fixes: ac9eafbe930a ("ACPI: PM: s2idle: Execute LPS0 _DSM functions with suspended devices") Reported-by: Marek Szyprowski Signed-off-by: Rafael J. Wysocki --- kernel/power/suspend.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index ed9ddef12b13..f3b7239f1892 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -253,10 +253,10 @@ static int platform_suspend_prepare_late(suspend_state_t state) static int platform_suspend_prepare_noirq(suspend_state_t state) { - if (state == PM_SUSPEND_TO_IDLE) { - if (s2idle_ops && s2idle_ops->prepare_late) - return s2idle_ops->prepare_late(); - } + if (state == PM_SUSPEND_TO_IDLE) + return s2idle_ops && s2idle_ops->prepare_late ? + s2idle_ops->prepare_late() : 0; + return suspend_ops->prepare_late ? suspend_ops->prepare_late() : 0; } @@ -265,8 +265,9 @@ static void platform_resume_noirq(suspend_state_t state) if (state == PM_SUSPEND_TO_IDLE) { if (s2idle_ops && s2idle_ops->restore_early) s2idle_ops->restore_early(); - } else if (suspend_ops->wake) + } else if (suspend_ops->wake) { suspend_ops->wake(); + } } static void platform_resume_early(suspend_state_t state) From 45dc1576e4575ba621cb6d017faf41531d8c1073 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 19 Aug 2019 12:35:03 +0200 Subject: [PATCH 20/21] ACPI: PM: s2idle: Avoid rearming SCI for wakeup unnecessarily It is only necessary to rearm the ACPI SCI for wakeup if pm_system_cancel_wakeup() has been called, so invoke rearm_wake_irq() only in that case. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/sleep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 8f7e95f97e1f..c52ecbda863f 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -1012,9 +1012,9 @@ static void acpi_s2idle_wake(void) acpi_os_wait_events_complete(); /* synchronize EC GPE processing */ acpi_ec_flush_work(); acpi_os_wait_events_complete(); /* synchronize Notify handling */ - } - rearm_wake_irq(acpi_sci_irq); + rearm_wake_irq(acpi_sci_irq); + } } static void acpi_s2idle_restore_early(void) From b90ff3554aa3e123bb7e6d08789f6fd92d86ddde Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 21 Aug 2019 11:40:19 +0200 Subject: [PATCH 21/21] ACPI: PM: s2idle: Always set up EC GPE for system wakeup Commit 10a08fd65ec1 ("ACPI: PM: Set up EC GPE for system wakeup from drivers that need it") assumed that the EC GPE would only need to be set up for system wakeup if either the intel-hid or the intel-vbtn driver was in use, but that turns out to be incorrect. In particular, on ASUS Zenbook UX430UNR/i7-8550U, if the EC GPE is not enabled while suspended, the system cannot be woken up by opening the lid or pressing a key, and that machine doesn't use any of the drivers mentioned above. For this reason, always set up the EC GPE for system wakeup from suspend-to-idle by setting and clearing its wake mask in the ACPI suspend-to-idle callbacks. Fixes: 10a08fd65ec1 ("ACPI: PM: Set up EC GPE for system wakeup from drivers that need it") Reported-by: Kristian Klausen Tested-by: Kristian Klausen Acked-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 1 - drivers/acpi/sleep.c | 15 +++++++++++++-- drivers/platform/x86/intel-hid.c | 6 +----- drivers/platform/x86/intel-vbtn.c | 6 +----- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 58597ec813eb..da1e5c5ce150 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1970,7 +1970,6 @@ void acpi_ec_set_gpe_wake_mask(u8 action) if (pm_suspend_no_platform() && first_ec && !ec_no_wakeup) acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action); } -EXPORT_SYMBOL_GPL(acpi_ec_set_gpe_wake_mask); bool acpi_ec_dispatch_gpe(void) { diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index c52ecbda863f..9fa77d72ef27 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -938,6 +938,13 @@ static int lps0_device_attach(struct acpi_device *adev, if (mem_sleep_default > PM_SUSPEND_MEM && !acpi_sleep_default_s3) mem_sleep_current = PM_SUSPEND_TO_IDLE; + /* + * Some LPS0 systems, like ASUS Zenbook UX430UNR/i7-8550U, require the + * EC GPE to be enabled while suspended for certain wakeup devices to + * work, so mark it as wakeup-capable. + */ + acpi_ec_mark_gpe_for_wake(); + return 0; } @@ -954,8 +961,10 @@ static int acpi_s2idle_begin(void) static int acpi_s2idle_prepare(void) { - if (acpi_sci_irq_valid()) + if (acpi_sci_irq_valid()) { enable_irq_wake(acpi_sci_irq); + acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE); + } acpi_enable_wakeup_devices(ACPI_STATE_S0); @@ -1034,8 +1043,10 @@ static void acpi_s2idle_restore(void) acpi_disable_wakeup_devices(ACPI_STATE_S0); - if (acpi_sci_irq_valid()) + if (acpi_sci_irq_valid()) { + acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); disable_irq_wake(acpi_sci_irq); + } } static void acpi_s2idle_end(void) diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index 18ac237114ff..ef6d4bd77b1a 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -257,7 +257,6 @@ static int intel_hid_pm_prepare(struct device *device) struct intel_hid_priv *priv = dev_get_drvdata(device); priv->wakeup_mode = true; - acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE); } return 0; } @@ -266,10 +265,7 @@ static void intel_hid_pm_complete(struct device *device) { struct intel_hid_priv *priv = dev_get_drvdata(device); - if (priv->wakeup_mode) { - acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); - priv->wakeup_mode = false; - } + priv->wakeup_mode = false; } static int intel_hid_pl_suspend_handler(struct device *device) diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c index b28e5519337e..b74932307d69 100644 --- a/drivers/platform/x86/intel-vbtn.c +++ b/drivers/platform/x86/intel-vbtn.c @@ -205,7 +205,6 @@ static int intel_vbtn_pm_prepare(struct device *dev) struct intel_vbtn_priv *priv = dev_get_drvdata(dev); priv->wakeup_mode = true; - acpi_ec_set_gpe_wake_mask(ACPI_GPE_ENABLE); } return 0; } @@ -214,10 +213,7 @@ static void intel_vbtn_pm_complete(struct device *dev) { struct intel_vbtn_priv *priv = dev_get_drvdata(dev); - if (priv->wakeup_mode) { - acpi_ec_set_gpe_wake_mask(ACPI_GPE_DISABLE); - priv->wakeup_mode = false; - } + priv->wakeup_mode = false; } static int intel_vbtn_pm_resume(struct device *dev)