Re: [RFC PATCH 3/6] cpufreq/amd-pstate: Repurpose "amd_dynamic_epp" cmdline and corresponding sysfs

[Mario Limonciello <superm1@kernel.org>]

On 6/30/26 13:59, K Prateek Nayak wrote:
> Since dynamic_epp has been converted to an
> "energy_performance_preference", toggling the feature via the sysfs file
> is now redundant.
> 
> Repurpose "amd_dynamic_epp=enable" command line to opt into dynamic EPP
> by default when the active driver is loaded, or switched into. The
> "disable" counterpart will "dynamic" option out of
> "energyy_performance_preference" to ensure dynamic EPP feature can never
> be toggled.
> 
> Use a tri-state enum to differentiate if the user has supplied a command
> line parameter or not to enable the feature by default vs keeping the
> current defaults as is and allowing users to toggle it later via the
> "dynamic" option in "energy_performance_preference" selection.
> 
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>

Honestly - I have to question if we even want to have the kernel command 
line option anymore.  If userspace can easily opt in and out as in this 
series, do we still need a kernel command line for default policy?

I'm of the opinion rip it out, less to maintain.

> ---
>   drivers/cpufreq/amd-pstate.c | 48 +++++++++---------------------------
>   1 file changed, 12 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index c7eec6b96dcc..61f30820d95a 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -82,12 +82,18 @@ struct quirk_entry {
>   	u32 lowest_freq;
>   };
>   
> +enum dynamic_epp_state {
> +	DYNAMIC_EPP_USER_TOGGLE = -1,	/* User can toggle dynamic EPP. (Default) */
> +	DYNAMIC_EPP_DISABLED,		/* Dynamic EPP disabled from command line. */
> +	DYNAMIC_EPP_DEFAULT_ENABLED,	/* Dynamic EPP enabled from command line. */
> +};
> +
> +static enum dynamic_epp_state dynamic_epp = DYNAMIC_EPP_USER_TOGGLE;
>   static struct cpufreq_driver *current_pstate_driver;
>   static struct cpufreq_driver amd_pstate_driver;
>   static struct cpufreq_driver amd_pstate_epp_driver;
>   static int cppc_state = AMD_PSTATE_UNDEFINED;
>   static bool amd_pstate_prefcore = true;
> -static bool dynamic_epp;
>   static struct quirk_entry *quirks;
>   
>   /*
> @@ -1839,39 +1845,9 @@ static ssize_t dynamic_epp_show(struct device *dev,
>   	return sysfs_emit(buf, "%s\n", str_enabled_disabled(dynamic_epp));
>   }
>   
> -static ssize_t dynamic_epp_store(struct device *a, struct device_attribute *b,
> -				 const char *buf, size_t count)
> -{
> -	bool enabled;
> -	int ret;
> -
> -	ret = kstrtobool(buf, &enabled);
> -	if (ret)
> -		return ret;
> -
> -	guard(mutex)(&amd_pstate_driver_lock);
> -
> -	if (cppc_state != AMD_PSTATE_ACTIVE) {
> -		pr_debug("dynamic_epp can only be toggled in active mode\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Nothing to do */
> -	if (dynamic_epp == enabled)
> -		return count;
> -
> -	/* reinitialize with desired dynamic EPP value */
> -	dynamic_epp = enabled;
> -	ret = amd_pstate_change_driver_mode(cppc_state);
> -	if (ret)
> -		dynamic_epp = false;
> -
> -	return ret ? ret : count;
> -}
> -
>   static DEVICE_ATTR_RW(status);
>   static DEVICE_ATTR_RO(prefcore);
> -static DEVICE_ATTR_RW(dynamic_epp);
> +static DEVICE_ATTR_RO(dynamic_epp);
>   
>   static struct attribute *pstate_global_attributes[] = {
>   	&dev_attr_status.attr,
> @@ -1983,7 +1959,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>   		cpudata->current_profile = PLATFORM_PROFILE_BALANCED;
>   	}
>   
> -	if (dynamic_epp) {
> +	if (dynamic_epp == DYNAMIC_EPP_DEFAULT_ENABLED) {
>   		/* Dynamic EPP is only available with the POWERSAVE policy. */
>   		policy->policy = CPUFREQ_POLICY_POWERSAVE;
>   		ret = amd_pstate_set_dynamic_epp(policy);
> @@ -2076,7 +2052,7 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>   		 * If dynamic_epp is enabled by default, toggle it on
>   		 * when switching to CPUFREQ_POLICY_POWERSAVE.
>   		 */
> -		if (dynamic_epp) {
> +		if (dynamic_epp == DYNAMIC_EPP_DEFAULT_ENABLED) {
>   			WARN_ON_ONCE(policy->policy != CPUFREQ_POLICY_POWERSAVE);
>   
>   			cpudata->current_profile = PLATFORM_PROFILE_BALANCED;
> @@ -2410,9 +2386,9 @@ static int __init amd_prefcore_param(char *str)
>   static int __init amd_dynamic_epp_param(char *str)
>   {
>   	if (!strcmp(str, "disable"))
> -		dynamic_epp = false;
> +		dynamic_epp = DYNAMIC_EPP_DISABLED;
>   	if (!strcmp(str, "enable"))
> -		dynamic_epp = true;
> +		dynamic_epp = DYNAMIC_EPP_DEFAULT_ENABLED;
>   
>   	return 0;
>   }