[PATCH v4 0/3] Dynamic power model from device tree

[PATCH v4 0/3] Dynamic power model from device tree

[Punit Agrawal]

Hi,

This patchset adds support to build a single-coefficient dynamic power
model for a CPU. The model is used by the CPU cooling device to
provide an estimate of power consumption and also translate allocated
power to performance cap.

Changes from previous posting -

v3 -> v4:
arm_big_little: Migrated to using static arrays
arm_big_little: Updated Kconfig to support building thermal as module

Patch 1 extends the CPU nodes binding to provide an optional dynamic
power coefficient which can be used to create a dynamic power model
for the CPUs. This model is used to constrain device power consumption
(using power_allocator governor) when the system is thermally
constrained.

Patches 2-3 extends the cpufreq-dt and arm_big_little driver to
register cpu cooling devices with the dynamic coefficient when
provided.

The patches were previously posted at [0][1][2]. Mediatek platform
8173 builds on these bindings to build the power model.

If there are no objections, I'd appreciate Acks from device tree
bindings maintainers.

Thanks,
Punit

[0] http://thread.gmane.org/gmane.linux.kernel/2002152
[1] http://thread.gmane.org/gmane.linux.kernel/2011466
[2] http://marc.info/?l=linux-kernel&m=144709020014884&w=2

Punit Agrawal (3):
  devicetree: bindings: Add optional dynamic-power-coefficient property
  cpufreq-dt: Supply power coefficient when registering cooling devices
  cpufreq: arm_big_little: Add support to register a cpufreq cooling
    device

 Documentation/devicetree/bindings/arm/cpus.txt | 17 +++++++++++++
 drivers/cpufreq/Kconfig.arm                    |  2 ++
 drivers/cpufreq/arm_big_little.c               | 35 ++++++++++++++++++++++++++
 drivers/cpufreq/cpufreq-dt.c                   |  9 +++++--
 4 files changed, 61 insertions(+), 2 deletions(-)

-- 
2.6.2

[PATCH v4 1/3] devicetree: bindings: Add optional dynamic-power-coefficient property

[Punit Agrawal]

The dynamic power consumption of a device is proportional to the
square of voltage (V) and the clock frequency (f). It can be expressed as

Pdyn = dynamic-power-coefficient * V^2 * f.

The coefficient represents the running time dynamic power consumption in
units of mw/MHz/uVolt^2 and can be used in the above formula to
calculate the dynamic power in mW.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/devicetree/bindings/arm/cpus.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 3a07a87..6aca64f 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -242,6 +242,23 @@ nodes to be present and contain the properties described below.
 		Definition: Specifies the syscon node controlling the cpu core
 			    power domains.
 
+	- dynamic-power-coefficient
+		Usage: optional
+		Value type: <prop-encoded-array>
+		Definition: A u32 value that represents the running time dynamic
+			    power coefficient in units of mW/MHz/uVolt^2. The
+			    coefficient can either be calculated from power
+			    measurements or derived by analysis.
+
+			    The dynamic power consumption of the CPU  is
+			    proportional to the square of the Voltage (V) and
+			    the clock frequency (f). The coefficient is used to
+			    calculate the dynamic power as below -
+
+			    Pdyn = dynamic-power-coefficient * V^2 * f
+
+			    where voltage is in uV, frequency is in MHz.
+
 Example 1 (dual-cluster big.LITTLE system 32-bit):
 
 	cpus {
-- 
2.6.2

[PATCH v4 2/3] cpufreq-dt: Supply power coefficient when registering cooling devices

[Punit Agrawal]

Support registering cooling devices with dynamic power coefficient
where provided by the device tree. This allows OF registered cooling
devices driver to be used with the power_allocator thermal governor.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Javi Merino <javi.merino@arm.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/cpufreq/cpufreq-dt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 90d6408..1ceece9 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -407,8 +407,13 @@ static void cpufreq_ready(struct cpufreq_policy *policy)
 	 * thermal DT code takes care of matching them.
 	 */
 	if (of_find_property(np, "#cooling-cells", NULL)) {
-		priv->cdev = of_cpufreq_cooling_register(np,
-							 policy->related_cpus);
+		u32 power_coefficient = 0;
+
+		of_property_read_u32(np, "dynamic-power-coefficient",
+				     &power_coefficient);
+
+		priv->cdev = of_cpufreq_power_cooling_register(np,
+				policy->related_cpus, power_coefficient, NULL);
 		if (IS_ERR(priv->cdev)) {
 			dev_err(priv->cpu_dev,
 				"running cpufreq without cooling device: %ld\n",
-- 
2.6.2

[PATCH v4 2/3] cpufreq-dt: Supply power coefficient when registering cooling devices

[Punit Agrawal]

Support registering cooling devices with dynamic power coefficient
where provided by the device tree. This allows OF registered cooling
devices driver to be used with the power_allocator thermal governor.

Signed-off-by: Punit Agrawal <punit.agrawal-5wv7dgnIgG8@public.gmane.org>
Acked-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Javi Merino <javi.merino-5wv7dgnIgG8@public.gmane.org>
Cc: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/cpufreq/cpufreq-dt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 90d6408..1ceece9 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -407,8 +407,13 @@ static void cpufreq_ready(struct cpufreq_policy *policy)
 	 * thermal DT code takes care of matching them.
 	 */
 	if (of_find_property(np, "#cooling-cells", NULL)) {
-		priv->cdev = of_cpufreq_cooling_register(np,
-							 policy->related_cpus);
+		u32 power_coefficient = 0;
+
+		of_property_read_u32(np, "dynamic-power-coefficient",
+				     &power_coefficient);
+
+		priv->cdev = of_cpufreq_power_cooling_register(np,
+				policy->related_cpus, power_coefficient, NULL);
 		if (IS_ERR(priv->cdev)) {
 			dev_err(priv->cpu_dev,
 				"running cpufreq without cooling device: %ld\n",
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device

[Punit Agrawal]

Register passive cooling devices when initialising cpufreq on
big.LITTLE systems. If the device tree provides a dynamic power
coefficient for the CPUs then the bound cooling device will support
the extensions that allow it to be used with all the existing thermal
governors including the power allocator governor.

A cooling device will be created per individual frequency domain and
can be bound to thermal zones via the thermal DT bindings.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/cpufreq/Kconfig.arm      |  2 ++
 drivers/cpufreq/arm_big_little.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 1582c1c..0e0052e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -6,6 +6,8 @@
 config ARM_BIG_LITTLE_CPUFREQ
 	tristate "Generic ARM big LITTLE CPUfreq driver"
 	depends on (ARM_CPU_TOPOLOGY || ARM64) && HAVE_CLK
+	# if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
+	depends on !CPU_THERMAL || THERMAL
 	select PM_OPP
 	help
 	  This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index c5d256c..60d09c0 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -23,6 +23,7 @@
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
+#include <linux/cpu_cooling.h>
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -55,6 +56,7 @@ static bool bL_switching_enabled;
 #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
 #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
 
+static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
 static struct cpufreq_arm_bL_ops *arm_bL_ops;
 static struct clk *clk[MAX_CLUSTERS];
 static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -493,6 +495,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 {
 	struct device *cpu_dev;
+	int domain;
 
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
@@ -501,12 +504,43 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 		return -ENODEV;
 	}
 
+	domain = topology_physical_package_id(cpu_dev->id);
+	cpufreq_cooling_unregister(cdev[domain]);
+	cdev[domain] = NULL;
+
 	put_cluster_clk_and_freq_table(cpu_dev);
 	dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
 
 	return 0;
 }
 
+static void bL_cpufreq_ready(struct cpufreq_policy *policy)
+{
+	struct device *cpu_dev = get_cpu_device(policy->cpu);
+	struct device_node *np = of_node_get(cpu_dev->of_node);
+	int domain = topology_physical_package_id(cpu_dev->id);
+
+	if (WARN_ON(!np))
+		return;
+
+	if (of_find_property(np, "#cooling-cells", NULL)) {
+		u32 power_coefficient = 0;
+
+		of_property_read_u32(np, "dynamic-power-coefficient",
+				     &power_coefficient);
+
+		cdev[domain] = of_cpufreq_power_cooling_register(np,
+				policy->related_cpus, power_coefficient, NULL);
+		if (IS_ERR(cdev[domain])) {
+			dev_err(cpu_dev,
+				"running cpufreq without cooling device: %ld\n",
+				PTR_ERR(cdev[domain]));
+			cdev[domain] = NULL;
+		}
+	}
+	of_node_put(np);
+}
+
 static struct cpufreq_driver bL_cpufreq_driver = {
 	.name			= "arm-big-little",
 	.flags			= CPUFREQ_STICKY |
@@ -517,6 +551,7 @@ static struct cpufreq_driver bL_cpufreq_driver = {
 	.get			= bL_cpufreq_get_rate,
 	.init			= bL_cpufreq_init,
 	.exit			= bL_cpufreq_exit,
+	.ready			= bL_cpufreq_ready,
 	.attr			= cpufreq_generic_attr,
 };
 
-- 
2.6.2

Re: [PATCH v4 1/3] devicetree: bindings: Add optional dynamic-power-coefficient property

[Rob Herring]

On Mon, Nov 16, 2015 at 07:30:23PM +0000, Punit Agrawal wrote:
> The dynamic power consumption of a device is proportional to the
> square of voltage (V) and the clock frequency (f). It can be expressed as
> 
> Pdyn = dynamic-power-coefficient * V^2 * f.
> 
> The coefficient represents the running time dynamic power consumption in
> units of mw/MHz/uVolt^2 and can be used in the above formula to
> calculate the dynamic power in mW.
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>

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

> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 3a07a87..6aca64f 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -242,6 +242,23 @@ nodes to be present and contain the properties described below.
>  		Definition: Specifies the syscon node controlling the cpu core
>  			    power domains.
>  
> +	- dynamic-power-coefficient
> +		Usage: optional
> +		Value type: <prop-encoded-array>
> +		Definition: A u32 value that represents the running time dynamic
> +			    power coefficient in units of mW/MHz/uVolt^2. The
> +			    coefficient can either be calculated from power
> +			    measurements or derived by analysis.
> +
> +			    The dynamic power consumption of the CPU  is
> +			    proportional to the square of the Voltage (V) and
> +			    the clock frequency (f). The coefficient is used to
> +			    calculate the dynamic power as below -
> +
> +			    Pdyn = dynamic-power-coefficient * V^2 * f
> +
> +			    where voltage is in uV, frequency is in MHz.
> +
>  Example 1 (dual-cluster big.LITTLE system 32-bit):
>  
>  	cpus {
> -- 
> 2.6.2
> 

Re: [PATCH v4 0/3] Dynamic power model from device tree

[Viresh Kumar]

On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
> Hi,
>
> This patchset adds support to build a single-coefficient dynamic power
> model for a CPU. The model is used by the CPU cooling device to
> provide an estimate of power consumption and also translate allocated
> power to performance cap.
>
> Changes from previous posting -
>
> v3 -> v4:
> arm_big_little: Migrated to using static arrays
> arm_big_little: Updated Kconfig to support building thermal as module
>
> Patch 1 extends the CPU nodes binding to provide an optional dynamic
> power coefficient which can be used to create a dynamic power model
> for the CPUs. This model is used to constrain device power consumption
> (using power_allocator governor) when the system is thermally
> constrained.
>
> Patches 2-3 extends the cpufreq-dt and arm_big_little driver to
> register cpu cooling devices with the dynamic coefficient when
> provided.
>
> The patches were previously posted at [0][1][2]. Mediatek platform
> 8173 builds on these bindings to build the power model.
>
> If there are no objections, I'd appreciate Acks from device tree
> bindings maintainers.

You forgot to cc cpufreq maintainers for this and there are good chances
that I may completely miss the series some times..

Re: [PATCH v4 1/3] devicetree: bindings: Add optional dynamic-power-coefficient property

[Viresh Kumar]

On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
> The dynamic power consumption of a device is proportional to the
> square of voltage (V) and the clock frequency (f). It can be expressed as
>
> Pdyn = dynamic-power-coefficient * V^2 * f.
>
> The coefficient represents the running time dynamic power consumption in
> units of mw/MHz/uVolt^2 and can be used in the above formula to
> calculate the dynamic power in mW.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 3a07a87..6aca64f 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -242,6 +242,23 @@ nodes to be present and contain the properties described below.
>                 Definition: Specifies the syscon node controlling the cpu core
>                             power domains.
>
> +       - dynamic-power-coefficient
> +               Usage: optional
> +               Value type: <prop-encoded-array>
> +               Definition: A u32 value that represents the running time dynamic
> +                           power coefficient in units of mW/MHz/uVolt^2. The
> +                           coefficient can either be calculated from power
> +                           measurements or derived by analysis.
> +
> +                           The dynamic power consumption of the CPU  is
> +                           proportional to the square of the Voltage (V) and
> +                           the clock frequency (f). The coefficient is used to
> +                           calculate the dynamic power as below -
> +
> +                           Pdyn = dynamic-power-coefficient * V^2 * f
> +
> +                           where voltage is in uV, frequency is in MHz.
> +
>  Example 1 (dual-cluster big.LITTLE system 32-bit):
>
>         cpus {

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH v4 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device

[Viresh Kumar]

On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
> Register passive cooling devices when initialising cpufreq on
> big.LITTLE systems. If the device tree provides a dynamic power
> coefficient for the CPUs then the bound cooling device will support
> the extensions that allow it to be used with all the existing thermal
> governors including the power allocator governor.
>
> A cooling device will be created per individual frequency domain and
> can be bound to thermal zones via the thermal DT bindings.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Remind me when did I Ack this version of your patch ..

> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> ---
>  drivers/cpufreq/Kconfig.arm      |  2 ++
>  drivers/cpufreq/arm_big_little.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 1582c1c..0e0052e 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -6,6 +6,8 @@
>  config ARM_BIG_LITTLE_CPUFREQ
>         tristate "Generic ARM big LITTLE CPUfreq driver"
>         depends on (ARM_CPU_TOPOLOGY || ARM64) && HAVE_CLK
> +       # if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
> +       depends on !CPU_THERMAL || THERMAL
>         select PM_OPP
>         help
>           This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> index c5d256c..60d09c0 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -23,6 +23,7 @@
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/cpumask.h>
> +#include <linux/cpu_cooling.h>
>  #include <linux/export.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -55,6 +56,7 @@ static bool bL_switching_enabled;
>  #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
>  #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
>
> +static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
>  static struct cpufreq_arm_bL_ops *arm_bL_ops;
>  static struct clk *clk[MAX_CLUSTERS];
>  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
> @@ -493,6 +495,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
>  static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>  {
>         struct device *cpu_dev;
> +       int domain;
>
>         cpu_dev = get_cpu_device(policy->cpu);
>         if (!cpu_dev) {
> @@ -501,12 +504,43 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>                 return -ENODEV;
>         }
>
> +       domain = topology_physical_package_id(cpu_dev->id);

And this is broken.

Have you tested this for IKS ? That's what the primary use-case of
this driver is.
And yeah, I would like to migrate the bL usecase to cpufreq-dt, now that it can
support multiple clusters.

This is broken, because exit() might get called for a CPU from big
cluster, while
read() was called for a CPU on little cluster.

--
viresh

Re: [PATCH v4 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device

[Viresh Kumar]

On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal-5wv7dgnIgG8@public.gmane.org> wrote:
> Register passive cooling devices when initialising cpufreq on
> big.LITTLE systems. If the device tree provides a dynamic power
> coefficient for the CPUs then the bound cooling device will support
> the extensions that allow it to be used with all the existing thermal
> governors including the power allocator governor.
>
> A cooling device will be created per individual frequency domain and
> can be bound to thermal zones via the thermal DT bindings.
>
> Signed-off-by: Punit Agrawal <punit.agrawal-5wv7dgnIgG8@public.gmane.org>
> Acked-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Remind me when did I Ack this version of your patch ..

> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/cpufreq/Kconfig.arm      |  2 ++
>  drivers/cpufreq/arm_big_little.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 1582c1c..0e0052e 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -6,6 +6,8 @@
>  config ARM_BIG_LITTLE_CPUFREQ
>         tristate "Generic ARM big LITTLE CPUfreq driver"
>         depends on (ARM_CPU_TOPOLOGY || ARM64) && HAVE_CLK
> +       # if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
> +       depends on !CPU_THERMAL || THERMAL
>         select PM_OPP
>         help
>           This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> index c5d256c..60d09c0 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -23,6 +23,7 @@
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/cpumask.h>
> +#include <linux/cpu_cooling.h>
>  #include <linux/export.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -55,6 +56,7 @@ static bool bL_switching_enabled;
>  #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
>  #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
>
> +static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
>  static struct cpufreq_arm_bL_ops *arm_bL_ops;
>  static struct clk *clk[MAX_CLUSTERS];
>  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
> @@ -493,6 +495,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
>  static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>  {
>         struct device *cpu_dev;
> +       int domain;
>
>         cpu_dev = get_cpu_device(policy->cpu);
>         if (!cpu_dev) {
> @@ -501,12 +504,43 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>                 return -ENODEV;
>         }
>
> +       domain = topology_physical_package_id(cpu_dev->id);

And this is broken.

Have you tested this for IKS ? That's what the primary use-case of
this driver is.
And yeah, I would like to migrate the bL usecase to cpufreq-dt, now that it can
support multiple clusters.

This is broken, because exit() might get called for a CPU from big
cluster, while
read() was called for a CPU on little cluster.

--
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/3] Dynamic power model from device tree

[Punit Agrawal]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
>> Hi,
>>
>> This patchset adds support to build a single-coefficient dynamic power
>> model for a CPU. The model is used by the CPU cooling device to
>> provide an estimate of power consumption and also translate allocated
>> power to performance cap.
>>
>> Changes from previous posting -
>>
>> v3 -> v4:
>> arm_big_little: Migrated to using static arrays
>> arm_big_little: Updated Kconfig to support building thermal as module
>>
>> Patch 1 extends the CPU nodes binding to provide an optional dynamic
>> power coefficient which can be used to create a dynamic power model
>> for the CPUs. This model is used to constrain device power consumption
>> (using power_allocator governor) when the system is thermally
>> constrained.
>>
>> Patches 2-3 extends the cpufreq-dt and arm_big_little driver to
>> register cpu cooling devices with the dynamic coefficient when
>> provided.
>>
>> The patches were previously posted at [0][1][2]. Mediatek platform
>> 8173 builds on these bindings to build the power model.
>>
>> If there are no objections, I'd appreciate Acks from device tree
>> bindings maintainers.
>
> You forgot to cc cpufreq maintainers for this and there are good chances
> that I may completely miss the series some times..

Yikes! I'll make sure to include both Rafael and you for future
postings.

> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/3] Dynamic power model from device tree

[Punit Agrawal]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
>> Hi,
>>
>> This patchset adds support to build a single-coefficient dynamic power
>> model for a CPU. The model is used by the CPU cooling device to
>> provide an estimate of power consumption and also translate allocated
>> power to performance cap.
>>
>> Changes from previous posting -
>>
>> v3 -> v4:
>> arm_big_little: Migrated to using static arrays
>> arm_big_little: Updated Kconfig to support building thermal as module
>>
>> Patch 1 extends the CPU nodes binding to provide an optional dynamic
>> power coefficient which can be used to create a dynamic power model
>> for the CPUs. This model is used to constrain device power consumption
>> (using power_allocator governor) when the system is thermally
>> constrained.
>>
>> Patches 2-3 extends the cpufreq-dt and arm_big_little driver to
>> register cpu cooling devices with the dynamic coefficient when
>> provided.
>>
>> The patches were previously posted at [0][1][2]. Mediatek platform
>> 8173 builds on these bindings to build the power model.
>>
>> If there are no objections, I'd appreciate Acks from device tree
>> bindings maintainers.
>
> You forgot to cc cpufreq maintainers for this and there are good chances
> that I may completely miss the series some times..

Yikes! I'll make sure to include both Rafael and you for future
postings.

> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device

[Punit Agrawal]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
>> Register passive cooling devices when initialising cpufreq on
>> big.LITTLE systems. If the device tree provides a dynamic power
>> coefficient for the CPUs then the bound cooling device will support
>> the extensions that allow it to be used with all the existing thermal
>> governors including the power allocator governor.
>>
>> A cooling device will be created per individual frequency domain and
>> can be bound to thermal zones via the thermal DT bindings.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Remind me when did I Ack this version of your patch ..

You didn't - I missed removing the tag before sending the patch
out. Dropped locally now.

>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>> ---
>>  drivers/cpufreq/Kconfig.arm      |  2 ++
>>  drivers/cpufreq/arm_big_little.c | 35 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 1582c1c..0e0052e 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -6,6 +6,8 @@
>>  config ARM_BIG_LITTLE_CPUFREQ
>>         tristate "Generic ARM big LITTLE CPUfreq driver"
>>         depends on (ARM_CPU_TOPOLOGY || ARM64) && HAVE_CLK
>> +       # if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
>> +       depends on !CPU_THERMAL || THERMAL
>>         select PM_OPP
>>         help
>>           This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
>> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
>> index c5d256c..60d09c0 100644
>> --- a/drivers/cpufreq/arm_big_little.c
>> +++ b/drivers/cpufreq/arm_big_little.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/cpu.h>
>>  #include <linux/cpufreq.h>
>>  #include <linux/cpumask.h>
>> +#include <linux/cpu_cooling.h>
>>  #include <linux/export.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>> @@ -55,6 +56,7 @@ static bool bL_switching_enabled;
>>  #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
>>  #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
>>
>> +static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
>>  static struct cpufreq_arm_bL_ops *arm_bL_ops;
>>  static struct clk *clk[MAX_CLUSTERS];
>>  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
>> @@ -493,6 +495,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
>>  static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>>  {
>>         struct device *cpu_dev;
>> +       int domain;
>>
>>         cpu_dev = get_cpu_device(policy->cpu);
>>         if (!cpu_dev) {
>> @@ -501,12 +504,43 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>>                 return -ENODEV;
>>         }
>>
>> +       domain = topology_physical_package_id(cpu_dev->id);
>
> And this is broken.
>
> Have you tested this for IKS ? That's what the primary use-case of
> this driver is.

Juno dvfs uses this driver - which is where I've been testing the
changes. Unfortunately, there is no IKS there.

> And yeah, I would like to migrate the bL usecase to cpufreq-dt, now that it can
> support multiple clusters.

It would be great to have a unified cpufreq driver servicing multiple
dvfs domains. If I understand correctly, we can drop $SUBJECT when that
happens. In the meanwhile...

>
> This is broken, because exit() might get called for a CPU from big
> cluster, while
> read() was called for a CPU on little cluster.

Good catch. The patch will definitely not do the right thing when IKS is
enabled. Looking at the code, it looks like you can switch in and out of
IKS at run time.

The cpu_cooling device doesn't cope with different types of cores, so
I'll just have to skip registering (and un-registering) the cooling
device when IKS is enabled.

I'll send out a new version addressing the issues you've pointed out.

Thanks for your comments.

Punit
>
> --
> viresh
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device

[Punit Agrawal]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On Tue, Nov 17, 2015 at 1:00 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
>> Register passive cooling devices when initialising cpufreq on
>> big.LITTLE systems. If the device tree provides a dynamic power
>> coefficient for the CPUs then the bound cooling device will support
>> the extensions that allow it to be used with all the existing thermal
>> governors including the power allocator governor.
>>
>> A cooling device will be created per individual frequency domain and
>> can be bound to thermal zones via the thermal DT bindings.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Remind me when did I Ack this version of your patch ..

You didn't - I missed removing the tag before sending the patch
out. Dropped locally now.

>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>> ---
>>  drivers/cpufreq/Kconfig.arm      |  2 ++
>>  drivers/cpufreq/arm_big_little.c | 35 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 1582c1c..0e0052e 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -6,6 +6,8 @@
>>  config ARM_BIG_LITTLE_CPUFREQ
>>         tristate "Generic ARM big LITTLE CPUfreq driver"
>>         depends on (ARM_CPU_TOPOLOGY || ARM64) && HAVE_CLK
>> +       # if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
>> +       depends on !CPU_THERMAL || THERMAL
>>         select PM_OPP
>>         help
>>           This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
>> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
>> index c5d256c..60d09c0 100644
>> --- a/drivers/cpufreq/arm_big_little.c
>> +++ b/drivers/cpufreq/arm_big_little.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/cpu.h>
>>  #include <linux/cpufreq.h>
>>  #include <linux/cpumask.h>
>> +#include <linux/cpu_cooling.h>
>>  #include <linux/export.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>> @@ -55,6 +56,7 @@ static bool bL_switching_enabled;
>>  #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
>>  #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
>>
>> +static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
>>  static struct cpufreq_arm_bL_ops *arm_bL_ops;
>>  static struct clk *clk[MAX_CLUSTERS];
>>  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
>> @@ -493,6 +495,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
>>  static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>>  {
>>         struct device *cpu_dev;
>> +       int domain;
>>
>>         cpu_dev = get_cpu_device(policy->cpu);
>>         if (!cpu_dev) {
>> @@ -501,12 +504,43 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>>                 return -ENODEV;
>>         }
>>
>> +       domain = topology_physical_package_id(cpu_dev->id);
>
> And this is broken.
>
> Have you tested this for IKS ? That's what the primary use-case of
> this driver is.

Juno dvfs uses this driver - which is where I've been testing the
changes. Unfortunately, there is no IKS there.

> And yeah, I would like to migrate the bL usecase to cpufreq-dt, now that it can
> support multiple clusters.

It would be great to have a unified cpufreq driver servicing multiple
dvfs domains. If I understand correctly, we can drop $SUBJECT when that
happens. In the meanwhile...

>
> This is broken, because exit() might get called for a CPU from big
> cluster, while
> read() was called for a CPU on little cluster.

Good catch. The patch will definitely not do the right thing when IKS is
enabled. Looking at the code, it looks like you can switch in and out of
IKS at run time.

The cpu_cooling device doesn't cope with different types of cores, so
I'll just have to skip registering (and un-registering) the cooling
device when IKS is enabled.

I'll send out a new version addressing the issues you've pointed out.

Thanks for your comments.

Punit
>
> --
> viresh
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html