Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1587449 > unrolled thread

[PATCH V3 0/7] PM / Domains: Implement domain performance states

Started byViresh Kumar <viresh.kumar@linaro.org>
First post2017-02-24 10:20 +0100
Last post2017-02-28 20:50 +0100
Articles 13 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH V3 0/7] PM / Domains: Implement domain performance states Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-24 10:20 +0100
    [PATCH V3 5/7] PM / domain: Register for PM QOS performance notifier Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-24 10:20 +0100
    [PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-24 10:20 +0100
      Re: [PATCH V3 1/7] PM / Domains: Introduce "performance-states"  binding Rob Herring <robh@kernel.org> - 2017-02-28 02:10 +0100
        Re: [PATCH V3 1/7] PM / Domains: Introduce "performance-states"  binding Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-28 06:40 +0100
    [PATCH V3 7/7] PM / OPP: Add support to parse domain-performance-state Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-24 10:40 +0100
    [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-24 11:20 +0100
      Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state"  binding to OPP nodes Rob Herring <robh@kernel.org> - 2017-02-28 01:50 +0100
        Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state"  binding to OPP nodes Viresh Kumar <viresh.kumar@linaro.org> - 2017-02-28 09:10 +0100
          Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state"  binding to OPP nodes Rob Herring <robh@kernel.org> - 2017-02-28 15:20 +0100
            Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state"  binding to OPP nodes Ulf Hansson <ulf.hansson@linaro.org> - 2017-02-28 16:20 +0100
              Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state"  binding to OPP nodes Rob Herring <robh@kernel.org> - 2017-02-28 17:10 +0100
                Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state"  binding to OPP nodes Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-28 20:50 +0100

#1587449 — [PATCH V3 0/7] PM / Domains: Implement domain performance states

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-24 10:20 +0100
Subject[PATCH V3 0/7] PM / Domains: Implement domain performance states
Message-ID<tekcF-6wz-3@gated-at.bofh.it>
Hi,

This series contain V3 of both the bindings and the code that implement
them. They were sent separately earlier.

Some platforms have the capability to configure the performance state of
their power domains. The process of configuring the performance state is
pretty much platform dependent and we may need to work with a wide range
of configurables.  For some platforms, like Qcom, it can be a positive
integer value alone, while in other cases it can be voltage levels, etc.

The power-domain framework until now was only designed for the idle
state management of the device and this needs to change in order to
reuse the power-domain framework for active state management of the
devices.

The first 2 patches update the DT bindings of the power-domains and OPP
tables. And the other 5 patches implement the details in QoS, genpd and
OPP frameworks.

This is tested currently by hacking the kernel a bit with virtual
power-domains for the dual A15 exynos platform. The earlier version of
patches was also tested by Rajendra Nayak (Qcom) on *real* Qualcomm
hardware for which this work is getting done. And so this version should
work as well.

Here is sample DT and C code we need to write for platforms:

DT:
---

        foo: foo-power-domain@08000000 {
                compatible = "foo,genpd";
                #power-domain-cells = <0>;

                performance-states {
                        compatible = "domain-performance-state";
                        pstate@1 {
                                reg = <1>;
                                domain-microvolt = <970000 975000 985000>;
                        };
                        pstate@2 {
                                reg = <2>;
                                domain-microvolt = <1000000 1075000 1085000>;
                        };
                };
        };

        cpu0_opp_table: opp_table0 {
                compatible = "operating-points-v2";
                opp-shared;

                opp00 {
                        opp-hz = /bits/ 64 <1700000000>;
                        clock-latency-ns = <30>;
                        domain-performance-state = <2>;
                        opp-suspend;
                };
                opp01 {
                        opp-hz = /bits/ 64 <1600000000>;
                        clock-latency-ns = <300>;
                        domain-performance-state = <1>;
                };
        }

Driver code:
------------

static int pd_performance(struct generic_pm_domain *domain, unsigned int state)
{
       int i = state - 1;

       pr_info("%d: %d: %u: %u: %u\n", state,
	       states[i].performance_state, states[i].u_volt,
	       states[i].u_volt_min, states[i].u_volt_max);
       return 0;
}

static const struct of_device_id pm_domain_of_match[] __initconst = {
       { .compatible = "foo,genpd", },
       { },
};

static int __init genpd_test_init(void)
{
       struct device *dev = get_cpu_device(0);
       struct device_node *np;
       const struct of_device_id *match;
       int n;
       int ret;

       for_each_matching_node_and_match(np, pm_domain_of_match, &match) {
               pd.name = kstrdup_const(strrchr(np->full_name, '/') + 1,
                               GFP_KERNEL);
               if (!pd.name) {
                       of_node_put(np);
                       return -ENOMEM;
               }

               pd.set_performance_state = pd_performance;

               pm_genpd_init(&pd, NULL, false);
               of_genpd_parse_performance_states(np, &states, &n);
               of_genpd_add_provider_simple(np, &pd);
       }

       ret = dev_pm_domain_attach(dev, false);

       return ret;
}


V2->V3:
- Based over latest pm/linux-next
- Bindings and code are merged together
- Lots of updates in bindings
  - the performance-states node is present within the power-domain now,
    instead of its phandle.
  - performance-level property is replaced by "reg".
  - domain-performance-state property of the consumers contain an
    integer value now instead of phandle.
- Lots of updates to the code as well
  - Patch "PM / QOS: Add default case to the switch" is merged with
    other patches and the code is changed a bit as well.
  - Don't pass 'type' to dev_pm_qos_add_notifier(), rather handle all
    notifiers with a single list. A new patch is added for that.
  - The OPP framework patch can be applied now and has proper SoB from
    me.
  - Dropped "PM / domain: Save/restore performance state at runtime
    suspend/resume".
  - Drop all WARN().
  - Tested-by Rajendra nayak.

V1->V2:
- Based over latest pm/linux-next
- It is mostly a resend of what is sent earlier as this series hasn't
  got any reviews so far and Rafael suggested that its better I resend
  it.
- Only the 4/6 patch got an update, which was shared earlier as reply to
  V1 as well. It has got several fixes for taking care of power domain
  hierarchy, etc.

--
viresh

Viresh Kumar (7):
  PM / Domains: Introduce "performance-states" binding
  PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
  PM / QOS: Keep common notifier list for genpd constraints
  PM / QOS: Add DEV_PM_QOS_PERFORMANCE request
  PM / domain: Register for PM QOS performance notifier
  PM / Domains: Allow domain performance states to be read from DT
  PM / OPP: Add support to parse domain-performance-state

 Documentation/devicetree/bindings/opp/opp.txt      |  64 +++++++
 .../devicetree/bindings/power/power_domain.txt     |  67 +++++++
 Documentation/power/pm_qos_interface.txt           |   2 +-
 drivers/base/power/domain.c                        | 204 ++++++++++++++++++++-
 drivers/base/power/opp/core.c                      |  73 ++++++++
 drivers/base/power/opp/debugfs.c                   |   4 +
 drivers/base/power/opp/of.c                        |  37 ++++
 drivers/base/power/opp/opp.h                       |  12 ++
 drivers/base/power/qos.c                           |  36 ++--
 include/linux/pm_domain.h                          |  19 ++
 include/linux/pm_qos.h                             |  17 ++
 kernel/power/qos.c                                 |   2 +-
 12 files changed, 517 insertions(+), 20 deletions(-)

-- 
2.7.1.410.g6faf27b

[toc] | [next] | [standalone]


#1587450 — [PATCH V3 5/7] PM / domain: Register for PM QOS performance notifier

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-24 10:20 +0100
Subject[PATCH V3 5/7] PM / domain: Register for PM QOS performance notifier
Message-ID<tekcG-6wz-39@gated-at.bofh.it>
In reply to#1587449
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are identified by positive
integer values, a lower value represents lower performance state. The
power domain driver should be able to retrieve all information required
to configure the performance state of the power domain, with the help of
the performance constraint's target value.

This patch implements performance state management in PM domain core.
The performance QOS uses the common QOS notifier list and we call
__performance_notifier() if the notifier is issued for performance
constraint.

This also allows the power domain drivers to implement a
->set_performance_state() callback, which will be called by the power
domain core from within the notifier routine. If a domain doesn't
implement ->set_performance_state() callback, then it is assumed that
its parents are responsible for performance state configuration. Both
devices and sub-domains are accounted for while finding the highest
performance state requested.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/base/power/domain.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |  4 +++
 2 files changed, 81 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 303490ab5ffd..202effbebfd1 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -452,6 +452,79 @@ static int __resume_latency_notifier(struct generic_pm_domain_data *gpd_data,
 	return NOTIFY_DONE;
 }
 
+static void __update_domain_performance_state(struct generic_pm_domain *genpd,
+					      int depth)
+{
+	struct generic_pm_domain_data *pd_data;
+	struct generic_pm_domain *subdomain;
+	struct pm_domain_data *pdd;
+	unsigned int state = 0;
+	struct gpd_link *link;
+
+	/* Traverse all devices within the domain */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		pd_data = to_gpd_data(pdd);
+
+		if (pd_data->performance_state > state)
+			state = pd_data->performance_state;
+	}
+
+	/* Traverse all subdomains within the domain */
+	list_for_each_entry(link, &genpd->master_links, master_node) {
+		subdomain = link->slave;
+
+		if (subdomain->performance_state > state)
+			state = subdomain->performance_state;
+	}
+
+	if (genpd->performance_state == state)
+		return;
+
+	genpd->performance_state = state;
+
+	if (genpd->set_performance_state) {
+		genpd->set_performance_state(genpd, state);
+		return;
+	}
+
+	/* Propagate to parent power domains */
+	list_for_each_entry(link, &genpd->slave_links, slave_node) {
+		struct generic_pm_domain *master = link->master;
+
+		genpd_lock_nested(master, depth + 1);
+		__update_domain_performance_state(master, depth + 1);
+		genpd_unlock(master);
+	}
+}
+
+static int __performance_notifier(struct generic_pm_domain_data *gpd_data,
+				  unsigned long val)
+{
+	struct generic_pm_domain *genpd = ERR_PTR(-ENODATA);
+	struct device *dev = gpd_data->base.dev;
+	struct pm_domain_data *pdd;
+
+	spin_lock_irq(&dev->power.lock);
+
+	pdd = dev->power.subsys_data ?
+		dev->power.subsys_data->domain_data : NULL;
+
+	if (pdd && pdd->dev)
+		genpd = dev_to_genpd(dev);
+
+	spin_unlock_irq(&dev->power.lock);
+
+	if (IS_ERR(genpd))
+		return NOTIFY_DONE;
+
+	genpd_lock(genpd);
+	gpd_data->performance_state = val;
+	__update_domain_performance_state(genpd, 0);
+	genpd_unlock(genpd);
+
+	return NOTIFY_DONE;
+}
+
 static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
 				     unsigned long val, void *ptr)
 {
@@ -464,6 +537,9 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
 	if (dev_pm_qos_notifier_is_resume_latency(dev, ptr))
 		return __resume_latency_notifier(gpd_data, val);
 
+	if (dev_pm_qos_notifier_is_performance(dev, ptr))
+		return __performance_notifier(gpd_data, val);
+
 	dev_err(dev, "%s: Unexpected notifier call\n", __func__);
 	return NOTIFY_BAD;
 }
@@ -1157,6 +1233,7 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 	gpd_data->td.constraint_changed = true;
 	gpd_data->td.effective_constraint_ns = -1;
 	gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
+	gpd_data->performance_state = 0;
 
 	spin_lock_irq(&dev->power.lock);
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 5339ed5bd6f9..83795935709e 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -62,8 +62,11 @@ struct generic_pm_domain {
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */
+	unsigned int performance_state;	/* Max requested performance state */
 	int (*power_off)(struct generic_pm_domain *domain);
 	int (*power_on)(struct generic_pm_domain *domain);
+	int (*set_performance_state)(struct generic_pm_domain *domain,
+				     unsigned int state);
 	struct gpd_dev_ops dev_ops;
 	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
 	bool max_off_time_changed;
@@ -117,6 +120,7 @@ struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_timing_data td;
 	struct notifier_block nb;
+	unsigned int performance_state;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
-- 
2.7.1.410.g6faf27b

[toc] | [prev] | [next] | [standalone]


#1587452 — [PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-24 10:20 +0100
Subject[PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding
Message-ID<tekcG-6wz-29@gated-at.bofh.it>
In reply to#1587449
Some platforms have the capability to configure the performance state of
their power domains. The process of configuring the performance state is
pretty much platform dependent and we may need to work with a wide range
of configurables. For some platforms, like Qcom, it can be a positive
integer value alone, while in other cases it can be voltage levels, etc.

The power-domain framework until now was only designed for the idle
state management of the device and this needs to change in order to
reuse the power-domain framework for active state management of the
devices.

This patch adds DT bindings to describe the performance states of a
power domain. The power domain node needs to contain a
"performance-states" node, which itself is an array of per-state nodes.
Each per-state node represents individual performance state of a device.
Individual nodes are identified by their (mandatory) "reg" field. These
nodes can also contain an optional "domain-microvolt" property. More
properties can be added later on once we have more platforms using it.

If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their node directly using the
"domain-performance-state" property. Otherwise the consumers can define
their requirements with help of other infrastructure, for example the
OPP table (added in a later commit).

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 .../devicetree/bindings/power/power_domain.txt     | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 723e1ad937da..9be09e576f68 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -38,6 +38,33 @@ phandle arguments (so called PM domain specifiers) of length specified by the
   domain's idle states. In the absence of this property, the domain would be
   considered as capable of being powered-on or powered-off.
 
+- performance-states : This describes the performance states of a PM domain.
+  The performance-states node reflects the performance states of this PM domain
+  and not the performance states of the devices or sub-domains in the PM domain.
+  Sub domains can have their own performance states. Sub domains without their
+  own performance states are governed by the performance states of the parent
+  domain and the "domain-performance-state" properties of their consumers refer
+  to the "reg" properties of the nodes in the parent domain.
+
+  Required properties of the performance-states node:
+  - compatible: Allow performance states to express their compatibility. It
+    should be: "domain-performance-state".
+
+  - nodes: The performance-states node should contain one or
+    more nodes, each representing a supported performance state.
+
+    Required properties of the performance state nodes:
+    - reg: A positive integer value representing the performance level
+      associated with a performance state. The integer value '0' represents the
+      lowest performance level and the highest value represents the highest
+      performance level. The exact meaning and performance implications of
+      individual values is left to be defined by the user platform.
+
+    Optional properties of performance state nodes:
+    - domain-microvolt: voltage in micro Volts. A single regulator's voltage is
+      specified with an array of size one or three.  Single entry is for target
+      voltage and three entries are for <target min max> voltages.
+
 Example:
 
 	power: power-controller@12340000 {
@@ -118,4 +145,44 @@ The node above defines a typical PM domain consumer device, which is located
 inside a PM domain with index 0 of a power controller represented by a node
 with the label "power".
 
+Optional properties:
+- domain-performance-state: A positive integer value representing the minimum
+  performance level (of the parent domain) required by the consumer for its
+  working. The integer value '0' represents the lowest performance level and the
+  highest value represents the highest performance level. The value of
+  domain-performance-state field should match one of the "reg" fields in the
+  "performance-states" table of the parent power domain.
+
+
+Example:
+
+	parent: power-controller@12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <0>;
+
+		performance-states {
+			compatible = "domain-performance-state";
+			pstate@1 {
+				reg = <1>;
+				domain-microvolt = <970000 975000 985000>;
+			};
+			pstate@2 {
+				reg = <2>;
+				domain-microvolt = <1000000 1075000 1085000>;
+			};
+			pstate@3 {
+				reg = <3>;
+				domain-microvolt = <1100000 1175000 1185000>;
+			};
+		};
+	};
+
+	leaky-device@12350000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12350000 0x1000>;
+		power-domains = <&power 0>;
+		domain-performance-state = <2>;
+	};
+
 [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
-- 
2.7.1.410.g6faf27b

[toc] | [prev] | [next] | [standalone]


#1589082 — Re: [PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding

FromRob Herring <robh@kernel.org>
Date2017-02-28 02:10 +0100
SubjectRe: [PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding
Message-ID<tfEsF-5rD-5@gated-at.bofh.it>
In reply to#1587452
On Fri, Feb 24, 2017 at 02:36:33PM +0530, Viresh Kumar wrote:
> Some platforms have the capability to configure the performance state of
> their power domains. The process of configuring the performance state is
> pretty much platform dependent and we may need to work with a wide range
> of configurables. For some platforms, like Qcom, it can be a positive
> integer value alone, while in other cases it can be voltage levels, etc.
> 
> The power-domain framework until now was only designed for the idle
> state management of the device and this needs to change in order to
> reuse the power-domain framework for active state management of the
> devices.
> 
> This patch adds DT bindings to describe the performance states of a
> power domain. The power domain node needs to contain a
> "performance-states" node, which itself is an array of per-state nodes.
> Each per-state node represents individual performance state of a device.
> Individual nodes are identified by their (mandatory) "reg" field. These
> nodes can also contain an optional "domain-microvolt" property. More
> properties can be added later on once we have more platforms using it.
> 
> If the consumers don't need the capability of switching to different
> domain performance states at runtime, then they can simply define their
> required domain performance state in their node directly using the
> "domain-performance-state" property. Otherwise the consumers can define
> their requirements with help of other infrastructure, for example the
> OPP table (added in a later commit).
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  .../devicetree/bindings/power/power_domain.txt     | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 723e1ad937da..9be09e576f68 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -38,6 +38,33 @@ phandle arguments (so called PM domain specifiers) of length specified by the
>    domain's idle states. In the absence of this property, the domain would be
>    considered as capable of being powered-on or powered-off.
>  
> +- performance-states : This describes the performance states of a PM domain.
> +  The performance-states node reflects the performance states of this PM domain
> +  and not the performance states of the devices or sub-domains in the PM domain.
> +  Sub domains can have their own performance states. Sub domains without their
> +  own performance states are governed by the performance states of the parent
> +  domain and the "domain-performance-state" properties of their consumers refer
> +  to the "reg" properties of the nodes in the parent domain.
> +
> +  Required properties of the performance-states node:
> +  - compatible: Allow performance states to express their compatibility. It
> +    should be: "domain-performance-state".
> +
> +  - nodes: The performance-states node should contain one or
> +    more nodes, each representing a supported performance state.
> +
> +    Required properties of the performance state nodes:
> +    - reg: A positive integer value representing the performance level
> +      associated with a performance state. The integer value '0' represents the
> +      lowest performance level and the highest value represents the highest
> +      performance level. The exact meaning and performance implications of
> +      individual values is left to be defined by the user platform.
> +
> +    Optional properties of performance state nodes:
> +    - domain-microvolt: voltage in micro Volts. A single regulator's voltage is
> +      specified with an array of size one or three.  Single entry is for target
> +      voltage and three entries are for <target min max> voltages.
> +
>  Example:
>  
>  	power: power-controller@12340000 {
> @@ -118,4 +145,44 @@ The node above defines a typical PM domain consumer device, which is located
>  inside a PM domain with index 0 of a power controller represented by a node
>  with the label "power".
>  
> +Optional properties:
> +- domain-performance-state: A positive integer value representing the minimum
> +  performance level (of the parent domain) required by the consumer for its
> +  working. The integer value '0' represents the lowest performance level and the
> +  highest value represents the highest performance level. The value of
> +  domain-performance-state field should match one of the "reg" fields in the
> +  "performance-states" table of the parent power domain.
> +
> +
> +Example:
> +
> +	parent: power-controller@12340000 {
> +		compatible = "foo,power-controller";
> +		reg = <0x12340000 0x1000>;
> +		#power-domain-cells = <0>;
> +
> +		performance-states {
> +			compatible = "domain-performance-state";
> +			pstate@1 {
> +				reg = <1>;
> +				domain-microvolt = <970000 975000 985000>;

This doesn't look like "<target> <min> <max>".

With that fixed,

Acked-by: Rob Herring <robh@kernel.org>


> +			};
> +			pstate@2 {
> +				reg = <2>;
> +				domain-microvolt = <1000000 1075000 1085000>;
> +			};
> +			pstate@3 {
> +				reg = <3>;
> +				domain-microvolt = <1100000 1175000 1185000>;
> +			};
> +		};
> +	};
> +
> +	leaky-device@12350000 {
> +		compatible = "foo,i-leak-current";
> +		reg = <0x12350000 0x1000>;
> +		power-domains = <&power 0>;
> +		domain-performance-state = <2>;
> +	};
> +
>  [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> -- 
> 2.7.1.410.g6faf27b
> 

[toc] | [prev] | [next] | [standalone]


#1589189 — Re: [PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-28 06:40 +0100
SubjectRe: [PATCH V3 1/7] PM / Domains: Introduce "performance-states" binding
Message-ID<tfIFX-8oQ-9@gated-at.bofh.it>
In reply to#1589082
On 27-02-17, 18:31, Rob Herring wrote:
> On Fri, Feb 24, 2017 at 02:36:33PM +0530, Viresh Kumar wrote:
> > Some platforms have the capability to configure the performance state of
> > their power domains. The process of configuring the performance state is
> > pretty much platform dependent and we may need to work with a wide range
> > of configurables. For some platforms, like Qcom, it can be a positive
> > integer value alone, while in other cases it can be voltage levels, etc.
> > 
> > The power-domain framework until now was only designed for the idle
> > state management of the device and this needs to change in order to
> > reuse the power-domain framework for active state management of the
> > devices.
> > 
> > This patch adds DT bindings to describe the performance states of a
> > power domain. The power domain node needs to contain a
> > "performance-states" node, which itself is an array of per-state nodes.
> > Each per-state node represents individual performance state of a device.
> > Individual nodes are identified by their (mandatory) "reg" field. These
> > nodes can also contain an optional "domain-microvolt" property. More
> > properties can be added later on once we have more platforms using it.
> > 
> > If the consumers don't need the capability of switching to different
> > domain performance states at runtime, then they can simply define their
> > required domain performance state in their node directly using the
> > "domain-performance-state" property. Otherwise the consumers can define
> > their requirements with help of other infrastructure, for example the
> > OPP table (added in a later commit).
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> > ---
> >  .../devicetree/bindings/power/power_domain.txt     | 67 ++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> > index 723e1ad937da..9be09e576f68 100644
> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> > @@ -38,6 +38,33 @@ phandle arguments (so called PM domain specifiers) of length specified by the
> >    domain's idle states. In the absence of this property, the domain would be
> >    considered as capable of being powered-on or powered-off.
> >  
> > +- performance-states : This describes the performance states of a PM domain.
> > +  The performance-states node reflects the performance states of this PM domain
> > +  and not the performance states of the devices or sub-domains in the PM domain.
> > +  Sub domains can have their own performance states. Sub domains without their
> > +  own performance states are governed by the performance states of the parent
> > +  domain and the "domain-performance-state" properties of their consumers refer
> > +  to the "reg" properties of the nodes in the parent domain.
> > +
> > +  Required properties of the performance-states node:
> > +  - compatible: Allow performance states to express their compatibility. It
> > +    should be: "domain-performance-state".
> > +
> > +  - nodes: The performance-states node should contain one or
> > +    more nodes, each representing a supported performance state.
> > +
> > +    Required properties of the performance state nodes:
> > +    - reg: A positive integer value representing the performance level
> > +      associated with a performance state. The integer value '0' represents the
> > +      lowest performance level and the highest value represents the highest
> > +      performance level. The exact meaning and performance implications of
> > +      individual values is left to be defined by the user platform.
> > +
> > +    Optional properties of performance state nodes:
> > +    - domain-microvolt: voltage in micro Volts. A single regulator's voltage is
> > +      specified with an array of size one or three.  Single entry is for target
> > +      voltage and three entries are for <target min max> voltages.
> > +
> >  Example:
> >  
> >  	power: power-controller@12340000 {
> > @@ -118,4 +145,44 @@ The node above defines a typical PM domain consumer device, which is located
> >  inside a PM domain with index 0 of a power controller represented by a node
> >  with the label "power".
> >  
> > +Optional properties:
> > +- domain-performance-state: A positive integer value representing the minimum
> > +  performance level (of the parent domain) required by the consumer for its
> > +  working. The integer value '0' represents the lowest performance level and the
> > +  highest value represents the highest performance level. The value of
> > +  domain-performance-state field should match one of the "reg" fields in the
> > +  "performance-states" table of the parent power domain.
> > +
> > +
> > +Example:
> > +
> > +	parent: power-controller@12340000 {
> > +		compatible = "foo,power-controller";
> > +		reg = <0x12340000 0x1000>;
> > +		#power-domain-cells = <0>;
> > +
> > +		performance-states {
> > +			compatible = "domain-performance-state";
> > +			pstate@1 {
> > +				reg = <1>;
> > +				domain-microvolt = <970000 975000 985000>;
> 
> This doesn't look like "<target> <min> <max>".

Wow, even the examples in the OPP document have these screwed up :(

> With that fixed,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks.

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1587464 — [PATCH V3 7/7] PM / OPP: Add support to parse domain-performance-state

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-24 10:40 +0100
Subject[PATCH V3 7/7] PM / OPP: Add support to parse domain-performance-state
Message-ID<tekw3-6Db-25@gated-at.bofh.it>
In reply to#1587449
This patch allows the OPP core to parse the "domain-performance-state"
property in the OPP nodes. The nodes are allowed to have the
"domain-performance-state" property, only if the device node contains a
"power-domains" property. The OPP nodes aren't allowed to contain the
property partially, i.e. Either all OPP nodes in the OPP table have the
"domain-performance-state" property or none of them have it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/base/power/opp/core.c    | 73 ++++++++++++++++++++++++++++++++++++++++
 drivers/base/power/opp/debugfs.c |  4 +++
 drivers/base/power/opp/of.c      | 37 ++++++++++++++++++++
 drivers/base/power/opp/opp.h     | 12 +++++++
 4 files changed, 126 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 91ec3232d630..211551f377e9 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -542,6 +542,63 @@ _generic_set_opp_clk_only(struct device *dev, struct clk *clk,
 	return ret;
 }
 
+static int _update_pm_qos_request(struct device *dev,
+				  struct dev_pm_qos_request *req,
+				  unsigned int perf)
+{
+	int ret;
+
+	if (likely(dev_pm_qos_request_active(req)))
+		ret = dev_pm_qos_update_request(req, perf);
+	else
+		ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_PERFORMANCE,
+					     perf);
+
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int _generic_set_opp_pd(struct device *dev, struct clk *clk,
+			       struct dev_pm_qos_request *req,
+			       unsigned long old_freq, unsigned long freq,
+			       unsigned int old_perf, unsigned int new_perf)
+{
+	int ret;
+
+	/* Scaling up? Scale voltage before frequency */
+	if (freq > old_freq) {
+		ret = _update_pm_qos_request(dev, req, new_perf);
+		if (ret)
+			return ret;
+	}
+
+	/* Change frequency */
+	ret = _generic_set_opp_clk_only(dev, clk, old_freq, freq);
+	if (ret)
+		goto restore_perf;
+
+	/* Scaling down? Scale voltage after frequency */
+	if (freq < old_freq) {
+		ret = _update_pm_qos_request(dev, req, new_perf);
+		if (ret)
+			goto restore_freq;
+	}
+
+	return 0;
+
+restore_freq:
+	if (_generic_set_opp_clk_only(dev, clk, freq, old_freq))
+		dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+			__func__, old_freq);
+restore_perf:
+	if (old_perf)
+		_update_pm_qos_request(dev, req, old_perf);
+
+	return ret;
+}
+
 static int _generic_set_opp(struct dev_pm_set_opp_data *data)
 {
 	struct dev_pm_opp_supply *old_supply = data->old_opp.supplies;
@@ -662,6 +719,19 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	regulators = opp_table->regulators;
 
+	/* Has power domains performance states */
+	if (opp_table->has_pd_perf_states) {
+		unsigned int old_perf = 0, new_perf;
+		struct dev_pm_qos_request *req = &opp_table->qos_request;
+
+		new_perf = opp->pd_perf_state;
+		if (!IS_ERR(old_opp))
+			old_perf = old_opp->pd_perf_state;
+
+		return _generic_set_opp_pd(dev, clk, req, old_freq, freq,
+					   old_perf, new_perf);
+	}
+
 	/* Only frequency scaling */
 	if (!regulators) {
 		ret = _generic_set_opp_clk_only(dev, clk, old_freq, freq);
@@ -807,6 +877,9 @@ static void _opp_table_kref_release(struct kref *kref)
 	struct opp_table *opp_table = container_of(kref, struct opp_table, kref);
 	struct opp_device *opp_dev;
 
+	if (dev_pm_qos_request_active(&opp_table->qos_request))
+		dev_pm_qos_remove_request(&opp_table->qos_request);
+
 	/* Release clk */
 	if (!IS_ERR(opp_table->clk))
 		clk_put(opp_table->clk);
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index 95f433db4ac7..264958ab3de9 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -104,6 +104,10 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
 	if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
 		return -ENOMEM;
 
+	if (!debugfs_create_u32("power_domain_perf_state", S_IRUGO, d,
+				&opp->pd_perf_state))
+		return -ENOMEM;
+
 	if (!opp_debug_create_supplies(opp, opp_table, d))
 		return -ENOMEM;
 
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 779428676f63..e3b5f10e7f25 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -311,6 +311,38 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
 	if (!of_property_read_u32(np, "clock-latency-ns", &val))
 		new_opp->clock_latency_ns = val;
 
+	/*
+	 * Make sure that all information is present around domain power states
+	 * and nothing is left out.
+	 */
+	if (!of_property_read_u32(np, "domain-performance-state",
+				  &new_opp->pd_perf_state)) {
+		if (!opp_table->has_pd) {
+			ret = -EINVAL;
+			dev_err(dev, "%s: OPP node can't have performance state as device doesn't have power-domain\n",
+				__func__);
+			goto free_opp;
+		}
+
+		if (opp_table->has_pd_perf_states == -1) {
+			opp_table->has_pd_perf_states = 1;
+		} else if (!opp_table->has_pd_perf_states) {
+			ret = -EINVAL;
+			dev_err(dev, "%s: Not all OPP nodes have performance state\n",
+				__func__);
+			goto free_opp;
+		}
+	} else {
+		if (opp_table->has_pd_perf_states == -1) {
+			opp_table->has_pd_perf_states = 0;
+		} else if (opp_table->has_pd_perf_states) {
+			ret = -EINVAL;
+			dev_err(dev, "%s: Not all OPP nodes have performance state\n",
+				__func__);
+			goto free_opp;
+		}
+	}
+
 	ret = opp_parse_supplies(new_opp, dev, opp_table);
 	if (ret)
 		goto free_opp;
@@ -375,6 +407,11 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 	if (!opp_table)
 		return -ENOMEM;
 
+	if (of_find_property(dev->of_node, "power-domains", NULL)) {
+		opp_table->has_pd = true;
+		opp_table->has_pd_perf_states = -1;
+	}
+
 	/* We have opp-table node now, iterate over it and add OPPs */
 	for_each_available_child_of_node(opp_np, np) {
 		count++;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 166eef990599..41a2c0a67031 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -20,6 +20,7 @@
 #include <linux/list.h>
 #include <linux/limits.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_qos.h>
 #include <linux/notifier.h>
 
 struct clk;
@@ -58,6 +59,7 @@ extern struct list_head opp_tables;
  * @dynamic:	not-created from static DT entries.
  * @turbo:	true if turbo (boost) OPP
  * @suspend:	true if suspend OPP
+ * @pd_perf_state: Performance state of power domain
  * @rate:	Frequency in hertz
  * @supplies:	Power supplies voltage/current values
  * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
@@ -76,6 +78,7 @@ struct dev_pm_opp {
 	bool dynamic;
 	bool turbo;
 	bool suspend;
+	unsigned int pd_perf_state;
 	unsigned long rate;
 
 	struct dev_pm_opp_supply *supplies;
@@ -137,6 +140,11 @@ enum opp_table_access {
  * @regulator_count: Number of power supply regulators
  * @set_opp: Platform specific set_opp callback
  * @set_opp_data: Data to be passed to set_opp callback
+ * @has_pd: True if the device node contains power-domain property
+ * @has_pd_perf_states: Can have value of 0, 1 or -1. -1 means uninitialized
+ * state, 0 means that OPP nodes don't have perf states and 1 means that OPP
+ * nodes have perf states.
+ * @qos_request: Qos request.
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
  *
@@ -174,6 +182,10 @@ struct opp_table {
 	int (*set_opp)(struct dev_pm_set_opp_data *data);
 	struct dev_pm_set_opp_data *set_opp_data;
 
+	bool has_pd;
+	int has_pd_perf_states;
+	struct dev_pm_qos_request qos_request;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	char dentry_name[NAME_MAX];
-- 
2.7.1.410.g6faf27b

[toc] | [prev] | [next] | [standalone]


#1587494 — [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-24 11:20 +0100
Subject[PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tel8J-79A-1@gated-at.bofh.it>
In reply to#1587449
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their nodes directly.

But if the device needs the capability of switching to different domain
performance states, as they may need to support different clock rates,
then the per OPP node can be used to contain that information.

This patch introduces the domain-performance-state (already defined by
Power Domain bindings) to the per OPP node.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 64 +++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 9f5ca4457b5f..7f6bb52521b6 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -154,6 +154,15 @@ properties.
 
 - status: Marks the node enabled/disabled.
 
+- domain-performance-state: A positive integer value representing the minimum
+  performance level (of the parent domain) required by the consumer as defined
+  by ../power/power_domain.txt binding document. The OPP nodes can contain the
+  "domain-performance-state" property, only if the device node contains a
+  "power-domains" property. The OPP nodes aren't allowed to contain the
+  "domain-performance-state" property partially, i.e. Either all OPP nodes in
+  the OPP table have the "domain-performance-state" property or none of them
+  have it.
+
 Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
 
 / {
@@ -528,3 +537,58 @@ Example 5: opp-supported-hw
 		};
 	};
 };
+
+Example 7: domain-Performance-state:
+(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
+
+/ {
+	cpu0_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp@1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			domain-performance-state = <1>;
+		};
+		opp@1100000000 {
+			opp-hz = /bits/ 64 <1100000000>;
+			domain-performance-state = <2>;
+		};
+		opp@1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			domain-performance-state = <2>;
+		};
+	};
+
+	foo_domain: power-controller@12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <0>;
+
+		performance-states {
+			compatible = "domain-performance-state";
+			pstate@1 {
+				reg = <1>;
+				domain-microvolt = <970000 975000 985000>;
+			};
+			pstate@2 {
+				reg = <2>;
+				domain-microvolt = <1000000 1075000 1085000>;
+			};
+		};
+	}
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			power-domains = <&foo_domain>;
+		};
+	};
+};
-- 
2.7.1.410.g6faf27b

[toc] | [prev] | [next] | [standalone]


#1589071 — Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromRob Herring <robh@kernel.org>
Date2017-02-28 01:50 +0100
SubjectRe: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tfE9j-53z-5@gated-at.bofh.it>
In reply to#1587494
On Fri, Feb 24, 2017 at 02:36:34PM +0530, Viresh Kumar wrote:
> If the consumers don't need the capability of switching to different
> domain performance states at runtime, then they can simply define their
> required domain performance state in their nodes directly.
> 
> But if the device needs the capability of switching to different domain
> performance states, as they may need to support different clock rates,
> then the per OPP node can be used to contain that information.
> 
> This patch introduces the domain-performance-state (already defined by
> Power Domain bindings) to the per OPP node.
> 

We already have OPP voltages, why are those not sufficient?

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 64 +++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 9f5ca4457b5f..7f6bb52521b6 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -154,6 +154,15 @@ properties.
>  
>  - status: Marks the node enabled/disabled.
>  
> +- domain-performance-state: A positive integer value representing the minimum
> +  performance level (of the parent domain) required by the consumer as defined
> +  by ../power/power_domain.txt binding document. The OPP nodes can contain the
> +  "domain-performance-state" property, only if the device node contains a
> +  "power-domains" property. The OPP nodes aren't allowed to contain the
> +  "domain-performance-state" property partially, i.e. Either all OPP nodes in
> +  the OPP table have the "domain-performance-state" property or none of them
> +  have it.
> +
>  Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
>  
>  / {
> @@ -528,3 +537,58 @@ Example 5: opp-supported-hw
>  		};
>  	};
>  };
> +
> +Example 7: domain-Performance-state:
> +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> +
> +/ {
> +	cpu0_opp_table: opp_table0 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp@1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +			domain-performance-state = <1>;

Thinking about this some more, there's a problem here that you have no 
link to foo_domain. I guess that resides in the cpu's node?

Perhaps instead of a number, this should be a phandle to pstate@1. Then 
you just get the parent if you need to know the domain.

> +		};
> +		opp@1100000000 {
> +			opp-hz = /bits/ 64 <1100000000>;
> +			domain-performance-state = <2>;
> +		};
> +		opp@1200000000 {
> +			opp-hz = /bits/ 64 <1200000000>;
> +			domain-performance-state = <2>;
> +		};
> +	};
> +
> +	foo_domain: power-controller@12340000 {
> +		compatible = "foo,power-controller";
> +		reg = <0x12340000 0x1000>;
> +		#power-domain-cells = <0>;
> +
> +		performance-states {
> +			compatible = "domain-performance-state";
> +			pstate@1 {
> +				reg = <1>;
> +				domain-microvolt = <970000 975000 985000>;
> +			};
> +			pstate@2 {
> +				reg = <2>;
> +				domain-microvolt = <1000000 1075000 1085000>;
> +			};
> +		};
> +	}
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,cortex-a9";
> +			reg = <0>;
> +			clocks = <&clk_controller 0>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			power-domains = <&foo_domain>;
> +		};
> +	};
> +};
> -- 
> 2.7.1.410.g6faf27b
> 

[toc] | [prev] | [next] | [standalone]


#1589266 — Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromViresh Kumar <viresh.kumar@linaro.org>
Date2017-02-28 09:10 +0100
SubjectRe: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tfL18-1He-7@gated-at.bofh.it>
In reply to#1589071
On 27-02-17, 18:39, Rob Herring wrote:
> On Fri, Feb 24, 2017 at 02:36:34PM +0530, Viresh Kumar wrote:
> > If the consumers don't need the capability of switching to different
> > domain performance states at runtime, then they can simply define their
> > required domain performance state in their nodes directly.
> > 
> > But if the device needs the capability of switching to different domain
> > performance states, as they may need to support different clock rates,
> > then the per OPP node can be used to contain that information.
> > 
> > This patch introduces the domain-performance-state (already defined by
> > Power Domain bindings) to the per OPP node.
> > 
> 
> We already have OPP voltages, why are those not sufficient?

Those are for the regulator that ONLY controls the device, and
domain-performance-state belongs to the parent domain which controls many
devices.

> > +Example 7: domain-Performance-state:
> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> > +
> > +/ {
> > +	cpu0_opp_table: opp_table0 {
> > +		compatible = "operating-points-v2";
> > +		opp-shared;
> > +
> > +		opp@1000000000 {
> > +			opp-hz = /bits/ 64 <1000000000>;
> 
> Thinking about this some more, there's a problem here that you have no 
> link to foo_domain. I guess that resides in the cpu's node?

Right, the "cpus" node below demonstrates that.

> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > +			compatible = "arm,cortex-a9";
> > +			reg = <0>;
> > +			clocks = <&clk_controller 0>;
> > +			clock-names = "cpu";
> > +			operating-points-v2 = <&cpu0_opp_table>;
> > +			power-domains = <&foo_domain>;
> > +		};
> > +	};
> > +};

> > +			domain-performance-state = <1>;

> Perhaps instead of a number, this should be a phandle to pstate@1. Then 
> you just get the parent if you need to know the domain.

That's what I did in V2, but then I turned it down considering the parent/child
relationships we may have.

There are multiple cases we can have:

A.) DeviceX  --->  Parent-domain-1 (Contains Perfomance states)

B.) DeviceX  --->  Parent-domain-1  ---> Parent domain-2 (Contains Perfomance states)

                                    ---> Parent domain-2 (Contains Perfomance states)
                                    |
                                    |
C.) DeviceX  --->  Parent-domain-1  |
                                    |
                                    |
                                    ---> Parent domain-3 (Contains Perfomance states)


The case A.) represents a simple case where the parent domain of the device
contains the performance states. The phandle can work pretty well in this case.
But the other cases B.) and C.) are a bit complicated as the direct parent
domain doesn't allow changing the performance states, but its parents. And so I
went ahead with numbers instead of phandles. Yes, we will still be able to get
to the performance state node with the help of phandles, but will that be the
right thing to do ?

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1589501 — Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromRob Herring <robh@kernel.org>
Date2017-02-28 15:20 +0100
SubjectRe: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tfQNc-5z5-5@gated-at.bofh.it>
In reply to#1589266
On Tue, Feb 28, 2017 at 12:57 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 27-02-17, 18:39, Rob Herring wrote:
>> On Fri, Feb 24, 2017 at 02:36:34PM +0530, Viresh Kumar wrote:
>> > If the consumers don't need the capability of switching to different
>> > domain performance states at runtime, then they can simply define their
>> > required domain performance state in their nodes directly.
>> >
>> > But if the device needs the capability of switching to different domain
>> > performance states, as they may need to support different clock rates,
>> > then the per OPP node can be used to contain that information.
>> >
>> > This patch introduces the domain-performance-state (already defined by
>> > Power Domain bindings) to the per OPP node.
>> >
>>
>> We already have OPP voltages, why are those not sufficient?
>
> Those are for the regulator that ONLY controls the device, and
> domain-performance-state belongs to the parent domain which controls many
> devices.
>
>> > +Example 7: domain-Performance-state:
>> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
>> > +
>> > +/ {
>> > +   cpu0_opp_table: opp_table0 {
>> > +           compatible = "operating-points-v2";
>> > +           opp-shared;
>> > +
>> > +           opp@1000000000 {
>> > +                   opp-hz = /bits/ 64 <1000000000>;
>>
>> Thinking about this some more, there's a problem here that you have no
>> link to foo_domain. I guess that resides in the cpu's node?
>
> Right, the "cpus" node below demonstrates that.
>
>> > +   cpus {
>> > +           #address-cells = <1>;
>> > +           #size-cells = <0>;
>> > +
>> > +           cpu@0 {
>> > +                   compatible = "arm,cortex-a9";
>> > +                   reg = <0>;
>> > +                   clocks = <&clk_controller 0>;
>> > +                   clock-names = "cpu";
>> > +                   operating-points-v2 = <&cpu0_opp_table>;
>> > +                   power-domains = <&foo_domain>;
>> > +           };
>> > +   };
>> > +};
>
>> > +                   domain-performance-state = <1>;
>
>> Perhaps instead of a number, this should be a phandle to pstate@1. Then
>> you just get the parent if you need to know the domain.
>
> That's what I did in V2, but then I turned it down considering the parent/child
> relationships we may have.
>
> There are multiple cases we can have:
>
> A.) DeviceX  --->  Parent-domain-1 (Contains Perfomance states)
>
> B.) DeviceX  --->  Parent-domain-1  ---> Parent domain-2 (Contains Perfomance states)
>
>                                     ---> Parent domain-2 (Contains Perfomance states)
>                                     |
>                                     |
> C.) DeviceX  --->  Parent-domain-1  |
>                                     |
>                                     |
>                                     ---> Parent domain-3 (Contains Perfomance states)

I'm a bit confused. How does a domain have 2 parent domains?

You have the same problem either way. If I have performance state 2
for the device, that corresponds to domain 2 or 3?

> The case A.) represents a simple case where the parent domain of the device
> contains the performance states. The phandle can work pretty well in this case.
> But the other cases B.) and C.) are a bit complicated as the direct parent
> domain doesn't allow changing the performance states, but its parents. And so I
> went ahead with numbers instead of phandles. Yes, we will still be able to get
> to the performance state node with the help of phandles, but will that be the
> right thing to do ?
>
> --
> viresh

[toc] | [prev] | [next] | [standalone]


#1589545 — Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromUlf Hansson <ulf.hansson@linaro.org>
Date2017-02-28 16:20 +0100
SubjectRe: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tfRJf-6ds-1@gated-at.bofh.it>
In reply to#1589501
[...]

>>                                     ---> Parent domain-2 (Contains Perfomance states)
>>                                     |
>>                                     |
>> C.) DeviceX  --->  Parent-domain-1  |
>>                                     |
>>                                     |
>>                                     ---> Parent domain-3 (Contains Perfomance states)
>
> I'm a bit confused. How does a domain have 2 parent domains?

This comes from the early design of the generic PM domain, thus I
assume we have some HW with such complex PM topology. However, I don't
know if it is actually being used.

Moreover, the corresponding DT bindings for "power-domains" parents,
can easily be extended to cover more than one parent. See more in
Documentation/devicetree/bindings/power/power_domain.txt

[...]

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1589585 — Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromRob Herring <robh@kernel.org>
Date2017-02-28 17:10 +0100
SubjectRe: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tfSvD-6NI-1@gated-at.bofh.it>
In reply to#1589545
On Tue, Feb 28, 2017 at 9:14 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]
>
>>>                                     ---> Parent domain-2 (Contains Perfomance states)
>>>                                     |
>>>                                     |
>>> C.) DeviceX  --->  Parent-domain-1  |
>>>                                     |
>>>                                     |
>>>                                     ---> Parent domain-3 (Contains Perfomance states)
>>
>> I'm a bit confused. How does a domain have 2 parent domains?
>
> This comes from the early design of the generic PM domain, thus I
> assume we have some HW with such complex PM topology. However, I don't
> know if it is actually being used.
>
> Moreover, the corresponding DT bindings for "power-domains" parents,
> can easily be extended to cover more than one parent. See more in
> Documentation/devicetree/bindings/power/power_domain.txt

I could easily see device having 2 power domains. For example a cpu
may have separate domains for RAM/caches and logic. And nesting of
power domains is certainly common, but a power domain being contained
in 2 different parents? I don't even see how that is possible in the
physical design. Now if we're mixing PM and power domains again and
the cpu device is pointing to the cpu PM domain which contains 2 power
domains, then certainly that is possible.

Rob

[toc] | [prev] | [next] | [standalone]


#1589731 — Re: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-02-28 20:50 +0100
SubjectRe: [PATCH V3 2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes
Message-ID<tfVWy-rZ-5@gated-at.bofh.it>
In reply to#1589585
Hi Rob,

On Tue, Feb 28, 2017 at 4:52 PM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 28, 2017 at 9:14 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> [...]
>>
>>>>                                     ---> Parent domain-2 (Contains Perfomance states)
>>>>                                     |
>>>>                                     |
>>>> C.) DeviceX  --->  Parent-domain-1  |
>>>>                                     |
>>>>                                     |
>>>>                                     ---> Parent domain-3 (Contains Perfomance states)
>>>
>>> I'm a bit confused. How does a domain have 2 parent domains?
>>
>> This comes from the early design of the generic PM domain, thus I
>> assume we have some HW with such complex PM topology. However, I don't
>> know if it is actually being used.
>>
>> Moreover, the corresponding DT bindings for "power-domains" parents,
>> can easily be extended to cover more than one parent. See more in
>> Documentation/devicetree/bindings/power/power_domain.txt
>
> I could easily see device having 2 power domains. For example a cpu
> may have separate domains for RAM/caches and logic. And nesting of
> power domains is certainly common, but a power domain being contained
> in 2 different parents? I don't even see how that is possible in the
> physical design. Now if we're mixing PM and power domains again and
> the cpu device is pointing to the cpu PM domain which contains 2 power
> domains, then certainly that is possible.

One of them could be a power area, the other a clock domain.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web