All of lore.kernel.org
 help / color / mirror / Atom feed
From: Claude Yen <claude.yen@mediatek.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Stephen Boyd <swboyd@chromium.org>,
	"Douglas Anderson" <dianders@chromium.org>
Cc: wsd_upstream@mediatek.com, linux-pm@vger.kernel.org,
	"claude.yen" <claude.yen@mediatek.com>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] PM: s2idle: Introduce syscore callbacks in s2idle flow
Date: Tue, 1 Sep 2020 15:04:19 +0800	[thread overview]
Message-ID: <1598943859-21857-2-git-send-email-claude.yen@mediatek.com> (raw)
In-Reply-To: <1598943859-21857-1-git-send-email-claude.yen@mediatek.com>

From: "claude.yen" <claude.yen@mediatek.com>

Adding syscore callbacks to s2idle makes the behavior of s2idle become
more similar to Suspend-to-Ram (S2R) and reduces potential porting
effort.

tick_freeze() in s2idle flow calls sched_clock_suspend() and
timekeeping_suspend(), which both functions are also registered as
syscore callback. sched_clock_suspend() introduced in
commit 3f2552f7e9c5 ("timers/sched_clock: Prevent generic sched_clock
wrap caused by tick_freeze()") is added due to clock wrap issue.
By adding syscore callbacks in s2idle, if there is any syscore callbacks
also required in s2idle, additional porting effort could be saved.

Besides, in both S2R and s2idle, before the system entering low power
state, there is a state when only one cpu alive with interrupt-disabled,
which is syscore callback designed for. Adding syscore callbacks in
s2idle is feasible option.

Scenarios to call syscore callback:
S2R: one cpu alive when nonboot cpus are hotplug-ed off
s2idle: one cpu alive when other cpus have enter idle state


Signed-off-by: claude.yen <claude.yen@mediatek.com>
---
 drivers/cpuidle/cpuidle.c |   36 ++++++++++++++++++++++++++++++++----
 kernel/cpu_pm.c           |   17 +++++++++++++++++
 kernel/time/tick-common.c |   17 ++---------------
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 8719731..be22174 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -23,6 +23,9 @@
 #include <linux/suspend.h>
 #include <linux/tick.h>
 #include <trace/events/power.h>
+#include <linux/cpumask.h>
+#include <linux/spinlock.h>
+#include <linux/syscore_ops.h>
 
 #include "cpuidle.h"
 
@@ -35,6 +38,8 @@
 static int enabled_devices;
 static int off __read_mostly;
 static int initialized __read_mostly;
+static struct cpumask s2idle_cpumask;
+static DEFINE_SPINLOCK(s2idle_spinlock);
 
 int cpuidle_disabled(void)
 {
@@ -137,15 +142,27 @@ static void enter_s2idle_proper(struct cpuidle_driver *drv,
 				struct cpuidle_device *dev, int index)
 {
 	ktime_t time_start, time_end;
+	int cpu;
 
 	time_start = ns_to_ktime(local_clock());
+	cpu = smp_processor_id();
+
+	tick_freeze();
 
 	/*
-	 * trace_suspend_resume() called by tick_freeze() for the last CPU
+	 * trace_suspend_resume() called by syscore_suepnd() for the last CPU
 	 * executing it contains RCU usage regarded as invalid in the idle
 	 * context, so tell RCU about that.
 	 */
-	RCU_NONIDLE(tick_freeze());
+	spin_lock(&s2idle_spinlock);
+	cpumask_set_cpu(cpu, &s2idle_cpumask);
+	if (cpumask_weight(&s2idle_cpumask) == num_online_cpus()) {
+		system_state = SYSTEM_SUSPEND;
+		RCU_NONIDLE(syscore_suspend());
+	}
+
+	spin_unlock(&s2idle_spinlock);
+
 	/*
 	 * The state used here cannot be a "coupled" one, because the "coupled"
 	 * cpuidle mechanism enables interrupts and doing that with timekeeping
@@ -154,12 +171,21 @@ static void enter_s2idle_proper(struct cpuidle_driver *drv,
 	stop_critical_timings();
 	drv->states[index].enter_s2idle(dev, drv, index);
 	WARN_ON(!irqs_disabled());
+
 	/*
-	 * timekeeping_resume() that will be called by tick_unfreeze() for the
+	 * timekeeping_resume() that will be called by syscore_reume() for the
 	 * first CPU executing it calls functions containing RCU read-side
 	 * critical sections, so tell RCU about that.
 	 */
-	RCU_NONIDLE(tick_unfreeze());
+	spin_lock(&s2idle_spinlock);
+	if (cpumask_weight(&s2idle_cpumask) == num_online_cpus()) {
+		RCU_NONIDLE(syscore_resume());
+		system_state = SYSTEM_RUNNING;
+	}
+	cpumask_clear_cpu(cpu, &s2idle_cpumask);
+	spin_unlock(&s2idle_spinlock);
+
+	tick_unfreeze();
 	start_critical_timings();
 
 	time_end = ns_to_ktime(local_clock());
@@ -745,6 +771,8 @@ static int __init cpuidle_init(void)
 	if (cpuidle_disabled())
 		return -ENODEV;
 
+	cpumask_clear(&s2idle_cpumask);
+
 	return cpuidle_add_interface(cpu_subsys.dev_root);
 }
 
diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c
index 44a2593..6c2f5ce 100644
--- a/kernel/cpu_pm.c
+++ b/kernel/cpu_pm.c
@@ -12,6 +12,7 @@
 #include <linux/notifier.h>
 #include <linux/spinlock.h>
 #include <linux/syscore_ops.h>
+#include <linux/suspend.h>
 
 static ATOMIC_NOTIFIER_HEAD(cpu_pm_notifier_chain);
 
@@ -172,6 +173,14 @@ static int cpu_pm_suspend(void)
 {
 	int ret;
 
+#ifdef CONFIG_SUSPEND
+	/*
+	 * Skip cpu_pm_suspend in s2idle flow because cpu_pm notifier callbacks
+	 * are triggered in idle framework
+	 */
+	if (s2idle_state != S2IDLE_STATE_NONE)
+		return 0;
+#endif
 	ret = cpu_pm_enter();
 	if (ret)
 		return ret;
@@ -182,6 +191,14 @@ static int cpu_pm_suspend(void)
 
 static void cpu_pm_resume(void)
 {
+#ifdef CONFIG_SUSPEND
+	/*
+	 * Skip cpu_pm_resume in s2idle flow because cpu_pm notifier callbacks
+	 * are triggered in idle framework
+	 */
+	if (s2idle_state != S2IDLE_STATE_NONE)
+		return;
+#endif
 	cpu_cluster_pm_exit();
 	cpu_pm_exit();
 }
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 6c9c342..19feeed 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -528,15 +528,8 @@ void tick_freeze(void)
 	raw_spin_lock(&tick_freeze_lock);
 
 	tick_freeze_depth++;
-	if (tick_freeze_depth == num_online_cpus()) {
-		trace_suspend_resume(TPS("timekeeping_freeze"),
-				     smp_processor_id(), true);
-		system_state = SYSTEM_SUSPEND;
-		sched_clock_suspend();
-		timekeeping_suspend();
-	} else {
+	if (tick_freeze_depth < num_online_cpus())
 		tick_suspend_local();
-	}
 
 	raw_spin_unlock(&tick_freeze_lock);
 }
@@ -554,13 +547,7 @@ void tick_unfreeze(void)
 {
 	raw_spin_lock(&tick_freeze_lock);
 
-	if (tick_freeze_depth == num_online_cpus()) {
-		timekeeping_resume();
-		sched_clock_resume();
-		system_state = SYSTEM_RUNNING;
-		trace_suspend_resume(TPS("timekeeping_freeze"),
-				     smp_processor_id(), false);
-	} else {
+	if (tick_freeze_depth < num_online_cpus()) {
 		touch_softlockup_watchdog();
 		tick_resume_local();
 	}
-- 
1.7.9.5
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Claude Yen <claude.yen@mediatek.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Stephen Boyd <swboyd@chromium.org>,
	"Douglas Anderson" <dianders@chromium.org>
Cc: wsd_upstream@mediatek.com, linux-pm@vger.kernel.org,
	"claude.yen" <claude.yen@mediatek.com>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] PM: s2idle: Introduce syscore callbacks in s2idle flow
Date: Tue, 1 Sep 2020 15:04:19 +0800	[thread overview]
Message-ID: <1598943859-21857-2-git-send-email-claude.yen@mediatek.com> (raw)
In-Reply-To: <1598943859-21857-1-git-send-email-claude.yen@mediatek.com>

From: "claude.yen" <claude.yen@mediatek.com>

Adding syscore callbacks to s2idle makes the behavior of s2idle become
more similar to Suspend-to-Ram (S2R) and reduces potential porting
effort.

tick_freeze() in s2idle flow calls sched_clock_suspend() and
timekeeping_suspend(), which both functions are also registered as
syscore callback. sched_clock_suspend() introduced in
commit 3f2552f7e9c5 ("timers/sched_clock: Prevent generic sched_clock
wrap caused by tick_freeze()") is added due to clock wrap issue.
By adding syscore callbacks in s2idle, if there is any syscore callbacks
also required in s2idle, additional porting effort could be saved.

Besides, in both S2R and s2idle, before the system entering low power
state, there is a state when only one cpu alive with interrupt-disabled,
which is syscore callback designed for. Adding syscore callbacks in
s2idle is feasible option.

Scenarios to call syscore callback:
S2R: one cpu alive when nonboot cpus are hotplug-ed off
s2idle: one cpu alive when other cpus have enter idle state


Signed-off-by: claude.yen <claude.yen@mediatek.com>
---
 drivers/cpuidle/cpuidle.c |   36 ++++++++++++++++++++++++++++++++----
 kernel/cpu_pm.c           |   17 +++++++++++++++++
 kernel/time/tick-common.c |   17 ++---------------
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 8719731..be22174 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -23,6 +23,9 @@
 #include <linux/suspend.h>
 #include <linux/tick.h>
 #include <trace/events/power.h>
+#include <linux/cpumask.h>
+#include <linux/spinlock.h>
+#include <linux/syscore_ops.h>
 
 #include "cpuidle.h"
 
@@ -35,6 +38,8 @@
 static int enabled_devices;
 static int off __read_mostly;
 static int initialized __read_mostly;
+static struct cpumask s2idle_cpumask;
+static DEFINE_SPINLOCK(s2idle_spinlock);
 
 int cpuidle_disabled(void)
 {
@@ -137,15 +142,27 @@ static void enter_s2idle_proper(struct cpuidle_driver *drv,
 				struct cpuidle_device *dev, int index)
 {
 	ktime_t time_start, time_end;
+	int cpu;
 
 	time_start = ns_to_ktime(local_clock());
+	cpu = smp_processor_id();
+
+	tick_freeze();
 
 	/*
-	 * trace_suspend_resume() called by tick_freeze() for the last CPU
+	 * trace_suspend_resume() called by syscore_suepnd() for the last CPU
 	 * executing it contains RCU usage regarded as invalid in the idle
 	 * context, so tell RCU about that.
 	 */
-	RCU_NONIDLE(tick_freeze());
+	spin_lock(&s2idle_spinlock);
+	cpumask_set_cpu(cpu, &s2idle_cpumask);
+	if (cpumask_weight(&s2idle_cpumask) == num_online_cpus()) {
+		system_state = SYSTEM_SUSPEND;
+		RCU_NONIDLE(syscore_suspend());
+	}
+
+	spin_unlock(&s2idle_spinlock);
+
 	/*
 	 * The state used here cannot be a "coupled" one, because the "coupled"
 	 * cpuidle mechanism enables interrupts and doing that with timekeeping
@@ -154,12 +171,21 @@ static void enter_s2idle_proper(struct cpuidle_driver *drv,
 	stop_critical_timings();
 	drv->states[index].enter_s2idle(dev, drv, index);
 	WARN_ON(!irqs_disabled());
+
 	/*
-	 * timekeeping_resume() that will be called by tick_unfreeze() for the
+	 * timekeeping_resume() that will be called by syscore_reume() for the
 	 * first CPU executing it calls functions containing RCU read-side
 	 * critical sections, so tell RCU about that.
 	 */
-	RCU_NONIDLE(tick_unfreeze());
+	spin_lock(&s2idle_spinlock);
+	if (cpumask_weight(&s2idle_cpumask) == num_online_cpus()) {
+		RCU_NONIDLE(syscore_resume());
+		system_state = SYSTEM_RUNNING;
+	}
+	cpumask_clear_cpu(cpu, &s2idle_cpumask);
+	spin_unlock(&s2idle_spinlock);
+
+	tick_unfreeze();
 	start_critical_timings();
 
 	time_end = ns_to_ktime(local_clock());
@@ -745,6 +771,8 @@ static int __init cpuidle_init(void)
 	if (cpuidle_disabled())
 		return -ENODEV;
 
+	cpumask_clear(&s2idle_cpumask);
+
 	return cpuidle_add_interface(cpu_subsys.dev_root);
 }
 
diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c
index 44a2593..6c2f5ce 100644
--- a/kernel/cpu_pm.c
+++ b/kernel/cpu_pm.c
@@ -12,6 +12,7 @@
 #include <linux/notifier.h>
 #include <linux/spinlock.h>
 #include <linux/syscore_ops.h>
+#include <linux/suspend.h>
 
 static ATOMIC_NOTIFIER_HEAD(cpu_pm_notifier_chain);
 
@@ -172,6 +173,14 @@ static int cpu_pm_suspend(void)
 {
 	int ret;
 
+#ifdef CONFIG_SUSPEND
+	/*
+	 * Skip cpu_pm_suspend in s2idle flow because cpu_pm notifier callbacks
+	 * are triggered in idle framework
+	 */
+	if (s2idle_state != S2IDLE_STATE_NONE)
+		return 0;
+#endif
 	ret = cpu_pm_enter();
 	if (ret)
 		return ret;
@@ -182,6 +191,14 @@ static int cpu_pm_suspend(void)
 
 static void cpu_pm_resume(void)
 {
+#ifdef CONFIG_SUSPEND
+	/*
+	 * Skip cpu_pm_resume in s2idle flow because cpu_pm notifier callbacks
+	 * are triggered in idle framework
+	 */
+	if (s2idle_state != S2IDLE_STATE_NONE)
+		return;
+#endif
 	cpu_cluster_pm_exit();
 	cpu_pm_exit();
 }
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 6c9c342..19feeed 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -528,15 +528,8 @@ void tick_freeze(void)
 	raw_spin_lock(&tick_freeze_lock);
 
 	tick_freeze_depth++;
-	if (tick_freeze_depth == num_online_cpus()) {
-		trace_suspend_resume(TPS("timekeeping_freeze"),
-				     smp_processor_id(), true);
-		system_state = SYSTEM_SUSPEND;
-		sched_clock_suspend();
-		timekeeping_suspend();
-	} else {
+	if (tick_freeze_depth < num_online_cpus())
 		tick_suspend_local();
-	}
 
 	raw_spin_unlock(&tick_freeze_lock);
 }
@@ -554,13 +547,7 @@ void tick_unfreeze(void)
 {
 	raw_spin_lock(&tick_freeze_lock);
 
-	if (tick_freeze_depth == num_online_cpus()) {
-		timekeeping_resume();
-		sched_clock_resume();
-		system_state = SYSTEM_RUNNING;
-		trace_suspend_resume(TPS("timekeeping_freeze"),
-				     smp_processor_id(), false);
-	} else {
+	if (tick_freeze_depth < num_online_cpus()) {
 		touch_softlockup_watchdog();
 		tick_resume_local();
 	}
-- 
1.7.9.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-09-01  7:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01  7:04 [PATCH] PM: s2idle: Introduce syscore callbacks in s2idle flow Claude Yen
2020-09-01  7:04 ` Claude Yen
2020-09-01  7:04 ` Claude Yen
2020-09-01  7:04 ` Claude Yen [this message]
2020-09-01  7:04   ` [PATCH 1/1] " Claude Yen
2020-09-01  7:40   ` Stephen Boyd
2020-09-01  7:40     ` Stephen Boyd
2020-09-03  3:43     ` Claude.Yen
2020-09-03  3:43       ` Claude.Yen
2020-09-01 11:57 ` [PATCH] " Rafael J. Wysocki
2020-09-01 11:57   ` Rafael J. Wysocki
2020-09-01 11:57   ` Rafael J. Wysocki
2020-09-03  2:14   ` Claude.Yen
2020-09-03  2:14     ` Claude.Yen
2020-09-10 12:55     ` Rafael J. Wysocki
2020-09-10 12:55       ` Rafael J. Wysocki
2020-09-10 12:55       ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1598943859-21857-2-git-send-email-claude.yen@mediatek.com \
    --to=claude.yen@mediatek.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=dianders@chromium.org \
    --cc=fweisbec@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mingo@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=swboyd@chromium.org \
    --cc=tglx@linutronix.de \
    --cc=wsd_upstream@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.