On 12/20/2017 12:20 PM, Viresh Kumar wrote:

On 20-12-17, 12:12, Abhishek Goel wrote:

quoted

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 ?

quoted

+
  /*
   * 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) ?

quoted

  	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.

quoted

+	}
+
+	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 ?

quoted

+
  	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 ?

quoted

+			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;
+

:(

quoted

  	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.

quoted

+		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