Re: [PATCH v3 2/2] cpufreq: intel_pstate: Set asymmetric CPU capacity on hybrid systems

[Shrikanth Hegde <sshegde@linux.ibm.com>]

On 8/28/24 17:18, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
[...]
>   
> +static void hybrid_update_capacity(struct cpudata *cpu)
> +{
> +	unsigned int max_cap_perf;
> +
> +	mutex_lock(&hybrid_capacity_lock);
> +
> +	if (!hybrid_max_perf_cpu)
> +		goto unlock;
> +
> +	/*
> +	 * The maximum performance of the CPU may have changed, but assume
> +	 * that the performance of the other CPUs has not changed.
> +	 */
> +	max_cap_perf = hybrid_max_perf_cpu->capacity_perf;
> +
> +	intel_pstate_get_hwp_cap(cpu);
> +
> +	hybrid_get_capacity_perf(cpu);
> +	/* Should hybrid_max_perf_cpu be replaced by this CPU? */
> +	if (cpu->capacity_perf > max_cap_perf) {
> +		hybrid_max_perf_cpu = cpu;
> +		hybrid_set_capacity_of_cpus();
> +		goto unlock;
> +	}
> +
> +	/* If this CPU is hybrid_max_perf_cpu, should it be replaced? */
> +	if (cpu == hybrid_max_perf_cpu && cpu->capacity_perf < max_cap_perf) {
> +		hybrid_update_cpu_scaling();
> +		goto unlock;
> +	}

I assume this CPU capacity is based on freq. It doesnt change based on 
irq, any upper scheduler classes such dl, rt right?

can capacity_perf change slightly or it can change such that it always 
changes to next possible level? The reason, if it can change slightly, 
but cpu is still hybrid_max_perf_cpu, it would end up accessing all the 
percpu structures and change it, that would be costly on larger systems.


> +
> +	hybrid_set_cpu_capacity(cpu);
> +
> +unlock:
> +	mutex_unlock(&hybrid_capacity_lock);
> +}
> +
>   static void intel_pstate_hwp_set(unsigned int cpu)
>   {
>   	struct cpudata *cpu_data = all_cpu_data[cpu];
> @@ -1070,6 +1240,22 @@ static void intel_pstate_hwp_offline(str
>   		value |= HWP_ENERGY_PERF_PREFERENCE(HWP_EPP_POWERSAVE);
>   
>   	wrmsrl_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
> +
> +	mutex_lock(&hybrid_capacity_lock);
> +
> +	if (!hybrid_max_perf_cpu) {
> +		mutex_unlock(&hybrid_capacity_lock);
> +
> +		return;
> +	}
> +
> +	if (hybrid_max_perf_cpu == cpu)
> +		hybrid_update_cpu_scaling();
> +
> +	mutex_unlock(&hybrid_capacity_lock);
> +
> +	/* Reset the capacity of the CPU going offline to the initial value. */
> +	hybrid_clear_cpu_capacity(cpu->cpu);
>