Re: [PATCH 1/2] cpuidle: Avoid calls to cpuidle_resume|pause() for s2idle
From: Ulf Hansson <hidden>
Date: 2021-10-11 10:05:26
Also in:
linux-pm, lkml
On Sat, 9 Oct 2021 at 17:39, Rafael J. Wysocki [off-list ref] wrote:
On Wednesday, September 29, 2021 4:44:50 PM CEST Ulf Hansson wrote:quoted
In s2idle_enter(), cpuidle_resume|pause() are invoked to re-allow calls to the cpuidle callbacks during s2idle operations. This is needed because cpuidle is paused in-between in dpm_suspend_noirq() and dpm_resume_noirq().Well, in fact, doing that last thing for s2idle is pointless, because cpuidle is going to be resumed eventually anyway in that case and the breakage expected to be prevented by the pausing will still occur. So I would rather do something like the patch below (untested).
Hi Rafael, From a standalone change point of view, what you suggest seems reasonable to me. However, the main issue I am really trying to fix in this series is being done in patch2/2. And unfortunately, the below change doesn't really fit with what I suggest in patch2/2. Can you please have a look at patch2 as well? If you think it may be better, I squash the two patches? Kind regards Uffe
quoted hunk ↗ jump to hunk
--- drivers/base/power/main.c | 11 ++++++----- kernel/power/suspend.c | 8 ++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) Index: linux-pm/drivers/base/power/main.c ===================================================================--- linux-pm.orig/drivers/base/power/main.c +++ linux-pm/drivers/base/power/main.c@@ -747,8 +747,6 @@ void dpm_resume_noirq(pm_message_t state resume_device_irqs(); device_wakeup_disarm_wake_irqs(); - - cpuidle_resume(); } /**@@ -881,6 +879,7 @@ void dpm_resume_early(pm_message_t state void dpm_resume_start(pm_message_t state) { dpm_resume_noirq(state); + cpuidle_resume(); dpm_resume_early(state); } EXPORT_SYMBOL_GPL(dpm_resume_start);@@ -1336,8 +1335,6 @@ int dpm_suspend_noirq(pm_message_t state { int ret; - cpuidle_pause(); - device_wakeup_arm_wake_irqs(); suspend_device_irqs();@@ -1521,9 +1518,13 @@ int dpm_suspend_end(pm_message_t state) if (error) goto out; + cpuidle_pause(); + error = dpm_suspend_noirq(state); - if (error) + if (error) { + cpuidle_resume(); dpm_resume_early(resume_event(state)); + } out: dpm_show_time(starttime, state, error, "end");Index: linux-pm/kernel/power/suspend.c ===================================================================--- linux-pm.orig/kernel/power/suspend.c +++ linux-pm/kernel/power/suspend.c@@ -97,7 +97,6 @@ static void s2idle_enter(void) raw_spin_unlock_irq(&s2idle_lock); cpus_read_lock(); - cpuidle_resume(); /* Push all the CPUs into the idle loop. */ wake_up_all_idle_cpus();@@ -105,7 +104,6 @@ static void s2idle_enter(void) swait_event_exclusive(s2idle_wait_head, s2idle_state == S2IDLE_STATE_WAKE); - cpuidle_pause(); cpus_read_unlock(); raw_spin_lock_irq(&s2idle_lock);@@ -405,6 +403,9 @@ static int suspend_enter(suspend_state_t if (error) goto Devices_early_resume; + if (state != PM_SUSPEND_TO_IDLE) + cpuidle_pause(); + error = dpm_suspend_noirq(PMSG_SUSPEND); if (error) { pr_err("noirq suspend of devices failed\n");@@ -459,6 +460,9 @@ static int suspend_enter(suspend_state_t dpm_resume_noirq(PMSG_RESUME); Platform_early_resume: + if (state != PM_SUSPEND_TO_IDLE) + cpuidle_resume(); + platform_resume_early(state); Devices_early_resume:
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel