On 12/20/2017 12:20 PM, Viresh Kumar wrote:
On 20-12-17, 12:12, Abhishek Goel wrote:
diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index b6d7c4c..fd642bc 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -37,6 +37,7 @@
  #include <asm/smp.h> /* Required for cpu_sibling_mask() in UP configs */
  #include <asm/opal.h>
  #include <linux/timer.h>
+#include <linux/hashtable.h>
#define POWERNV_MAX_PSTATES 256
  #define PMSR_PSAFE_ENABLE     (1UL << 30)
@@ -130,6 +131,9 @@ static struct chip {
  static int nr_chips;
  static DEFINE_PER_CPU(struct chip *, chip_info);
+static u32 freq_domain_indicator;
+static u32 flag;
I wouldn't name it as flag, its unreadable. Maybe its better to name
it based on the quirk you are trying to workaround with ?

+
  /*
   * Note:
   * The set of pstates consists of contiguous integers.
@@ -194,6 +198,38 @@ static inline void reset_gpstates(struct cpufreq_policy 
*policy)
        gpstates->last_gpstate_idx = 0;
  }
+#define SIZE NR_CPUS
+#define ORDER_FREQ_MAP ilog2(SIZE)
+
+static DEFINE_HASHTABLE(freq_domain_map, ORDER_FREQ_MAP);
+
+struct hashmap {
+       cpumask_t mask;
+       int chip_id;
+       u32 pir_key;
+       struct hlist_node hash_node;
+};
+
+static void insert(u32 key, int cpu)
+{
+       struct hashmap *data;
+
+       hash_for_each_possible(freq_domain_map, data, hash_node, key%SIZE) {
+               if (data->chip_id == cpu_to_chip_id(cpu) &&
+                       data->pir_key == key) {
+                       cpumask_set_cpu(cpu, &data->mask);
+                       return;
+               }
+       }
+
+       data = kzalloc(sizeof(*data), GFP_KERNEL);
+       hash_add(freq_domain_map, &data->hash_node, key%SIZE);
+       cpumask_set_cpu(cpu, &data->mask);
+       data->chip_id = cpu_to_chip_id(cpu);
+       data->pir_key = key;
+
+}
+
  /*
   * Initialize the freq table based on data obtained
   * from the firmware passed via device-tree
@@ -206,7 +242,9 @@ static int init_powernv_pstates(void)
        u32 len_ids, len_freqs;
        u32 pstate_min, pstate_max, pstate_nominal;
        u32 pstate_turbo, pstate_ultra_turbo;
+       u32 key;
+ flag = 0;
Isn't flag already 0 (global-uninitialized) ?

        power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
        if (!power_mgt) {
                pr_warn("power-mgt node not found\n");
@@ -229,6 +267,17 @@ static int init_powernv_pstates(void)
                return -ENODEV;
        }
+ if (of_device_is_compatible(power_mgt, "freq-domain-v1") &&
+               of_property_read_u32(power_mgt, "ibm,freq-domain-indicator",
+                                &freq_domain_indicator)) {
+               pr_warn("ibm,freq-domain-indicator not found\n");
+               freq_domain_indicator = 0;
You shouldn't be required to set it to 0 here.

+       }
+
+       if (of_device_is_compatible(power_mgt, "P9-occ-quirk")) {
+               flag = 1;
+       }
Remove {} and a better name like p9_occ_quirk would be good for flag.
Also making it a bool may be better ?

+
        if (of_property_read_u32(power_mgt, "ibm,pstate-ultra-turbo",
                                 &pstate_ultra_turbo)) {
                powernv_pstate_info.wof_enabled = false;
@@ -249,6 +298,7 @@ static int init_powernv_pstates(void)
  next:
        pr_info("cpufreq pstate min %d nominal %d max %d\n", pstate_min,
                pstate_nominal, pstate_max);
+       pr_info("frequency domain indicator %d", freq_domain_indicator);
        pr_info("Workload Optimized Frequency is %s in the platform\n",
                (powernv_pstate_info.wof_enabled) ? "enabled" : "disabled");
@@ -276,6 +326,15 @@ static int init_powernv_pstates(void)
                return -ENODEV;
        }
+ if (freq_domain_indicator) {
+               hash_init(freq_domain_map);
+               for_each_possible_cpu(i) {
+                       key = ((u32) get_hard_smp_processor_id(i) &
+                               freq_domain_indicator);
Maybe break it like:

                        key = (u32) get_hard_smp_processor_id(i);
                         key &= freq_domain_indicator;

to make it easily readable ?

+                       insert(key, i);
+               }
+       }
+
        powernv_pstate_info.nr_pstates = nr_pstates;
        pr_debug("NR PStates %d\n", nr_pstates);
        for (i = 0; i < nr_pstates; i++) {
@@ -693,6 +752,7 @@ static int powernv_cpufreq_target_index(struct 
cpufreq_policy *policy,
  {
        struct powernv_smp_call_data freq_data;
        unsigned int cur_msec, gpstate_idx;
+
:(

        struct global_pstate_info *gpstates = policy->driver_data;
if (unlikely(rebooting) && new_index != get_nominal_index())
@@ -760,25 +820,55 @@ static int powernv_cpufreq_target_index(struct 
cpufreq_policy *policy,
spin_unlock(&gpstates->gpstate_lock); - /*
-        * Use smp_call_function to send IPI and execute the
-        * mtspr on target CPU.  We could do that without IPI
-        * if current CPU is within policy->cpus (core)
-        */
-       smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1);
+       if (flag) {
Maybe add a comment over this on why you need to do things differently
here, as it isn't obvious.

+               cpumask_t temp;
+               u32 cpu;
+
+              /*
+               * Use smp_call_function to send IPI and execute the mtspr
+               * on CPU. This needs to be done on every core of the policy.
+               */
+               cpumask_copy(&temp, policy->cpus);
+               while (!cpumask_empty(&temp)) {
+                       cpu = cpumask_first(&temp);
+                       smp_call_function_any(cpu_sibling_mask(cpu),
+                                       set_pstate, &freq_data, 1);
+                       cpumask_andnot(&temp, &temp, cpu_sibling_mask(cpu));
+               }
+       } else {
+               smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1);
+       }
+
        return 0;
  }
static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
  {
-       int base, i, ret;
+       int ret;
        struct kernfs_node *kn;
        struct global_pstate_info *gpstates;
- base = cpu_first_thread_sibling(policy->cpu);
+       if (!freq_domain_indicator) {
+               int base, i;
- for (i = 0; i < threads_per_core; i++)
-               cpumask_set_cpu(base + i, policy->cpus);
+               base = cpu_first_thread_sibling(policy->cpu);
+               for (i = 0; i < threads_per_core; i++)
+                       cpumask_set_cpu(base + i, policy->cpus);
+       } else {
+               u32 key;
+               struct hashmap *data;
+
+               key = ((u32) get_hard_smp_processor_id(policy->cpu) &
+                               freq_domain_indicator);
+               hash_for_each_possible(freq_domain_map, data, hash_node,
+                                                                key%SIZE) {
+                       if (data->chip_id == cpu_to_chip_id(policy->cpu) &&
+                               data->pir_key == key) {
+                               cpumask_copy(policy->cpus, &data->mask);
+                               break;
+                       }
+               }
+       }
kn = kernfs_find_and_get(policy->kobj.sd, throttle_attr_grp.name);
        if (!kn) {
--
2.9.3
Have posted the next version with the changes made as suggested. Also the skiboot patch required for the device tree changes made is posted here : http://patchwork.ozlabs.org/patch/862256/

-Abhishek

Reply via email to