Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1240124 > unrolled thread
| Started by | Lina Iyer <lina.iyer@linaro.org> |
|---|---|
| First post | 2015-10-06 04:30 +0200 |
| Last post | 2015-10-14 10:20 +0200 |
| Articles | 18 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RFC 1/7] arm64: pm/domains: try mutualize CPU domains init between arm/arm64 Lina Iyer <lina.iyer@linaro.org> - 2015-10-06 04:30 +0200
Re: [RFC 1/7] arm64: pm/domains: try mutualize CPU domains init between arm/arm64 Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 11:00 +0200
[RFC v2 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:30 +0200
[RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states. Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:30 +0200
Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states. Lina Iyer <lina.iyer@linaro.org> - 2015-10-08 18:30 +0200
Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states. Marc Titinger <mtitinger@baylibre.com> - 2015-10-09 12:10 +0200
[RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states' Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:30 +0200
[RFC v2 0/6] Managing cluser-level c-states with generic power domains Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:30 +0200
[RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:40 +0200
Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Lina Iyer <lina.iyer@linaro.org> - 2015-10-08 18:20 +0200
Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Marc Titinger <mtitinger@baylibre.com> - 2015-10-09 11:40 +0200
Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Lina Iyer <lina.iyer@linaro.org> - 2015-10-09 20:30 +0200
Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Marc Titinger <mtitinger@baylibre.com> - 2015-10-13 12:40 +0200
Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state Kevin Hilman <khilman@kernel.org> - 2015-10-13 23:10 +0200
[RFC v2 1/6] arm64: Juno: declare generic power domains for both clusters. Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:40 +0200
[RFC v2 4/6] PM / Domains: succeed & warn when attaching non-irqsafe devices to an irq-safe domain. Marc Titinger <mtitinger@baylibre.com> - 2015-10-06 16:40 +0200
Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains Kevin Hilman <khilman@kernel.org> - 2015-10-14 01:20 +0200
Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains Axel Haslam <ahaslam@baylibre.com> - 2015-10-14 10:20 +0200
| From | Lina Iyer <lina.iyer@linaro.org> |
|---|---|
| Date | 2015-10-06 04:30 +0200 |
| Subject | Re: [RFC 1/7] arm64: pm/domains: try mutualize CPU domains init between arm/arm64 |
| Message-ID | <qgqaS-Yd-1@gated-at.bofh.it> |
On Fri, Sep 25 2015 at 07:04 -0600, Marc Titinger wrote:
>From: Marc Titinger <mtitinger@baylibre.com>
>
>fake path to start testing, eventually move this out of /arch/.
>incidently enable PM_GENERIC_DOMAINS for VExpress.
>
In fact, this could be moved out of ARM. My last series moved it to
drivers/base/power/.
-- Lina
>Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
>---
> arch/arm/common/domains.c | 4 ++--
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/arm-pd.h | 1 +
> arch/arm64/kernel/Makefile | 6 ++++++
> arch/arm64/kernel/domains.c | 1 +
> 5 files changed, 11 insertions(+), 2 deletions(-)
> create mode 120000 arch/arm64/include/asm/arm-pd.h
> create mode 120000 arch/arm64/kernel/domains.c
>
>diff --git a/arch/arm/common/domains.c b/arch/arm/common/domains.c
>index d3207da..68908d4 100644
>--- a/arch/arm/common/domains.c
>+++ b/arch/arm/common/domains.c
>@@ -20,7 +20,7 @@
>
> #include <asm/arm-pd.h>
>
>-#define NAME_MAX 36
>+#define GENPD_NAME_MAX 36
>
> struct arm_pm_domain {
> struct generic_pm_domain genpd;
>@@ -182,7 +182,7 @@ static int __init arm_domain_init(void)
> }
>
> /* Initialize rest of CPU PM domain specifics */
>- pd->genpd.name = kstrndup(np->name, NAME_MAX, GFP_KERNEL);
>+ pd->genpd.name = kstrndup(np->name, GENPD_NAME_MAX, GFP_KERNEL);
> pd->genpd.power_off = arm_pd_power_down;
> pd->genpd.power_on = arm_pd_power_up;
> pd->genpd.flags |= GENPD_FLAG_IRQ_SAFE;
>diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>index 7c55a63..d35f213 100644
>--- a/arch/arm64/Kconfig
>+++ b/arch/arm64/Kconfig
>@@ -250,6 +250,7 @@ config ARCH_VEXPRESS
> select COMMON_CLK_VERSATILE
> select POWER_RESET_VEXPRESS
> select VEXPRESS_CONFIG
>+ select PM_GENERIC_DOMAINS if PM
> help
> This enables support for the ARMv8 software model (Versatile
> Express).
>diff --git a/arch/arm64/include/asm/arm-pd.h b/arch/arm64/include/asm/arm-pd.h
>new file mode 120000
>index 0000000..ecc5437
>--- /dev/null
>+++ b/arch/arm64/include/asm/arm-pd.h
>@@ -0,0 +1 @@
>+../../../arm/include/asm/arm-pd.h
>\ No newline at end of file
>diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>index 426d076..4689565 100644
>--- a/arch/arm64/kernel/Makefile
>+++ b/arch/arm64/kernel/Makefile
>@@ -30,6 +30,12 @@ arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o
> arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o
> arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>+
>+# CPU domains: try mutualize for arm/arm64 in a first step.
>+# Eventually make this more generic.
>+#
>+arm64-obj-$(CONFIG_PM_GENERIC_DOMAINS) += domains.o
>+
> arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> arm64-obj-$(CONFIG_KGDB) += kgdb.o
> arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o
>diff --git a/arch/arm64/kernel/domains.c b/arch/arm64/kernel/domains.c
>new file mode 120000
>index 0000000..bf17c69
>--- /dev/null
>+++ b/arch/arm64/kernel/domains.c
>@@ -0,0 +1 @@
>+../../arm/common/domains.c
>\ No newline at end of file
>--
>1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 11:00 +0200 |
| Message-ID | <qgwgi-1eJ-11@gated-at.bofh.it> |
| In reply to | #1240124 |
On 06/10/2015 04:27, Lina Iyer wrote: > On Fri, Sep 25 2015 at 07:04 -0600, Marc Titinger wrote: >> From: Marc Titinger <mtitinger@baylibre.com> >> >> fake path to start testing, eventually move this out of /arch/. >> incidently enable PM_GENERIC_DOMAINS for VExpress. >> > In fact, this could be moved out of ARM. My last series moved it to > drivers/base/power/. > > -- Lina > Hi Lina, thanks for the heads-up, rebase of my patch-set in progress :) Marc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:30 +0200 |
| Subject | [RFC v2 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files |
| Message-ID | <qgBpD-hk-9@gated-at.bofh.it> |
| In reply to | #1240124 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:30 +0200 |
| Subject | [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states. |
| Message-ID | <qgBpE-hk-19@gated-at.bofh.it> |
| In reply to | #1240124 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lina Iyer <lina.iyer@linaro.org> |
|---|---|
| Date | 2015-10-08 18:30 +0200 |
| Subject | Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states. |
| Message-ID | <qhmeS-mN-11@gated-at.bofh.it> |
| In reply to | #1240467 |
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-09 12:10 +0200 |
| Subject | Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent with c-states. |
| Message-ID | <qhCMG-7p0-15@gated-at.bofh.it> |
| In reply to | #1242588 |
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
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:30 +0200 |
| Subject | [RFC v2 5/6] arm: cpuidle: let genpd handle the cluster power transition with 'power-states' |
| Message-ID | <qgBpE-hk-25@gated-at.bofh.it> |
| In reply to | #1240124 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:30 +0200 |
| Subject | [RFC v2 0/6] Managing cluser-level c-states with generic power domains |
| Message-ID | <qgBpE-hk-11@gated-at.bofh.it> |
| In reply to | #1240124 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:40 +0200 |
| Subject | [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state |
| Message-ID | <qgBzj-sN-7@gated-at.bofh.it> |
| In reply to | #1240481 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lina Iyer <lina.iyer@linaro.org> |
|---|---|
| Date | 2015-10-08 18:20 +0200 |
| Subject | Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state |
| Message-ID | <qhm5c-bp-5@gated-at.bofh.it> |
| In reply to | #1240485 |
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-09 11:40 +0200 |
| Subject | Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state |
| Message-ID | <qhCjE-6BQ-15@gated-at.bofh.it> |
| In reply to | #1242585 |
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
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lina Iyer <lina.iyer@linaro.org> |
|---|---|
| Date | 2015-10-09 20:30 +0200 |
| Subject | Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state |
| Message-ID | <qhKAy-1Gw-15@gated-at.bofh.it> |
| In reply to | #1243190 |
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
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-13 12:40 +0200 |
| Subject | Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state |
| Message-ID | <qj59U-3SV-19@gated-at.bofh.it> |
| In reply to | #1243586 |
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
>>>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Kevin Hilman <khilman@kernel.org> |
|---|---|
| Date | 2015-10-13 23:10 +0200 |
| Subject | Re: [RFC v2 2/6] PM / Domains: prepare for devices that might register a power state |
| Message-ID | <qjeZB-1zx-21@gated-at.bofh.it> |
| In reply to | #1245563 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:40 +0200 |
| Subject | [RFC v2 1/6] arm64: Juno: declare generic power domains for both clusters. |
| Message-ID | <qgBzj-sN-9@gated-at.bofh.it> |
| In reply to | #1240481 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Marc Titinger <mtitinger@baylibre.com> |
|---|---|
| Date | 2015-10-06 16:40 +0200 |
| Subject | [RFC v2 4/6] PM / Domains: succeed & warn when attaching non-irqsafe devices to an irq-safe domain. |
| Message-ID | <qgBzk-sN-25@gated-at.bofh.it> |
| In reply to | #1240481 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Kevin Hilman <khilman@kernel.org> |
|---|---|
| Date | 2015-10-14 01:20 +0200 |
| Subject | Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains |
| Message-ID | <qjh1n-4xv-3@gated-at.bofh.it> |
| In reply to | #1240481 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Axel Haslam <ahaslam@baylibre.com> |
|---|---|
| Date | 2015-10-14 10:20 +0200 |
| Subject | Re: [RFC v2 0/6] Managing cluser-level c-states with generic power domains |
| Message-ID | <qjprX-15w-9@gated-at.bofh.it> |
| In reply to | #1246216 |
> @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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web