diff mbox series

[V2,2/2] base/drivers/arch_topology: Default dmips-mhz if they are not set in DT

Message ID 1542890209-3263-2-git-send-email-daniel.lezcano@linaro.org
State Superseded
Headers show
Series [V2,1/2] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE | expand

Commit Message

Daniel Lezcano Nov. 22, 2018, 12:36 p.m. UTC
In the case of asymmetric SoC with the same micro-architecture, we
have a group of CPUs with smaller OPPs than the other group. One
example is the 96boards dragonboard 820c. There is no dmips/MHz
difference between both groups, so no need to specify the values in
the DT. Unfortunately, without these defined, there is no scaling
capacity computation triggered, so we need to write
'capacity-dmips-mhz' for each CPU with the same value in order to
force the scaled capacity computation.

Fix this by setting a default capacity to SCHED_CAPACITY_SCALE, if no
'capacity-dmips-mhz' is defined in the DT.

This was tested on db820c:
 - specified values in the DT (correct results)
 - partial values defined in the DT (error + fallback to defaults)
 - no specified values in the DT (correct results)

correct results are:
  cat /sys/devices/system/cpu/cpu*/cpu_capacity
   758
   758
  1024
  1024

  ... respectively for CPU0, CPU1, CPU2 and CPU3.

That reflects the capacity for the max frequencies 1593600 and 2150400.

Cc: Chris Redpath <chris.redpath@linaro.org>
Cc: Quentin Perret <quentin.perret@linaro.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Amit Kucheria <amit.kucheria@linaro.org>
Cc: Nicolas Dechesne <nicolas.dechesne@linaro.org>
Cc: Niklas Cassel <niklas.cassel@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

---
 drivers/base/arch_topology.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.7.4

Comments

Viresh Kumar Nov. 23, 2018, 10:04 a.m. UTC | #1
On 22-11-18, 13:36, Daniel Lezcano wrote:
> In the case of asymmetric SoC with the same micro-architecture, we

> have a group of CPUs with smaller OPPs than the other group. One

> example is the 96boards dragonboard 820c. There is no dmips/MHz

> difference between both groups, so no need to specify the values in

> the DT. Unfortunately, without these defined, there is no scaling

> capacity computation triggered, so we need to write

> 'capacity-dmips-mhz' for each CPU with the same value in order to

> force the scaled capacity computation.

> 

> Fix this by setting a default capacity to SCHED_CAPACITY_SCALE, if no

> 'capacity-dmips-mhz' is defined in the DT.


We aren't doing this anymore. You should rather explain that we just
allocate raw_capacity now and rest is left for
init_cpu_capacity_callback() to fix.

-- 
viresh
Daniel Lezcano Nov. 23, 2018, 10:32 a.m. UTC | #2
On 23/11/2018 11:04, Viresh Kumar wrote:
> On 22-11-18, 13:36, Daniel Lezcano wrote:

>> In the case of asymmetric SoC with the same micro-architecture, we

>> have a group of CPUs with smaller OPPs than the other group. One

>> example is the 96boards dragonboard 820c. There is no dmips/MHz

>> difference between both groups, so no need to specify the values in

>> the DT. Unfortunately, without these defined, there is no scaling

>> capacity computation triggered, so we need to write

>> 'capacity-dmips-mhz' for each CPU with the same value in order to

>> force the scaled capacity computation.

>>

>> Fix this by setting a default capacity to SCHED_CAPACITY_SCALE, if no

>> 'capacity-dmips-mhz' is defined in the DT.

> 

> We aren't doing this anymore. You should rather explain that we just

> allocate raw_capacity now and rest is left for

> init_cpu_capacity_callback() to fix.


What about?

"In the case of asymmetric SoC with the same micro-architecture, we
have a group of CPUs with smaller OPPs than the other group. One
example is the 96boards dragonboard 820c. There is no dmips/MHz
difference between both groups, so no need to specify the values in
the DT. Unfortunately, without these defined, there is no scaling
capacity computation triggered, so we need to write
'capacity-dmips-mhz' for each CPU with the same value in order to
force the scaled capacity computation.

In order to fix this situation, allocate 'raw_capacity' so the pointer
is set and the init_cpu_capacity_callback() function can be called."

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Viresh Kumar Nov. 26, 2018, 6:01 a.m. UTC | #3
On 23-11-18, 11:32, Daniel Lezcano wrote:
> On 23/11/2018 11:04, Viresh Kumar wrote:

> > On 22-11-18, 13:36, Daniel Lezcano wrote:

> >> In the case of asymmetric SoC with the same micro-architecture, we

> >> have a group of CPUs with smaller OPPs than the other group. One

> >> example is the 96boards dragonboard 820c. There is no dmips/MHz

> >> difference between both groups, so no need to specify the values in

> >> the DT. Unfortunately, without these defined, there is no scaling

> >> capacity computation triggered, so we need to write

> >> 'capacity-dmips-mhz' for each CPU with the same value in order to

> >> force the scaled capacity computation.

> >>

> >> Fix this by setting a default capacity to SCHED_CAPACITY_SCALE, if no

> >> 'capacity-dmips-mhz' is defined in the DT.

> > 

> > We aren't doing this anymore. You should rather explain that we just

> > allocate raw_capacity now and rest is left for

> > init_cpu_capacity_callback() to fix.

> 

> What about?

> 

> "In the case of asymmetric SoC with the same micro-architecture, we

> have a group of CPUs with smaller OPPs than the other group. One

> example is the 96boards dragonboard 820c. There is no dmips/MHz

> difference between both groups, so no need to specify the values in

> the DT. Unfortunately, without these defined, there is no scaling

> capacity computation triggered, so we need to write

> 'capacity-dmips-mhz' for each CPU with the same value in order to

> force the scaled capacity computation.

> 

> In order to fix this situation, allocate 'raw_capacity' so the pointer

> is set and the init_cpu_capacity_callback() function can be called."


LGTM

-- 
viresh
diff mbox series

Patch

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index fd5325b..e0c5b60 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -243,9 +243,20 @@  static int __init register_cpufreq_notifier(void)
 	 * until we have the necessary code to parse the cpu capacity, so
 	 * skip registering cpufreq notifier.
 	 */
-	if (!acpi_disabled || !raw_capacity)
+	if (!acpi_disabled)
 		return -EINVAL;
 
+	if (!raw_capacity) {
+
+		pr_info("cpu_capacity: No capacity defined in DT, set default "
+		       "values to %ld\n", SCHED_CAPACITY_SCALE);
+
+		raw_capacity = kmalloc_array(num_possible_cpus(),
+					     sizeof(*raw_capacity), GFP_KERNEL);
+		if (!raw_capacity)
+			return -ENOMEM;
+	}
+
 	if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
 		pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
 		return -ENOMEM;