* [RFC v2 1/6] arm64: Juno: declare generic power domains for both clusters.
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
@ 2015-10-06 14:27 ` Marc Titinger
2015-10-06 14:27 ` [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Marc Titinger
` (6 subsequent siblings)
7 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-06 14:27 UTC (permalink / raw)
To: khilman, rjw
Cc: lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
arch/arm64/boot/dts/arm/juno.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index 342bb99..499f035 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -63,6 +63,7 @@
enable-method = "psci";
next-level-cache = <&A57_L2>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a57_pd>;
};
A57_1: cpu@1 {
@@ -72,6 +73,7 @@
enable-method = "psci";
next-level-cache = <&A57_L2>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a57_pd>;
};
A53_0: cpu@100 {
@@ -81,6 +83,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A53_1: cpu@101 {
@@ -90,6 +93,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A53_2: cpu@102 {
@@ -99,6 +103,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A53_3: cpu@103 {
@@ -108,6 +113,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A57_L2: l2-cache0 {
@@ -119,6 +125,19 @@
};
};
+ pm-domains {
+
+ a57_pd: a57_pd@ {
+ compatible = "arm,pd";
+ #power-domain-cells = <0>;
+ };
+
+ a53_pd: a53_pd@ {
+ compatible = "arm,pd";
+ #power-domain-cells = <0>;
+ };
+ };
+
pmu {
compatible = "arm,armv8-pmuv3";
interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
2015-10-06 14:27 ` [RFC v2 1/6] arm64: Juno: declare generic power domains for both clusters Marc Titinger
@ 2015-10-06 14:27 ` Marc Titinger
2015-10-08 16:11 ` Lina Iyer
2015-10-06 14:27 ` [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states Marc Titinger
` (5 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Marc Titinger @ 2015-10-06 14:27 UTC (permalink / raw)
To: khilman, rjw
Cc: lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Devices may register an intermediate retention state into the domain upon
attaching. Currently generic domain would register an array of states upon
init. This patch prepares for later insertion (sort per depth, remove).
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
drivers/base/power/domain.c | 189 +++++++++++++++++++-------------------------
include/linux/pm_domain.h | 18 ++++-
2 files changed, 97 insertions(+), 110 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 3e27a2b..e5f4c00b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -19,6 +19,7 @@
#include <linux/sched.h>
#include <linux/suspend.h>
#include <linux/export.h>
+#include <linux/sort.h>
#define GENPD_RETRY_MAX_MS 250 /* Approximate */
@@ -50,12 +51,6 @@
__retval; \
})
-#define GENPD_MAX_NAME_SIZE 20
-
-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count);
-
static LIST_HEAD(gpd_list);
static DEFINE_MUTEX(gpd_list_lock);
@@ -1364,46 +1359,6 @@ static void genpd_free_dev_data(struct device *dev,
dev_pm_put_subsys_data(dev);
}
-static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count)
-{
- int ret = 0;
- unsigned int i;
-
- if (IS_ERR_OR_NULL(genpd)) {
- ret = -EINVAL;
- goto err;
- }
-
- if (!st || (st_count < 1)) {
- ret = -EINVAL;
- goto err;
- }
-
- /* Allocate the local memory to keep the states for this genpd */
- genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
- if (!genpd->states) {
- ret = -ENOMEM;
- goto err;
- }
-
- for (i = 0; i < st_count; i++) {
- genpd->states[i].power_on_latency_ns =
- st[i].power_on_latency_ns;
- genpd->states[i].power_off_latency_ns =
- st[i].power_off_latency_ns;
- }
-
- genpd->state_count = st_count;
-
- /* to save memory, Name allocation will happen if debug is enabled */
- pm_genpd_alloc_states_names(genpd, st, st_count);
-
-err:
- return ret;
-}
-
/**
* __pm_genpd_add_device - Add a device to an I/O PM domain.
* @genpd: PM domain to add the device to.
@@ -1833,6 +1788,75 @@ static void genpd_lock_init(struct generic_pm_domain *genpd)
}
}
+
+/*
+* state depth comparison function.
+*/
+static int state_cmp(const void *a, const void *b)
+{
+ struct genpd_power_state *state_a = (struct genpd_power_state *)(a);
+ struct genpd_power_state *state_b = (struct genpd_power_state *)(b);
+
+ s64 depth_a =
+ state_a->power_on_latency_ns + state_a->power_off_latency_ns;
+ s64 depth_b =
+ state_b->power_on_latency_ns + state_b->power_off_latency_ns;
+
+ return (depth_a > depth_b) ? 0 : -1;
+}
+
+/*
+* TODO: antagonist routine.
+*/
+int pm_genpd_insert_state(struct generic_pm_domain *genpd,
+ const struct genpd_power_state *state)
+{
+ int ret = 0;
+ int state_count = genpd->state_count;
+
+ if (IS_ERR_OR_NULL(genpd) || (!state))
+ ret = -EINVAL;
+
+ if (state_count >= GENPD_POWER_STATES_MAX)
+ ret = -ENOMEM;
+
+#ifdef CONFIG_PM_ADVANCED_DEBUG
+ /* to save memory, Name allocation will happen if debug is enabled */
+ genpd->states[state_count].name = kstrndup(state->name,
+ GENPD_MAX_NAME_SIZE,
+ GFP_KERNEL);
+ if (!genpd->states[state_count].name) {
+ pr_err("%s Failed to allocate state '%s' name.\n",
+ genpd->name, state->name);
+ ret = -ENOMEM;
+ }
+#endif
+ genpd_lock(genpd);
+
+ if (!ret) {
+ genpd->states[state_count].power_on_latency_ns =
+ state->power_on_latency_ns;
+ genpd->states[state_count].power_off_latency_ns =
+ state->power_off_latency_ns;
+ genpd->state_count++;
+ }
+
+ /* sort from shallowest to deepest */
+ sort(genpd->states, genpd->state_count,
+ sizeof(genpd->states[0]), state_cmp, NULL /*generic swap */);
+
+ /* Sanity check for current state index */
+ if (genpd->state_idx >= genpd->state_count) {
+ pr_warn("pm domain %s Invalid initial state.\n", genpd->name);
+ genpd->state_idx = genpd->state_count - 1;
+ }
+
+ genpd_unlock(genpd);
+
+ return ret;
+}
+
+
/**
* pm_genpd_init - Initialize a generic I/O PM domain object.
* @genpd: PM domain object to initialize.
@@ -1846,36 +1870,24 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
const struct genpd_power_state *states,
unsigned int state_count, bool is_off)
{
- static const struct genpd_power_state genpd_default_state[] = {
- {
- .name = "OFF",
- .power_off_latency_ns = 0,
- .power_on_latency_ns = 0,
- },
- };
- int ret;
+ int i;
if (IS_ERR_OR_NULL(genpd))
return;
- /* If no states defined, use the default OFF state */
- if (!states || (state_count < 1))
- ret = genpd_alloc_states_data(genpd, genpd_default_state,
- ARRAY_SIZE(genpd_default_state));
- else
- ret = genpd_alloc_states_data(genpd, states, state_count);
-
- if (ret) {
- pr_err("Fail to allocate states for %s\n", genpd->name);
- return;
- }
+ /* simply use an array, we wish to add/remove new retention states
+ from later device init/exit. */
+ memset(genpd->states, 0, GENPD_POWER_STATES_MAX
+ * sizeof(struct genpd_power_state));
- /* Sanity check for initial state */
- if (genpd->state_idx >= genpd->state_count) {
- pr_warn("pm domain %s Invalid initial state.\n",
- genpd->name);
- genpd->state_idx = genpd->state_count - 1;
- }
+ if (!states || !state_count) {
+ /* require a provider for a default state */
+ genpd->state_count = 0;
+ genpd->state_idx = 0;
+ } else
+ for (i = 0; i < state_count; i++)
+ if (pm_genpd_insert_state(genpd, &states[i]))
+ return;
INIT_LIST_HEAD(&genpd->master_links);
INIT_LIST_HEAD(&genpd->slave_links);
@@ -2233,33 +2245,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
#include <linux/kobject.h>
static struct dentry *pm_genpd_debugfs_dir;
-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count)
-{
- unsigned int i;
-
- if (IS_ERR_OR_NULL(genpd))
- return -EINVAL;
-
- if (genpd->state_count != st_count) {
- pr_err("Invalid allocated state count\n");
- return -EINVAL;
- }
-
- for (i = 0; i < st_count; i++) {
- genpd->states[i].name = kstrndup(st[i].name,
- GENPD_MAX_NAME_SIZE, GFP_KERNEL);
- if (!genpd->states[i].name) {
- pr_err("%s Failed to allocate state %d name.\n",
- genpd->name, i);
- return -ENOMEM;
- }
- }
-
- return 0;
-}
-
/*
* TODO: This function is a slightly modified version of rtpm_status_show
* from sysfs.c, so generalize it.
@@ -2398,12 +2383,4 @@ static void __exit pm_genpd_debug_exit(void)
{
debugfs_remove_recursive(pm_genpd_debugfs_dir);
}
-__exitcall(pm_genpd_debug_exit);
-#else
-static inline int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count)
-{
- return 0;
-}
#endif /* CONFIG_PM_ADVANCED_DEBUG */
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9d37292..8a4eab0 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -45,6 +45,13 @@ struct gpd_cpuidle_data {
struct cpuidle_state *idle_state;
};
+
+/* Arbitrary max number of devices registering a special
+ * retention state with the PD, to keep things simple.
+ */
+#define GENPD_POWER_STATES_MAX 12
+#define GENPD_MAX_NAME_SIZE 40
+
struct genpd_power_state {
char *name;
s64 power_off_latency_ns;
@@ -80,7 +87,8 @@ struct generic_pm_domain {
struct device *dev);
unsigned int flags; /* Bit field of configs for genpd */
- struct genpd_power_state *states;
+ struct genpd_power_state states[GENPD_POWER_STATES_MAX];
+
unsigned int state_count; /* number of states */
unsigned int state_idx; /* state that genpd will go to when off */
@@ -159,10 +167,12 @@ extern int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state);
extern int pm_genpd_name_attach_cpuidle(const char *name, int state);
extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd);
extern int pm_genpd_name_detach_cpuidle(const char *name);
+extern int pm_genpd_insert_state(struct generic_pm_domain *genpd,
+ const struct genpd_power_state *state);
extern void pm_genpd_init(struct generic_pm_domain *genpd,
- struct dev_power_governor *gov,
- const struct genpd_power_state *states,
- unsigned int state_count, bool is_off);
+ struct dev_power_governor *gov,
+ const struct genpd_power_state *states,
+ unsigned int state_count, bool is_off);
extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
extern int pm_genpd_name_poweron(const char *domain_name);
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state
2015-10-06 14:27 ` [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Marc Titinger
@ 2015-10-08 16:11 ` Lina Iyer
2015-10-09 9:39 ` Marc Titinger
0 siblings, 1 reply; 39+ messages in thread
From: Lina Iyer @ 2015-10-08 16:11 UTC (permalink / raw)
To: Marc Titinger
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Hi Marc,
Thanks for rebasing on top of my latest series.
On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>Devices may register an intermediate retention state into the domain upon
>
I may agree with the usability of dynamic adding a state to the domain,
but I dont see why a device attaching to a domain should bring about a
new domain state.
A domain should define its power states, independent of the devices that
may attach. The way I see it, devices have their own idle states and
domains have their own. I do see a relationship between possible domain
states depending on the states of the individual devices in the domain.
For ex, a CPU domain can only be in a retention state (low voltage,
memory retained), if its CPU devices are in retention state, i.e, the
domain cannot be powered off; alternately, the domain may be in
retention or power down if the CPU devices are in power down state.
Could you elaborate on why this is a need?
Thanks,
Lina
>attaching. Currently generic domain would register an array of states upon
>init. This patch prepares for later insertion (sort per depth, remove).
>
>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>---
> drivers/base/power/domain.c | 189 +++++++++++++++++++-------------------------
> include/linux/pm_domain.h | 18 ++++-
> 2 files changed, 97 insertions(+), 110 deletions(-)
>
>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>index 3e27a2b..e5f4c00b 100644
>--- a/drivers/base/power/domain.c
>+++ b/drivers/base/power/domain.c
>@@ -19,6 +19,7 @@
> #include <linux/sched.h>
> #include <linux/suspend.h>
> #include <linux/export.h>
>+#include <linux/sort.h>
>
> #define GENPD_RETRY_MAX_MS 250 /* Approximate */
>
>@@ -50,12 +51,6 @@
> __retval; \
> })
>
>-#define GENPD_MAX_NAME_SIZE 20
>-
>-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>- const struct genpd_power_state *st,
>- unsigned int st_count);
>-
> static LIST_HEAD(gpd_list);
> static DEFINE_MUTEX(gpd_list_lock);
>
>@@ -1364,46 +1359,6 @@ static void genpd_free_dev_data(struct device *dev,
> dev_pm_put_subsys_data(dev);
> }
>
>-static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
>- const struct genpd_power_state *st,
>- unsigned int st_count)
>-{
>- int ret = 0;
>- unsigned int i;
>-
>- if (IS_ERR_OR_NULL(genpd)) {
>- ret = -EINVAL;
>- goto err;
>- }
>-
>- if (!st || (st_count < 1)) {
>- ret = -EINVAL;
>- goto err;
>- }
>-
>- /* Allocate the local memory to keep the states for this genpd */
>- genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
>- if (!genpd->states) {
>- ret = -ENOMEM;
>- goto err;
>- }
>-
>- for (i = 0; i < st_count; i++) {
>- genpd->states[i].power_on_latency_ns =
>- st[i].power_on_latency_ns;
>- genpd->states[i].power_off_latency_ns =
>- st[i].power_off_latency_ns;
>- }
>-
>- genpd->state_count = st_count;
>-
>- /* to save memory, Name allocation will happen if debug is enabled */
>- pm_genpd_alloc_states_names(genpd, st, st_count);
>-
>-err:
>- return ret;
>-}
>-
> /**
> * __pm_genpd_add_device - Add a device to an I/O PM domain.
> * @genpd: PM domain to add the device to.
>@@ -1833,6 +1788,75 @@ static void genpd_lock_init(struct generic_pm_domain *genpd)
> }
> }
>
>+
>+/*
>+* state depth comparison function.
>+*/
>+static int state_cmp(const void *a, const void *b)
>+{
>+ struct genpd_power_state *state_a = (struct genpd_power_state *)(a);
>+ struct genpd_power_state *state_b = (struct genpd_power_state *)(b);
>+
>+ s64 depth_a =
>+ state_a->power_on_latency_ns + state_a->power_off_latency_ns;
>+ s64 depth_b =
>+ state_b->power_on_latency_ns + state_b->power_off_latency_ns;
>+
>+ return (depth_a > depth_b) ? 0 : -1;
>+}
>+
>+/*
>+* TODO: antagonist routine.
>+*/
>+int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>+ const struct genpd_power_state *state)
>+{
>+ int ret = 0;
>+ int state_count = genpd->state_count;
>+
>+ if (IS_ERR_OR_NULL(genpd) || (!state))
>+ ret = -EINVAL;
>+
>+ if (state_count >= GENPD_POWER_STATES_MAX)
>+ ret = -ENOMEM;
>+
>+#ifdef CONFIG_PM_ADVANCED_DEBUG
>+ /* to save memory, Name allocation will happen if debug is enabled */
>+ genpd->states[state_count].name = kstrndup(state->name,
>+ GENPD_MAX_NAME_SIZE,
>+ GFP_KERNEL);
>+ if (!genpd->states[state_count].name) {
>+ pr_err("%s Failed to allocate state '%s' name.\n",
>+ genpd->name, state->name);
>+ ret = -ENOMEM;
>+ }
>+#endif
>+ genpd_lock(genpd);
>+
>+ if (!ret) {
>+ genpd->states[state_count].power_on_latency_ns =
>+ state->power_on_latency_ns;
>+ genpd->states[state_count].power_off_latency_ns =
>+ state->power_off_latency_ns;
>+ genpd->state_count++;
>+ }
>+
>+ /* sort from shallowest to deepest */
>+ sort(genpd->states, genpd->state_count,
>+ sizeof(genpd->states[0]), state_cmp, NULL /*generic swap */);
>+
>+ /* Sanity check for current state index */
>+ if (genpd->state_idx >= genpd->state_count) {
>+ pr_warn("pm domain %s Invalid initial state.\n", genpd->name);
>+ genpd->state_idx = genpd->state_count - 1;
>+ }
>+
>+ genpd_unlock(genpd);
>+
>+ return ret;
>+}
>+
>+
> /**
> * pm_genpd_init - Initialize a generic I/O PM domain object.
> * @genpd: PM domain object to initialize.
>@@ -1846,36 +1870,24 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
> const struct genpd_power_state *states,
> unsigned int state_count, bool is_off)
> {
>- static const struct genpd_power_state genpd_default_state[] = {
>- {
>- .name = "OFF",
>- .power_off_latency_ns = 0,
>- .power_on_latency_ns = 0,
>- },
>- };
>- int ret;
>+ int i;
>
> if (IS_ERR_OR_NULL(genpd))
> return;
>
>- /* If no states defined, use the default OFF state */
>- if (!states || (state_count < 1))
>- ret = genpd_alloc_states_data(genpd, genpd_default_state,
>- ARRAY_SIZE(genpd_default_state));
>- else
>- ret = genpd_alloc_states_data(genpd, states, state_count);
>-
>- if (ret) {
>- pr_err("Fail to allocate states for %s\n", genpd->name);
>- return;
>- }
>+ /* simply use an array, we wish to add/remove new retention states
>+ from later device init/exit. */
>+ memset(genpd->states, 0, GENPD_POWER_STATES_MAX
>+ * sizeof(struct genpd_power_state));
>
>- /* Sanity check for initial state */
>- if (genpd->state_idx >= genpd->state_count) {
>- pr_warn("pm domain %s Invalid initial state.\n",
>- genpd->name);
>- genpd->state_idx = genpd->state_count - 1;
>- }
>+ if (!states || !state_count) {
>+ /* require a provider for a default state */
>+ genpd->state_count = 0;
>+ genpd->state_idx = 0;
>+ } else
>+ for (i = 0; i < state_count; i++)
>+ if (pm_genpd_insert_state(genpd, &states[i]))
>+ return;
>
> INIT_LIST_HEAD(&genpd->master_links);
> INIT_LIST_HEAD(&genpd->slave_links);
>@@ -2233,33 +2245,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> #include <linux/kobject.h>
> static struct dentry *pm_genpd_debugfs_dir;
>
>-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>- const struct genpd_power_state *st,
>- unsigned int st_count)
>-{
>- unsigned int i;
>-
>- if (IS_ERR_OR_NULL(genpd))
>- return -EINVAL;
>-
>- if (genpd->state_count != st_count) {
>- pr_err("Invalid allocated state count\n");
>- return -EINVAL;
>- }
>-
>- for (i = 0; i < st_count; i++) {
>- genpd->states[i].name = kstrndup(st[i].name,
>- GENPD_MAX_NAME_SIZE, GFP_KERNEL);
>- if (!genpd->states[i].name) {
>- pr_err("%s Failed to allocate state %d name.\n",
>- genpd->name, i);
>- return -ENOMEM;
>- }
>- }
>-
>- return 0;
>-}
>-
> /*
> * TODO: This function is a slightly modified version of rtpm_status_show
> * from sysfs.c, so generalize it.
>@@ -2398,12 +2383,4 @@ static void __exit pm_genpd_debug_exit(void)
> {
> debugfs_remove_recursive(pm_genpd_debugfs_dir);
> }
>-__exitcall(pm_genpd_debug_exit);
>-#else
>-static inline int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>- const struct genpd_power_state *st,
>- unsigned int st_count)
>-{
>- return 0;
>-}
> #endif /* CONFIG_PM_ADVANCED_DEBUG */
>diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>index 9d37292..8a4eab0 100644
>--- a/include/linux/pm_domain.h
>+++ b/include/linux/pm_domain.h
>@@ -45,6 +45,13 @@ struct gpd_cpuidle_data {
> struct cpuidle_state *idle_state;
> };
>
>+
>+/* Arbitrary max number of devices registering a special
>+ * retention state with the PD, to keep things simple.
>+ */
>+#define GENPD_POWER_STATES_MAX 12
>+#define GENPD_MAX_NAME_SIZE 40
>+
> struct genpd_power_state {
> char *name;
> s64 power_off_latency_ns;
>@@ -80,7 +87,8 @@ struct generic_pm_domain {
> struct device *dev);
> unsigned int flags; /* Bit field of configs for genpd */
>
>- struct genpd_power_state *states;
>+ struct genpd_power_state states[GENPD_POWER_STATES_MAX];
>+
> unsigned int state_count; /* number of states */
> unsigned int state_idx; /* state that genpd will go to when off */
>
>@@ -159,10 +167,12 @@ extern int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state);
> extern int pm_genpd_name_attach_cpuidle(const char *name, int state);
> extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd);
> extern int pm_genpd_name_detach_cpuidle(const char *name);
>+extern int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>+ const struct genpd_power_state *state);
> extern void pm_genpd_init(struct generic_pm_domain *genpd,
>- struct dev_power_governor *gov,
>- const struct genpd_power_state *states,
>- unsigned int state_count, bool is_off);
>+ struct dev_power_governor *gov,
>+ const struct genpd_power_state *states,
>+ unsigned int state_count, bool is_off);
>
> extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
> extern int pm_genpd_name_poweron(const char *domain_name);
>--
>1.9.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state
2015-10-08 16:11 ` Lina Iyer
@ 2015-10-09 9:39 ` Marc Titinger
2015-10-09 18:22 ` Lina Iyer
0 siblings, 1 reply; 39+ messages in thread
From: Marc Titinger @ 2015-10-09 9:39 UTC (permalink / raw)
To: Lina Iyer
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
On 08/10/2015 18:11, Lina Iyer wrote:
> Hi Marc,
>
> Thanks for rebasing on top of my latest series.
>
> On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>> Devices may register an intermediate retention state into the domain upon
>>
> I may agree with the usability of dynamic adding a state to the domain,
> but I dont see why a device attaching to a domain should bring about a
> new domain state.
Hi Lina,
thanks a lot for taking the time to look into this. The initial
discussion behind it was about to see how a device like a PMU, FPU,
cache, or a Healthcheck IP in the same power domain than CPUs, with
special retention states can be handled in a way 'unified' with CPUs.
Also I admit it is partly an attempt from us to create something useful
out of the "collision" of Axel's multiple states and your
CPUs-as-generic-power-domain-devices, hence the RFC!
Looking at Juno for instance, she currently has a platform-initiated
mode implemented in the arm-trusted-firmware through psci as a
cpuidle-driver. the menu governor will select a possibly deep c-state,
but the last-man CPU and actual power state is known to ATF. Similarly
my idea was to have a genpd-initiated mode so to say, where the actual
power state results from the cpu-domain's governor choice based on
possible retention states, and their latency.
A Health-check IP or Cache will not (to my current knowledge) have a
driver calling runtime_put, but may have a retention state "D1_RET" with
a off/on latency between CPU_SLEEP and CLUSTER_SLEEP, so that
CLUSTER_SLEEP may be ruled out by the governor, but D1_RET is ok given
the time slot available. Some platform code can be called so that the
cluster goes to D1_RET in that case, upon the last-man CPU
waiting-for-interrupt. Note that in the case of a Health-Check HIP, the
state my actually be a working state (all CPUs power down, and time slot
OK for sampling stuff).
>
> A domain should define its power states, independent of the devices that
> may attach. The way I see it, devices have their own idle states and
> domains have their own. I do see a relationship between possible domain
> states depending on the states of the individual devices in the domain.
> For ex, a CPU domain can only be in a retention state (low voltage,
> memory retained), if its CPU devices are in retention state, i.e, the
> domain cannot be powered off; alternately, the domain may be in
> retention or power down if the CPU devices are in power down state.
>
> Could you elaborate on why this is a need?
Well, it may not be a need TBH, it is an attempt to have cluster-level
devices act like hotplugged CPUs but with heterogeneous c-states and
latencies. I hope it makes some sense :)
Thanks,
Marc.
>
> Thanks,
> Lina
>
>> attaching. Currently generic domain would register an array of states
>> upon
>> init. This patch prepares for later insertion (sort per depth, remove).
>>
>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>> ---
>> drivers/base/power/domain.c | 189
>> +++++++++++++++++++-------------------------
>> include/linux/pm_domain.h | 18 ++++-
>> 2 files changed, 97 insertions(+), 110 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 3e27a2b..e5f4c00b 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -19,6 +19,7 @@
>> #include <linux/sched.h>
>> #include <linux/suspend.h>
>> #include <linux/export.h>
>> +#include <linux/sort.h>
>>
>> #define GENPD_RETRY_MAX_MS 250 /* Approximate */
>>
>> @@ -50,12 +51,6 @@
>> __retval; \
>> })
>>
>> -#define GENPD_MAX_NAME_SIZE 20
>> -
>> -static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>> - const struct genpd_power_state *st,
>> - unsigned int st_count);
>> -
>> static LIST_HEAD(gpd_list);
>> static DEFINE_MUTEX(gpd_list_lock);
>>
>> @@ -1364,46 +1359,6 @@ static void genpd_free_dev_data(struct device
>> *dev,
>> dev_pm_put_subsys_data(dev);
>> }
>>
>> -static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
>> - const struct genpd_power_state *st,
>> - unsigned int st_count)
>> -{
>> - int ret = 0;
>> - unsigned int i;
>> -
>> - if (IS_ERR_OR_NULL(genpd)) {
>> - ret = -EINVAL;
>> - goto err;
>> - }
>> -
>> - if (!st || (st_count < 1)) {
>> - ret = -EINVAL;
>> - goto err;
>> - }
>> -
>> - /* Allocate the local memory to keep the states for this genpd */
>> - genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
>> - if (!genpd->states) {
>> - ret = -ENOMEM;
>> - goto err;
>> - }
>> -
>> - for (i = 0; i < st_count; i++) {
>> - genpd->states[i].power_on_latency_ns =
>> - st[i].power_on_latency_ns;
>> - genpd->states[i].power_off_latency_ns =
>> - st[i].power_off_latency_ns;
>> - }
>> -
>> - genpd->state_count = st_count;
>> -
>> - /* to save memory, Name allocation will happen if debug is
>> enabled */
>> - pm_genpd_alloc_states_names(genpd, st, st_count);
>> -
>> -err:
>> - return ret;
>> -}
>> -
>> /**
>> * __pm_genpd_add_device - Add a device to an I/O PM domain.
>> * @genpd: PM domain to add the device to.
>> @@ -1833,6 +1788,75 @@ static void genpd_lock_init(struct
>> generic_pm_domain *genpd)
>> }
>> }
>>
>> +
>> +/*
>> +* state depth comparison function.
>> +*/
>> +static int state_cmp(const void *a, const void *b)
>> +{
>> + struct genpd_power_state *state_a = (struct genpd_power_state *)(a);
>> + struct genpd_power_state *state_b = (struct genpd_power_state *)(b);
>> +
>> + s64 depth_a =
>> + state_a->power_on_latency_ns + state_a->power_off_latency_ns;
>> + s64 depth_b =
>> + state_b->power_on_latency_ns + state_b->power_off_latency_ns;
>> +
>> + return (depth_a > depth_b) ? 0 : -1;
>> +}
>> +
>> +/*
>> +* TODO: antagonist routine.
>> +*/
>> +int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>> + const struct genpd_power_state *state)
>> +{
>> + int ret = 0;
>> + int state_count = genpd->state_count;
>> +
>> + if (IS_ERR_OR_NULL(genpd) || (!state))
>> + ret = -EINVAL;
>> +
>> + if (state_count >= GENPD_POWER_STATES_MAX)
>> + ret = -ENOMEM;
>> +
>> +#ifdef CONFIG_PM_ADVANCED_DEBUG
>> + /* to save memory, Name allocation will happen if debug is
>> enabled */
>> + genpd->states[state_count].name = kstrndup(state->name,
>> + GENPD_MAX_NAME_SIZE,
>> + GFP_KERNEL);
>> + if (!genpd->states[state_count].name) {
>> + pr_err("%s Failed to allocate state '%s' name.\n",
>> + genpd->name, state->name);
>> + ret = -ENOMEM;
>> + }
>> +#endif
>> + genpd_lock(genpd);
>> +
>> + if (!ret) {
>> + genpd->states[state_count].power_on_latency_ns =
>> + state->power_on_latency_ns;
>> + genpd->states[state_count].power_off_latency_ns =
>> + state->power_off_latency_ns;
>> + genpd->state_count++;
>> + }
>> +
>> + /* sort from shallowest to deepest */
>> + sort(genpd->states, genpd->state_count,
>> + sizeof(genpd->states[0]), state_cmp, NULL /*generic swap */);
>> +
>> + /* Sanity check for current state index */
>> + if (genpd->state_idx >= genpd->state_count) {
>> + pr_warn("pm domain %s Invalid initial state.\n", genpd->name);
>> + genpd->state_idx = genpd->state_count - 1;
>> + }
>> +
>> + genpd_unlock(genpd);
>> +
>> + return ret;
>> +}
>> +
>> +
>> /**
>> * pm_genpd_init - Initialize a generic I/O PM domain object.
>> * @genpd: PM domain object to initialize.
>> @@ -1846,36 +1870,24 @@ void pm_genpd_init(struct generic_pm_domain
>> *genpd,
>> const struct genpd_power_state *states,
>> unsigned int state_count, bool is_off)
>> {
>> - static const struct genpd_power_state genpd_default_state[] = {
>> - {
>> - .name = "OFF",
>> - .power_off_latency_ns = 0,
>> - .power_on_latency_ns = 0,
>> - },
>> - };
>> - int ret;
>> + int i;
>>
>> if (IS_ERR_OR_NULL(genpd))
>> return;
>>
>> - /* If no states defined, use the default OFF state */
>> - if (!states || (state_count < 1))
>> - ret = genpd_alloc_states_data(genpd, genpd_default_state,
>> - ARRAY_SIZE(genpd_default_state));
>> - else
>> - ret = genpd_alloc_states_data(genpd, states, state_count);
>> -
>> - if (ret) {
>> - pr_err("Fail to allocate states for %s\n", genpd->name);
>> - return;
>> - }
>> + /* simply use an array, we wish to add/remove new retention states
>> + from later device init/exit. */
>> + memset(genpd->states, 0, GENPD_POWER_STATES_MAX
>> + * sizeof(struct genpd_power_state));
>>
>> - /* Sanity check for initial state */
>> - if (genpd->state_idx >= genpd->state_count) {
>> - pr_warn("pm domain %s Invalid initial state.\n",
>> - genpd->name);
>> - genpd->state_idx = genpd->state_count - 1;
>> - }
>> + if (!states || !state_count) {
>> + /* require a provider for a default state */
>> + genpd->state_count = 0;
>> + genpd->state_idx = 0;
>> + } else
>> + for (i = 0; i < state_count; i++)
>> + if (pm_genpd_insert_state(genpd, &states[i]))
>> + return;
>>
>> INIT_LIST_HEAD(&genpd->master_links);
>> INIT_LIST_HEAD(&genpd->slave_links);
>> @@ -2233,33 +2245,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>> #include <linux/kobject.h>
>> static struct dentry *pm_genpd_debugfs_dir;
>>
>> -static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>> - const struct genpd_power_state *st,
>> - unsigned int st_count)
>> -{
>> - unsigned int i;
>> -
>> - if (IS_ERR_OR_NULL(genpd))
>> - return -EINVAL;
>> -
>> - if (genpd->state_count != st_count) {
>> - pr_err("Invalid allocated state count\n");
>> - return -EINVAL;
>> - }
>> -
>> - for (i = 0; i < st_count; i++) {
>> - genpd->states[i].name = kstrndup(st[i].name,
>> - GENPD_MAX_NAME_SIZE, GFP_KERNEL);
>> - if (!genpd->states[i].name) {
>> - pr_err("%s Failed to allocate state %d name.\n",
>> - genpd->name, i);
>> - return -ENOMEM;
>> - }
>> - }
>> -
>> - return 0;
>> -}
>> -
>> /*
>> * TODO: This function is a slightly modified version of rtpm_status_show
>> * from sysfs.c, so generalize it.
>> @@ -2398,12 +2383,4 @@ static void __exit pm_genpd_debug_exit(void)
>> {
>> debugfs_remove_recursive(pm_genpd_debugfs_dir);
>> }
>> -__exitcall(pm_genpd_debug_exit);
>> -#else
>> -static inline int pm_genpd_alloc_states_names(struct
>> generic_pm_domain *genpd,
>> - const struct genpd_power_state *st,
>> - unsigned int st_count)
>> -{
>> - return 0;
>> -}
>> #endif /* CONFIG_PM_ADVANCED_DEBUG */
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 9d37292..8a4eab0 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -45,6 +45,13 @@ struct gpd_cpuidle_data {
>> struct cpuidle_state *idle_state;
>> };
>>
>> +
>> +/* Arbitrary max number of devices registering a special
>> + * retention state with the PD, to keep things simple.
>> + */
>> +#define GENPD_POWER_STATES_MAX 12
>> +#define GENPD_MAX_NAME_SIZE 40
>> +
>> struct genpd_power_state {
>> char *name;
>> s64 power_off_latency_ns;
>> @@ -80,7 +87,8 @@ struct generic_pm_domain {
>> struct device *dev);
>> unsigned int flags; /* Bit field of configs for genpd */
>>
>> - struct genpd_power_state *states;
>> + struct genpd_power_state states[GENPD_POWER_STATES_MAX];
>> +
>> unsigned int state_count; /* number of states */
>> unsigned int state_idx; /* state that genpd will go to when off */
>>
>> @@ -159,10 +167,12 @@ extern int pm_genpd_attach_cpuidle(struct
>> generic_pm_domain *genpd, int state);
>> extern int pm_genpd_name_attach_cpuidle(const char *name, int state);
>> extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd);
>> extern int pm_genpd_name_detach_cpuidle(const char *name);
>> +extern int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>> + const struct genpd_power_state *state);
>> extern void pm_genpd_init(struct generic_pm_domain *genpd,
>> - struct dev_power_governor *gov,
>> - const struct genpd_power_state *states,
>> - unsigned int state_count, bool is_off);
>> + struct dev_power_governor *gov,
>> + const struct genpd_power_state *states,
>> + unsigned int state_count, bool is_off);
>>
>> extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
>> extern int pm_genpd_name_poweron(const char *domain_name);
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state
2015-10-09 9:39 ` Marc Titinger
@ 2015-10-09 18:22 ` Lina Iyer
2015-10-13 10:29 ` Marc Titinger
0 siblings, 1 reply; 39+ messages in thread
From: Lina Iyer @ 2015-10-09 18:22 UTC (permalink / raw)
To: Marc Titinger
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
On Fri, Oct 09 2015 at 03:39 -0600, Marc Titinger wrote:
>On 08/10/2015 18:11, Lina Iyer wrote:
>>Hi Marc,
>>
>>Thanks for rebasing on top of my latest series.
>>
>>On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>>>Devices may register an intermediate retention state into the domain upon
>>>
>>I may agree with the usability of dynamic adding a state to the domain,
>>but I dont see why a device attaching to a domain should bring about a
>>new domain state.
>
>Hi Lina,
>
>thanks a lot for taking the time to look into this. The initial
>discussion behind it was about to see how a device like a PMU, FPU,
>cache, or a Healthcheck IP in the same power domain than CPUs, with
>special retention states can be handled in a way 'unified' with CPUs.
>Also I admit it is partly an attempt from us to create something
>useful out of the "collision" of Axel's multiple states and your
>CPUs-as-generic-power-domain-devices, hence the RFC!
>
>Looking at Juno for instance, she currently has a platform-initiated
>mode implemented in the arm-trusted-firmware through psci as a
>cpuidle-driver. the menu governor will select a possibly deep c-state,
>but the last-man CPU and actual power state is known to ATF. Similarly
>my idea was to have a genpd-initiated mode so to say, where the actual
>power state results from the cpu-domain's governor choice based on
>possible retention states, and their latency.
>
Okay, I must admit that my ideas are quite partial to OS-initiated PSCI
(v1.0 onwards). So you have C-States that allow domains to enter idle as
well. Fair.
>A Health-check IP or Cache will not (to my current knowledge) have a
>driver calling runtime_put, but may have a retention state "D1_RET"
>with a off/on latency between CPU_SLEEP and CLUSTER_SLEEP, so that
>CLUSTER_SLEEP may be ruled out by the governor, but D1_RET is ok given
>the time slot available.
>
A couple of questions here.
You say there is no driver for HIP, is there a device for it atleast?
How is the CPU/Domain going to know if the HIP is running or not?
To me this seems like you want to set a QoS on the PM domain here.
>Some platform code can be called so that the
>cluster goes to D1_RET in that case, upon the last-man CPU
>waiting-for-interrupt. Note that in the case of a Health-Check HIP,
>the state my actually be a working state (all CPUs power down, and
>time slot OK for sampling stuff).
>
Building on my previous statement, if you have a QoS for a domain and a
domain governor, it could consider the QoS requirement and choose to do
retention. However that needs a driver or some entity that know the HIP
is requesting a D1_RET mode only.
>>
>>A domain should define its power states, independent of the devices that
>>may attach. The way I see it, devices have their own idle states and
>>domains have their own. I do see a relationship between possible domain
>>states depending on the states of the individual devices in the domain.
>>For ex, a CPU domain can only be in a retention state (low voltage,
>>memory retained), if its CPU devices are in retention state, i.e, the
>>domain cannot be powered off; alternately, the domain may be in
>>retention or power down if the CPU devices are in power down state.
>>
>>Could you elaborate on why this is a need?
>
>Well, it may not be a need TBH, it is an attempt to have cluster-level
>devices act like hotplugged CPUs but with heterogeneous c-states and
>latencies. I hope it makes some sense :)
>
Hmm.. Let me think about it.
What would be a difference between a cluster-level device and a CPU in
the same domain?
-- Lina
>Thanks,
>Marc.
>
>
>>
>>Thanks,
>>Lina
>>
>>>attaching. Currently generic domain would register an array of states
>>>upon
>>>init. This patch prepares for later insertion (sort per depth, remove).
>>>
>>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>>---
>>>drivers/base/power/domain.c | 189
>>>+++++++++++++++++++-------------------------
>>>include/linux/pm_domain.h | 18 ++++-
>>>2 files changed, 97 insertions(+), 110 deletions(-)
>>>
>>>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>>index 3e27a2b..e5f4c00b 100644
>>>--- a/drivers/base/power/domain.c
>>>+++ b/drivers/base/power/domain.c
>>>@@ -19,6 +19,7 @@
>>>#include <linux/sched.h>
>>>#include <linux/suspend.h>
>>>#include <linux/export.h>
>>>+#include <linux/sort.h>
>>>
>>>#define GENPD_RETRY_MAX_MS 250 /* Approximate */
>>>
>>>@@ -50,12 +51,6 @@
>>> __retval; \
>>>})
>>>
>>>-#define GENPD_MAX_NAME_SIZE 20
>>>-
>>>-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>>>- const struct genpd_power_state *st,
>>>- unsigned int st_count);
>>>-
>>>static LIST_HEAD(gpd_list);
>>>static DEFINE_MUTEX(gpd_list_lock);
>>>
>>>@@ -1364,46 +1359,6 @@ static void genpd_free_dev_data(struct device
>>>*dev,
>>> dev_pm_put_subsys_data(dev);
>>>}
>>>
>>>-static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
>>>- const struct genpd_power_state *st,
>>>- unsigned int st_count)
>>>-{
>>>- int ret = 0;
>>>- unsigned int i;
>>>-
>>>- if (IS_ERR_OR_NULL(genpd)) {
>>>- ret = -EINVAL;
>>>- goto err;
>>>- }
>>>-
>>>- if (!st || (st_count < 1)) {
>>>- ret = -EINVAL;
>>>- goto err;
>>>- }
>>>-
>>>- /* Allocate the local memory to keep the states for this genpd */
>>>- genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
>>>- if (!genpd->states) {
>>>- ret = -ENOMEM;
>>>- goto err;
>>>- }
>>>-
>>>- for (i = 0; i < st_count; i++) {
>>>- genpd->states[i].power_on_latency_ns =
>>>- st[i].power_on_latency_ns;
>>>- genpd->states[i].power_off_latency_ns =
>>>- st[i].power_off_latency_ns;
>>>- }
>>>-
>>>- genpd->state_count = st_count;
>>>-
>>>- /* to save memory, Name allocation will happen if debug is
>>>enabled */
>>>- pm_genpd_alloc_states_names(genpd, st, st_count);
>>>-
>>>-err:
>>>- return ret;
>>>-}
>>>-
>>>/**
>>> * __pm_genpd_add_device - Add a device to an I/O PM domain.
>>> * @genpd: PM domain to add the device to.
>>>@@ -1833,6 +1788,75 @@ static void genpd_lock_init(struct
>>>generic_pm_domain *genpd)
>>> }
>>>}
>>>
>>>+
>>>+/*
>>>+* state depth comparison function.
>>>+*/
>>>+static int state_cmp(const void *a, const void *b)
>>>+{
>>>+ struct genpd_power_state *state_a = (struct genpd_power_state *)(a);
>>>+ struct genpd_power_state *state_b = (struct genpd_power_state *)(b);
>>>+
>>>+ s64 depth_a =
>>>+ state_a->power_on_latency_ns + state_a->power_off_latency_ns;
>>>+ s64 depth_b =
>>>+ state_b->power_on_latency_ns + state_b->power_off_latency_ns;
>>>+
>>>+ return (depth_a > depth_b) ? 0 : -1;
>>>+}
>>>+
>>>+/*
>>>+* TODO: antagonist routine.
>>>+*/
>>>+int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>>>+ const struct genpd_power_state *state)
>>>+{
>>>+ int ret = 0;
>>>+ int state_count = genpd->state_count;
>>>+
>>>+ if (IS_ERR_OR_NULL(genpd) || (!state))
>>>+ ret = -EINVAL;
>>>+
>>>+ if (state_count >= GENPD_POWER_STATES_MAX)
>>>+ ret = -ENOMEM;
>>>+
>>>+#ifdef CONFIG_PM_ADVANCED_DEBUG
>>>+ /* to save memory, Name allocation will happen if debug is
>>>enabled */
>>>+ genpd->states[state_count].name = kstrndup(state->name,
>>>+ GENPD_MAX_NAME_SIZE,
>>>+ GFP_KERNEL);
>>>+ if (!genpd->states[state_count].name) {
>>>+ pr_err("%s Failed to allocate state '%s' name.\n",
>>>+ genpd->name, state->name);
>>>+ ret = -ENOMEM;
>>>+ }
>>>+#endif
>>>+ genpd_lock(genpd);
>>>+
>>>+ if (!ret) {
>>>+ genpd->states[state_count].power_on_latency_ns =
>>>+ state->power_on_latency_ns;
>>>+ genpd->states[state_count].power_off_latency_ns =
>>>+ state->power_off_latency_ns;
>>>+ genpd->state_count++;
>>>+ }
>>>+
>>>+ /* sort from shallowest to deepest */
>>>+ sort(genpd->states, genpd->state_count,
>>>+ sizeof(genpd->states[0]), state_cmp, NULL /*generic swap */);
>>>+
>>>+ /* Sanity check for current state index */
>>>+ if (genpd->state_idx >= genpd->state_count) {
>>>+ pr_warn("pm domain %s Invalid initial state.\n", genpd->name);
>>>+ genpd->state_idx = genpd->state_count - 1;
>>>+ }
>>>+
>>>+ genpd_unlock(genpd);
>>>+
>>>+ return ret;
>>>+}
>>>+
>>>+
>>>/**
>>> * pm_genpd_init - Initialize a generic I/O PM domain object.
>>> * @genpd: PM domain object to initialize.
>>>@@ -1846,36 +1870,24 @@ void pm_genpd_init(struct generic_pm_domain
>>>*genpd,
>>> const struct genpd_power_state *states,
>>> unsigned int state_count, bool is_off)
>>>{
>>>- static const struct genpd_power_state genpd_default_state[] = {
>>>- {
>>>- .name = "OFF",
>>>- .power_off_latency_ns = 0,
>>>- .power_on_latency_ns = 0,
>>>- },
>>>- };
>>>- int ret;
>>>+ int i;
>>>
>>> if (IS_ERR_OR_NULL(genpd))
>>> return;
>>>
>>>- /* If no states defined, use the default OFF state */
>>>- if (!states || (state_count < 1))
>>>- ret = genpd_alloc_states_data(genpd, genpd_default_state,
>>>- ARRAY_SIZE(genpd_default_state));
>>>- else
>>>- ret = genpd_alloc_states_data(genpd, states, state_count);
>>>-
>>>- if (ret) {
>>>- pr_err("Fail to allocate states for %s\n", genpd->name);
>>>- return;
>>>- }
>>>+ /* simply use an array, we wish to add/remove new retention states
>>>+ from later device init/exit. */
>>>+ memset(genpd->states, 0, GENPD_POWER_STATES_MAX
>>>+ * sizeof(struct genpd_power_state));
>>>
>>>- /* Sanity check for initial state */
>>>- if (genpd->state_idx >= genpd->state_count) {
>>>- pr_warn("pm domain %s Invalid initial state.\n",
>>>- genpd->name);
>>>- genpd->state_idx = genpd->state_count - 1;
>>>- }
>>>+ if (!states || !state_count) {
>>>+ /* require a provider for a default state */
>>>+ genpd->state_count = 0;
>>>+ genpd->state_idx = 0;
>>>+ } else
>>>+ for (i = 0; i < state_count; i++)
>>>+ if (pm_genpd_insert_state(genpd, &states[i]))
>>>+ return;
>>>
>>> INIT_LIST_HEAD(&genpd->master_links);
>>> INIT_LIST_HEAD(&genpd->slave_links);
>>>@@ -2233,33 +2245,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>>#include <linux/kobject.h>
>>>static struct dentry *pm_genpd_debugfs_dir;
>>>
>>>-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
>>>- const struct genpd_power_state *st,
>>>- unsigned int st_count)
>>>-{
>>>- unsigned int i;
>>>-
>>>- if (IS_ERR_OR_NULL(genpd))
>>>- return -EINVAL;
>>>-
>>>- if (genpd->state_count != st_count) {
>>>- pr_err("Invalid allocated state count\n");
>>>- return -EINVAL;
>>>- }
>>>-
>>>- for (i = 0; i < st_count; i++) {
>>>- genpd->states[i].name = kstrndup(st[i].name,
>>>- GENPD_MAX_NAME_SIZE, GFP_KERNEL);
>>>- if (!genpd->states[i].name) {
>>>- pr_err("%s Failed to allocate state %d name.\n",
>>>- genpd->name, i);
>>>- return -ENOMEM;
>>>- }
>>>- }
>>>-
>>>- return 0;
>>>-}
>>>-
>>>/*
>>> * TODO: This function is a slightly modified version of rtpm_status_show
>>> * from sysfs.c, so generalize it.
>>>@@ -2398,12 +2383,4 @@ static void __exit pm_genpd_debug_exit(void)
>>>{
>>> debugfs_remove_recursive(pm_genpd_debugfs_dir);
>>>}
>>>-__exitcall(pm_genpd_debug_exit);
>>>-#else
>>>-static inline int pm_genpd_alloc_states_names(struct
>>>generic_pm_domain *genpd,
>>>- const struct genpd_power_state *st,
>>>- unsigned int st_count)
>>>-{
>>>- return 0;
>>>-}
>>>#endif /* CONFIG_PM_ADVANCED_DEBUG */
>>>diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>>index 9d37292..8a4eab0 100644
>>>--- a/include/linux/pm_domain.h
>>>+++ b/include/linux/pm_domain.h
>>>@@ -45,6 +45,13 @@ struct gpd_cpuidle_data {
>>> struct cpuidle_state *idle_state;
>>>};
>>>
>>>+
>>>+/* Arbitrary max number of devices registering a special
>>>+ * retention state with the PD, to keep things simple.
>>>+ */
>>>+#define GENPD_POWER_STATES_MAX 12
>>>+#define GENPD_MAX_NAME_SIZE 40
>>>+
>>>struct genpd_power_state {
>>> char *name;
>>> s64 power_off_latency_ns;
>>>@@ -80,7 +87,8 @@ struct generic_pm_domain {
>>> struct device *dev);
>>> unsigned int flags; /* Bit field of configs for genpd */
>>>
>>>- struct genpd_power_state *states;
>>>+ struct genpd_power_state states[GENPD_POWER_STATES_MAX];
>>>+
>>> unsigned int state_count; /* number of states */
>>> unsigned int state_idx; /* state that genpd will go to when off */
>>>
>>>@@ -159,10 +167,12 @@ extern int pm_genpd_attach_cpuidle(struct
>>>generic_pm_domain *genpd, int state);
>>>extern int pm_genpd_name_attach_cpuidle(const char *name, int state);
>>>extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd);
>>>extern int pm_genpd_name_detach_cpuidle(const char *name);
>>>+extern int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>>>+ const struct genpd_power_state *state);
>>>extern void pm_genpd_init(struct generic_pm_domain *genpd,
>>>- struct dev_power_governor *gov,
>>>- const struct genpd_power_state *states,
>>>- unsigned int state_count, bool is_off);
>>>+ struct dev_power_governor *gov,
>>>+ const struct genpd_power_state *states,
>>>+ unsigned int state_count, bool is_off);
>>>
>>>extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
>>>extern int pm_genpd_name_poweron(const char *domain_name);
>>>--
>>>1.9.1
>>>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state
2015-10-09 18:22 ` Lina Iyer
@ 2015-10-13 10:29 ` Marc Titinger
2015-10-13 21:03 ` Kevin Hilman
0 siblings, 1 reply; 39+ messages in thread
From: Marc Titinger @ 2015-10-13 10:29 UTC (permalink / raw)
To: Lina Iyer
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
On 09/10/2015 20:22, Lina Iyer wrote:
> On Fri, Oct 09 2015 at 03:39 -0600, Marc Titinger wrote:
>> On 08/10/2015 18:11, Lina Iyer wrote:
>>> Hi Marc,
>>>
>>> Thanks for rebasing on top of my latest series.
>>>
>>> On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>>>> Devices may register an intermediate retention state into the domain
>>>> upon
>>>>
>>> I may agree with the usability of dynamic adding a state to the domain,
>>> but I dont see why a device attaching to a domain should bring about a
>>> new domain state.
>>
>> Hi Lina,
>>
>> thanks a lot for taking the time to look into this. The initial
>> discussion behind it was about to see how a device like a PMU, FPU,
>> cache, or a Healthcheck IP in the same power domain than CPUs, with
>> special retention states can be handled in a way 'unified' with CPUs.
>> Also I admit it is partly an attempt from us to create something
>> useful out of the "collision" of Axel's multiple states and your
>> CPUs-as-generic-power-domain-devices, hence the RFC!
>>
>> Looking at Juno for instance, she currently has a platform-initiated
>> mode implemented in the arm-trusted-firmware through psci as a
>> cpuidle-driver. the menu governor will select a possibly deep c-state,
>> but the last-man CPU and actual power state is known to ATF. Similarly
>> my idea was to have a genpd-initiated mode so to say, where the actual
>> power state results from the cpu-domain's governor choice based on
>> possible retention states, and their latency.
>>
> Okay, I must admit that my ideas are quite partial to OS-initiated PSCI
> (v1.0 onwards). So you have C-States that allow domains to enter idle as
> well. Fair.
>
>> A Health-check IP or Cache will not (to my current knowledge) have a
>> driver calling runtime_put, but may have a retention state "D1_RET"
>> with a off/on latency between CPU_SLEEP and CLUSTER_SLEEP, so that
>> CLUSTER_SLEEP may be ruled out by the governor, but D1_RET is ok given
>> the time slot available.
> A couple of questions here.
>
> You say there is no driver for HIP, is there a device for it atleast?
> How is the CPU/Domain going to know if the HIP is running or not?
>
> To me this seems like you want to set a QoS on the PM domain here.
>
>> Some platform code can be called so that the cluster goes to D1_RET in
>> that case, upon the last-man CPU waiting-for-interrupt. Note that in
>> the case of a Health-Check HIP, the state my actually be a working
>> state (all CPUs power down, and time slot OK for sampling stuff).
>>
> Building on my previous statement, if you have a QoS for a domain and a
> domain governor, it could consider the QoS requirement and choose to do
> retention. However that needs a driver or some entity that know the HIP
> is requesting a D1_RET mode only.
lets' consider a device like an L2-cache with a RAM retention state,
(for instance looking at
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0500f/DDI0500F_cortex_a53_r0p4_trm.pdf
page 41). the platform code and suspend sequence that will allow for
setup of this L2 RAM Retention state will be partly common to handling
deep c-states like 'CLUSTER_SLEEP'. Typically for a53, the manual
describes a parent power domain PDCORTEXA53, child CPU domains PDCPUn
and a child domain PDL2 that allows for L2 RAM retention. We can have
all CPUs 'off' and PDL2 in retention.
In terms of 'multiple states', each CPU as a genpd device can
independently set a constraint for the domain 'simple governor', ok it's
not done through pm_qos_add_request, but through runtime_put, but since
the c-states are soaked into the power domain as possible power-domain
states, the domain will chose for the deepest possible c-state based on :
- cpus runtime_put (for c-states deeper than state0 "WFI")
- qos_requests from regular devices in the domain, or subdomains
- .. what about L2 or devices with their own power domain, that will not
hook to pm_runtime ?
Beyond L2 controllers, you could have hard IPs for debug, monitoring,
that will have a child power domain like above, but not necessarily hook
to pm_runtime. Since the platform code for handling the CPU constraints
on the domain QoS is the one for handling c-states, and the same for the
L2-retention state, why not expose those constraints as all-c-states ?
I reckon that alternatively, it could be interesting to model L2-cc as a
regular peripheral on its own, and hook to pm_runtime instead, but then
eventually will call the same monitor code code that handles the
cpu-suspend. But it's maybe less architecture dependent and more in the
initial spirit of "statement-of-work" motivating this series ?
M.
>
>>>
>>> A domain should define its power states, independent of the devices that
>>> may attach. The way I see it, devices have their own idle states and
>>> domains have their own. I do see a relationship between possible domain
>>> states depending on the states of the individual devices in the domain.
>>> For ex, a CPU domain can only be in a retention state (low voltage,
>>> memory retained), if its CPU devices are in retention state, i.e, the
>>> domain cannot be powered off; alternately, the domain may be in
>>> retention or power down if the CPU devices are in power down state.
>>>
>>> Could you elaborate on why this is a need?
>>
>> Well, it may not be a need TBH, it is an attempt to have cluster-level
>> devices act like hotplugged CPUs but with heterogeneous c-states and
>> latencies. I hope it makes some sense :)
>>
> Hmm.. Let me think about it.
>
> What would be a difference between a cluster-level device and a CPU in
> the same domain?
>
> -- Lina
>
>> Thanks,
>> Marc.
>>
>>
>>>
>>> Thanks,
>>> Lina
>>>
>>>> attaching. Currently generic domain would register an array of states
>>>> upon
>>>> init. This patch prepares for later insertion (sort per depth, remove).
>>>>
>>>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>>> ---
>>>> drivers/base/power/domain.c | 189
>>>> +++++++++++++++++++-------------------------
>>>> include/linux/pm_domain.h | 18 ++++-
>>>> 2 files changed, 97 insertions(+), 110 deletions(-)
>>>>
>>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>>> index 3e27a2b..e5f4c00b 100644
>>>> --- a/drivers/base/power/domain.c
>>>> +++ b/drivers/base/power/domain.c
>>>> @@ -19,6 +19,7 @@
>>>> #include <linux/sched.h>
>>>> #include <linux/suspend.h>
>>>> #include <linux/export.h>
>>>> +#include <linux/sort.h>
>>>>
>>>> #define GENPD_RETRY_MAX_MS 250 /* Approximate */
>>>>
>>>> @@ -50,12 +51,6 @@
>>>> __retval; \
>>>> })
>>>>
>>>> -#define GENPD_MAX_NAME_SIZE 20
>>>> -
>>>> -static int pm_genpd_alloc_states_names(struct generic_pm_domain
>>>> *genpd,
>>>> - const struct genpd_power_state *st,
>>>> - unsigned int st_count);
>>>> -
>>>> static LIST_HEAD(gpd_list);
>>>> static DEFINE_MUTEX(gpd_list_lock);
>>>>
>>>> @@ -1364,46 +1359,6 @@ static void genpd_free_dev_data(struct device
>>>> *dev,
>>>> dev_pm_put_subsys_data(dev);
>>>> }
>>>>
>>>> -static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
>>>> - const struct genpd_power_state *st,
>>>> - unsigned int st_count)
>>>> -{
>>>> - int ret = 0;
>>>> - unsigned int i;
>>>> -
>>>> - if (IS_ERR_OR_NULL(genpd)) {
>>>> - ret = -EINVAL;
>>>> - goto err;
>>>> - }
>>>> -
>>>> - if (!st || (st_count < 1)) {
>>>> - ret = -EINVAL;
>>>> - goto err;
>>>> - }
>>>> -
>>>> - /* Allocate the local memory to keep the states for this genpd */
>>>> - genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
>>>> - if (!genpd->states) {
>>>> - ret = -ENOMEM;
>>>> - goto err;
>>>> - }
>>>> -
>>>> - for (i = 0; i < st_count; i++) {
>>>> - genpd->states[i].power_on_latency_ns =
>>>> - st[i].power_on_latency_ns;
>>>> - genpd->states[i].power_off_latency_ns =
>>>> - st[i].power_off_latency_ns;
>>>> - }
>>>> -
>>>> - genpd->state_count = st_count;
>>>> -
>>>> - /* to save memory, Name allocation will happen if debug is
>>>> enabled */
>>>> - pm_genpd_alloc_states_names(genpd, st, st_count);
>>>> -
>>>> -err:
>>>> - return ret;
>>>> -}
>>>> -
>>>> /**
>>>> * __pm_genpd_add_device - Add a device to an I/O PM domain.
>>>> * @genpd: PM domain to add the device to.
>>>> @@ -1833,6 +1788,75 @@ static void genpd_lock_init(struct
>>>> generic_pm_domain *genpd)
>>>> }
>>>> }
>>>>
>>>> +
>>>> +/*
>>>> +* state depth comparison function.
>>>> +*/
>>>> +static int state_cmp(const void *a, const void *b)
>>>> +{
>>>> + struct genpd_power_state *state_a = (struct genpd_power_state
>>>> *)(a);
>>>> + struct genpd_power_state *state_b = (struct genpd_power_state
>>>> *)(b);
>>>> +
>>>> + s64 depth_a =
>>>> + state_a->power_on_latency_ns + state_a->power_off_latency_ns;
>>>> + s64 depth_b =
>>>> + state_b->power_on_latency_ns + state_b->power_off_latency_ns;
>>>> +
>>>> + return (depth_a > depth_b) ? 0 : -1;
>>>> +}
>>>> +
>>>> +/*
>>>> +* TODO: antagonist routine.
>>>> +*/
>>>> +int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>>>> + const struct genpd_power_state *state)
>>>> +{
>>>> + int ret = 0;
>>>> + int state_count = genpd->state_count;
>>>> +
>>>> + if (IS_ERR_OR_NULL(genpd) || (!state))
>>>> + ret = -EINVAL;
>>>> +
>>>> + if (state_count >= GENPD_POWER_STATES_MAX)
>>>> + ret = -ENOMEM;
>>>> +
>>>> +#ifdef CONFIG_PM_ADVANCED_DEBUG
>>>> + /* to save memory, Name allocation will happen if debug is
>>>> enabled */
>>>> + genpd->states[state_count].name = kstrndup(state->name,
>>>> + GENPD_MAX_NAME_SIZE,
>>>> + GFP_KERNEL);
>>>> + if (!genpd->states[state_count].name) {
>>>> + pr_err("%s Failed to allocate state '%s' name.\n",
>>>> + genpd->name, state->name);
>>>> + ret = -ENOMEM;
>>>> + }
>>>> +#endif
>>>> + genpd_lock(genpd);
>>>> +
>>>> + if (!ret) {
>>>> + genpd->states[state_count].power_on_latency_ns =
>>>> + state->power_on_latency_ns;
>>>> + genpd->states[state_count].power_off_latency_ns =
>>>> + state->power_off_latency_ns;
>>>> + genpd->state_count++;
>>>> + }
>>>> +
>>>> + /* sort from shallowest to deepest */
>>>> + sort(genpd->states, genpd->state_count,
>>>> + sizeof(genpd->states[0]), state_cmp, NULL /*generic swap */);
>>>> +
>>>> + /* Sanity check for current state index */
>>>> + if (genpd->state_idx >= genpd->state_count) {
>>>> + pr_warn("pm domain %s Invalid initial state.\n", genpd->name);
>>>> + genpd->state_idx = genpd->state_count - 1;
>>>> + }
>>>> +
>>>> + genpd_unlock(genpd);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +
>>>> /**
>>>> * pm_genpd_init - Initialize a generic I/O PM domain object.
>>>> * @genpd: PM domain object to initialize.
>>>> @@ -1846,36 +1870,24 @@ void pm_genpd_init(struct generic_pm_domain
>>>> *genpd,
>>>> const struct genpd_power_state *states,
>>>> unsigned int state_count, bool is_off)
>>>> {
>>>> - static const struct genpd_power_state genpd_default_state[] = {
>>>> - {
>>>> - .name = "OFF",
>>>> - .power_off_latency_ns = 0,
>>>> - .power_on_latency_ns = 0,
>>>> - },
>>>> - };
>>>> - int ret;
>>>> + int i;
>>>>
>>>> if (IS_ERR_OR_NULL(genpd))
>>>> return;
>>>>
>>>> - /* If no states defined, use the default OFF state */
>>>> - if (!states || (state_count < 1))
>>>> - ret = genpd_alloc_states_data(genpd, genpd_default_state,
>>>> - ARRAY_SIZE(genpd_default_state));
>>>> - else
>>>> - ret = genpd_alloc_states_data(genpd, states, state_count);
>>>> -
>>>> - if (ret) {
>>>> - pr_err("Fail to allocate states for %s\n", genpd->name);
>>>> - return;
>>>> - }
>>>> + /* simply use an array, we wish to add/remove new retention states
>>>> + from later device init/exit. */
>>>> + memset(genpd->states, 0, GENPD_POWER_STATES_MAX
>>>> + * sizeof(struct genpd_power_state));
>>>>
>>>> - /* Sanity check for initial state */
>>>> - if (genpd->state_idx >= genpd->state_count) {
>>>> - pr_warn("pm domain %s Invalid initial state.\n",
>>>> - genpd->name);
>>>> - genpd->state_idx = genpd->state_count - 1;
>>>> - }
>>>> + if (!states || !state_count) {
>>>> + /* require a provider for a default state */
>>>> + genpd->state_count = 0;
>>>> + genpd->state_idx = 0;
>>>> + } else
>>>> + for (i = 0; i < state_count; i++)
>>>> + if (pm_genpd_insert_state(genpd, &states[i]))
>>>> + return;
>>>>
>>>> INIT_LIST_HEAD(&genpd->master_links);
>>>> INIT_LIST_HEAD(&genpd->slave_links);
>>>> @@ -2233,33 +2245,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>>> #include <linux/kobject.h>
>>>> static struct dentry *pm_genpd_debugfs_dir;
>>>>
>>>> -static int pm_genpd_alloc_states_names(struct generic_pm_domain
>>>> *genpd,
>>>> - const struct genpd_power_state *st,
>>>> - unsigned int st_count)
>>>> -{
>>>> - unsigned int i;
>>>> -
>>>> - if (IS_ERR_OR_NULL(genpd))
>>>> - return -EINVAL;
>>>> -
>>>> - if (genpd->state_count != st_count) {
>>>> - pr_err("Invalid allocated state count\n");
>>>> - return -EINVAL;
>>>> - }
>>>> -
>>>> - for (i = 0; i < st_count; i++) {
>>>> - genpd->states[i].name = kstrndup(st[i].name,
>>>> - GENPD_MAX_NAME_SIZE, GFP_KERNEL);
>>>> - if (!genpd->states[i].name) {
>>>> - pr_err("%s Failed to allocate state %d name.\n",
>>>> - genpd->name, i);
>>>> - return -ENOMEM;
>>>> - }
>>>> - }
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> /*
>>>> * TODO: This function is a slightly modified version of
>>>> rtpm_status_show
>>>> * from sysfs.c, so generalize it.
>>>> @@ -2398,12 +2383,4 @@ static void __exit pm_genpd_debug_exit(void)
>>>> {
>>>> debugfs_remove_recursive(pm_genpd_debugfs_dir);
>>>> }
>>>> -__exitcall(pm_genpd_debug_exit);
>>>> -#else
>>>> -static inline int pm_genpd_alloc_states_names(struct
>>>> generic_pm_domain *genpd,
>>>> - const struct genpd_power_state *st,
>>>> - unsigned int st_count)
>>>> -{
>>>> - return 0;
>>>> -}
>>>> #endif /* CONFIG_PM_ADVANCED_DEBUG */
>>>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>>> index 9d37292..8a4eab0 100644
>>>> --- a/include/linux/pm_domain.h
>>>> +++ b/include/linux/pm_domain.h
>>>> @@ -45,6 +45,13 @@ struct gpd_cpuidle_data {
>>>> struct cpuidle_state *idle_state;
>>>> };
>>>>
>>>> +
>>>> +/* Arbitrary max number of devices registering a special
>>>> + * retention state with the PD, to keep things simple.
>>>> + */
>>>> +#define GENPD_POWER_STATES_MAX 12
>>>> +#define GENPD_MAX_NAME_SIZE 40
>>>> +
>>>> struct genpd_power_state {
>>>> char *name;
>>>> s64 power_off_latency_ns;
>>>> @@ -80,7 +87,8 @@ struct generic_pm_domain {
>>>> struct device *dev);
>>>> unsigned int flags; /* Bit field of configs for genpd */
>>>>
>>>> - struct genpd_power_state *states;
>>>> + struct genpd_power_state states[GENPD_POWER_STATES_MAX];
>>>> +
>>>> unsigned int state_count; /* number of states */
>>>> unsigned int state_idx; /* state that genpd will go to when off */
>>>>
>>>> @@ -159,10 +167,12 @@ extern int pm_genpd_attach_cpuidle(struct
>>>> generic_pm_domain *genpd, int state);
>>>> extern int pm_genpd_name_attach_cpuidle(const char *name, int state);
>>>> extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd);
>>>> extern int pm_genpd_name_detach_cpuidle(const char *name);
>>>> +extern int pm_genpd_insert_state(struct generic_pm_domain *genpd,
>>>> + const struct genpd_power_state *state);
>>>> extern void pm_genpd_init(struct generic_pm_domain *genpd,
>>>> - struct dev_power_governor *gov,
>>>> - const struct genpd_power_state *states,
>>>> - unsigned int state_count, bool is_off);
>>>> + struct dev_power_governor *gov,
>>>> + const struct genpd_power_state *states,
>>>> + unsigned int state_count, bool is_off);
>>>>
>>>> extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
>>>> extern int pm_genpd_name_poweron(const char *domain_name);
>>>> --
>>>> 1.9.1
>>>>
>>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state
2015-10-13 10:29 ` Marc Titinger
@ 2015-10-13 21:03 ` Kevin Hilman
0 siblings, 0 replies; 39+ messages in thread
From: Kevin Hilman @ 2015-10-13 21:03 UTC (permalink / raw)
To: Marc Titinger
Cc: Lina Iyer, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Marc Titinger <mtitinger@baylibre.com> writes:
> On 09/10/2015 20:22, Lina Iyer wrote:
>> On Fri, Oct 09 2015 at 03:39 -0600, Marc Titinger wrote:
>>> On 08/10/2015 18:11, Lina Iyer wrote:
>>>> Hi Marc,
>>>>
>>>> Thanks for rebasing on top of my latest series.
>>>>
>>>> On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>>>>> Devices may register an intermediate retention state into the domain
>>>>> upon
>>>>>
>>>> I may agree with the usability of dynamic adding a state to the domain,
>>>> but I dont see why a device attaching to a domain should bring about a
>>>> new domain state.
>>>
>>> Hi Lina,
>>>
>>> thanks a lot for taking the time to look into this. The initial
>>> discussion behind it was about to see how a device like a PMU, FPU,
>>> cache, or a Healthcheck IP in the same power domain than CPUs, with
>>> special retention states can be handled in a way 'unified' with CPUs.
>>> Also I admit it is partly an attempt from us to create something
>>> useful out of the "collision" of Axel's multiple states and your
>>> CPUs-as-generic-power-domain-devices, hence the RFC!
>>>
>>> Looking at Juno for instance, she currently has a platform-initiated
>>> mode implemented in the arm-trusted-firmware through psci as a
>>> cpuidle-driver. the menu governor will select a possibly deep c-state,
>>> but the last-man CPU and actual power state is known to ATF. Similarly
>>> my idea was to have a genpd-initiated mode so to say, where the actual
>>> power state results from the cpu-domain's governor choice based on
>>> possible retention states, and their latency.
>>>
>> Okay, I must admit that my ideas are quite partial to OS-initiated PSCI
>> (v1.0 onwards). So you have C-States that allow domains to enter idle as
>> well. Fair.
>>
>>> A Health-check IP or Cache will not (to my current knowledge) have a
>>> driver calling runtime_put, but may have a retention state "D1_RET"
>>> with a off/on latency between CPU_SLEEP and CLUSTER_SLEEP, so that
>>> CLUSTER_SLEEP may be ruled out by the governor, but D1_RET is ok given
>>> the time slot available.
>> A couple of questions here.
>>
>> You say there is no driver for HIP, is there a device for it atleast?
>> How is the CPU/Domain going to know if the HIP is running or not?
>>
>> To me this seems like you want to set a QoS on the PM domain here.
>>
>>> Some platform code can be called so that the cluster goes to D1_RET in
>>> that case, upon the last-man CPU waiting-for-interrupt. Note that in
>>> the case of a Health-Check HIP, the state my actually be a working
>>> state (all CPUs power down, and time slot OK for sampling stuff).
>>>
>> Building on my previous statement, if you have a QoS for a domain and a
>> domain governor, it could consider the QoS requirement and choose to do
>> retention. However that needs a driver or some entity that know the HIP
>> is requesting a D1_RET mode only.
>
> lets' consider a device like an L2-cache with a RAM retention state,
> (for instance looking at
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0500f/DDI0500F_cortex_a53_r0p4_trm.pdf
> page 41). the platform code and suspend sequence that will allow for
> setup of this L2 RAM Retention state will be partly common to
> handling deep c-states like 'CLUSTER_SLEEP'. Typically for a53, the
> manual describes a parent power domain PDCORTEXA53, child CPU domains
> PDCPUn and a child domain PDL2 that allows for L2 RAM retention. We
> can have all CPUs 'off' and PDL2 in retention.
>
> In terms of 'multiple states', each CPU as a genpd device can
> independently set a constraint for the domain 'simple governor', ok
> it's not done through pm_qos_add_request, but through runtime_put, but
> since the c-states are soaked into the power domain as possible
> power-domain states, the domain will chose for the deepest possible
> c-state based on :
>
> - cpus runtime_put (for c-states deeper than state0 "WFI")
> - qos_requests from regular devices in the domain, or subdomains
> - .. what about L2 or devices with their own power domain, that will
> not hook to pm_runtime ?
>
> Beyond L2 controllers, you could have hard IPs for debug, monitoring,
> that will have a child power domain like above, but not necessarily
> hook to pm_runtime. Since the platform code for handling the CPU
> constraints on the domain QoS is the one for handling c-states, and
> the same for the L2-retention state, why not expose those constraints
> as all-c-states ?
>
> I reckon that alternatively, it could be interesting to model L2-cc as
> a regular peripheral on its own, and hook to pm_runtime instead, but
> then eventually will call the same monitor code code that handles the
> cpu-suspend. But it's maybe less architecture dependent and more in
> the initial spirit of "statement-of-work" motivating this series ?
Correct.
I think has a first pass, rather than add the additional complexity
required for a dynamic set of genpd states, I think it's much better to
start by assuming that all devices in the domain that affect the domain
state should have an associated device and a driver using runtime PM.
For example, performance montitoring units (PMUs) like CoreSight have
this same issue, and the upstream support for those is already using
runtime PM.
For really simple/dump devices like L2$ or similar, it might be that we
don't need a real driver, but instead the CPU "devices" could do
gets/puts on any dependent devices directly.
Kevin
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states.
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
2015-10-06 14:27 ` [RFC v2 1/6] arm64: Juno: declare generic power domains for both clusters Marc Titinger
2015-10-06 14:27 ` [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Marc Titinger
@ 2015-10-06 14:27 ` Marc Titinger
2015-10-08 16:27 ` Lina Iyer
2015-10-06 14:27 ` [RFC v2 4/6] PM / Domains: succeed & warn when attaching non-irqsafe devices to an irq-safe domain Marc Titinger
` (4 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Marc Titinger @ 2015-10-06 14:27 UTC (permalink / raw)
To: khilman, rjw
Cc: lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
This patch allows cluster-level C-states to being soaked in as generic
domain power states, in order for the domain governor to chose the most
efficient power state compatible with the device constraints. Similarly,
devices can register power-states into the cluster domain, in a manner
consistent with c-states.
With Juno, in this example the c-state 'cluster-sleep-0 ' is known from
each cluster generic domain, as the deepest sate.
cat /sys/kernel/debug/pm_genpd/*
Domain State name Enter (ns) / Exit (ns)
-------------------------------------------------------------
a53_pd cluster-sleep-0 1500000 / 800000
a57_pd cluster-sleep-0 1500000 / 800000
domain status pstate slaves
/device runtime status
-----------------------------------------------------------------------
a53_pd on
/devices/system/cpu/cpu0 active
/devices/system/cpu/cpu3 suspended
/devices/system/cpu/cpu4 suspended
/devices/system/cpu/cpu5 suspended
/devices/platform/D1 suspended
a57_pd cluster-sleep-0
/devices/system/cpu/cpu1 suspended
/devices/system/cpu/cpu2 suspended
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
.../devicetree/bindings/arm/idle-states.txt | 21 ++++-
.../devicetree/bindings/power/power_domain.txt | 29 ++++++
arch/arm64/boot/dts/arm/juno.dts | 10 ++-
drivers/base/power/cpu-pd.c | 5 ++
drivers/base/power/domain.c | 100 +++++++++++++++++++++
include/linux/pm_domain.h | 3 +
6 files changed, 163 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt
index a8274ea..18fdeaf 100644
--- a/Documentation/devicetree/bindings/arm/idle-states.txt
+++ b/Documentation/devicetree/bindings/arm/idle-states.txt
@@ -270,7 +270,8 @@ follows:
- compatible
Usage: Required
Value type: <stringlist>
- Definition: Must be "arm,idle-state".
+ Definition: Must be "arm,idle-state",
+ or "arm,power-state" (see section 5)
- local-timer-stop
Usage: See definition
@@ -680,7 +681,23 @@ cpus {
};
===========================================
-5 - References
+5 - power state
+===========================================
+
+Device in a generic power domain may expose an intermediate retention
+state that can be opted to by the domain governor when the last-man
+CPU is powered off. Those power-states will not be entered by the
+cpuidle.ops based on a state index, but instead can be elected by the
+domain governor and entered to by the generic domain.
+
+ - compatible
+ Usage: Required
+ Value type: <stringlist>
+ Definition: Must be "arm,power-state".
+
+
+===========================================
+6 - References
===========================================
[1] ARM Linux Kernel documentation - CPUs bindings
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 0f8ed37..d437385 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -29,6 +29,16 @@ Optional properties:
specified by this binding. More details about power domain specifier are
available in the next section.
+ - power-states : a phandle of an idle-state that shall be soaked into a
+ generic domain power state.
+ CPU domains: Deep c-states that match a cluster power-off can be delegated to the
+ generic power domain. Device other than CPUs may have register intermediate
+ power states in the same domain. The domain governor can do a good job in
+ electing a power state when the last cpu is powered off as devices in the
+ same genpd may register intermediate states.
+ Devices : a device may register an intermediate c-state matching a memory
+ retention feature for instance.
+
Example:
power: power-controller@12340000 {
@@ -55,6 +65,25 @@ Example 2:
#power-domain-cells = <1>;
};
+Example 3:
+
+ pm-domains {
+ a57_pd: a57_pd@ {
+ /* will have a57 platform ARM_PD_METHOD_OF_DECLARE*/
+ compatible = "arm,pd","arm,cortex-a57";
+ #power-domain-cells = <0>;
+ power-states = <&CLUSTER_SLEEP_0>;
+ };
+
+ a53_pd: a53_pd@ {
+ /* will have a a53 platform ARM_PD_METHOD_OF_DECLARE*/
+ compatible = "arm,pd","arm,cortex-a53";
+ #power-domain-cells = <0>;
+ power-states = <&CLUSTER_SLEEP_0>;
+ };
+ };
+
+
The nodes above define two power controllers: 'parent' and 'child'.
Domains created by the 'child' power controller are subdomains of '0' power
domain provided by the 'parent' power controller.
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index 499f035..cadc5de 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -47,7 +47,7 @@
};
CLUSTER_SLEEP_0: cluster-sleep-0 {
- compatible = "arm,idle-state";
+ compatible = "arm,idle-state","arm,power-state";
arm,psci-suspend-param = <0x1010000>;
local-timer-stop;
entry-latency-us = <800>;
@@ -128,13 +128,17 @@
pm-domains {
a57_pd: a57_pd@ {
- compatible = "arm,pd";
+ /* will have the a53 platform ARM_PD_METHOD_OF_DECLARE*/
+ compatible = "arm,pd","arm,cortex-a57";
#power-domain-cells = <0>;
+ power-states = <&CLUSTER_SLEEP_0>;
};
a53_pd: a53_pd@ {
- compatible = "arm,pd";
+ /* will have the a53 platform ARM_PD_METHOD_OF_DECLARE*/
+ compatible = "arm,pd","arm,cortex-a57";
#power-domain-cells = <0>;
+ power-states = <&CLUSTER_SLEEP_0>;
};
};
diff --git a/drivers/base/power/cpu-pd.c b/drivers/base/power/cpu-pd.c
index 5f30025..3c4cb12 100644
--- a/drivers/base/power/cpu-pd.c
+++ b/drivers/base/power/cpu-pd.c
@@ -163,6 +163,11 @@ int of_register_cpu_pm_domain(struct device_node *dn,
pm_genpd_init(pd->genpd, &simple_qos_governor, false);
of_genpd_add_provider_simple(dn, pd->genpd);
+ /* if a cpuidle-state is declared in this domain node, it will
+ * be the domain's job to enter/exit this state, if the device
+ * subdomain constraints are compatible */
+ of_genpd_device_parse_states(dn, &pd->genpd);
+
/* Attach the CPUs to the CPU PM domain */
return of_pm_domain_attach_cpus();
}
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e5f4c00b..1ca28a2 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2156,6 +2156,104 @@ static void genpd_dev_pm_sync(struct device *dev)
genpd_queue_power_off_work(pd);
}
+static int dt_cpuidle_to_genpd_power_state(struct genpd_power_state
+ *genpd_state,
+ const struct of_device_id *matches,
+ struct device_node *state_node)
+{
+ const struct of_device_id *match_id;
+ int err = 0;
+ u32 latency;
+
+ match_id = of_match_node(matches, state_node);
+ if (!match_id)
+ return -ENODEV;
+
+ err = of_property_read_u32(state_node, "wakeup-latency-us", &latency);
+ if (err) {
+ u32 entry_latency, exit_latency;
+
+ err = of_property_read_u32(state_node, "entry-latency-us",
+ &entry_latency);
+ if (err) {
+ pr_debug(" * %s missing entry-latency-us property\n",
+ state_node->full_name);
+ return -EINVAL;
+ }
+
+ err = of_property_read_u32(state_node, "exit-latency-us",
+ &exit_latency);
+ if (err) {
+ pr_debug(" * %s missing exit-latency-us property\n",
+ state_node->full_name);
+ return -EINVAL;
+ }
+ /*
+ * If wakeup-latency-us is missing, default to entry+exit
+ * latencies as defined in idle states bindings
+ */
+ latency = entry_latency + exit_latency;
+ }
+
+ genpd_state->power_on_latency_ns = 1000 * latency;
+
+ err = of_property_read_u32(state_node, "entry-latency-us", &latency);
+ if (err) {
+ pr_debug(" * %s missing min-residency-us property\n",
+ state_node->full_name);
+ return -EINVAL;
+ }
+
+ genpd_state->power_off_latency_ns = 1000 * latency;
+
+ return 0;
+}
+
+static const struct of_device_id power_state_match[] = {
+ {.compatible = "arm,power-state",
+ },
+};
+
+int of_genpd_device_parse_states(struct device_node *np,
+ struct generic_pm_domain *genpd)
+{
+ struct device_node *state_node;
+ int i, err = 0;
+
+ for (i = 0;; i++) {
+ struct genpd_power_state genpd_state;
+
+ state_node = of_parse_phandle(np, "power-states", i);
+ if (!state_node)
+ break;
+
+ err = dt_cpuidle_to_genpd_power_state(&genpd_state,
+ power_state_match,
+ state_node);
+ if (err) {
+ pr_err
+ ("Parsing idle state node %s failed with err %d\n",
+ state_node->full_name, err);
+ err = -EINVAL;
+ break;
+ }
+#ifdef CONFIG_PM_ADVANCED_DEBUG
+ genpd_state.name = kstrndup(state_node->name,
+ GENPD_MAX_NAME_SIZE, GFP_KERNEL);
+ if (!genpd_state.name)
+ err = -ENOMEM;
+#endif
+ of_node_put(state_node);
+ err = pm_genpd_insert_state(genpd, &genpd_state);
+ if (err)
+ break;
+#ifdef CONFIG_PM_ADVANCED_DEBUG
+ kfree(genpd_state.name);
+#endif
+ }
+ return err;
+}
+
/**
* genpd_dev_pm_attach - Attach a device to its PM domain using DT.
* @dev: Device to attach.
@@ -2224,6 +2322,8 @@ int genpd_dev_pm_attach(struct device *dev)
return ret;
}
+ of_genpd_device_parse_states(dev->of_node, pd);
+
dev->pm_domain->detach = genpd_dev_pm_detach;
dev->pm_domain->sync = genpd_dev_pm_sync;
pm_genpd_poweron(pd);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 8a4eab0..791a8ac 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -302,6 +302,9 @@ struct generic_pm_domain *__of_genpd_xlate_onecell(
struct of_phandle_args *genpdspec,
void *data);
+int of_genpd_device_parse_states(struct device_node *np,
+ struct generic_pm_domain *genpd);
+
int genpd_dev_pm_attach(struct device *dev);
#else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
static inline int __of_genpd_add_provider(struct device_node *np,
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states.
2015-10-06 14:27 ` [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states Marc Titinger
@ 2015-10-08 16:27 ` Lina Iyer
2015-10-09 10:04 ` Marc Titinger
0 siblings, 1 reply; 39+ messages in thread
From: Lina Iyer @ 2015-10-08 16:27 UTC (permalink / raw)
To: Marc Titinger
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>This patch allows cluster-level C-states to being soaked in as generic
>domain power states, in order for the domain governor to chose the most
>efficient power state compatible with the device constraints. Similarly,
>devices can register power-states into the cluster domain, in a manner
>consistent with c-states.
>
A domain power state as depicted in the DT is no different than the CPU
idle state. I think you can achieve this without adding another
compatible - arm,power-state.
I think I am still at loss trying to understand why a device is
populating the domain's power states.
>With Juno, in this example the c-state 'cluster-sleep-0 ' is known from
>each cluster generic domain, as the deepest sate.
>
>cat /sys/kernel/debug/pm_genpd/*
>
> Domain State name Enter (ns) / Exit (ns)
>-------------------------------------------------------------
>a53_pd cluster-sleep-0 1500000 / 800000
>a57_pd cluster-sleep-0 1500000 / 800000
>
> domain status pstate slaves
> /device runtime status
>-----------------------------------------------------------------------
>a53_pd on
> /devices/system/cpu/cpu0 active
> /devices/system/cpu/cpu3 suspended
> /devices/system/cpu/cpu4 suspended
> /devices/system/cpu/cpu5 suspended
> /devices/platform/D1 suspended
>a57_pd cluster-sleep-0
> /devices/system/cpu/cpu1 suspended
> /devices/system/cpu/cpu2 suspended
>
>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>---
> .../devicetree/bindings/arm/idle-states.txt | 21 ++++-
> .../devicetree/bindings/power/power_domain.txt | 29 ++++++
> arch/arm64/boot/dts/arm/juno.dts | 10 ++-
> drivers/base/power/cpu-pd.c | 5 ++
> drivers/base/power/domain.c | 100 +++++++++++++++++++++
> include/linux/pm_domain.h | 3 +
> 6 files changed, 163 insertions(+), 5 deletions(-)
>
>diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt
>index a8274ea..18fdeaf 100644
>--- a/Documentation/devicetree/bindings/arm/idle-states.txt
>+++ b/Documentation/devicetree/bindings/arm/idle-states.txt
>@@ -270,7 +270,8 @@ follows:
> - compatible
> Usage: Required
> Value type: <stringlist>
>- Definition: Must be "arm,idle-state".
>+ Definition: Must be "arm,idle-state",
>+ or "arm,power-state" (see section 5)
>
> - local-timer-stop
> Usage: See definition
>@@ -680,7 +681,23 @@ cpus {
> };
>
> ===========================================
>-5 - References
>+5 - power state
>+===========================================
>+
>+Device in a generic power domain may expose an intermediate retention
>+state that can be opted to by the domain governor when the last-man
>+CPU is powered off. Those power-states will not be entered by the
>+cpuidle.ops based on a state index, but instead can be elected by the
>+domain governor and entered to by the generic domain.
>+
Agreed.
>+ - compatible
>+ Usage: Required
>+ Value type: <stringlist>
>+ Definition: Must be "arm,power-state".
>+
>+
>+===========================================
>+6 - References
> ===========================================
>
> [1] ARM Linux Kernel documentation - CPUs bindings
>diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>index 0f8ed37..d437385 100644
>--- a/Documentation/devicetree/bindings/power/power_domain.txt
>+++ b/Documentation/devicetree/bindings/power/power_domain.txt
>@@ -29,6 +29,16 @@ Optional properties:
> specified by this binding. More details about power domain specifier are
> available in the next section.
>
>+ - power-states : a phandle of an idle-state that shall be soaked into a
>+ generic domain power state.
>+ CPU domains: Deep c-states that match a cluster power-off can be delegated to the
>+ generic power domain. Device other than CPUs may have register intermediate
>+ power states in the same domain. The domain governor can do a good job in
>+ electing a power state when the last cpu is powered off as devices in the
>+ same genpd may register intermediate states.
>
Devices may enable a certain domain state, but should not be defining
the domain state. The domain its state and the domain governor may
choose to enter that state on a vote from its devices.
>+ Devices : a device may register an intermediate c-state matching a memory
>+ retention feature for instance.
>
This point onwards is where I need clarity.
Thanks,
Lina
>+
> Example:
>
> power: power-controller@12340000 {
>@@ -55,6 +65,25 @@ Example 2:
> #power-domain-cells = <1>;
> };
>
>+Example 3:
>+
>+ pm-domains {
>+ a57_pd: a57_pd@ {
>+ /* will have a57 platform ARM_PD_METHOD_OF_DECLARE*/
>+ compatible = "arm,pd","arm,cortex-a57";
>+ #power-domain-cells = <0>;
>+ power-states = <&CLUSTER_SLEEP_0>;
>+ };
>+
>+ a53_pd: a53_pd@ {
>+ /* will have a a53 platform ARM_PD_METHOD_OF_DECLARE*/
>+ compatible = "arm,pd","arm,cortex-a53";
>+ #power-domain-cells = <0>;
>+ power-states = <&CLUSTER_SLEEP_0>;
>+ };
>+ };
>+
>+
> The nodes above define two power controllers: 'parent' and 'child'.
> Domains created by the 'child' power controller are subdomains of '0' power
> domain provided by the 'parent' power controller.
>diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
>index 499f035..cadc5de 100644
>--- a/arch/arm64/boot/dts/arm/juno.dts
>+++ b/arch/arm64/boot/dts/arm/juno.dts
>@@ -47,7 +47,7 @@
> };
>
> CLUSTER_SLEEP_0: cluster-sleep-0 {
>- compatible = "arm,idle-state";
>+ compatible = "arm,idle-state","arm,power-state";
> arm,psci-suspend-param = <0x1010000>;
> local-timer-stop;
> entry-latency-us = <800>;
>@@ -128,13 +128,17 @@
> pm-domains {
>
> a57_pd: a57_pd@ {
>- compatible = "arm,pd";
>+ /* will have the a53 platform ARM_PD_METHOD_OF_DECLARE*/
>+ compatible = "arm,pd","arm,cortex-a57";
> #power-domain-cells = <0>;
>+ power-states = <&CLUSTER_SLEEP_0>;
> };
>
> a53_pd: a53_pd@ {
>- compatible = "arm,pd";
>+ /* will have the a53 platform ARM_PD_METHOD_OF_DECLARE*/
>+ compatible = "arm,pd","arm,cortex-a57";
> #power-domain-cells = <0>;
>+ power-states = <&CLUSTER_SLEEP_0>;
> };
> };
>
>diff --git a/drivers/base/power/cpu-pd.c b/drivers/base/power/cpu-pd.c
>index 5f30025..3c4cb12 100644
>--- a/drivers/base/power/cpu-pd.c
>+++ b/drivers/base/power/cpu-pd.c
>@@ -163,6 +163,11 @@ int of_register_cpu_pm_domain(struct device_node *dn,
> pm_genpd_init(pd->genpd, &simple_qos_governor, false);
> of_genpd_add_provider_simple(dn, pd->genpd);
>
>+ /* if a cpuidle-state is declared in this domain node, it will
>+ * be the domain's job to enter/exit this state, if the device
>+ * subdomain constraints are compatible */
>+ of_genpd_device_parse_states(dn, &pd->genpd);
>+
> /* Attach the CPUs to the CPU PM domain */
> return of_pm_domain_attach_cpus();
> }
>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>index e5f4c00b..1ca28a2 100644
>--- a/drivers/base/power/domain.c
>+++ b/drivers/base/power/domain.c
>@@ -2156,6 +2156,104 @@ static void genpd_dev_pm_sync(struct device *dev)
> genpd_queue_power_off_work(pd);
> }
>
>+static int dt_cpuidle_to_genpd_power_state(struct genpd_power_state
>+ *genpd_state,
>+ const struct of_device_id *matches,
>+ struct device_node *state_node)
>+{
>+ const struct of_device_id *match_id;
>+ int err = 0;
>+ u32 latency;
>+
>+ match_id = of_match_node(matches, state_node);
>+ if (!match_id)
>+ return -ENODEV;
>+
>+ err = of_property_read_u32(state_node, "wakeup-latency-us", &latency);
>+ if (err) {
>+ u32 entry_latency, exit_latency;
>+
>+ err = of_property_read_u32(state_node, "entry-latency-us",
>+ &entry_latency);
>+ if (err) {
>+ pr_debug(" * %s missing entry-latency-us property\n",
>+ state_node->full_name);
>+ return -EINVAL;
>+ }
>+
>+ err = of_property_read_u32(state_node, "exit-latency-us",
>+ &exit_latency);
>+ if (err) {
>+ pr_debug(" * %s missing exit-latency-us property\n",
>+ state_node->full_name);
>+ return -EINVAL;
>+ }
>+ /*
>+ * If wakeup-latency-us is missing, default to entry+exit
>+ * latencies as defined in idle states bindings
>+ */
>+ latency = entry_latency + exit_latency;
>+ }
>+
>+ genpd_state->power_on_latency_ns = 1000 * latency;
>+
>+ err = of_property_read_u32(state_node, "entry-latency-us", &latency);
>+ if (err) {
>+ pr_debug(" * %s missing min-residency-us property\n",
>+ state_node->full_name);
>+ return -EINVAL;
>+ }
>+
>+ genpd_state->power_off_latency_ns = 1000 * latency;
>+
>+ return 0;
>+}
>+
>+static const struct of_device_id power_state_match[] = {
>+ {.compatible = "arm,power-state",
>+ },
>+};
>+
>+int of_genpd_device_parse_states(struct device_node *np,
>+ struct generic_pm_domain *genpd)
>+{
>+ struct device_node *state_node;
>+ int i, err = 0;
>+
>+ for (i = 0;; i++) {
>+ struct genpd_power_state genpd_state;
>+
>+ state_node = of_parse_phandle(np, "power-states", i);
>+ if (!state_node)
>+ break;
>+
>+ err = dt_cpuidle_to_genpd_power_state(&genpd_state,
>+ power_state_match,
>+ state_node);
>+ if (err) {
>+ pr_err
>+ ("Parsing idle state node %s failed with err %d\n",
>+ state_node->full_name, err);
>+ err = -EINVAL;
>+ break;
>+ }
>+#ifdef CONFIG_PM_ADVANCED_DEBUG
>+ genpd_state.name = kstrndup(state_node->name,
>+ GENPD_MAX_NAME_SIZE, GFP_KERNEL);
>+ if (!genpd_state.name)
>+ err = -ENOMEM;
>+#endif
>+ of_node_put(state_node);
>+ err = pm_genpd_insert_state(genpd, &genpd_state);
>+ if (err)
>+ break;
>+#ifdef CONFIG_PM_ADVANCED_DEBUG
>+ kfree(genpd_state.name);
>+#endif
>+ }
>+ return err;
>+}
>+
> /**
> * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> * @dev: Device to attach.
>@@ -2224,6 +2322,8 @@ int genpd_dev_pm_attach(struct device *dev)
> return ret;
> }
>
>+ of_genpd_device_parse_states(dev->of_node, pd);
>+
> dev->pm_domain->detach = genpd_dev_pm_detach;
> dev->pm_domain->sync = genpd_dev_pm_sync;
> pm_genpd_poweron(pd);
>diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>index 8a4eab0..791a8ac 100644
>--- a/include/linux/pm_domain.h
>+++ b/include/linux/pm_domain.h
>@@ -302,6 +302,9 @@ struct generic_pm_domain *__of_genpd_xlate_onecell(
> struct of_phandle_args *genpdspec,
> void *data);
>
>+int of_genpd_device_parse_states(struct device_node *np,
>+ struct generic_pm_domain *genpd);
>+
> int genpd_dev_pm_attach(struct device *dev);
> #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
> static inline int __of_genpd_add_provider(struct device_node *np,
>--
>1.9.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states.
2015-10-08 16:27 ` Lina Iyer
@ 2015-10-09 10:04 ` Marc Titinger
0 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-09 10:04 UTC (permalink / raw)
To: Lina Iyer
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
On 08/10/2015 18:27, Lina Iyer wrote:
> On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>> This patch allows cluster-level C-states to being soaked in as generic
>> domain power states, in order for the domain governor to chose the most
>> efficient power state compatible with the device constraints. Similarly,
>> devices can register power-states into the cluster domain, in a manner
>> consistent with c-states.
>>
> A domain power state as depicted in the DT is no different than the CPU
> idle state. I think you can achieve this without adding another
> compatible - arm,power-state.
>
yes, thats'a bit hacky, I was facing the genpd_lock'ing issue I believe
you mentioned recently and lead to your simplification work in the case
of CPUs. the power-state discrimination was to allow cpu-idle to call
runtime_put/get only for those states where genpd is to call the
platform code and select the optimal c-state. Eventually this
discrimination would be useless. Alternatively I've been wondering if we
could have one domain (and one lock) per CPU, and have a parent domain
for the cluster.
> I think I am still at loss trying to understand why a device is
> populating the domain's power states.
>
>> With Juno, in this example the c-state 'cluster-sleep-0 ' is known from
>> each cluster generic domain, as the deepest sate.
>>
>> cat /sys/kernel/debug/pm_genpd/*
>>
>> Domain State name Enter (ns) / Exit (ns)
>> -------------------------------------------------------------
>> a53_pd cluster-sleep-0 1500000 / 800000
>> a57_pd cluster-sleep-0 1500000 / 800000
>>
>> domain status pstate slaves
>> /device runtime status
>> -----------------------------------------------------------------------
>> a53_pd on
>> /devices/system/cpu/cpu0 active
>> /devices/system/cpu/cpu3 suspended
>> /devices/system/cpu/cpu4 suspended
>> /devices/system/cpu/cpu5 suspended
>> /devices/platform/D1 suspended
>> a57_pd cluster-sleep-0
>> /devices/system/cpu/cpu1 suspended
>> /devices/system/cpu/cpu2 suspended
>>
>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>> ---
>> .../devicetree/bindings/arm/idle-states.txt | 21 ++++-
>> .../devicetree/bindings/power/power_domain.txt | 29 ++++++
>> arch/arm64/boot/dts/arm/juno.dts | 10 ++-
>> drivers/base/power/cpu-pd.c | 5 ++
>> drivers/base/power/domain.c | 100
>> +++++++++++++++++++++
>> include/linux/pm_domain.h | 3 +
>> 6 files changed, 163 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt
>> b/Documentation/devicetree/bindings/arm/idle-states.txt
>> index a8274ea..18fdeaf 100644
>> --- a/Documentation/devicetree/bindings/arm/idle-states.txt
>> +++ b/Documentation/devicetree/bindings/arm/idle-states.txt
>> @@ -270,7 +270,8 @@ follows:
>> - compatible
>> Usage: Required
>> Value type: <stringlist>
>> - Definition: Must be "arm,idle-state".
>> + Definition: Must be "arm,idle-state",
>> + or "arm,power-state" (see section 5)
>>
>> - local-timer-stop
>> Usage: See definition
>> @@ -680,7 +681,23 @@ cpus {
>> };
>>
>> ===========================================
>> -5 - References
>> +5 - power state
>> +===========================================
>> +
>> +Device in a generic power domain may expose an intermediate retention
>> +state that can be opted to by the domain governor when the last-man
>> +CPU is powered off. Those power-states will not be entered by the
>> +cpuidle.ops based on a state index, but instead can be elected by the
>> +domain governor and entered to by the generic domain.
>> +
> Agreed.
>
>> + - compatible
>> + Usage: Required
>> + Value type: <stringlist>
>> + Definition: Must be "arm,power-state".
>> +
>> +
>> +===========================================
>> +6 - References
>> ===========================================
>>
>> [1] ARM Linux Kernel documentation - CPUs bindings
>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt
>> b/Documentation/devicetree/bindings/power/power_domain.txt
>> index 0f8ed37..d437385 100644
>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> @@ -29,6 +29,16 @@ Optional properties:
>> specified by this binding. More details about power domain
>> specifier are
>> available in the next section.
>>
>> + - power-states : a phandle of an idle-state that shall be soaked into a
>> + generic domain power state.
>> + CPU domains: Deep c-states that match a cluster power-off can be
>> delegated to the
>> + generic power domain. Device other than CPUs may have register
>> intermediate
>> + power states in the same domain. The domain governor can do a good
>> job in
>> + electing a power state when the last cpu is powered off as devices
>> in the
>> + same genpd may register intermediate states.
>>
> Devices may enable a certain domain state, but should not be defining
> the domain state. The domain its state and the domain governor may
> choose to enter that state on a vote from its devices.
>
>> + Devices : a device may register an intermediate c-state matching a
>> memory
>> + retention feature for instance.
>>
> This point onwards is where I need clarity.
>
> Thanks,
> Lina
>
>> +
>> Example:
>>
>> power: power-controller@12340000 {
>> @@ -55,6 +65,25 @@ Example 2:
>> #power-domain-cells = <1>;
>> };
>>
>> +Example 3:
>> +
>> + pm-domains {
>> + a57_pd: a57_pd@ {
>> + /* will have a57 platform
>> ARM_PD_METHOD_OF_DECLARE*/
>> + compatible = "arm,pd","arm,cortex-a57";
>> + #power-domain-cells = <0>;
>> + power-states = <&CLUSTER_SLEEP_0>;
>> + };
>> +
>> + a53_pd: a53_pd@ {
>> + /* will have a a53 platform
>> ARM_PD_METHOD_OF_DECLARE*/
>> + compatible = "arm,pd","arm,cortex-a53";
>> + #power-domain-cells = <0>;
>> + power-states = <&CLUSTER_SLEEP_0>;
>> + };
>> + };
>> +
>> +
>> The nodes above define two power controllers: 'parent' and 'child'.
>> Domains created by the 'child' power controller are subdomains of '0'
>> power
>> domain provided by the 'parent' power controller.
>> diff --git a/arch/arm64/boot/dts/arm/juno.dts
>> b/arch/arm64/boot/dts/arm/juno.dts
>> index 499f035..cadc5de 100644
>> --- a/arch/arm64/boot/dts/arm/juno.dts
>> +++ b/arch/arm64/boot/dts/arm/juno.dts
>> @@ -47,7 +47,7 @@
>> };
>>
>> CLUSTER_SLEEP_0: cluster-sleep-0 {
>> - compatible = "arm,idle-state";
>> + compatible = "arm,idle-state","arm,power-state";
>> arm,psci-suspend-param = <0x1010000>;
>> local-timer-stop;
>> entry-latency-us = <800>;
>> @@ -128,13 +128,17 @@
>> pm-domains {
>>
>> a57_pd: a57_pd@ {
>> - compatible = "arm,pd";
>> + /* will have the a53 platform ARM_PD_METHOD_OF_DECLARE*/
>> + compatible = "arm,pd","arm,cortex-a57";
>> #power-domain-cells = <0>;
>> + power-states = <&CLUSTER_SLEEP_0>;
>> };
>>
>> a53_pd: a53_pd@ {
>> - compatible = "arm,pd";
>> + /* will have the a53 platform ARM_PD_METHOD_OF_DECLARE*/
>> + compatible = "arm,pd","arm,cortex-a57";
>> #power-domain-cells = <0>;
>> + power-states = <&CLUSTER_SLEEP_0>;
>> };
>> };
>>
>> diff --git a/drivers/base/power/cpu-pd.c b/drivers/base/power/cpu-pd.c
>> index 5f30025..3c4cb12 100644
>> --- a/drivers/base/power/cpu-pd.c
>> +++ b/drivers/base/power/cpu-pd.c
>> @@ -163,6 +163,11 @@ int of_register_cpu_pm_domain(struct device_node
>> *dn,
>> pm_genpd_init(pd->genpd, &simple_qos_governor, false);
>> of_genpd_add_provider_simple(dn, pd->genpd);
>>
>> + /* if a cpuidle-state is declared in this domain node, it will
>> + * be the domain's job to enter/exit this state, if the device
>> + * subdomain constraints are compatible */
>> + of_genpd_device_parse_states(dn, &pd->genpd);
>> +
>> /* Attach the CPUs to the CPU PM domain */
>> return of_pm_domain_attach_cpus();
>> }
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index e5f4c00b..1ca28a2 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2156,6 +2156,104 @@ static void genpd_dev_pm_sync(struct device *dev)
>> genpd_queue_power_off_work(pd);
>> }
>>
>> +static int dt_cpuidle_to_genpd_power_state(struct genpd_power_state
>> + *genpd_state,
>> + const struct of_device_id *matches,
>> + struct device_node *state_node)
>> +{
>> + const struct of_device_id *match_id;
>> + int err = 0;
>> + u32 latency;
>> +
>> + match_id = of_match_node(matches, state_node);
>> + if (!match_id)
>> + return -ENODEV;
>> +
>> + err = of_property_read_u32(state_node, "wakeup-latency-us",
>> &latency);
>> + if (err) {
>> + u32 entry_latency, exit_latency;
>> +
>> + err = of_property_read_u32(state_node, "entry-latency-us",
>> + &entry_latency);
>> + if (err) {
>> + pr_debug(" * %s missing entry-latency-us property\n",
>> + state_node->full_name);
>> + return -EINVAL;
>> + }
>> +
>> + err = of_property_read_u32(state_node, "exit-latency-us",
>> + &exit_latency);
>> + if (err) {
>> + pr_debug(" * %s missing exit-latency-us property\n",
>> + state_node->full_name);
>> + return -EINVAL;
>> + }
>> + /*
>> + * If wakeup-latency-us is missing, default to entry+exit
>> + * latencies as defined in idle states bindings
>> + */
>> + latency = entry_latency + exit_latency;
>> + }
>> +
>> + genpd_state->power_on_latency_ns = 1000 * latency;
>> +
>> + err = of_property_read_u32(state_node, "entry-latency-us",
>> &latency);
>> + if (err) {
>> + pr_debug(" * %s missing min-residency-us property\n",
>> + state_node->full_name);
>> + return -EINVAL;
>> + }
>> +
>> + genpd_state->power_off_latency_ns = 1000 * latency;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id power_state_match[] = {
>> + {.compatible = "arm,power-state",
>> + },
>> +};
>> +
>> +int of_genpd_device_parse_states(struct device_node *np,
>> + struct generic_pm_domain *genpd)
>> +{
>> + struct device_node *state_node;
>> + int i, err = 0;
>> +
>> + for (i = 0;; i++) {
>> + struct genpd_power_state genpd_state;
>> +
>> + state_node = of_parse_phandle(np, "power-states", i);
>> + if (!state_node)
>> + break;
>> +
>> + err = dt_cpuidle_to_genpd_power_state(&genpd_state,
>> + power_state_match,
>> + state_node);
>> + if (err) {
>> + pr_err
>> + ("Parsing idle state node %s failed with err %d\n",
>> + state_node->full_name, err);
>> + err = -EINVAL;
>> + break;
>> + }
>> +#ifdef CONFIG_PM_ADVANCED_DEBUG
>> + genpd_state.name = kstrndup(state_node->name,
>> + GENPD_MAX_NAME_SIZE, GFP_KERNEL);
>> + if (!genpd_state.name)
>> + err = -ENOMEM;
>> +#endif
>> + of_node_put(state_node);
>> + err = pm_genpd_insert_state(genpd, &genpd_state);
>> + if (err)
>> + break;
>> +#ifdef CONFIG_PM_ADVANCED_DEBUG
>> + kfree(genpd_state.name);
>> +#endif
>> + }
>> + return err;
>> +}
>> +
>> /**
>> * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
>> * @dev: Device to attach.
>> @@ -2224,6 +2322,8 @@ int genpd_dev_pm_attach(struct device *dev)
>> return ret;
>> }
>>
>> + of_genpd_device_parse_states(dev->of_node, pd);
>> +
>> dev->pm_domain->detach = genpd_dev_pm_detach;
>> dev->pm_domain->sync = genpd_dev_pm_sync;
>> pm_genpd_poweron(pd);
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 8a4eab0..791a8ac 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -302,6 +302,9 @@ struct generic_pm_domain *__of_genpd_xlate_onecell(
>> struct of_phandle_args *genpdspec,
>> void *data);
>>
>> +int of_genpd_device_parse_states(struct device_node *np,
>> + struct generic_pm_domain *genpd);
>> +
>> int genpd_dev_pm_attach(struct device *dev);
>> #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
>> static inline int __of_genpd_add_provider(struct device_node *np,
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC v2 4/6] PM / Domains: succeed & warn when attaching non-irqsafe devices to an irq-safe domain.
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
` (2 preceding siblings ...)
2015-10-06 14:27 ` [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states Marc Titinger
@ 2015-10-06 14:27 ` Marc Titinger
2015-10-06 14:27 ` [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states' Marc Titinger
` (3 subsequent siblings)
7 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-06 14:27 UTC (permalink / raw)
To: khilman, rjw
Cc: lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
This patch checks for irq-safe compatibility in suspend/resume instead of
failing the attach operation early on. Non-cpu devices attaching to an
irq-safe power domain will have to call pm_runtime_irq_safe from their
probe function.
Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
drivers/base/power/domain.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1ca28a2..ebf1299 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -635,6 +635,14 @@ static int pm_genpd_runtime_resume(struct device *dev)
goto out;
}
+ /* The device probe may have missed calling pm_runtime_irq_safe.
+ */
+ if (!dev->power.irq_safe && genpd->irq_safe) {
+ dev_err(dev, "trying to %s a non-irqsafe device in an irq-safe domain\n",
+ __func__);
+ return -EINVAL;
+ }
+
genpd_lock(genpd);
ret = __pm_genpd_poweron(genpd);
genpd_unlock(genpd);
@@ -1376,12 +1384,11 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
- if (genpd->irq_safe && !dev->power.irq_safe) {
- dev_err(dev,
- "PM Domain %s is IRQ safe; device has to IRQ safe.\n",
- genpd->name);
- return -EINVAL;
- }
+ /* Only issue a warning, runtime_irqsafe may be called later on
+ * from the driver probe. */
+ if (genpd->irq_safe && !dev->power.irq_safe)
+ dev_warn(dev,
+ "Devices in an IRQ safe domain have to be IRQ safe.\n");
gpd_data = genpd_alloc_dev_data(dev, genpd, td);
if (IS_ERR(gpd_data))
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states'
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
` (3 preceding siblings ...)
2015-10-06 14:27 ` [RFC v2 4/6] PM / Domains: succeed & warn when attaching non-irqsafe devices to an irq-safe domain Marc Titinger
@ 2015-10-06 14:27 ` Marc Titinger
2015-11-11 9:10 ` Zhaoyang Huang
2015-10-06 14:27 ` [RFC v2 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files Marc Titinger
` (2 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Marc Titinger @ 2015-10-06 14:27 UTC (permalink / raw)
To: khilman, rjw
Cc: lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
Cpuidle now handles c-states and power-states differently. c-states do not decrement
the reference count for the CPUs in the cluster, while power-states i.e.
cluster level states like 'CLUSTER_SLEEP_0' in the case of juno, will.
The 'D1' fake device also registers intermediate power-state,
for experimentation.
Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
arch/arm64/boot/dts/arm/juno.dts | 2 +-
drivers/cpuidle/cpuidle-arm.c | 52 ++++++++++++++++++++++++++++++++--------
2 files changed, 43 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index cadc5de..0bb0dd7 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -47,7 +47,7 @@
};
CLUSTER_SLEEP_0: cluster-sleep-0 {
- compatible = "arm,idle-state","arm,power-state";
+ compatible = "arm,power-state";
arm,psci-suspend-param = <0x1010000>;
local-timer-stop;
entry-latency-us = <800>;
diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index 7c791f9..8dd5dc3 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c
@@ -40,7 +40,6 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)
{
int ret;
- struct device *cpu_dev = get_cpu_device(dev->cpu);
if (!idx) {
cpu_do_idle();
@@ -50,18 +49,49 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
ret = cpu_pm_enter();
if (!ret) {
/*
- * Notify runtime PM as well of this cpu powering down
- * TODO: Merge CPU_PM and runtime PM.
- */
- RCU_NONIDLE(pm_runtime_put_sync(cpu_dev));
-
- /*
* Pass idle state index to cpu_suspend which in turn will
* call the CPU ops suspend protocol with idle index as a
* parameter.
*/
arm_cpuidle_suspend(idx);
+ cpu_pm_exit();
+ }
+
+ return ret ? -1 : idx;
+}
+
+/*
+ * arm_enter_power_state - delegate state trasition to genpd
+ *
+ * dev: cpuidle device
+ * drv: cpuidle driver
+ * idx: state index
+ *
+ * Called from the CPUidle framework to delegate a state transition
+ * to the generic domain. This will be a cluster poweroff state
+ * the Domain will chose to actually turn off the cluster based on
+ * the status of other CPUs, and devices and subdomains in the Cluster
+ * domain.
+*/
+static int arm_enter_power_state(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int idx)
+{
+ int ret;
+ struct device *cpu_dev = get_cpu_device(dev->cpu);
+
+ BUG_ON(idx == 0);
+
+ ret = cpu_pm_enter();
+ if (!ret) {
+ /*
+ * Notify runtime PM as well of this cpu powering down
+ * TODO: Merge CPU_PM and runtime PM.
+ */
+ RCU_NONIDLE(pm_runtime_put_sync(cpu_dev));
+
+ arm_cpuidle_suspend(idx);
+
RCU_NONIDLE(pm_runtime_get_sync(cpu_dev));
cpu_pm_exit();
}
@@ -69,6 +99,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
return ret ? -1 : idx;
}
+
static struct cpuidle_driver arm_idle_driver = {
.name = "arm_idle",
.owner = THIS_MODULE,
@@ -90,9 +121,10 @@ static struct cpuidle_driver arm_idle_driver = {
};
static const struct of_device_id arm_idle_state_match[] __initconst = {
- { .compatible = "arm,idle-state",
- .data = arm_enter_idle_state },
- { },
+ {.compatible = "arm,idle-state",
+ .data = arm_enter_idle_state},
+ {.compatible = "arm,power-state",
+ .data = arm_enter_power_state},
};
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states'
2015-10-06 14:27 ` [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states' Marc Titinger
@ 2015-11-11 9:10 ` Zhaoyang Huang
2015-11-11 17:27 ` Lina Iyer
0 siblings, 1 reply; 39+ messages in thread
From: Zhaoyang Huang @ 2015-11-11 9:10 UTC (permalink / raw)
To: Marc Titinger
Cc: khilman, rjw, Lina Iyer, Axel Haslam, Benoit Cousson, linux-pm,
linux-kernel
On 6 October 2015 at 22:27, Marc Titinger <mtitinger@baylibre.com> wrote:
> From: Marc Titinger <mtitinger@baylibre.com>
>
> Cpuidle now handles c-states and power-states differently. c-states do not decrement
> the reference count for the CPUs in the cluster, while power-states i.e.
> cluster level states like 'CLUSTER_SLEEP_0' in the case of juno, will.
>
> The 'D1' fake device also registers intermediate power-state,
> for experimentation.
>
> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
> ---
> arch/arm64/boot/dts/arm/juno.dts | 2 +-
> drivers/cpuidle/cpuidle-arm.c | 52 ++++++++++++++++++++++++++++++++--------
> 2 files changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
> index cadc5de..0bb0dd7 100644
> --- a/arch/arm64/boot/dts/arm/juno.dts
> +++ b/arch/arm64/boot/dts/arm/juno.dts
> @@ -47,7 +47,7 @@
> };
>
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> - compatible = "arm,idle-state","arm,power-state";
> + compatible = "arm,power-state";
> arm,psci-suspend-param = <0x1010000>;
> local-timer-stop;
> entry-latency-us = <800>;
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index 7c791f9..8dd5dc3 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -40,7 +40,6 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int idx)
> {
> int ret;
> - struct device *cpu_dev = get_cpu_device(dev->cpu);
>
> if (!idx) {
> cpu_do_idle();
> @@ -50,18 +49,49 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
> ret = cpu_pm_enter();
> if (!ret) {
> /*
> - * Notify runtime PM as well of this cpu powering down
> - * TODO: Merge CPU_PM and runtime PM.
> - */
> - RCU_NONIDLE(pm_runtime_put_sync(cpu_dev));
> -
> - /*
> * Pass idle state index to cpu_suspend which in turn will
> * call the CPU ops suspend protocol with idle index as a
> * parameter.
> */
> arm_cpuidle_suspend(idx);
>
> + cpu_pm_exit();
> + }
> +
> + return ret ? -1 : idx;
> +}
> +
> +/*
> + * arm_enter_power_state - delegate state trasition to genpd
> + *
> + * dev: cpuidle device
> + * drv: cpuidle driver
> + * idx: state index
> + *
> + * Called from the CPUidle framework to delegate a state transition
> + * to the generic domain. This will be a cluster poweroff state
> + * the Domain will chose to actually turn off the cluster based on
> + * the status of other CPUs, and devices and subdomains in the Cluster
> + * domain.
> +*/
> +static int arm_enter_power_state(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int idx)
> +{
> + int ret;
> + struct device *cpu_dev = get_cpu_device(dev->cpu);
> +
> + BUG_ON(idx == 0);
> +
> + ret = cpu_pm_enter();
> + if (!ret) {
> + /*
> + * Notify runtime PM as well of this cpu powering down
> + * TODO: Merge CPU_PM and runtime PM.
> + */
> + RCU_NONIDLE(pm_runtime_put_sync(cpu_dev));
[question]: Does it mean that above function will use the gpd->rpm->idle?
> +
> + arm_cpuidle_suspend(idx);
> +
> RCU_NONIDLE(pm_runtime_get_sync(cpu_dev));
> cpu_pm_exit();
> }
> @@ -69,6 +99,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
> return ret ? -1 : idx;
> }
>
> +
> static struct cpuidle_driver arm_idle_driver = {
> .name = "arm_idle",
> .owner = THIS_MODULE,
> @@ -90,9 +121,10 @@ static struct cpuidle_driver arm_idle_driver = {
> };
>
> static const struct of_device_id arm_idle_state_match[] __initconst = {
> - { .compatible = "arm,idle-state",
> - .data = arm_enter_idle_state },
> - { },
> + {.compatible = "arm,idle-state",
> + .data = arm_enter_idle_state},
> + {.compatible = "arm,power-state",
> + .data = arm_enter_power_state},
> };
>
> /*
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states'
2015-11-11 9:10 ` Zhaoyang Huang
@ 2015-11-11 17:27 ` Lina Iyer
0 siblings, 0 replies; 39+ messages in thread
From: Lina Iyer @ 2015-11-11 17:27 UTC (permalink / raw)
To: Zhaoyang Huang
Cc: Marc Titinger, khilman, rjw, Axel Haslam, Benoit Cousson,
linux-pm, linux-kernel
On Wed, Nov 11 2015 at 02:10 -0700, Zhaoyang Huang wrote:
>On 6 October 2015 at 22:27, Marc Titinger <mtitinger@baylibre.com> wrote:
>> From: Marc Titinger <mtitinger@baylibre.com>
>>
>> Cpuidle now handles c-states and power-states differently. c-states do not decrement
>> the reference count for the CPUs in the cluster, while power-states i.e.
>> cluster level states like 'CLUSTER_SLEEP_0' in the case of juno, will.
>>
>> The 'D1' fake device also registers intermediate power-state,
>> for experimentation.
>>
>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
>> ---
>> arch/arm64/boot/dts/arm/juno.dts | 2 +-
>> drivers/cpuidle/cpuidle-arm.c | 52 ++++++++++++++++++++++++++++++++--------
>> 2 files changed, 43 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
>> index cadc5de..0bb0dd7 100644
>> --- a/arch/arm64/boot/dts/arm/juno.dts
>> +++ b/arch/arm64/boot/dts/arm/juno.dts
>> @@ -47,7 +47,7 @@
>> };
>>
>> CLUSTER_SLEEP_0: cluster-sleep-0 {
>> - compatible = "arm,idle-state","arm,power-state";
>> + compatible = "arm,power-state";
>> arm,psci-suspend-param = <0x1010000>;
>> local-timer-stop;
>> entry-latency-us = <800>;
>> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
>> index 7c791f9..8dd5dc3 100644
>> --- a/drivers/cpuidle/cpuidle-arm.c
>> +++ b/drivers/cpuidle/cpuidle-arm.c
>> @@ -40,7 +40,6 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
>> struct cpuidle_driver *drv, int idx)
>> {
>> int ret;
>> - struct device *cpu_dev = get_cpu_device(dev->cpu);
>>
>> if (!idx) {
>> cpu_do_idle();
>> @@ -50,18 +49,49 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
>> ret = cpu_pm_enter();
>> if (!ret) {
>> /*
>> - * Notify runtime PM as well of this cpu powering down
>> - * TODO: Merge CPU_PM and runtime PM.
>> - */
>> - RCU_NONIDLE(pm_runtime_put_sync(cpu_dev));
>> -
>> - /*
>> * Pass idle state index to cpu_suspend which in turn will
>> * call the CPU ops suspend protocol with idle index as a
>> * parameter.
>> */
>> arm_cpuidle_suspend(idx);
>>
>> + cpu_pm_exit();
>> + }
>> +
>> + return ret ? -1 : idx;
>> +}
>> +
>> +/*
>> + * arm_enter_power_state - delegate state trasition to genpd
>> + *
>> + * dev: cpuidle device
>> + * drv: cpuidle driver
>> + * idx: state index
>> + *
>> + * Called from the CPUidle framework to delegate a state transition
>> + * to the generic domain. This will be a cluster poweroff state
>> + * the Domain will chose to actually turn off the cluster based on
>> + * the status of other CPUs, and devices and subdomains in the Cluster
>> + * domain.
>> +*/
>> +static int arm_enter_power_state(struct cpuidle_device *dev,
>> + struct cpuidle_driver *drv, int idx)
>> +{
>> + int ret;
>> + struct device *cpu_dev = get_cpu_device(dev->cpu);
>> +
>> + BUG_ON(idx == 0);
>> +
>> + ret = cpu_pm_enter();
>> + if (!ret) {
>> + /*
>> + * Notify runtime PM as well of this cpu powering down
>> + * TODO: Merge CPU_PM and runtime PM.
>> + */
>> + RCU_NONIDLE(pm_runtime_put_sync(cpu_dev));
>[question]: Does it mean that above function will use the gpd->rpm->idle?
Will end up at rpm_suspend() in runtime.c which will callinto genpd. Its
not a direct call to genpd.
Thanks,
Lina
>> +
>> + arm_cpuidle_suspend(idx);
>> +
>> RCU_NONIDLE(pm_runtime_get_sync(cpu_dev));
>> cpu_pm_exit();
>> }
>> @@ -69,6 +99,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
>> return ret ? -1 : idx;
>> }
>>
>> +
>> static struct cpuidle_driver arm_idle_driver = {
>> .name = "arm_idle",
>> .owner = THIS_MODULE,
>> @@ -90,9 +121,10 @@ static struct cpuidle_driver arm_idle_driver = {
>> };
>>
>> static const struct of_device_id arm_idle_state_match[] __initconst = {
>> - { .compatible = "arm,idle-state",
>> - .data = arm_enter_idle_state },
>> - { },
>> + {.compatible = "arm,idle-state",
>> + .data = arm_enter_idle_state},
>> + {.compatible = "arm,power-state",
>> + .data = arm_enter_power_state},
>> };
>>
>> /*
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC v2 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
` (4 preceding siblings ...)
2015-10-06 14:27 ` [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states' Marc Titinger
@ 2015-10-06 14:27 ` Marc Titinger
2015-10-13 23:10 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Kevin Hilman
2015-10-19 20:58 ` Lina Iyer
7 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-06 14:27 UTC (permalink / raw)
To: khilman, rjw
Cc: lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
This purpose of these debug seq-files is to help investigate
generic power domain state transitions, based on device constraints.
requires the "multiple states" patches from Axel Haslam.
also rename 'summary' from 'pm_genpd_summary'
sample output for 'states'
==========================
Domain State name Eval = 2200nter + Exit = Min_off_on (ns)
-----------------------------------------------------------------------
a53_pd cluster-sleep-0 1500000+800000=2300000
a57_pd d1-retention 1000000+800000=1800000
a57_pd cluster-sleep-0 1500000+800000=2300000
sample output for 'timings'
===========================
Domain Devices, Timings in ns
Stop/Start Save/Restore, Effective
---------------------------------------------------- ---
a53_pd
/cpus/cpu@100 1060 /660 1580 /1940 ,0 (cached stop)
/cpus/cpu@101 1060 /740 1520 /1600 ,0 (cached stop)
/cpus/cpu@102 880 /620 1380 /1780 ,0 (cached stop)
/cpus/cpu@103 1080 /640 1340 /1600 ,0 (cached stop)
a57_pd
/cpus/cpu@0 1160 /740 3280 /1800 ,0 (cached stop)
/cpus/cpu@1 780 /1400 1440 /2080 ,0 (cached stop)
/D1 600 /540 7140 /6420 ,2199460 (cached stop)
Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
drivers/base/power/domain.c | 107 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 105 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index ebf1299..5de2112 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2468,6 +2468,96 @@ static const struct file_operations pm_genpd_summary_fops = {
.release = single_release,
};
+static int pm_genpd_states_show(struct seq_file *s, void *data)
+{
+ struct generic_pm_domain *genpd;
+
+ seq_puts(s,
+ "\n Domain State name Enter + Exit = Min_off_on (ns)\n");
+ seq_puts(s,
+ "-----------------------------------------------------------------------\n");
+
+ list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+
+ int i;
+
+ for (i = 0; i < genpd->state_count; i++) {
+ seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n",
+ genpd->name,
+ genpd->states[i].name,
+ genpd->states[i].power_on_latency_ns,
+ genpd->states[i].power_off_latency_ns,
+ genpd->states[i].power_off_latency_ns
+ + genpd->states[i].power_on_latency_ns);
+ }
+
+ }
+
+ seq_puts(s, "\n");
+
+ return 0;
+}
+
+static int pm_genpd_states_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, pm_genpd_states_show, NULL);
+}
+
+static const struct file_operations pm_genpd_states_fops = {
+ .open = pm_genpd_states_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static int pm_genpd_timing_show(struct seq_file *s, void *data)
+{
+ struct generic_pm_domain *genpd;
+
+ seq_puts(s, "\n Domain Devices, Timings in ns\n");
+ seq_puts(s,
+ " Stop/Start Save/Restore, Effective\n");
+ seq_puts(s,
+ "---------------------------------------------------- ---\n");
+
+ list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+ struct pm_domain_data *pm_data;
+
+ seq_printf(s, "%-30s", genpd->name);
+
+ list_for_each_entry(pm_data, &genpd->dev_list, list_node) {
+ struct gpd_timing_data *td = &to_gpd_data(pm_data)->td;
+
+ if (!pm_data->dev->of_node)
+ continue;
+
+ seq_printf(s,
+ "\n %-20s %-6lld/%-6lld %-6lld/%-6lld,%lld %s%s",
+ pm_data->dev->of_node->full_name,
+ td->stop_latency_ns, td->start_latency_ns,
+ td->save_state_latency_ns,
+ td->restore_state_latency_ns,
+ td->effective_constraint_ns,
+ td->cached_stop_ok ? "(cached stop) " : "",
+ td->constraint_changed ? "(changed)" : "");
+ }
+ seq_puts(s, "\n");
+ }
+ return 0;
+}
+
+static int pm_genpd_timing_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, pm_genpd_timing_show, NULL);
+}
+
+static const struct file_operations pm_genpd_timing_fops = {
+ .open = pm_genpd_timing_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
static int __init pm_genpd_debug_init(void)
{
struct dentry *d;
@@ -2477,8 +2567,21 @@ static int __init pm_genpd_debug_init(void)
if (!pm_genpd_debugfs_dir)
return -ENOMEM;
- d = debugfs_create_file("pm_genpd_summary", S_IRUGO,
- pm_genpd_debugfs_dir, NULL, &pm_genpd_summary_fops);
+ d = debugfs_create_file("summary", S_IRUGO,
+ pm_genpd_debugfs_dir, NULL,
+ &pm_genpd_summary_fops);
+ if (!d)
+ return -ENOMEM;
+
+ d = debugfs_create_file("states", S_IRUGO,
+ pm_genpd_debugfs_dir, NULL,
+ &pm_genpd_states_fops);
+ if (!d)
+ return -ENOMEM;
+
+ d = debugfs_create_file("timings", S_IRUGO,
+ pm_genpd_debugfs_dir, NULL,
+ &pm_genpd_timing_fops);
if (!d)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
` (5 preceding siblings ...)
2015-10-06 14:27 ` [RFC v2 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files Marc Titinger
@ 2015-10-13 23:10 ` Kevin Hilman
2015-10-14 8:10 ` Axel Haslam
2015-10-19 20:58 ` Lina Iyer
7 siblings, 1 reply; 39+ messages in thread
From: Kevin Hilman @ 2015-10-13 23:10 UTC (permalink / raw)
To: Marc Titinger
Cc: rjw, lina.iyer, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Hi Marc,
Sorry for the lag in reviewing this. I've been on the road for a couple
weeks of conference related travel. I did have a look through this on
the plane, but didn't have time to reply until now, and then had to
rework my thoughts a bit since Lina already said some of the things I
was thinking. Anyways, some overall thoughts below...
Marc Titinger <mtitinger@baylibre.com> writes:
> v2:
> - rebase on Lina Iyer's latest series
> - remove unnecessary dependency on perf-state patches from Axel Haslam
>
> -----------------------
>
> Summary
>
> 1) DESCRIPTION
> 2) DEPENDENCIES
> 3) URL
> ------------------------
>
>
> 1) DESCRIPTION
>
>
> This patch set's underlying idea is that cluster-level c-states can be managed
nit: can we drop the use of C-states throughout and just refer to idle
states. C-states is technically an ACPI specific term.
> by the power domain, building upon Lina Iyers recent work on CPU-domain, and Axel Haslam's
> genpd multiple states. The power domain may contain CPU devices and non-CPU devices.
@Axel: any plans to repost your series now that we have a little more
momentum here?
> Non-CPU Devices may expose latency constraints by registering intermediate power-states upon
> probing, for instance shallower states than the deepest cluster-off state. The generic
> power domain governor may chose a device retention state in place of the cluster-sleep
> state demanded by the menu governor, and call the platform specific handling to enter/leave
> that retention state.
>
>
> power-states
> -----------
>
>
> The proposed way how cluster-level c-states are declared as manageable by the
> power domain, rather than through the cpuidle-ops, relies on the introduction of
> "power-states", consistent with c-states. Here is an example of the DT bindings,
> the c-state CLUSTER_SLEEP_0 is exposed as a power-state in the compatible property:
>
> juno.dts: idle-states {
> entry-method = "arm,psci";
>
> CPU_SLEEP_0: cpu-sleep-0 {
> compatible = "arm,idle-state";
> arm,psci-suspend-param = <0x0010000>;
> local-timer-stop;
> entry-latency-us = <100>;
> exit-latency-us = <250>;
> min-residency-us = <2000>;
> };
>
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,power-state";
> arm,psci-suspend-param = <0x1010000>;
> local-timer-stop;
> entry-latency-us = <800>;
> exit-latency-us = <700>;
> min-residency-us = <2500>;
> };
> }
>
> This will tell cpuidle runtime_put/get the CPU devices for this c-state. Eventually, the
> actual platform handlers may be called from the genpd platform ops (in place of cpuidle_ops).
>
> "drivers/cpuidle/cpuidle-arm.c":
>
> static const struct of_device_id arm_idle_state_match[] __initconst = {
> {.compatible = "arm,idle-state",
> .data = arm_enter_idle_state},
> {.compatible = "arm,power-state",
> .data = arm_enter_power_state},
> };
>
>
> In case of a power-state, arm_enter_power_state will only call pm_runtime_put/get_sync
> The power doamin will handle the power off,
I'm not crazy about this part of the design. Essentially it delegates
the cluster level decision making to the CPUidle driver. IMO, all of
the decision making should be at the domain level.
> currently this patch set lacks the final
> call to the psci interface to have a fully fonctionnal setup
> (and there are some genpd_lock'ing issues if put/get actually suspend the CPU device.)
> Ultimately, we would like the Power Domain's simple governor to being able to chose
> the cluster power-state based on the c-states defered to it (power-states) and constraints
> added by the devices. Consequently, we need to "soak" those power-states into the
> power-domain intermediate states from Axel. Since power-states are declared and handled
> the same manner than c-states (idle-states in DT), these patches add a soaking used when
> attaching to a genpd, where power-states are parsed from the DT into the genpd states:
>
>
> "drivers/base/power/domain.c":
>
> static const struct of_device_id power_state_match[] = {
> {.compatible = "arm,power-state",
> },
> };
>
> int of_genpd_device_parse_states(struct device_node *np,
> struct generic_pm_domain *genpd)
This approach results in some strange (IMO) sharing of the
responsibility, as can be seen because both the genpd and the cpuidle
driver end up parsing the new arm,power-state states.
IMO, the CPUidle driver should not have to be aware of what cluster
states are availble, nor what other devices may or may not be in the
same domain as a given CPU.
Maybe another way to differentiate between CPU idle states and cluster
idle states is to associate them with a power-domain node in the DT
(instead of a CPU node as is done currently.) Then there would also not
be a need for another compatible string. As Lina pointed out, your
arm,power-state has identical properties to an arm,idle-state so doesn't
really justify a new compatible.
> debugfs addition
> ---------------
>
> To easy debug, this patch set adds a seq-file names "states" to the pm_genpd debugfs:
>
> cat /sys/kernel/debug/pm_genpd/*
>
> Domain State name Enter (ns) / Exit (ns)
> -------------------------------------------------------------
> a53_pd cluster-sleep-0 1500000 / 800000
> a57_pd cluster-sleep-0 1500000 / 800000
I guess this should probably be part of Axel's "multiple state" series.
> And also a seq-file "timings", to help visualize the constrains of the non-CPU
> devices in a cluster PD.
>
> Domain Devices, Timings in ns
> Stop/Start Save/Restore, Effective
> ---------------------------------------------------- ---
> a57_pd
> /cpus/cpu@0 800 /740 1320 /1720 ,0 (cached stop)
> /cpus/cpu@1 800 /740 1420 /1780 ,0 (cached stop)
> /D1 660 /580 16560 /6080 ,2199420 (cached stop)
Nice!
> Device power-states
> -------------------
>
> some devices, like L2 caches, may feature a shallower retention mode, between CPU_SLEEP_0
> and CLUSTER_SLEEP_0, in which mode the L2 memory is not powered off, leading to faster
> resume than CLUSTER_SLEEP_0.
>
> One way to handle device constrains and retention features in the power-domain, is to
> allow devices to register a new power-state (consistent with a c-state).
The problem with this is that the power-state is not technically a
property of the device. It's a property of the power domain.
> idle-states:
>
> D1_RETENTION: d1-retention {
> compatible = "arm,power-state";
> /*leave the psci param, for demo/testing:
> * the psci cpuidle driver will not currently
> * understand that a c-state shall not have it's
> * table entry with a firmware command.
> * the actual .power_on/off would be registered
> * by the DECLARE macro for a given domain*/
> arm,psci-suspend-param = <0x1010000>;
> local-timer-stop;
> entry-latency-us = <800>;
> exit-latency-us = <200>;
> min-residency-us = <2500>;
> };
>
>
> D1 {
> compatible = "fake,fake-driver";
> name = "D1";
> constraint = <30000>;
> power-domains = <&a53_pd>;
> power-states =<&D1_RETENTION>;
> };
>
>
> The genpd simple governor can now upon suspend of the last-man CPU chose a shallower
> retention state than CLUSTER_SLEEP_0.
>
> In order to achieve this, this patch set added the power-state parsing during the
> genpd_dev_pm_attach call. Multiple genpd states are now inserted in a sorted manner
> according to their depth: see pm_genpd_insert_state in "drivers/base/power/domain.c".
As mentioned elsewhere, I think the ability to dynamically add (and
presumably remove) genpd states is a bit overkill right now. I think
the list of power states should be statically defined along with the
power-domain itself, and then it should simply rely on the runtime PM
usage of all the devices in the domain (as well as the genpd governors)
to select the right state.
Kevin
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains
2015-10-13 23:10 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Kevin Hilman
@ 2015-10-14 8:10 ` Axel Haslam
0 siblings, 0 replies; 39+ messages in thread
From: Axel Haslam @ 2015-10-14 8:10 UTC (permalink / raw)
To: Kevin Hilman
Cc: Marc Titinger, Rafael J. Wysocki, Lina Iyer, Benoit Cousson,
Linux PM list, linux-kernel, Marc Titinger
> @Axel: any plans to repost your series now that we have a little more
> momentum here?
>
sure, no problem, ill rebase adding Marc's patch for the debugfs file
and re-post them.
Regards,
Axel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains
2015-10-06 14:27 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger
` (6 preceding siblings ...)
2015-10-13 23:10 ` [RFC v2 0/6] Managing cluser-level c-states with generic power domains Kevin Hilman
@ 2015-10-19 20:58 ` Lina Iyer
2015-10-20 9:10 ` Marc Titinger
` (8 more replies)
7 siblings, 9 replies; 39+ messages in thread
From: Lina Iyer @ 2015-10-19 20:58 UTC (permalink / raw)
To: Marc Titinger
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Hi Marc,
I am trying to apply this on top of Axel's patches on linux-next (after
fixing issues I saw with his v9), and running to issues applying your
patches. Could you rebase on top of his v10 (he said he would send to
the ML soon) ?
Thanks,
Lina
On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>v2:
> - rebase on Lina Iyer's latest series
> - remove unnecessary dependency on perf-state patches from Axel Haslam
>
>-----------------------
>
>Summary
>
>1) DESCRIPTION
>2) DEPENDENCIES
>3) URL
>------------------------
>
>
>1) DESCRIPTION
>
>
> This patch set's underlying idea is that cluster-level c-states can be managed
>by the power domain, building upon Lina Iyers recent work on CPU-domain, and Axel Haslam's
>genpd multiple states. The power domain may contain CPU devices and non-CPU devices.
>
>Non-CPU Devices may expose latency constraints by registering intermediate power-states upon
>probing, for instance shallower states than the deepest cluster-off state. The generic
>power domain governor may chose a device retention state in place of the cluster-sleep
>state demanded by the menu governor, and call the platform specific handling to enter/leave
>that retention state.
>
>
>power-states
>-----------
>
>
>The proposed way how cluster-level c-states are declared as manageable by the
>power domain, rather than through the cpuidle-ops, relies on the introduction of
>"power-states", consistent with c-states. Here is an example of the DT bindings,
>the c-state CLUSTER_SLEEP_0 is exposed as a power-state in the compatible property:
>
>juno.dts: idle-states {
> entry-method = "arm,psci";
>
> CPU_SLEEP_0: cpu-sleep-0 {
> compatible = "arm,idle-state";
> arm,psci-suspend-param = <0x0010000>;
> local-timer-stop;
> entry-latency-us = <100>;
> exit-latency-us = <250>;
> min-residency-us = <2000>;
> };
>
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,power-state";
> arm,psci-suspend-param = <0x1010000>;
> local-timer-stop;
> entry-latency-us = <800>;
> exit-latency-us = <700>;
> min-residency-us = <2500>;
> };
> }
>
>This will tell cpuidle runtime_put/get the CPU devices for this c-state. Eventually, the
>actual platform handlers may be called from the genpd platform ops (in place of cpuidle_ops).
>
>"drivers/cpuidle/cpuidle-arm.c":
>
>static const struct of_device_id arm_idle_state_match[] __initconst = {
> {.compatible = "arm,idle-state",
> .data = arm_enter_idle_state},
> {.compatible = "arm,power-state",
> .data = arm_enter_power_state},
>};
>
>
>In case of a power-state, arm_enter_power_state will only call pm_runtime_put/get_sync
>The power doamin will handle the power off, currently this patch set lacks the final
>call to the psci interface to have a fully fonctionnal setup
>(and there are some genpd_lock'ing issues if put/get actually suspend the CPU device.)
>
>Ultimately, we would like the Power Domain's simple governor to being able to chose
>the cluster power-state based on the c-states defered to it (power-states) and constraints
>added by the devices. Consequently, we need to "soak" those power-states into the
>power-domain intermediate states from Axel. Since power-states are declared and handled
>the same manner than c-states (idle-states in DT), these patches add a soaking used when
>attaching to a genpd, where power-states are parsed from the DT into the genpd states:
>
>
>"drivers/base/power/domain.c":
>
>static const struct of_device_id power_state_match[] = {
> {.compatible = "arm,power-state",
> },
>};
>
>int of_genpd_device_parse_states(struct device_node *np,
> struct generic_pm_domain *genpd)
>
>debugfs addition
>---------------
>
>To easy debug, this patch set adds a seq-file names "states" to the pm_genpd debugfs:
>
> cat /sys/kernel/debug/pm_genpd/*
>
> Domain State name Enter (ns) / Exit (ns)
> -------------------------------------------------------------
> a53_pd cluster-sleep-0 1500000 / 800000
> a57_pd cluster-sleep-0 1500000 / 800000
>
>And also a seq-file "timings", to help visualize the constrains of the non-CPU
>devices in a cluster PD.
>
> Domain Devices, Timings in ns
> Stop/Start Save/Restore, Effective
>---------------------------------------------------- ---
>a57_pd
> /cpus/cpu@0 800 /740 1320 /1720 ,0 (cached stop)
> /cpus/cpu@1 800 /740 1420 /1780 ,0 (cached stop)
> /D1 660 /580 16560 /6080 ,2199420 (cached stop)
>
>
>Device power-states
>-------------------
>
>some devices, like L2 caches, may feature a shallower retention mode, between CPU_SLEEP_0
>and CLUSTER_SLEEP_0, in which mode the L2 memory is not powered off, leading to faster
>resume than CLUSTER_SLEEP_0.
>
>One way to handle device constrains and retention features in the power-domain, is to
>allow devices to register a new power-state (consistent with a c-state).
>
>idle-states:
>
> D1_RETENTION: d1-retention {
> compatible = "arm,power-state";
> /*leave the psci param, for demo/testing:
> * the psci cpuidle driver will not currently
> * understand that a c-state shall not have it's
> * table entry with a firmware command.
> * the actual .power_on/off would be registered
> * by the DECLARE macro for a given domain*/
> arm,psci-suspend-param = <0x1010000>;
> local-timer-stop;
> entry-latency-us = <800>;
> exit-latency-us = <200>;
> min-residency-us = <2500>;
> };
>
>
> D1 {
> compatible = "fake,fake-driver";
> name = "D1";
> constraint = <30000>;
> power-domains = <&a53_pd>;
> power-states =<&D1_RETENTION>;
> };
>
>
>The genpd simple governor can now upon suspend of the last-man CPU chose a shallower
>retention state than CLUSTER_SLEEP_0.
>
>In order to achieve this, this patch set added the power-state parsing during the
>genpd_dev_pm_attach call. Multiple genpd states are now inserted in a sorted manner
>according to their depth: see pm_genpd_insert_state in "drivers/base/power/domain.c".
>
>
>
>2) DEPENDENCIES
>
> This patch set applies over linux-4.2rc5 plus the following ordered dependencies:
>
> * Ulf Hansson:
>
>6637131 New [V4] PM / Domains: Remove intermediate states from the power off sequence
>
> * Lina Iyer's patch series:
>
>7118981 Not Applicable [v2,1/7] PM / Domains: Allocate memory outside domain locks
>7118991 Not Applicable [v2,2/7] PM / Domains: Support IRQ safe PM domains
>7119001 Not Applicable [v2,3/7] drivers: cpu: Define CPU devices as IRQ safe
>7119011 Not Applicable [v2,4/7] PM / Domains: Introduce PM domains for CPUs/clusters
>7119021 Not Applicable [v2,5/7] ARM: cpuidle: Add runtime PM support for CPU idle
>7119031 Not Applicable [v2,6/7] ARM64: smp: Add runtime PM support for CPU hotplug
>7119041 Not Applicable [v2,7/7] ARM: smp: Add runtime PM support for CPU hotplug
>
> * John Medhurst:
>
>6303671 New arm64: dts: Add idle-states for Juno
>
> * Axel Haslam:
>
>6301741 Not Applicable [v7,1/5] PM / Domains: prepare for multiple states
>6301751 Not Applicable [v7,2/5] PM / Domains: core changes for multiple states
>6301781 Not Applicable [v7,3/5] PM / Domains: make governor select deepest state
>6301771 Not Applicable [v7,4/5] ARM: imx6: pm: declare pm domain latency on power_state struct.
>6301761 Not Applicable [v7,5/5] PM / Domains: remove old power on/off latencies.
>
>2) URL
>
>playable from https://github.com/mtitinger/linux-pm.git
>
>by adding the "fake driver D1" and launching the test-dev-state.sh script.
>this will show the power domain suspending to an intermediate state, based on the
>device constraints.
>
> domain status pstate slaves
> /device runtime status
>-----------------------------------------------------------------------------------
>a53_pd on
> /devices/system/cpu/cpu0 active
> /devices/system/cpu/cpu3 suspended
> /devices/system/cpu/cpu4 suspended
> /devices/system/cpu/cpu5 suspended
>a57_pd d1-retention
> /devices/system/cpu/cpu1 suspended
> /devices/system/cpu/cpu2 suspended
> /devices/platform/D1
>
>----------------------------------------------------------------------
>
>Marc Titinger (6):
> arm64: Juno: declare generic power domains for both clusters.
> PM / Domains: prepare for devices that might register a power state
> PM / Domains: introduce power-states consistent with c-states.
> PM / Domains: succeed & warn when attaching non-irqsafe devices to an
> irq-safe domain.
> arm: cpuidle: let genpd handle the cluster power transition with
> 'power-states'
> PM / Domains: add debugfs 'states' and 'timings' seq files
>
> .../devicetree/bindings/arm/idle-states.txt | 21 +-
> .../devicetree/bindings/power/power_domain.txt | 29 ++
> arch/arm64/boot/dts/arm/juno.dts | 25 +-
> drivers/base/power/cpu-pd.c | 5 +
> drivers/base/power/domain.c | 415 +++++++++++++++------
> drivers/cpuidle/cpuidle-arm.c | 52 ++-
> include/linux/pm_domain.h | 21 +-
> 7 files changed, 437 insertions(+), 131 deletions(-)
>
>--
>1.9.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains
2015-10-19 20:58 ` Lina Iyer
@ 2015-10-20 9:10 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 0/7] Managing cluser-level idle-states " Marc Titinger
` (7 subsequent siblings)
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-20 9:10 UTC (permalink / raw)
To: Lina Iyer
Cc: khilman, rjw, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
On 19/10/2015 22:58, Lina Iyer wrote:
> Hi Marc,
>
> I am trying to apply this on top of Axel's patches on linux-next (after
> fixing issues I saw with his v9), and running to issues applying your
> patches. Could you rebase on top of his v10 (he said he would send to
> the ML soon) ?
>
Hi Lina,
I want to replay this with Juno this afternoon first, I'll post ASAP.
Also, based on Kevin's comment I was wondering if I should drop this
path already and try the other way as discussed (hook l2 devices to
runtime-pm, through the CPU device), but I still need to think about
this first.
Cheers,
Marc.
> Thanks,
> Lina
>
> On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
>> v2:
>> - rebase on Lina Iyer's latest series
>> - remove unnecessary dependency on perf-state patches from Axel Haslam
>>
>> -----------------------
>>
>> Summary
>>
>> 1) DESCRIPTION
>> 2) DEPENDENCIES
>> 3) URL
>> ------------------------
>>
>>
>> 1) DESCRIPTION
>>
>>
>> This patch set's underlying idea is that cluster-level c-states
>> can be managed
>> by the power domain, building upon Lina Iyers recent work on
>> CPU-domain, and Axel Haslam's
>> genpd multiple states. The power domain may contain CPU devices and
>> non-CPU devices.
>>
>> Non-CPU Devices may expose latency constraints by registering
>> intermediate power-states upon
>> probing, for instance shallower states than the deepest cluster-off
>> state. The generic
>> power domain governor may chose a device retention state in place of
>> the cluster-sleep
>> state demanded by the menu governor, and call the platform specific
>> handling to enter/leave
>> that retention state.
>>
>>
>> power-states
>> -----------
>>
>>
>> The proposed way how cluster-level c-states are declared as manageable
>> by the
>> power domain, rather than through the cpuidle-ops, relies on the
>> introduction of
>> "power-states", consistent with c-states. Here is an example of the DT
>> bindings,
>> the c-state CLUSTER_SLEEP_0 is exposed as a power-state in the
>> compatible property:
>>
>> juno.dts: idle-states {
>> entry-method = "arm,psci";
>>
>> CPU_SLEEP_0: cpu-sleep-0 {
>> compatible = "arm,idle-state";
>> arm,psci-suspend-param = <0x0010000>;
>> local-timer-stop;
>> entry-latency-us = <100>;
>> exit-latency-us = <250>;
>> min-residency-us = <2000>;
>> };
>>
>> CLUSTER_SLEEP_0: cluster-sleep-0 {
>> compatible = "arm,power-state";
>> arm,psci-suspend-param = <0x1010000>;
>> local-timer-stop;
>> entry-latency-us = <800>;
>> exit-latency-us = <700>;
>> min-residency-us = <2500>;
>> };
>> }
>>
>> This will tell cpuidle runtime_put/get the CPU devices for this
>> c-state. Eventually, the
>> actual platform handlers may be called from the genpd platform ops (in
>> place of cpuidle_ops).
>>
>> "drivers/cpuidle/cpuidle-arm.c":
>>
>> static const struct of_device_id arm_idle_state_match[] __initconst = {
>> {.compatible = "arm,idle-state",
>> .data = arm_enter_idle_state},
>> {.compatible = "arm,power-state",
>> .data = arm_enter_power_state},
>> };
>>
>>
>> In case of a power-state, arm_enter_power_state will only call
>> pm_runtime_put/get_sync
>> The power doamin will handle the power off, currently this patch set
>> lacks the final
>> call to the psci interface to have a fully fonctionnal setup
>> (and there are some genpd_lock'ing issues if put/get actually suspend
>> the CPU device.)
>>
>> Ultimately, we would like the Power Domain's simple governor to being
>> able to chose
>> the cluster power-state based on the c-states defered to it
>> (power-states) and constraints
>> added by the devices. Consequently, we need to "soak" those
>> power-states into the
>> power-domain intermediate states from Axel. Since power-states are
>> declared and handled
>> the same manner than c-states (idle-states in DT), these patches add a
>> soaking used when
>> attaching to a genpd, where power-states are parsed from the DT into
>> the genpd states:
>>
>>
>> "drivers/base/power/domain.c":
>>
>> static const struct of_device_id power_state_match[] = {
>> {.compatible = "arm,power-state",
>> },
>> };
>>
>> int of_genpd_device_parse_states(struct device_node *np,
>> struct generic_pm_domain *genpd)
>>
>> debugfs addition
>> ---------------
>>
>> To easy debug, this patch set adds a seq-file names "states" to the
>> pm_genpd debugfs:
>>
>> cat /sys/kernel/debug/pm_genpd/*
>>
>> Domain State name Enter (ns) / Exit (ns)
>> -------------------------------------------------------------
>> a53_pd cluster-sleep-0 1500000 / 800000
>> a57_pd cluster-sleep-0 1500000 / 800000
>>
>> And also a seq-file "timings", to help visualize the constrains of the
>> non-CPU
>> devices in a cluster PD.
>>
>> Domain Devices, Timings in ns
>> Stop/Start Save/Restore, Effective
>> ---------------------------------------------------- ---
>> a57_pd
>> /cpus/cpu@0 800 /740 1320 /1720 ,0 (cached stop)
>> /cpus/cpu@1 800 /740 1420 /1780 ,0 (cached stop)
>> /D1 660 /580 16560 /6080 ,2199420 (cached stop)
>>
>>
>> Device power-states
>> -------------------
>>
>> some devices, like L2 caches, may feature a shallower retention mode,
>> between CPU_SLEEP_0
>> and CLUSTER_SLEEP_0, in which mode the L2 memory is not powered off,
>> leading to faster
>> resume than CLUSTER_SLEEP_0.
>>
>> One way to handle device constrains and retention features in the
>> power-domain, is to
>> allow devices to register a new power-state (consistent with a c-state).
>>
>> idle-states:
>>
>> D1_RETENTION: d1-retention {
>> compatible = "arm,power-state";
>> /*leave the psci param, for demo/testing:
>> * the psci cpuidle driver will not
>> currently
>> * understand that a c-state shall not
>> have it's
>> * table entry with a firmware command.
>> * the actual .power_on/off would be
>> registered
>> * by the DECLARE macro for a given
>> domain*/
>> arm,psci-suspend-param = <0x1010000>;
>> local-timer-stop;
>> entry-latency-us = <800>;
>> exit-latency-us = <200>;
>> min-residency-us = <2500>;
>> };
>>
>>
>> D1 {
>> compatible = "fake,fake-driver";
>> name = "D1";
>> constraint = <30000>;
>> power-domains = <&a53_pd>;
>> power-states =<&D1_RETENTION>;
>> };
>>
>>
>> The genpd simple governor can now upon suspend of the last-man CPU
>> chose a shallower
>> retention state than CLUSTER_SLEEP_0.
>>
>> In order to achieve this, this patch set added the power-state parsing
>> during the
>> genpd_dev_pm_attach call. Multiple genpd states are now inserted in a
>> sorted manner
>> according to their depth: see pm_genpd_insert_state in
>> "drivers/base/power/domain.c".
>>
>>
>>
>> 2) DEPENDENCIES
>>
>> This patch set applies over linux-4.2rc5 plus the following
>> ordered dependencies:
>>
>> * Ulf Hansson:
>>
>> 6637131 New [V4] PM / Domains: Remove intermediate states
>> from the power off sequence
>>
>> * Lina Iyer's patch series:
>>
>> 7118981 Not Applicable [v2,1/7] PM / Domains: Allocate memory outside
>> domain locks
>> 7118991 Not Applicable [v2,2/7] PM / Domains: Support IRQ safe PM domains
>> 7119001 Not Applicable [v2,3/7] drivers: cpu: Define CPU devices as
>> IRQ safe
>> 7119011 Not Applicable [v2,4/7] PM / Domains: Introduce PM domains for
>> CPUs/clusters
>> 7119021 Not Applicable [v2,5/7] ARM: cpuidle: Add runtime PM support
>> for CPU idle
>> 7119031 Not Applicable [v2,6/7] ARM64: smp: Add runtime PM support for
>> CPU hotplug
>> 7119041 Not Applicable [v2,7/7] ARM: smp: Add runtime PM support for
>> CPU hotplug
>>
>> * John Medhurst:
>>
>> 6303671 New arm64: dts: Add idle-states for Juno
>>
>> * Axel Haslam:
>>
>> 6301741 Not Applicable [v7,1/5] PM / Domains: prepare for multiple states
>> 6301751 Not Applicable [v7,2/5] PM / Domains: core changes for
>> multiple states
>> 6301781 Not Applicable [v7,3/5] PM / Domains: make governor select
>> deepest state
>> 6301771 Not Applicable [v7,4/5] ARM: imx6: pm: declare pm domain
>> latency on power_state struct.
>> 6301761 Not Applicable [v7,5/5] PM / Domains: remove old power on/off
>> latencies.
>>
>> 2) URL
>>
>> playable from https://github.com/mtitinger/linux-pm.git
>>
>> by adding the "fake driver D1" and launching the test-dev-state.sh
>> script.
>> this will show the power domain suspending to an intermediate state,
>> based on the
>> device constraints.
>>
>> domain status pstate slaves
>> /device runtime status
>> -----------------------------------------------------------------------------------
>>
>> a53_pd on
>> /devices/system/cpu/cpu0 active
>> /devices/system/cpu/cpu3 suspended
>> /devices/system/cpu/cpu4 suspended
>> /devices/system/cpu/cpu5 suspended
>> a57_pd d1-retention
>> /devices/system/cpu/cpu1 suspended
>> /devices/system/cpu/cpu2 suspended
>> /devices/platform/D1
>>
>> ----------------------------------------------------------------------
>>
>> Marc Titinger (6):
>> arm64: Juno: declare generic power domains for both clusters.
>> PM / Domains: prepare for devices that might register a power state
>> PM / Domains: introduce power-states consistent with c-states.
>> PM / Domains: succeed & warn when attaching non-irqsafe devices to an
>> irq-safe domain.
>> arm: cpuidle: let genpd handle the cluster power transition with
>> 'power-states'
>> PM / Domains: add debugfs 'states' and 'timings' seq files
>>
>> .../devicetree/bindings/arm/idle-states.txt | 21 +-
>> .../devicetree/bindings/power/power_domain.txt | 29 ++
>> arch/arm64/boot/dts/arm/juno.dts | 25 +-
>> drivers/base/power/cpu-pd.c | 5 +
>> drivers/base/power/domain.c | 415
>> +++++++++++++++------
>> drivers/cpuidle/cpuidle-arm.c | 52 ++-
>> include/linux/pm_domain.h | 21 +-
>> 7 files changed, 437 insertions(+), 131 deletions(-)
>>
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 39+ messages in thread* [RFC v3 0/7] Managing cluser-level idle-states with generic power domains
2015-10-19 20:58 ` Lina Iyer
2015-10-20 9:10 ` Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 1/7] PM / Domains: prepare for devices that might register a power state Marc Titinger
` (6 subsequent siblings)
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger
Hi Lina,
this is the rebased version of my patches to handle cluster sleep with genpd.
I rebased over your current patches in the linaro git, and Axels v10, and
finally linux-next.
The arm64/Juno stuff is mainly for testing, it may not be 100% up-to-date with
the CPU-pd discussion outcomes, and I've left the patch for dynamic states
registration by devices in this series for now (will do things differently
using the CPU devices as suggested by Kevin).
Changes in v3 (based on comments from you and Kevin)
- removed the "power-states" binding. Instead, look for a ppty called
cpu-idle-states in each PD node. this is a list of idle-states that will be
mapped as domain states.
- quit referring to c-states, but refer to idle-states instead.
Jon Medhurst (1):
arm64: dts: Add idle-states for Juno
Marc Titinger (6):
PM / Domains: prepare for devices that might register a power state
PM / Domains: support idle-states as genpd multiple-state.
arm64: Juno: declare generic power domains for both clusters.
drivers: cpu-pd: allow calling of_cpu_pd_init from platform code.
arm64: PM /Domains: Initialize CPU-domains from DT.
arm64: Juno: declare idle-state cluster-sleep-0 as genpd state
.../devicetree/bindings/power/power_domain.txt | 29 +++
arch/arm64/boot/dts/arm/juno.dts | 50 ++++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/cpu_domain.c | 19 ++
drivers/base/power/cpu-pd.c | 31 +--
drivers/base/power/domain.c | 278 ++++++++++++++-------
include/linux/cpu-pd.h | 2 +
include/linux/pm_domain.h | 13 +-
8 files changed, 308 insertions(+), 115 deletions(-)
create mode 100644 arch/arm64/kernel/cpu_domain.c
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread* [RFC v3 1/7] PM / Domains: prepare for devices that might register a power state
2015-10-19 20:58 ` Lina Iyer
2015-10-20 9:10 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 0/7] Managing cluser-level idle-states " Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 2/7] PM / Domains: support idle-states as genpd multiple-state Marc Titinger
` (5 subsequent siblings)
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger, Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
Devices may register an intermediate retention state into the domain
upon attaching. Currently generic domain would register an array of
states upon init. This patch prepares for later insertion (sort per
depth, remove).
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
- Lina: Rebase related changes]
- Lina: Initialize domain states after the domain is initalized to
ensure the domain lock is initialized before calling the function
to add states dynamically.
---
drivers/base/power/cpu-pd.c | 2 -
drivers/base/power/domain.c | 178 +++++++++++++++++++++-----------------------
include/linux/pm_domain.h | 10 ++-
3 files changed, 94 insertions(+), 96 deletions(-)
diff --git a/drivers/base/power/cpu-pd.c b/drivers/base/power/cpu-pd.c
index aa276fc..eddee98 100644
--- a/drivers/base/power/cpu-pd.c
+++ b/drivers/base/power/cpu-pd.c
@@ -21,8 +21,6 @@
#include <linux/rculist.h>
#include <linux/slab.h>
-#define NAME_MAX 36
-
/* List of CPU PM domains we care about */
static LIST_HEAD(of_cpu_pd_list);
static DEFINE_SPINLOCK(cpu_pd_list_lock);
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index cc8134d..6b2d771 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -19,6 +19,7 @@
#include <linux/sched.h>
#include <linux/suspend.h>
#include <linux/export.h>
+#include <linux/sort.h>
#define GENPD_RETRY_MAX_MS 250 /* Approximate */
@@ -50,12 +51,6 @@
__retval; \
})
-#define GENPD_MAX_NAME_SIZE 20
-
-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count);
-
static LIST_HEAD(gpd_list);
static DEFINE_MUTEX(gpd_list_lock);
@@ -1297,45 +1292,6 @@ static void genpd_free_dev_data(struct device *dev,
dev_pm_put_subsys_data(dev);
}
-static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count)
-{
- int ret = 0;
- unsigned int i;
-
- if (IS_ERR_OR_NULL(genpd)) {
- ret = -EINVAL;
- goto err;
- }
-
- if (!st || (st_count < 1)) {
- ret = -EINVAL;
- goto err;
- }
-
- /* Allocate the local memory to keep the states for this genpd */
- genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
- if (!genpd->states) {
- ret = -ENOMEM;
- goto err;
- }
-
- for (i = 0; i < st_count; i++) {
- genpd->states[i].power_on_latency_ns =
- st[i].power_on_latency_ns;
- genpd->states[i].power_off_latency_ns =
- st[i].power_off_latency_ns;
- }
-
- genpd->state_count = st_count;
-
- /* to save memory, Name allocation will happen if debug is enabled */
- pm_genpd_alloc_states_names(genpd, st, st_count);
-
-err:
- return ret;
-}
/**
* __pm_genpd_add_device - Add a device to an I/O PM domain.
@@ -1614,6 +1570,73 @@ static void genpd_lock_init(struct generic_pm_domain *genpd)
}
}
+/*
+ * state depth comparison function.
+ */
+static int state_cmp(const void *a, const void *b)
+{
+ struct genpd_power_state *state_a = (struct genpd_power_state *)(a);
+ struct genpd_power_state *state_b = (struct genpd_power_state *)(b);
+
+ s64 depth_a =
+ state_a->power_on_latency_ns + state_a->power_off_latency_ns;
+ s64 depth_b =
+ state_b->power_on_latency_ns + state_b->power_off_latency_ns;
+
+ return (depth_a > depth_b) ? 0 : -1;
+}
+
+/*
+ * TODO: antagonist routine.
+ */
+int pm_genpd_insert_state(struct generic_pm_domain *genpd,
+ const struct genpd_power_state *state)
+{
+ int ret = 0;
+ int state_count = genpd->state_count;
+
+ if (IS_ERR_OR_NULL(genpd) || (!state))
+ ret = -EINVAL;
+
+ if (state_count >= GENPD_POWER_STATES_MAX)
+ ret = -ENOMEM;
+
+#ifdef CONFIG_PM_ADVANCED_DEBUG
+ /* to save memory, Name allocation will happen if debug is enabled */
+ genpd->states[state_count].name = kstrndup(state->name,
+ GENPD_MAX_NAME_SIZE,
+ GFP_KERNEL);
+ if (!genpd->states[state_count].name) {
+ pr_err("%s Failed to allocate state '%s' name.\n",
+ genpd->name, state->name);
+ ret = -ENOMEM;
+ }
+#endif
+ genpd_lock(genpd);
+
+ if (!ret) {
+ genpd->states[state_count].power_on_latency_ns =
+ state->power_on_latency_ns;
+ genpd->states[state_count].power_off_latency_ns =
+ state->power_off_latency_ns;
+ genpd->state_count++;
+ }
+
+ /* sort from shallowest to deepest */
+ sort(genpd->states, genpd->state_count,
+ sizeof(genpd->states[0]), state_cmp, NULL);
+
+ /* Sanity check for current state index */
+ if (genpd->state_idx >= genpd->state_count) {
+ pr_warn("pm domain %s Invalid initial state.\n", genpd->name);
+ genpd->state_idx = genpd->state_count - 1;
+ }
+
+ genpd_unlock(genpd);
+
+ return ret;
+}
+
/**
* pm_genpd_init - Initialize a generic I/O PM domain object.
* @genpd: PM domain object to initialize.
@@ -1627,23 +1650,11 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
const struct genpd_power_state *states,
unsigned int state_count, bool is_off)
{
- int ret;
+ int i;
if (IS_ERR_OR_NULL(genpd))
return;
- /* State data should be provided */
- if (!states || (state_count < 1)) {
- pr_err("Invalid state data\n");
- return;
- }
-
- ret = genpd_alloc_states_data(genpd, states, state_count);
- if (ret) {
- pr_err("Failed to allocate states for %s\n", genpd->name);
- return;
- }
-
INIT_LIST_HEAD(&genpd->master_links);
INIT_LIST_HEAD(&genpd->slave_links);
INIT_LIST_HEAD(&genpd->dev_list);
@@ -1688,9 +1699,25 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
genpd->dev_ops.start = pm_clk_resume;
}
+ /* simply use an array, we wish to add/remove new retention states
+ * from later device init/exit.
+ */
+ memset(genpd->states, 0, GENPD_POWER_STATES_MAX
+ * sizeof(struct genpd_power_state));
+
+ if (!states || !state_count) {
+ /* require a provider for a default state */
+ genpd->state_count = 0;
+ genpd->state_idx = 0;
+ } else
+ for (i = 0; i < state_count; i++)
+ if (pm_genpd_insert_state(genpd, &states[i]))
+ return;
+
mutex_lock(&gpd_list_lock);
list_add(&genpd->gpd_list_node, &gpd_list);
mutex_unlock(&gpd_list_lock);
+
}
EXPORT_SYMBOL_GPL(pm_genpd_init);
@@ -2029,33 +2056,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
#include <linux/kobject.h>
static struct dentry *pm_genpd_debugfs_dir;
-static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count)
-{
- unsigned int i;
-
- if (IS_ERR_OR_NULL(genpd))
- return -EINVAL;
-
- if (genpd->state_count != st_count) {
- pr_err("Invalid allocated state count\n");
- return -EINVAL;
- }
-
- for (i = 0; i < st_count; i++) {
- genpd->states[i].name = kstrndup(st[i].name,
- GENPD_MAX_NAME_SIZE, GFP_KERNEL);
- if (!genpd->states[i].name) {
- pr_err("%s Failed to allocate state %d name.\n",
- genpd->name, i);
- return -ENOMEM;
- }
- }
-
- return 0;
-}
-
/*
* TODO: This function is a slightly modified version of rtpm_status_show
* from sysfs.c, so generalize it.
@@ -2292,12 +2292,4 @@ static void __exit pm_genpd_debug_exit(void)
{
debugfs_remove_recursive(pm_genpd_debugfs_dir);
}
-__exitcall(pm_genpd_debug_exit);
-#else
-static inline int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
- const struct genpd_power_state *st,
- unsigned int st_count)
-{
- return 0;
-}
#endif /* CONFIG_PM_ADVANCED_DEBUG */
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 57ee8d8..24621be 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -44,6 +44,12 @@ struct gpd_cpuidle_data {
struct cpuidle_state *idle_state;
};
+/* Arbitrary max number of devices registering a special
+ * retention state with the PD, to keep things simple.
+ */
+#define GENPD_POWER_STATES_MAX 12
+#define GENPD_MAX_NAME_SIZE 40
+
struct genpd_power_state {
char *name;
s64 power_off_latency_ns;
@@ -77,7 +83,7 @@ struct generic_pm_domain {
void (*detach_dev)(struct generic_pm_domain *domain,
struct device *dev);
unsigned int flags; /* Bit field of configs for genpd */
- struct genpd_power_state *states;
+ struct genpd_power_state states[GENPD_POWER_STATES_MAX];
unsigned int state_count; /* number of states */
unsigned int state_idx; /* state that genpd will go to when off */
bool irq_safe;
@@ -154,6 +160,8 @@ extern void pm_genpd_init_simple(struct generic_pm_domain *genpd,
extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
extern int pm_genpd_name_poweron(const char *domain_name);
extern void pm_genpd_poweroff_unused(void);
+extern int pm_genpd_insert_state(struct generic_pm_domain *genpd,
+ const struct genpd_power_state *state);
extern struct dev_power_governor simple_qos_governor;
extern struct dev_power_governor pm_domain_always_on_gov;
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v3 2/7] PM / Domains: support idle-states as genpd multiple-state.
2015-10-19 20:58 ` Lina Iyer
` (2 preceding siblings ...)
2015-10-27 17:40 ` [RFC v3 1/7] PM / Domains: prepare for devices that might register a power state Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-11-13 5:56 ` Zhaoyang Huang
2015-10-27 17:40 ` [RFC v3 3/7] arm64: dts: Add idle-states for Juno Marc Titinger
` (4 subsequent siblings)
8 siblings, 1 reply; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger, Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
This patch allows cluster-level idle-states to being soaked in as generic
domain power states, in order for the domain governor to chose the most
efficient power state compatible with the device constraints. Similarly,
devices can register power-states into the cluster domain, in a manner
consistent with idle-states.
This is a attempt to address device-retention states for devices that
are not hooked to runtime-pm, but feature a retention state handled by
the same firmware that handles idle-states. For instance a L2 caches.
With Juno, in this example the idle-state 'cluster-sleep-0 ' is known from
each cluster generic domain, as the deepest sate.
cat /sys/kernel/debug/pm_genpd/*
Domain State name Enter (ns) / Exit (ns)
-------------------------------------------------------------
a53_pd cluster-sleep-0 1500000 / 800000
a57_pd cluster-sleep-0 1500000 / 800000
domain status pstate slaves
/device runtime status
-----------------------------------------------------------------------
a53_pd on
/devices/system/cpu/cpu0 active
/devices/system/cpu/cpu3 suspended
/devices/system/cpu/cpu4 suspended
/devices/system/cpu/cpu5 suspended
/devices/platform/D1 suspended
a57_pd cluster-sleep-0
/devices/system/cpu/cpu1 suspended
/devices/system/cpu/cpu2 suspended
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
.../devicetree/bindings/power/power_domain.txt | 29 ++++++
drivers/base/power/domain.c | 102 ++++++++++++++++++++-
include/linux/pm_domain.h | 3 +
3 files changed, 131 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 025b5e7..2657e19 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -29,6 +29,16 @@ Optional properties:
specified by this binding. More details about power domain specifier are
available in the next section.
+ - cpu-idle-states : a phandle of an idle-state that shall be soaked into a
+ generic domain power state.
+ CPU domains: Deep c-states that match a cluster power-off can be delegated to the
+ generic power domain. Device other than CPUs may have register intermediate
+ power states in the same domain. The domain governor can do a good job in
+ electing a power state when the last cpu is powered off as devices in the
+ same genpd may register intermediate states.
+ Devices : a device may register an intermediate c-state matching a memory
+ retention feature for instance.
+
Example:
power: power-controller@12340000 {
@@ -55,6 +65,25 @@ Example 2:
#power-domain-cells = <1>;
};
+Example 3:
+
+ pm-domains {
+ a57_pd: a57_pd@ {
+ /* will have a57 platform ARM_PD_METHOD_OF_DECLARE*/
+ compatible = "arm,pd","arm,cortex-a57";
+ #power-domain-cells = <0>;
+ cpu-idle-states = <&CLUSTER_SLEEP_0>;
+ };
+
+ a53_pd: a53_pd@ {
+ /* will have a a53 platform ARM_PD_METHOD_OF_DECLARE*/
+ compatible = "arm,pd","arm,cortex-a53";
+ #power-domain-cells = <0>;
+ cpu-idle-states = <&CLUSTER_SLEEP_0>;
+ };
+ };
+
+
The nodes above define two power controllers: 'parent' and 'child'.
Domains created by the 'child' power controller are subdomains of '0' power
domain provided by the 'parent' power controller.
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6b2d771..8512e28 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1345,7 +1345,8 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
else {
dev_pm_qos_add_notifier(dev, &gpd_data->nb);
atomic_inc(&genpd->usage_count);
- printk("Add device %d\n", atomic_read(&genpd->usage_count));
+ dev_info(dev, "Add device %d\n",
+ atomic_read(&genpd->usage_count));
}
return ret;
}
@@ -1592,7 +1593,7 @@ static int state_cmp(const void *a, const void *b)
int pm_genpd_insert_state(struct generic_pm_domain *genpd,
const struct genpd_power_state *state)
{
- int ret = 0;
+ int i, ret = 0;
int state_count = genpd->state_count;
if (IS_ERR_OR_NULL(genpd) || (!state))
@@ -1601,11 +1602,18 @@ int pm_genpd_insert_state(struct generic_pm_domain *genpd,
if (state_count >= GENPD_POWER_STATES_MAX)
ret = -ENOMEM;
+ /* Bail out, this state was already registered.*/
+ for (i = 0; i < state_count; i++)
+ if (!strncmp(state->name, genpd->states[i].name,
+ GENPD_MAX_NAME_SIZE))
+ return 0;
+
#ifdef CONFIG_PM_ADVANCED_DEBUG
/* to save memory, Name allocation will happen if debug is enabled */
genpd->states[state_count].name = kstrndup(state->name,
GENPD_MAX_NAME_SIZE,
GFP_KERNEL);
+
if (!genpd->states[state_count].name) {
pr_err("%s Failed to allocate state '%s' name.\n",
genpd->name, state->name);
@@ -1963,6 +1971,93 @@ static void genpd_dev_pm_sync(struct device *dev)
genpd_queue_power_off_work(pd);
}
+
+static int dt_cpuidle_to_genpd_power_state(struct genpd_power_state
+ *genpd_state,
+ struct device_node *state_node)
+{
+ int err = 0;
+ u32 latency;
+
+ err = of_property_read_u32(state_node, "wakeup-latency-us", &latency);
+ if (err) {
+ u32 entry_latency, exit_latency;
+
+ err = of_property_read_u32(state_node, "entry-latency-us",
+ &entry_latency);
+ if (err) {
+ pr_debug(" * %s missing entry-latency-us property\n",
+ state_node->full_name);
+ return -EINVAL;
+ }
+
+ err = of_property_read_u32(state_node, "exit-latency-us",
+ &exit_latency);
+ if (err) {
+ pr_debug(" * %s missing exit-latency-us property\n",
+ state_node->full_name);
+ return -EINVAL;
+ }
+ /*
+ * If wakeup-latency-us is missing, default to entry+exit
+ * latencies as defined in idle states bindings
+ */
+ latency = entry_latency + exit_latency;
+ }
+
+ genpd_state->power_on_latency_ns = 1000 * latency;
+
+ err = of_property_read_u32(state_node, "entry-latency-us", &latency);
+ if (err) {
+ pr_debug(" * %s missing min-residency-us property\n",
+ state_node->full_name);
+ return -EINVAL;
+ }
+
+ genpd_state->power_off_latency_ns = 1000 * latency;
+
+ return 0;
+}
+
+int of_genpd_device_parse_states(struct device_node *np,
+ struct generic_pm_domain *genpd)
+{
+ struct device_node *state_node;
+ int i, err = 0;
+
+ for (i = 0;; i++) {
+ struct genpd_power_state genpd_state;
+
+ state_node = of_parse_phandle(np, "domain-idle-states", i);
+ if (!state_node)
+ break;
+
+ err = dt_cpuidle_to_genpd_power_state(&genpd_state,
+ state_node);
+ if (err) {
+ pr_err
+ ("Parsing idle state node %s failed with err %d\n",
+ state_node->full_name, err);
+ err = -EINVAL;
+ break;
+ }
+#ifdef CONFIG_PM_ADVANCED_DEBUG
+ genpd_state.name = kstrndup(state_node->name,
+ GENPD_MAX_NAME_SIZE, GFP_KERNEL);
+ if (!genpd_state.name)
+ err = -ENOMEM;
+#endif
+ of_node_put(state_node);
+ err = pm_genpd_insert_state(genpd, &genpd_state);
+ if (err)
+ break;
+#ifdef CONFIG_PM_ADVANCED_DEBUG
+ kfree(genpd_state.name);
+#endif
+ }
+ return err;
+}
+
/**
* genpd_dev_pm_attach - Attach a device to its PM domain using DT.
* @dev: Device to attach.
@@ -1996,7 +2091,6 @@ int genpd_dev_pm_attach(struct device *dev)
if (ret < 0) {
if (ret != -ENOENT)
return ret;
-
/*
* Try legacy Samsung-specific bindings
* (for backwards compatibility of DT ABI)
@@ -2034,6 +2128,8 @@ int genpd_dev_pm_attach(struct device *dev)
goto out;
}
+ of_genpd_device_parse_states(pd_args.np, pd);
+
dev->pm_domain->detach = genpd_dev_pm_detach;
dev->pm_domain->sync = genpd_dev_pm_sync;
ret = genpd_poweron(pd);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 24621be..48ab3b1 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -248,6 +248,9 @@ struct generic_pm_domain *__of_genpd_xlate_onecell(
struct of_phandle_args *genpdspec,
void *data);
+int of_genpd_device_parse_states(struct device_node *np,
+ struct generic_pm_domain *genpd);
+
int genpd_dev_pm_attach(struct device *dev);
#else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
static inline int __of_genpd_add_provider(struct device_node *np,
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC v3 2/7] PM / Domains: support idle-states as genpd multiple-state.
2015-10-27 17:40 ` [RFC v3 2/7] PM / Domains: support idle-states as genpd multiple-state Marc Titinger
@ 2015-11-13 5:56 ` Zhaoyang Huang
0 siblings, 0 replies; 39+ messages in thread
From: Zhaoyang Huang @ 2015-11-13 5:56 UTC (permalink / raw)
To: Marc Titinger
Cc: Lina Iyer, rjw, khilman, Axel Haslam, Benoit Cousson, linux-pm,
linux-kernel, Marc Titinger
On 28 October 2015 at 01:40, Marc Titinger <mtitinger@baylibre.com> wrote:
> From: Marc Titinger <mtitinger@baylibre.com>
>
> This patch allows cluster-level idle-states to being soaked in as generic
> domain power states, in order for the domain governor to chose the most
> efficient power state compatible with the device constraints. Similarly,
> devices can register power-states into the cluster domain, in a manner
> consistent with idle-states.
>
> This is a attempt to address device-retention states for devices that
> are not hooked to runtime-pm, but feature a retention state handled by
> the same firmware that handles idle-states. For instance a L2 caches.
>
> With Juno, in this example the idle-state 'cluster-sleep-0 ' is known from
> each cluster generic domain, as the deepest sate.
>
> cat /sys/kernel/debug/pm_genpd/*
>
> Domain State name Enter (ns) / Exit (ns)
> -------------------------------------------------------------
> a53_pd cluster-sleep-0 1500000 / 800000
> a57_pd cluster-sleep-0 1500000 / 800000
>
> domain status pstate slaves
> /device runtime status
> -----------------------------------------------------------------------
> a53_pd on
> /devices/system/cpu/cpu0 active
> /devices/system/cpu/cpu3 suspended
> /devices/system/cpu/cpu4 suspended
> /devices/system/cpu/cpu5 suspended
> /devices/platform/D1 suspended
> a57_pd cluster-sleep-0
> /devices/system/cpu/cpu1 suspended
> /devices/system/cpu/cpu2 suspended
>
> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
> ---
> .../devicetree/bindings/power/power_domain.txt | 29 ++++++
> drivers/base/power/domain.c | 102 ++++++++++++++++++++-
> include/linux/pm_domain.h | 3 +
> 3 files changed, 131 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 025b5e7..2657e19 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -29,6 +29,16 @@ Optional properties:
> specified by this binding. More details about power domain specifier are
> available in the next section.
>
> + - cpu-idle-states : a phandle of an idle-state that shall be soaked into a
> + generic domain power state.
> + CPU domains: Deep c-states that match a cluster power-off can be delegated to the
> + generic power domain. Device other than CPUs may have register intermediate
> + power states in the same domain. The domain governor can do a good job in
> + electing a power state when the last cpu is powered off as devices in the
> + same genpd may register intermediate states.
> + Devices : a device may register an intermediate c-state matching a memory
> + retention feature for instance.
> +
> Example:
>
> power: power-controller@12340000 {
> @@ -55,6 +65,25 @@ Example 2:
> #power-domain-cells = <1>;
> };
>
> +Example 3:
> +
> + pm-domains {
> + a57_pd: a57_pd@ {
> + /* will have a57 platform ARM_PD_METHOD_OF_DECLARE*/
> + compatible = "arm,pd","arm,cortex-a57";
> + #power-domain-cells = <0>;
> + cpu-idle-states = <&CLUSTER_SLEEP_0>;
Should the "cpu_idle_states" be "domain-idle-states", which is
consistent to the following code.
> + };
> +
> + a53_pd: a53_pd@ {
> + /* will have a a53 platform ARM_PD_METHOD_OF_DECLARE*/
> + compatible = "arm,pd","arm,cortex-a53";
> + #power-domain-cells = <0>;
> + cpu-idle-states = <&CLUSTER_SLEEP_0>;
> + };
> + };
> +
> +
> The nodes above define two power controllers: 'parent' and 'child'.
> Domains created by the 'child' power controller are subdomains of '0' power
> domain provided by the 'parent' power controller.
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6b2d771..8512e28 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1345,7 +1345,8 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
> else {
> dev_pm_qos_add_notifier(dev, &gpd_data->nb);
> atomic_inc(&genpd->usage_count);
> - printk("Add device %d\n", atomic_read(&genpd->usage_count));
> + dev_info(dev, "Add device %d\n",
> + atomic_read(&genpd->usage_count));
> }
> return ret;
> }
> @@ -1592,7 +1593,7 @@ static int state_cmp(const void *a, const void *b)
> int pm_genpd_insert_state(struct generic_pm_domain *genpd,
> const struct genpd_power_state *state)
> {
> - int ret = 0;
> + int i, ret = 0;
> int state_count = genpd->state_count;
>
> if (IS_ERR_OR_NULL(genpd) || (!state))
> @@ -1601,11 +1602,18 @@ int pm_genpd_insert_state(struct generic_pm_domain *genpd,
> if (state_count >= GENPD_POWER_STATES_MAX)
> ret = -ENOMEM;
>
> + /* Bail out, this state was already registered.*/
> + for (i = 0; i < state_count; i++)
> + if (!strncmp(state->name, genpd->states[i].name,
> + GENPD_MAX_NAME_SIZE))
> + return 0;
> +
> #ifdef CONFIG_PM_ADVANCED_DEBUG
> /* to save memory, Name allocation will happen if debug is enabled */
> genpd->states[state_count].name = kstrndup(state->name,
> GENPD_MAX_NAME_SIZE,
> GFP_KERNEL);
> +
> if (!genpd->states[state_count].name) {
> pr_err("%s Failed to allocate state '%s' name.\n",
> genpd->name, state->name);
> @@ -1963,6 +1971,93 @@ static void genpd_dev_pm_sync(struct device *dev)
> genpd_queue_power_off_work(pd);
> }
>
> +
> +static int dt_cpuidle_to_genpd_power_state(struct genpd_power_state
> + *genpd_state,
> + struct device_node *state_node)
> +{
> + int err = 0;
> + u32 latency;
> +
> + err = of_property_read_u32(state_node, "wakeup-latency-us", &latency);
> + if (err) {
> + u32 entry_latency, exit_latency;
> +
> + err = of_property_read_u32(state_node, "entry-latency-us",
> + &entry_latency);
> + if (err) {
> + pr_debug(" * %s missing entry-latency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> +
> + err = of_property_read_u32(state_node, "exit-latency-us",
> + &exit_latency);
> + if (err) {
> + pr_debug(" * %s missing exit-latency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> + /*
> + * If wakeup-latency-us is missing, default to entry+exit
> + * latencies as defined in idle states bindings
> + */
> + latency = entry_latency + exit_latency;
> + }
> +
> + genpd_state->power_on_latency_ns = 1000 * latency;
> +
> + err = of_property_read_u32(state_node, "entry-latency-us", &latency);
> + if (err) {
> + pr_debug(" * %s missing min-residency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> +
> + genpd_state->power_off_latency_ns = 1000 * latency;
> +
> + return 0;
> +}
> +
> +int of_genpd_device_parse_states(struct device_node *np,
> + struct generic_pm_domain *genpd)
> +{
> + struct device_node *state_node;
> + int i, err = 0;
> +
> + for (i = 0;; i++) {
> + struct genpd_power_state genpd_state;
> +
> + state_node = of_parse_phandle(np, "domain-idle-states", i);
> + if (!state_node)
> + break;
> +
> + err = dt_cpuidle_to_genpd_power_state(&genpd_state,
> + state_node);
> + if (err) {
> + pr_err
> + ("Parsing idle state node %s failed with err %d\n",
> + state_node->full_name, err);
> + err = -EINVAL;
> + break;
> + }
> +#ifdef CONFIG_PM_ADVANCED_DEBUG
> + genpd_state.name = kstrndup(state_node->name,
> + GENPD_MAX_NAME_SIZE, GFP_KERNEL);
> + if (!genpd_state.name)
> + err = -ENOMEM;
> +#endif
> + of_node_put(state_node);
> + err = pm_genpd_insert_state(genpd, &genpd_state);
> + if (err)
> + break;
> +#ifdef CONFIG_PM_ADVANCED_DEBUG
> + kfree(genpd_state.name);
> +#endif
> + }
> + return err;
> +}
> +
> /**
> * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> * @dev: Device to attach.
> @@ -1996,7 +2091,6 @@ int genpd_dev_pm_attach(struct device *dev)
> if (ret < 0) {
> if (ret != -ENOENT)
> return ret;
> -
> /*
> * Try legacy Samsung-specific bindings
> * (for backwards compatibility of DT ABI)
> @@ -2034,6 +2128,8 @@ int genpd_dev_pm_attach(struct device *dev)
> goto out;
> }
>
> + of_genpd_device_parse_states(pd_args.np, pd);
> +
> dev->pm_domain->detach = genpd_dev_pm_detach;
> dev->pm_domain->sync = genpd_dev_pm_sync;
> ret = genpd_poweron(pd);
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 24621be..48ab3b1 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -248,6 +248,9 @@ struct generic_pm_domain *__of_genpd_xlate_onecell(
> struct of_phandle_args *genpdspec,
> void *data);
>
> +int of_genpd_device_parse_states(struct device_node *np,
> + struct generic_pm_domain *genpd);
> +
> int genpd_dev_pm_attach(struct device *dev);
> #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
> static inline int __of_genpd_add_provider(struct device_node *np,
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC v3 3/7] arm64: dts: Add idle-states for Juno
2015-10-19 20:58 ` Lina Iyer
` (3 preceding siblings ...)
2015-10-27 17:40 ` [RFC v3 2/7] PM / Domains: support idle-states as genpd multiple-state Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 4/7] arm64: Juno: declare generic power domains for both clusters Marc Titinger
` (3 subsequent siblings)
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Jon Medhurst
From: Jon Medhurst <tixy@linaro.org>
Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
arch/arm64/boot/dts/arm/juno.dts | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index 53442b5..b3fcee8 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -60,6 +60,29 @@
};
};
+ idle-states {
+
+ entry-method = "arm,psci";
+
+ CPU_SLEEP_0: cpu-sleep-0 {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x0010000>;
+ local-timer-stop;
+ entry-latency-us = <100>;
+ exit-latency-us = <250>;
+ min-residency-us = <2000>;
+ };
+
+ CLUSTER_SLEEP_0: cluster-sleep-0 {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x1010000>;
+ local-timer-stop;
+ entry-latency-us = <800>;
+ exit-latency-us = <700>;
+ min-residency-us = <2500>;
+ };
+ };
+
A57_0: cpu@0 {
compatible = "arm,cortex-a57","arm,armv8";
reg = <0x0 0x0>;
@@ -67,6 +90,7 @@
enable-method = "psci";
next-level-cache = <&A57_L2>;
clocks = <&scpi_dvfs 0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
A57_1: cpu@1 {
@@ -76,6 +100,7 @@
enable-method = "psci";
next-level-cache = <&A57_L2>;
clocks = <&scpi_dvfs 0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
A53_0: cpu@100 {
@@ -85,6 +110,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
A53_1: cpu@101 {
@@ -94,6 +120,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
A53_2: cpu@102 {
@@ -103,6 +130,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
A53_3: cpu@103 {
@@ -112,6 +140,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
A57_L2: l2-cache0 {
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v3 4/7] arm64: Juno: declare generic power domains for both clusters.
2015-10-19 20:58 ` Lina Iyer
` (4 preceding siblings ...)
2015-10-27 17:40 ` [RFC v3 3/7] arm64: dts: Add idle-states for Juno Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 5/7] drivers: cpu-pd: allow calling of_cpu_pd_init from platform code Marc Titinger
` (2 subsequent siblings)
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger, Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
arch/arm64/boot/dts/arm/juno.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index b3fcee8..0a72c07 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -91,6 +91,7 @@
next-level-cache = <&A57_L2>;
clocks = <&scpi_dvfs 0>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a57_pd>;
};
A57_1: cpu@1 {
@@ -101,6 +102,7 @@
next-level-cache = <&A57_L2>;
clocks = <&scpi_dvfs 0>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a57_pd>;
};
A53_0: cpu@100 {
@@ -111,6 +113,7 @@
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A53_1: cpu@101 {
@@ -121,6 +124,7 @@
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A53_2: cpu@102 {
@@ -131,6 +135,7 @@
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A53_3: cpu@103 {
@@ -141,6 +146,7 @@
next-level-cache = <&A53_L2>;
clocks = <&scpi_dvfs 1>;
cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ power-domains = <&a53_pd>;
};
A57_L2: l2-cache0 {
@@ -152,6 +158,19 @@
};
};
+ CPU_PD: cpu-domains {
+
+ a57_pd: a57_pd@ {
+ compatible = "arm,cpu-pd";
+ #power-domain-cells = <0>;
+ };
+
+ a53_pd: a53_pd@ {
+ compatible = "arm,cpu-pd";
+ #power-domain-cells = <0>;
+ };
+ };
+
pmu_a57 {
compatible = "arm,cortex-a57-pmu";
interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v3 5/7] drivers: cpu-pd: allow calling of_cpu_pd_init from platform code.
2015-10-19 20:58 ` Lina Iyer
` (5 preceding siblings ...)
2015-10-27 17:40 ` [RFC v3 4/7] arm64: Juno: declare generic power domains for both clusters Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 6/7] arm64: PM /Domains: Initialize CPU-domains from DT Marc Titinger
2015-10-27 17:40 ` [RFC v3 7/7] arm64: Juno: declare idle-state cluster-sleep-0 as genpd state Marc Titinger
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger, Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
re-instate a two step init, first register the CPU-power domains, then
attach the CPUs, because CPU ordering does not match cluster/PD boundaries.
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
drivers/base/power/cpu-pd.c | 29 ++++++++++++-----------------
include/linux/cpu-pd.h | 2 ++
2 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/base/power/cpu-pd.c b/drivers/base/power/cpu-pd.c
index eddee98..701a68f 100644
--- a/drivers/base/power/cpu-pd.c
+++ b/drivers/base/power/cpu-pd.c
@@ -8,8 +8,6 @@
* published by the Free Software Foundation.
*/
-#define DEBUG
-
#include <linux/kernel.h>
#include <linux/export.h>
#include <linux/cpu.h>
@@ -160,8 +158,6 @@ static int cpu_hotplug(struct notifier_block *nb,
int of_register_cpu_pm_domain(struct device_node *dn,
struct cpu_pm_domain *pd)
{
- int ret;
-
if (!pd || !pd->genpd)
return -EINVAL;
@@ -187,14 +183,6 @@ int of_register_cpu_pm_domain(struct device_node *dn,
pm_genpd_init_simple(pd->genpd, &simple_qos_governor, false);
of_genpd_add_provider_simple(dn, pd->genpd);
- /* Attach the CPUs to the CPU PM domain */
- ret = of_pm_domain_attach_cpus();
- if (ret) {
- of_genpd_del_provider(dn);
- return ret;
- }
-
- hotcpu_notifier(cpu_hotplug, 0)
return 0;
}
EXPORT_SYMBOL(of_register_cpu_pm_domain);
@@ -245,21 +233,28 @@ int of_init_cpu_pm_domain(struct device_node *dn, struct cpu_pm_ops *ops)
EXPORT_SYMBOL(of_init_cpu_pm_domain);
-static int __init of_cpu_pd_init(void)
+int __init of_cpu_pd_init(const char *compatible)
{
struct device_node *dn;
int ret;
- for_each_compatible_node(dn, NULL, "cpu,pd") {
+ if (!compatible)
+ return -EINVAL;
+
+ for_each_compatible_node(dn, NULL, compatible) {
if (!of_device_is_available(dn))
continue;
ret = of_init_cpu_pm_domain(dn, NULL);
- if (!ret)
+ if (ret)
return ret;
}
- return 0;
+
+ ret = of_pm_domain_attach_cpus();
+ if (!ret)
+ hotcpu_notifier(cpu_hotplug, 0);
+
+ return ret;
}
-device_initcall(of_cpu_pd_init);
diff --git a/include/linux/cpu-pd.h b/include/linux/cpu-pd.h
index 9ae6f5b..f3066d0 100644
--- a/include/linux/cpu-pd.h
+++ b/include/linux/cpu-pd.h
@@ -32,4 +32,6 @@ extern int of_register_cpu_pm_domain(struct device_node *dn,
extern int of_init_cpu_pm_domain(struct device_node *dn,
struct cpu_pm_ops *ops);
+extern int __init of_cpu_pd_init(const char *compatible);
+
#endif /* __CPU_PD_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v3 6/7] arm64: PM /Domains: Initialize CPU-domains from DT.
2015-10-19 20:58 ` Lina Iyer
` (6 preceding siblings ...)
2015-10-27 17:40 ` [RFC v3 5/7] drivers: cpu-pd: allow calling of_cpu_pd_init from platform code Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
2015-10-27 17:40 ` [RFC v3 7/7] arm64: Juno: declare idle-state cluster-sleep-0 as genpd state Marc Titinger
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger, Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/cpu_domain.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
create mode 100644 arch/arm64/kernel/cpu_domain.c
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 1b6bda2..5f8b59f 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -46,6 +46,7 @@ arm64-obj-$(CONFIG_EFI) += efi.o $(stub-obj)
arm64-obj-$(CONFIG_PCI) += pci.o
arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
arm64-obj-$(CONFIG_ACPI) += acpi.o
+arm64-obj-$(CONFIG_PM_GENERIC_DOMAINS) += cpu_domain.o
obj-y += $(arm64-obj-y) vdso/
obj-m += $(arm64-obj-m)
diff --git a/arch/arm64/kernel/cpu_domain.c b/arch/arm64/kernel/cpu_domain.c
new file mode 100644
index 0000000..06a3949
--- /dev/null
+++ b/arch/arm64/kernel/cpu_domain.c
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2015, Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/cpu-pd.h>
+
+static int __init arm64_cpu_pd_init(void)
+{
+ return of_cpu_pd_init("arm,cpu-pd");
+}
+device_initcall(arm64_cpu_pd_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v3 7/7] arm64: Juno: declare idle-state cluster-sleep-0 as genpd state
2015-10-19 20:58 ` Lina Iyer
` (7 preceding siblings ...)
2015-10-27 17:40 ` [RFC v3 6/7] arm64: PM /Domains: Initialize CPU-domains from DT Marc Titinger
@ 2015-10-27 17:40 ` Marc Titinger
8 siblings, 0 replies; 39+ messages in thread
From: Marc Titinger @ 2015-10-27 17:40 UTC (permalink / raw)
To: lina.iyer
Cc: rjw, khilman, ahaslam, bcousson, linux-pm, linux-kernel,
Marc Titinger, Marc Titinger
From: Marc Titinger <mtitinger@baylibre.com>
Using Juno to exercise the code that prepares for OS-initiated idle-state
handling, using genpd platform callbacks. In opposition to platform-
initiated mode as currently with pcsi/ATF.
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
---
arch/arm64/boot/dts/arm/juno.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index 0a72c07..f42d5f9 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -163,11 +163,13 @@
a57_pd: a57_pd@ {
compatible = "arm,cpu-pd";
#power-domain-cells = <0>;
+ domain-idle-states = <&CLUSTER_SLEEP_0>;
};
a53_pd: a53_pd@ {
compatible = "arm,cpu-pd";
#power-domain-cells = <0>;
+ domain-idle-states = <&CLUSTER_SLEEP_0>;
};
};
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread