Re: [PATCH v2 8/9] cpufreq/amd-pstate-ut: Add unit test for "dynamic" EPP mode

[Mario Limonciello <superm1@kernel.org>]

On 7/23/26 15:12, K Prateek Nayak wrote:
> Extend the EPP unit test to cover the "dynamic" epp mode. Since
> "dynamic_epp" is no longer a system-wide toggle, remove the legacy
> "dynamic_epp" bits from the unit test.
> 
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> Changelog rfc v1..v2:
> 
> o Simplified some aspect of test now that dynamic_epp global knob is
>    removed.
> ---
>   drivers/cpufreq/amd-pstate-ut.c | 45 ++++++++++++++++++---------------
>   1 file changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index 2142838ad6cc..d1f730e3502b 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -275,6 +275,7 @@ static int amd_pstate_set_mode(enum amd_pstate_mode mode)
>   static int amd_pstate_ut_epp(u32 index)
>   {
>   	static const char * const epp_strings[] = {
> +		"dynamic",
>   		"power",
>   		"balance_power",
>   		"balance_performance",
> @@ -282,10 +283,10 @@ static int amd_pstate_ut_epp(u32 index)
>   	};
>   	char *buf __free(cleanup_page) = NULL;
>   	struct cpufreq_policy *policy = NULL;
> +	unsigned long orig_dynamic_epp = 0;
>   	enum amd_pstate_mode orig_mode;
>   	struct amd_cpudata *cpudata;
>   	unsigned long orig_policy;
> -	bool orig_dynamic_epp;
>   	int ret, cpu = 0;
>   	u16 epp;
>   	int i;
> @@ -294,9 +295,11 @@ static int amd_pstate_ut_epp(u32 index)
>   	if (!policy)
>   		return -ENODEV;
>   
> -	cpudata = policy->driver_data;
>   	orig_mode = amd_pstate_get_status();
> -	orig_dynamic_epp = cpudata->dynamic_epp;
> +	if (policy->driver_data) {
> +		cpudata = policy->driver_data;
> +		orig_dynamic_epp = cpudata->dynamic_epp;
> +	}
>   
>   	/* Drop reference before potential driver change. */
>   	cpufreq_cpu_put(policy);
> @@ -321,16 +324,6 @@ static int amd_pstate_ut_epp(u32 index)
>   	orig_policy = cpudata->policy;
>   	cpudata->policy = CPUFREQ_POLICY_POWERSAVE;
>   
> -	/*
> -	 * Disable dynamic EPP before running test. If "orig_dynamic_epp" is
> -	 * true, the  driver will do a redundant switch at the end and there
> -	 * is no need for enabling it again at the end of the test.
> -	 */
> -	if (cpudata->dynamic_epp) {
> -		pr_debug("Dynamic EPP is enabled, disabling it\n");
> -		amd_pstate_clear_dynamic_epp(policy);
> -	}
> -
>   	for (epp = 0; epp <= U8_MAX; epp++) {
>   		u8 val;
>   
> @@ -367,6 +360,11 @@ static int amd_pstate_ut_epp(u32 index)
>   		if (ret < 0)
>   			goto out;
>   		strreplace(buf, '\n', '\0');
> +		/*
> +		 * "dynamic" mode reports the EPP as "dynamic(profile:X)"
> +		 * Trim at "(" and just compare tie the epp string.
> +		 */
> +		strreplace(buf, '(', '\0');
>   
>   		if (strcmp(buf, epp_strings[i])) {
>   			pr_err("String EPP value mismatch: %s != %s\n", buf, epp_strings[i]);
> @@ -380,18 +378,23 @@ static int amd_pstate_ut_epp(u32 index)
>   out:
>   	if (policy) {
>   		cpudata->policy = orig_policy;
> +		/*
> +		 * If the driver had enabled dynamic_epp to brgin with,
begin
> +		 * restore it here before dropping policy reference.
> +		 */
> +		if (orig_dynamic_epp) {
> +			int ret2;
> +
> +			ret2 = store_energy_performance_preference(policy,
> +								  epp_strings[0],
> +								  strlen(epp_strings[0]));
> +			if (!ret && (ret2 < 0))
> +				ret = ret2;
> +		}
>   		up_write(&policy->rwsem);
>   		cpufreq_cpu_put(policy);
>   	}
>   
> -	if (orig_dynamic_epp) {
> -		int ret2;
> -
> -		ret2 = amd_pstate_set_mode(AMD_PSTATE_DISABLE);
> -		if (!ret && ret2)
> -			ret = ret2;
> -	}
> -
>   	if (orig_mode != amd_pstate_get_status()) {
>   		int ret2;
>