cpufreq: exynos: Broadcast frequency change notifications for all cores

PATCH New Tomasz Figa 4 participants 13 messages Nov 13, 2012 – Feb 1, 2013

Subsystems (2)

Arm Samsung Exynos CPU frequency scaling framework

lore

Share & tools

lore

Tomasz Figa

author

2012-11-13 09:26 UTC #0 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

On Exynos SoCs all cores share the same frequency setting, so changing frequency of one core will affect rest of cores.

This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/cpufreq/exynos-cpufreq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/cpufreq/exynos-cpufreq.c +5 -2

index af2d81e..c0d54a8 100644
@@ -100,7 +100,8 @@ static int exynos_target(struct cpufreq_policy *policy,
 	}
 	arm_volt = volt_table[index];
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	/* When the new frequency is higher than current frequency */
 	if ((freqs.new > freqs.old) && !safe_arm_volt) {

@@ -115,7 +116,8 @@ static int exynos_target(struct cpufreq_policy *policy,
 	if (freqs.new != freqs.old)
 		exynos_info->set_freq(old_index, index);
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 
 	/* When the new frequency is lower than current frequency */
 	if ((freqs.new < freqs.old) ||

@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		cpumask_copy(policy->related_cpus, cpu_possible_mask);
 		cpumask_copy(policy->cpus, cpu_online_mask);
 	} else {
+		policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
 		cpumask_setall(policy->cpus);
 	}

Signature · 1 line

1.8.0

Tomasz Figa

author

2012-11-21 13:52 UTC #1 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 38 lines · On Tuesday 13 of November 2012 10:26:12 Tomasz Figa wrote:

On Exynos SoCs all cores share the same frequency setting, so changing frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.
Signed-off-by Tomasz Figa <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/cpufreq/exynos-cpufreq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/cpufreq/exynos-cpufreq.c index af2d81e..c0d54a8 100644
drivers/cpufreq/exynos-cpufreq.c +4 -2
@@ -100,7 +100,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, }
 	arm_volt = volt_table[index];
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	/* When the new frequency is higher than current frequency */
 	if ((freqs.new > freqs.old) && !safe_arm_volt) {
@@ -115,7 +116,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, if (freqs.new != freqs.old)
 		exynos_info->set_freq(old_index, index);
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 
 	/* When the new frequency is lower than current frequency */
 	if ((freqs.new < freqs.old) ||

drivers/cpufreq/exynos-cpufreq.c static int exynos_cpufreq_cpu_init(struct · line 243

Quoted 7 lines

drivers/cpufreq/exynos-cpufreq.c +1 -0

@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct
 cpufreq_policy *policy) cpumask_copy(policy->related_cpus,
 cpu_possible_mask);
 		cpumask_copy(policy->cpus, cpu_online_mask);
 	} else {
+		policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
 		cpumask_setall(policy->cpus);
 	}

Ping.

Best regards,

Signature · 3 lines

Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

Rafael J. Wysocki

2012-11-21 20:43 UTC #2 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 36 lines · Tomasz Figa

Quoted 35 lines · On Tuesday 13 of November 2012 10:26:12 Tomasz Figa wrote:
On Exynos SoCs all cores share the same frequency setting, so changing frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.
Signed-off-by Tomasz Figa <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/cpufreq/exynos-cpufreq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/cpufreq/exynos-cpufreq.c index af2d81e..c0d54a8 100644
drivers/cpufreq/exynos-cpufreq.c +4 -2
@@ -100,7 +100,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, }
 	arm_volt = volt_table[index];
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	/* When the new frequency is higher than current frequency */
 	if ((freqs.new > freqs.old) && !safe_arm_volt) {
@@ -115,7 +116,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, if (freqs.new != freqs.old)
 		exynos_info->set_freq(old_index, index);
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 

Quoted 12 lines

drivers/cpufreq/exynos-cpufreq.c static int exynos_cpufreq_cpu_init(struct · line 243

Quoted 10 lines

drivers/cpufreq/exynos-cpufreq.c +1 -0

 	/* When the new frequency is lower than current frequency */
 	if ((freqs.new < freqs.old) ||

@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct
 cpufreq_policy *policy) cpumask_copy(policy->related_cpus,
 cpu_possible_mask);
 		cpumask_copy(policy->cpus, cpu_online_mask);
 	} else {
+		policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
 		cpumask_setall(policy->cpus);
 	}

Ping.

Am I supposed to handle this?

Rafael

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Tomasz Figa

2012-11-21 21:23 UTC #3 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Hi Rafael,

Quoted 55 lines · On Wednesday 21 of November 2012 21:47:42 Rafael J. Wysocki wrote:

Quoted 54 lines · Tomasz Figa
Quoted 53 lines · On Tuesday 13 of November 2012 10:26:12 Tomasz Figa wrote:
On Exynos SoCs all cores share the same frequency setting, so changing
frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.
Signed-off-by Tomasz Figa <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/cpufreq/exynos-cpufreq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/cpufreq/exynos-cpufreq.c index af2d81e..c0d54a8 100644
drivers/cpufreq/exynos-cpufreq.c +2 -1
@@ -100,7 +100,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, }
 
 	arm_volt = volt_table[index];
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	/* When the new frequency is higher than current frequency */
drivers/cpufreq/exynos-cpufreq.c +2 -1
 	if ((freqs.new > freqs.old) && !safe_arm_volt) {
@@ -115,7 +116,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, if (freqs.new != freqs.old)
 
 		exynos_info->set_freq(old_index, index);
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 
 	/* When the new frequency is lower than current frequency */
drivers/cpufreq/exynos-cpufreq.c +1 -0
 	if ((freqs.new < freqs.old) ||
@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct
 cpufreq_policy *policy) cpumask_copy(policy->related_cpus,
 cpu_possible_mask);
 
 		cpumask_copy(policy->cpus, cpu_online_mask);
 
 	} else {
 
+		policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;

Quoted 7 lines

Quoted 5 lines
drivers/cpufreq/exynos-cpufreq.c static int exynos_cpufreq_cpu_init(struct cpufreq_policy *policy)
Quoted 3 lines
drivers/cpufreq/exynos-cpufreq.c +0 -0
 		cpumask_setall(policy->cpus);
 
 	}
Ping.
Am I supposed to handle this?

I guess. Would be really nice if you could apply this patch for 3.8.

Best regards,
Tomasz Figa

Rafael J. Wysocki

Applied

2012-11-21 23:41 UTC #4 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 60 lines · Tomasz Figa

Hi Rafael,
Quoted 57 lines · On Wednesday 21 of November 2012 21:47:42 Rafael J. Wysocki wrote:
Quoted 56 lines · Tomasz Figa
Quoted 55 lines · On Tuesday 13 of November 2012 10:26:12 Tomasz Figa wrote:
On Exynos SoCs all cores share the same frequency setting, so changing
frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.
Signed-off-by Tomasz Figa <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/cpufreq/exynos-cpufreq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/cpufreq/exynos-cpufreq.c index af2d81e..c0d54a8 100644
drivers/cpufreq/exynos-cpufreq.c +2 -1
@@ -100,7 +100,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, }
 
 	arm_volt = volt_table[index];
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	/* When the new frequency is higher than current frequency */
drivers/cpufreq/exynos-cpufreq.c +2 -1
 	if ((freqs.new > freqs.old) && !safe_arm_volt) {
@@ -115,7 +116,8 @@ static int exynos_target(struct cpufreq_policy
 *policy, if (freqs.new != freqs.old)
 
 		exynos_info->set_freq(old_index, index);
 
-	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	for_each_cpu(freqs.cpu, policy->cpus)
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 
 	/* When the new frequency is lower than current frequency */
drivers/cpufreq/exynos-cpufreq.c +1 -0
 	if ((freqs.new < freqs.old) ||
@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct
 cpufreq_policy *policy) cpumask_copy(policy->related_cpus,
 cpu_possible_mask);
 
 		cpumask_copy(policy->cpus, cpu_online_mask);
 
 	} else {
 
+		policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
 
 		cpumask_setall(policy->cpus);

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
}
Ping.
Am I supposed to handle this?
I guess. Would be really nice if you could apply this patch for 3.8.

Applied to linux-pm.git/linux-next as v3.8 material.

Thanks,
Rafael

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Viresh Kumar

maintainer

2013-01-31 04:20 UTC #5 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

In reply to Tomasz Figa

Quoted 10 lines · Tomasz Figa

On Exynos SoCs all cores share the same frequency setting, so changing frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.
        /* When the new frequency is lower than current frequency */
        if ((freqs.new < freqs.old) ||

drivers/cpufreq/exynos-cpufreq.c static int exynos_cpufreq_cpu_init(struct cpufreq_policy *policy) · line 241

Quoted 5 lines

drivers/cpufreq/exynos-cpufreq.c +1 -0

@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct cpufreq_policy *policy)
                cpumask_copy(policy->related_cpus, cpu_possible_mask);
                cpumask_copy(policy->cpus, cpu_online_mask);
        } else {
+               policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
                cpumask_setall(policy->cpus);

I couldn't understand the use of this change. Can you please explain ?

Tomasz Figa

author

2013-01-31 14:45 UTC #6 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Hi Viresh,

Quoted 16 lines · On Thursday 31 of January 2013 09:50:37 Viresh Kumar wrote:

Quoted 15 lines · Tomasz Figa
On Exynos SoCs all cores share the same frequency setting, so changing frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core about this behavior and broadcast frequency change notifications for all cores.
        /* When the new frequency is lower than current frequency */
        if ((freqs.new < freqs.old) ||
(quoted diff) +0 -0
@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct
cpufreq_policy *policy)>
                cpumask_copy(policy->related_cpus, cpu_possible_mask);
                cpumask_copy(policy->cpus, cpu_online_mask);

Quoted 7 lines

Quoted 5 lines
        } else {
+ policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
                cpumask_setall(policy->cpus);
I couldn't understand the use of this change. Can you please explain ?

All cores on Exynos SoCs share the same clock source, which means that they all always run at the same frequency. So setting frequency on one core sets the frequency on all cores and so it doesn't matter on which core the frequency setting operation is done.

As you can see in include/linux/cpufreq.h:

#define CPUFREQ_SHARED_TYPE_ANY (3) /* Freq can be set from any dependent CPU*/

which basically tells (together with rest of the patch) the cpufreq subsystem about what I wrote earlier.

Best regards,

Signature · 3 lines

Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

Viresh Kumar

maintainer

2013-01-31 14:52 UTC #7 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 9 lines · Tomasz Figa

Quoted 3 lines · On Thursday 31 of January 2013 09:50:37 Viresh Kumar wrote:
Quoted 1 line
+ policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
I couldn't understand the use of this change. Can you please explain ?
All cores on Exynos SoCs share the same clock source, which means that they all always run at the same frequency. So setting frequency on one core sets the frequency on all cores and so it doesn't matter on which core the frequency setting operation is done.

Great, that's like most other platforms and for this you just need to set mask of all the possible cpus (online or offline) into policy->cpus

Quoted 7 lines

As you can see in include/linux/cpufreq.h:
#define CPUFREQ_SHARED_TYPE_ANY (3) /* Freq can be set from any dependent CPU*/
which basically tells (together with rest of the patch) the cpufreq subsystem about what I wrote earlier.

I don't think so. Because i wasn't using it on my system and it was working well.
Then i grepped it in kernel source

viresh at blr-linut-001:$ git grep shared_type drivers/cpufreq/ drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type != CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type = perf->shared_type; drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL ||
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type ==
CPUFREQ_SHARED_TYPE_ANY) {
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_HW;
drivers/cpufreq/cpufreq-cpu0.c: policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
drivers/cpufreq/db8500-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/omap-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ANY;
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY)

I couldn't see any core file using this stuff other than acpi-cpufreq.c, which is for x86 only systems if i am not wrong.

@Rafael: You view on this.. ? Actually i believe it was just used by many ARM drivers without knowing the real use of it and new drivers have just followed it.

I can get a patch to fix it at all places if you agree.

BTW, grep in full kernel source gave, which is again acpi only:

viresh at blr-linut-001:$ git grep shared_type

arch/arm/mach-tegra/cpu-tegra.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL; drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_HW; drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ANY; drivers/acpi/processor_perflib.c:
match_pr->performance->shared_type = drivers/acpi/processor_perflib.c:
pr->performance->shared_type;
drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL; drivers/acpi/processor_throttling.c:
pthrottling->shared_type = DOMAIN_COORD_TYPE_SW_ALL;
drivers/acpi/processor_throttling.c: pthrottling->shared_type =
pdomain->coord_type;
drivers/acpi/processor_throttling.c:
pthrottling->shared_type = DOMAIN_COORD_TYPE_SW_ALL;
drivers/acpi/processor_throttling.c: if (p_throttling->shared_type
== DOMAIN_COORD_TYPE_SW_ANY) { drivers/acpi/processor_throttling.c:
pthrottling->shared_type = DOMAIN_COORD_TYPE_SW_ALL; drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type != CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type = perf->shared_type; drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL ||
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type ==
CPUFREQ_SHARED_TYPE_ANY) {
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_HW;
drivers/cpufreq/cpufreq-cpu0.c: policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
drivers/cpufreq/db8500-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/omap-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ANY;
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) drivers/net/wireless/rtlwifi/rtl8192ce/hw.c:
rtlpcipriv->bt_coexist.bt_radio_shared_type = drivers/net/wireless/rtlwifi/rtl8723ae/hw.c:
pcipriv->bt_coexist.bt_radio_shared_type = drivers/net/wireless/rtlwifi/rtl8723ae/hw.c:
pcipriv->bt_coexist.bt_radio_shared_type);
drivers/net/wireless/rtlwifi/wifi.h: u8 bt_radio_shared_type;
drivers/xen/xen-acpi-processor.c: dst->shared_type =
_pr->performance->shared_type;
drivers/xen/xen-acpi-processor.c:
dst->shared_type = CPUFREQ_SHARED_TYPE_ALL; drivers/xen/xen-acpi-processor.c:
dst->shared_type = CPUFREQ_SHARED_TYPE_HW; drivers/xen/xen-acpi-processor.c:
dst->shared_type = CPUFREQ_SHARED_TYPE_ANY;

include/acpi/processor.h:       unsigned int shared_type;
include/acpi/processor.h:       unsigned int shared_type;
include/linux/cpufreq.h:        unsigned int            shared_type;

/* ANY or ALL affected CPUs
include/xen/interface/platform.h: uint32_t shared_type; /*
coordination type of this processor */
tools/perf/util/dwarf-aux.c: tag == DW_TAG_shared_type);

Tomasz Figa

author

2013-01-31 15:04 UTC #8 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 122 lines · On Thursday 31 of January 2013 20:22:50 Viresh Kumar wrote:

Quoted 10 lines · Tomasz Figa
Quoted 4 lines · On Thursday 31 of January 2013 09:50:37 Viresh Kumar wrote:
Quoted 1 line
+ policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
I couldn't understand the use of this change. Can you please explain ?
All cores on Exynos SoCs share the same clock source, which means that they all always run at the same frequency. So setting frequency on one core sets the frequency on all cores and so it doesn't matter on which core the frequency setting operation is done.
Great, that's like most other platforms and for this you just need to set mask of all the possible cpus (online or offline) into policy->cpus
Quoted 7 lines
As you can see in include/linux/cpufreq.h:
#define CPUFREQ_SHARED_TYPE_ANY (3) /* Freq can be set from any dependent CPU*/
which basically tells (together with rest of the patch) the cpufreq subsystem about what I wrote earlier.
I don't think so. Because i wasn't using it on my system and it was working well.
Then i grepped it in kernel source
viresh at blr-linut-001:$ git grep shared_type drivers/cpufreq/ drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type != CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type = perf->shared_type; drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL ||
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type ==
CPUFREQ_SHARED_TYPE_ANY) {
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_HW;
drivers/cpufreq/cpufreq-cpu0.c: policy->shared_type = CPUFREQ_SHARED_TYPE_ANY; drivers/cpufreq/db8500-cpufreq.c:
policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/omap-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ANY;
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY)
I couldn't see any core file using this stuff other than acpi-cpufreq.c, which is for x86 only systems if i am not wrong.
@Rafael: You view on this.. ? Actually i believe it was just used by many ARM drivers without knowing the real use of it and new drivers have just followed it.
I can get a patch to fix it at all places if you agree.
BTW, grep in full kernel source gave, which is again acpi only:
viresh at blr-linut-001:$ git grep shared_type
arch/arm/mach-tegra/cpu-tegra.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL; drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_HW; drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ANY; drivers/acpi/processor_perflib.c:
match_pr->performance->shared_type = drivers/acpi/processor_perflib.c:
pr->performance->shared_type;
drivers/acpi/processor_perflib.c:
pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL; drivers/acpi/processor_throttling.c:
pthrottling->shared_type = DOMAIN_COORD_TYPE_SW_ALL;
drivers/acpi/processor_throttling.c: pthrottling->shared_type =
pdomain->coord_type;
drivers/acpi/processor_throttling.c:
pthrottling->shared_type = DOMAIN_COORD_TYPE_SW_ALL;
drivers/acpi/processor_throttling.c: if (p_throttling->shared_type
== DOMAIN_COORD_TYPE_SW_ANY) { drivers/acpi/processor_throttling.c:
pthrottling->shared_type = DOMAIN_COORD_TYPE_SW_ALL; drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type != CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type = perf->shared_type; drivers/cpufreq/acpi-cpufreq.c: if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL ||
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type ==
CPUFREQ_SHARED_TYPE_ANY) {
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/acpi-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_HW;
drivers/cpufreq/cpufreq-cpu0.c: policy->shared_type = CPUFREQ_SHARED_TYPE_ANY; drivers/cpufreq/db8500-cpufreq.c:
policy->shared_type =
CPUFREQ_SHARED_TYPE_ALL;
drivers/cpufreq/omap-cpufreq.c: policy->shared_type =
CPUFREQ_SHARED_TYPE_ANY;
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY)
drivers/cpufreq/speedstep-centrino.c: if
(policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) drivers/net/wireless/rtlwifi/rtl8192ce/hw.c:
rtlpcipriv->bt_coexist.bt_radio_shared_type = drivers/net/wireless/rtlwifi/rtl8723ae/hw.c:
pcipriv->bt_coexist.bt_radio_shared_type = drivers/net/wireless/rtlwifi/rtl8723ae/hw.c:
pcipriv->bt_coexist.bt_radio_shared_type);
drivers/net/wireless/rtlwifi/wifi.h: u8 bt_radio_shared_type;
drivers/xen/xen-acpi-processor.c: dst->shared_type =
_pr->performance->shared_type; drivers/xen/xen-acpi-processor.c:
dst->shared_type = CPUFREQ_SHARED_TYPE_ALL; drivers/xen/xen-acpi-processor.c:
dst->shared_type = CPUFREQ_SHARED_TYPE_HW; drivers/xen/xen-acpi-processor.c:
dst->shared_type = CPUFREQ_SHARED_TYPE_ANY;

Quoted 7 lines

include/acpi/processor.h:       unsigned int shared_type;
include/acpi/processor.h:       unsigned int shared_type;
include/linux/cpufreq.h:        unsigned int            shared_type;
/* ANY or ALL affected CPUs
include/xen/interface/platform.h: uint32_t shared_type; /*
coordination type of this processor */
tools/perf/util/dwarf-aux.c: tag == DW_TAG_shared_type);

Well, the fact that it isn't used at the moment doesn't mean that it shouldn't be set correctly. The field is present in the structure and has a set of defined values - one of which should be selected. For example, I can imagine some governor taking this information into account.

Best regards,

Signature · 3 lines

Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

Viresh Kumar

maintainer

2013-01-31 15:16 UTC #9 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 4 lines · Tomasz Figa

Well, the fact that it isn't used at the moment doesn't mean that it shouldn't be set correctly. The field is present in the structure and has a set of defined values - one of which should be selected. For example, I can imagine some governor taking this information into account.

Governors already take this information but from a different variable:
policy->cpus.

Look at the patch which added it:

commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36

Author: Venkatesh Pallipadi <[email protected]>
Date:   Wed Dec 14 15:05:00 2005 -0500

P-state software coordination for ACPI core

http://bugzilla.kernel.org/show_bug.cgi?id=5737

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/processor_perflib.c | 228
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/processor.h         |  27 ++++++++++++-
 include/linux/cpufreq.h          |   4 ++
 3 files changed, 258 insertions(+), 1 deletion(-)

It was clearly for ACPI, but was probably named badly and we people got confused that it is for our use.

--
viresh

Tomasz Figa

author

2013-01-31 16:07 UTC #10 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 30 lines · On Thursday 31 of January 2013 20:46:10 Viresh Kumar wrote:

Quoted 5 lines · Tomasz Figa
Well, the fact that it isn't used at the moment doesn't mean that it shouldn't be set correctly. The field is present in the structure and has a set of defined values - one of which should be selected. For example, I can imagine some governor taking this information into account.
Governors already take this information but from a different variable:
policy->cpus.
Look at the patch which added it:
commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36
Author: Venkatesh Pallipadi <[email protected]>
Date:   Wed Dec 14 15:05:00 2005 -0500
P-state software coordination for ACPI core
http://bugzilla.kernel.org/show_bug.cgi?id=5737
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Len Brown <[email protected]>
---
 drivers/acpi/processor_perflib.c | 228
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++++++++++++++++++++++++++++++ include/acpi/processor.h         |
 27 ++++++++++++-
 include/linux/cpufreq.h          |   4 ++
 3 files changed, 258 insertions(+), 1 deletion(-)

Quoted 2 lines

It was clearly for ACPI, but was probably named badly and we people got confused that it is for our use.

Hmm. Now as I think of it, there might be another confusing aspect:

Could you explain what "sw coordination" and "any coordination" mean? I mean specifically cpufreq_policy.cpus and cpufreq_policy.related_cpus masks.

Best regards,

Signature · 3 lines

Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

Viresh Kumar

maintainer

2013-01-31 16:12 UTC #11 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 5 lines · Tomasz Figa

Hmm. Now as I think of it, there might be another confusing aspect:
Could you explain what "sw coordination" and "any coordination" mean? I mean specifically cpufreq_policy.cpus and cpufreq_policy.related_cpus masks.

Check this out:

https://lkml.org/lkml/2013/1/30/587

Viresh Kumar

maintainer

2013-02-01 03:47 UTC #12 lore raw

PATCH 1/1 cpufreq: exynos: Broadcast frequency change notifications for all cores

Quoted 10 lines · Viresh Kumar

Quoted 5 lines · Tomasz Figa
Hmm. Now as I think of it, there might be another confusing aspect:
Could you explain what "sw coordination" and "any coordination" mean? I mean specifically cpufreq_policy.cpus and cpufreq_policy.related_cpus masks.
Check this out:
https://lkml.org/lkml/2013/1/30/587

You would really like to look and ack this one too:

https://lkml.org/lkml/2013/1/30/592