Re: [PATCH 12/14] cpufreq/amd-pstate: Cache a pointer to policy in cpudata

[Dhananjay Ugwekar]

On 2/7/2025 3:26 AM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@xxxxxxx>
> 
> In order to access the policy from a notification block it will
> need to be stored in cpudata.

This might break the cpufreq_policy ref counting right?, if we cache the pointer 
and use it independent of the ref counting framework.

> 
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
>  drivers/cpufreq/amd-pstate.c | 13 +++++++------
>  drivers/cpufreq/amd-pstate.h |  3 ++-
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 689de385d06da..5945b6c7f7e56 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -388,7 +388,7 @@ static int amd_pstate_set_energy_pref_index(struct cpufreq_policy *policy,
>  	else
>  		epp = epp_values[pref_index];
>  
> -	if (epp > 0 && cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
> +	if (epp > 0 && policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
>  		pr_debug("EPP cannot be set under performance policy\n");
>  		return -EBUSY;
>  	}
> @@ -689,7 +689,7 @@ static void amd_pstate_update_min_max_limit(struct cpufreq_policy *policy)
>  	perf.max_limit_perf = freq_to_perf(perf, cpudata->nominal_freq, policy->max);
>  	perf.min_limit_perf = freq_to_perf(perf, cpudata->nominal_freq, policy->min);
>  
> -	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> +	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
>  		perf.min_limit_perf = min(perf.nominal_perf, perf.max_limit_perf);
>  
>  	WRITE_ONCE(cpudata->perf, perf);
> @@ -1042,6 +1042,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>  		return -ENOMEM;
>  
>  	cpudata->cpu = policy->cpu;
> +	cpudata->policy = policy;
>  
>  	mutex_init(&cpudata->lock);
>  	guard(mutex)(&cpudata->lock);
> @@ -1224,9 +1225,8 @@ static ssize_t show_energy_performance_available_preferences(
>  {
>  	int i = 0;
>  	int offset = 0;
> -	struct amd_cpudata *cpudata = policy->driver_data;
>  
> -	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> +	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
>  		return sysfs_emit_at(buf, offset, "%s\n",
>  				energy_perf_strings[EPP_INDEX_PERFORMANCE]);
>  
> @@ -1543,6 +1543,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  		return -ENOMEM;
>  
>  	cpudata->cpu = policy->cpu;
> +	cpudata->policy = policy;
>  
>  	mutex_init(&cpudata->lock);
>  	guard(mutex)(&cpudata->lock);
> @@ -1632,7 +1633,7 @@ static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy)
>  
>  	amd_pstate_update_min_max_limit(policy);
>  
> -	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> +	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
>  		epp = 0;
>  	else
>  		epp = READ_ONCE(cpudata->epp_cached);
> @@ -1651,7 +1652,7 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>  	if (!policy->cpuinfo.max_freq)
>  		return -ENODEV;
>  
> -	cpudata->policy = policy->policy;
> +	cpudata->policy = policy;
>  
>  	ret = amd_pstate_epp_update_limit(policy);
>  	if (ret)
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index 7501d30db9953..16ce631a6c3d5 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -97,9 +97,10 @@ struct amd_cpudata {
>  
>  	struct mutex	lock;
>  
> +	struct cpufreq_policy *policy;
> +
>  	/* EPP feature related attributes*/
>  	u8	epp_cached;
> -	u32	policy;
>  	bool	suspended;
>  	u8	epp_default;
>  };