Re: [PATCH v2 4/9] cpufreq/amd-pstate: Add dynamic EPP as an "energy_performance_preference" mode

[Mario Limonciello <mario.limonciello@amd.com>]

On 7/23/26 15:12, K Prateek Nayak wrote:
> Conver the global "dynamic_epp" toggle into a per-CPU
Convert

> "energy_performance_preference" mode "dynamic" that allows toggling the
> functionality of "dynamic_epp" at a per-CPU level.
> 
> Instead of being a system-wide toggle, users can opt into the
> functionality of dynamic EPP on a per-CPU basis by switching to the
> powersave governor and selecting the "dynamic" mode from the available
> performance preferences.
> 
> Unlike the previous implementation that had to check for driver mode
> before toggling on the functionality, block writes to certain sysfs
> files, potentially disallow policy change, etc. the per-CPU toggle fits
> naturally into the intended design and provides more granular control to
> the user.
> 
> The dynamic_epp file is now redundant as the option to toggle it on is
> controlled via energy_performance_preference, and the dynamic_epp file
> will be removed in the subsequent commit.
> 
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> Changelog rfc v1..v2:
> 
> o Removed "dynamic_epp" global control related changes since it will be
>    removed in the subsequent commit.
> ---
>   drivers/cpufreq/amd-pstate.c | 57 +++++++++++++++++++++++++++++++-----
>   1 file changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 7d1feed0e11e..fc90e4108c22 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -106,6 +106,7 @@ static struct quirk_entry *quirks;
>    *	3		balance_power
>    *	4		power
>    *	5		custom (for raw EPP values)
> + *	6		dynamic (platform profile driven selection)
>    */
>   enum energy_perf_value_index {
>   	EPP_INDEX_DEFAULT = 0,
> @@ -114,6 +115,7 @@ enum energy_perf_value_index {
>   	EPP_INDEX_BALANCE_POWERSAVE,
>   	EPP_INDEX_POWERSAVE,
>   	EPP_INDEX_CUSTOM,
> +	EPP_INDEX_DYNAMIC,
>   	EPP_INDEX_MAX,
>   };
>   
> @@ -124,6 +126,7 @@ static const char * const energy_perf_strings[] = {
>   	[EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
>   	[EPP_INDEX_POWERSAVE] = "power",
>   	[EPP_INDEX_CUSTOM] = "custom",
> +	[EPP_INDEX_DYNAMIC] = "dynamic",
>   };
>   static_assert(ARRAY_SIZE(energy_perf_strings) == EPP_INDEX_MAX);
>   
> @@ -134,7 +137,7 @@ static unsigned int epp_values[] = {
>   	[EPP_INDEX_BALANCE_POWERSAVE] = AMD_CPPC_EPP_BALANCE_POWERSAVE,
>   	[EPP_INDEX_POWERSAVE] = AMD_CPPC_EPP_POWERSAVE,
>   };
> -static_assert(ARRAY_SIZE(epp_values) == EPP_INDEX_MAX - 1);
> +static_assert(ARRAY_SIZE(epp_values) == EPP_INDEX_MAX - 2);
>   
>   typedef int (*cppc_mode_transition_fn)(int);
>   
> @@ -1274,6 +1277,7 @@ EXPORT_SYMBOL_GPL(amd_pstate_clear_dynamic_epp);
>   static int amd_pstate_set_dynamic_epp(struct cpufreq_policy *policy)
>   {
>   	struct amd_cpudata *cpudata = policy->driver_data;
> +	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
>   	int ret;
>   	u8 epp;
>   
> @@ -1314,6 +1318,9 @@ static int amd_pstate_set_dynamic_epp(struct cpufreq_policy *policy)
>   cleanup:
>   	amd_pstate_clear_dynamic_epp(policy);
>   
> +	epp = FIELD_GET(AMD_CPPC_EPP_PERF_MASK, prev);
> +	/* Restore previous EPP if toggling Dynamic EPP failed. */
> +	amd_pstate_set_epp(policy, epp);
>   	return ret;
>   }
>   
> @@ -1384,7 +1391,7 @@ static ssize_t show_amd_pstate_hw_prefcore(struct cpufreq_policy *policy,
>   static ssize_t show_energy_performance_available_preferences(
>   				struct cpufreq_policy *policy, char *buf)
>   {
> -	int offset = 0, i;
> +	int i, offset = 0;
>   	struct amd_cpudata *cpudata = policy->driver_data;
>   
>   	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> @@ -1407,11 +1414,6 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
>   	bool raw_epp = false;
>   	u8 epp;
>   
> -	if (cpudata->dynamic_epp) {
> -		pr_debug("EPP cannot be set when dynamic EPP is enabled\n");
> -		return -EBUSY;
> -	}
> -
>   	/*
>   	 * if the value matches a number, use that, otherwise see if
>   	 * matches an index in the energy_perf_strings array
> @@ -1422,6 +1424,23 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
>   		ret = sysfs_match_string(energy_perf_strings, buf);
>   		if (ret < 0 || ret == EPP_INDEX_CUSTOM)
>   			return -EINVAL;
> +
> +		if (ret == EPP_INDEX_DYNAMIC) {
> +			/*
> +			 * Dynamic EPP was already enabled for this CPU.
> +			 * Nothing to do.
> +			 */
> +			if (cpudata->dynamic_epp)
> +				return count;
> +
> +			cpudata->current_profile = PLATFORM_PROFILE_BALANCED;
> +			ret = amd_pstate_set_dynamic_epp(policy);
> +			if (ret)
> +				return ret;
> +
> +			return count;
> +		}
> +
>   		if (ret)
>   			epp = epp_values[ret];
>   		else
> @@ -1433,6 +1452,13 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
>   		return -EBUSY;
>   	}
>   
> +	/*
> +	 * Dynamic EPP was enabled previously!
> +	 * Switch back to the static EPP mode.
> +	 */
> +	if (cpudata->dynamic_epp)
> +		amd_pstate_clear_dynamic_epp(policy);
> +
>   	ret = amd_pstate_set_epp(policy, epp);
>   	if (ret)
>   		return ret;
> @@ -1450,7 +1476,7 @@ ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *
>   
>   	epp = FIELD_GET(AMD_CPPC_EPP_PERF_MASK, cpudata->cppc_req_cached);
>   
> -	if (cpudata->raw_epp)
> +	if (!cpudata->dynamic_epp && cpudata->raw_epp)
>   		return sysfs_emit(buf, "%u\n", epp);
>   
>   	switch (epp) {
> @@ -1470,6 +1496,9 @@ ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *
>   		return -EINVAL;
>   	}
>   
> +	if (cpudata->dynamic_epp)
> +		return sysfs_emit(buf, "dynamic(profile:%s)\n", energy_perf_strings[preference]);
> +
>   	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
>   }
>   EXPORT_SYMBOL_GPL(show_energy_performance_preference);
> @@ -2030,6 +2059,18 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>   	if (!policy->cpuinfo.max_freq)
>   		return -ENODEV;
>   
> +	/* Must be a switch between PERFORMANCE and POWERSAVE */
> +	if (cpudata->policy != policy->policy) {
> +		/*
> +		 * Disable dynamic_epp when switching
> +		 * out of CPUFREQ_POLICY_POWERSAVE.
> +		 */
> +		if (cpudata->dynamic_epp) {
> +			WARN_ON_ONCE(cpudata->policy != CPUFREQ_POLICY_POWERSAVE);
> +			amd_pstate_clear_dynamic_epp(policy);
> +		}
> +	}
> +
>   	cpudata->policy = policy->policy;
>   
>   	ret = amd_pstate_epp_update_limit(policy, true);