| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20151008162704.GB921@linaro.org>
Date: Thu, 8 Oct 2015 10:27:04 -0600
From: Lina Iyer <lina.iyer@...aro.org>
To: Marc Titinger <mtitinger@...libre.com>
Cc: khilman@...nel.org, rjw@...ysocki.net, ahaslam@...libre.com,
bcousson@...libre.com, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org,
Marc Titinger <mtitinger+renesas@...libre.com>
Subject: Re: [RFC v2 3/6] PM / Domains: introduce power-states consistent
with c-states.
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@...libre.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@...40000 {
>@@ -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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists