Re: [PATCH v6 5/5] cpufreq/amd-pstate-ut: Add a unit test for raw EPP

From: Gautham R. Shenoy

Date: Tue Mar 31 2026 - 13:47:00 EST


On Sun, Mar 29, 2026 at 03:38:11PM -0500, Mario Limonciello (AMD) wrote:
> Ensure that all supported raw EPP values work properly.
>
> Export the driver helpers used by the test module so the test can drive
> raw EPP writes and temporarily disable dynamic EPP while it runs.
>
> Signed-off-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>

Looks good to me.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@xxxxxxx>

--
Thanks and Regards
gautham.
> ---
> v5->v6:
> * Also verify that string EPP writes read back as strings
> * Save and restore the original driver mode and dynamic EPP state
> * Export the helper interfaces used by the unit test module
> ---
> drivers/cpufreq/amd-pstate-ut.c | 109 ++++++++++++++++++++++++++++++++
> drivers/cpufreq/amd-pstate.c | 11 ++--
> drivers/cpufreq/amd-pstate.h | 4 ++
> 3 files changed, 120 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index 1f62ab6438b4e..aa8a464fab47a 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -28,6 +28,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> +#include <linux/mm.h>
> #include <linux/fs.h>
> #include <linux/cleanup.h>
>
> @@ -41,6 +42,7 @@ static char *test_list;
> module_param(test_list, charp, 0444);
> MODULE_PARM_DESC(test_list,
> "Comma-delimited list of tests to run (empty means run all tests)");
> +DEFINE_FREE(cleanup_page, void *, if (_T) free_page((unsigned long)_T))
>
> struct amd_pstate_ut_struct {
> const char *name;
> @@ -54,6 +56,7 @@ static int amd_pstate_ut_acpi_cpc_valid(u32 index);
> static int amd_pstate_ut_check_enabled(u32 index);
> static int amd_pstate_ut_check_perf(u32 index);
> static int amd_pstate_ut_check_freq(u32 index);
> +static int amd_pstate_ut_epp(u32 index);
> static int amd_pstate_ut_check_driver(u32 index);
> static int amd_pstate_ut_check_freq_attrs(u32 index);
>
> @@ -62,6 +65,7 @@ static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
> {"amd_pstate_ut_check_enabled", amd_pstate_ut_check_enabled },
> {"amd_pstate_ut_check_perf", amd_pstate_ut_check_perf },
> {"amd_pstate_ut_check_freq", amd_pstate_ut_check_freq },
> + {"amd_pstate_ut_epp", amd_pstate_ut_epp },
> {"amd_pstate_ut_check_driver", amd_pstate_ut_check_driver },
> {"amd_pstate_ut_check_freq_attrs", amd_pstate_ut_check_freq_attrs },
> };
> @@ -268,6 +272,111 @@ static int amd_pstate_set_mode(enum amd_pstate_mode mode)
> return amd_pstate_update_status(mode_str, strlen(mode_str));
> }
>
> +static int amd_pstate_ut_epp(u32 index)
> +{
> + struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
> + char *buf __free(cleanup_page) = NULL;
> + static const char * const epp_strings[] = {
> + "performance",
> + "balance_performance",
> + "balance_power",
> + "power",
> + };
> + struct amd_cpudata *cpudata;
> + enum amd_pstate_mode orig_mode;
> + bool orig_dynamic_epp;
> + int ret, cpu = 0;
> + int i;
> + u16 epp;
> +
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy)
> + return -ENODEV;
> +
> + cpudata = policy->driver_data;
> + orig_mode = amd_pstate_get_status();
> + orig_dynamic_epp = cpudata->dynamic_epp;
> +
> + /* disable dynamic EPP before running test */
> + if (cpudata->dynamic_epp) {
> + pr_debug("Dynamic EPP is enabled, disabling it\n");
> + amd_pstate_clear_dynamic_epp(policy);
> + }
> +
> + buf = (char *)__get_free_page(GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = amd_pstate_set_mode(AMD_PSTATE_ACTIVE);
> + if (ret)
> + goto out;
> +
> + for (epp = 0; epp <= U8_MAX; epp++) {
> + u8 val;
> +
> + /* write all EPP values */
> + memset(buf, 0, PAGE_SIZE);
> + snprintf(buf, PAGE_SIZE, "%d", epp);
> + ret = store_energy_performance_preference(policy, buf, strlen(buf));
> + if (ret < 0)
> + goto out;
> +
> + /* check if the EPP value reads back correctly for raw numbers */
> + memset(buf, 0, PAGE_SIZE);
> + ret = show_energy_performance_preference(policy, buf);
> + if (ret < 0)
> + goto out;
> + strreplace(buf, '\n', '\0');
> + ret = kstrtou8(buf, 0, &val);
> + if (!ret && epp != val) {
> + pr_err("Raw EPP value mismatch: %d != %d\n", epp, val);
> + ret = -EINVAL;
> + goto out;
> + }
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(epp_strings); i++) {
> + memset(buf, 0, PAGE_SIZE);
> + snprintf(buf, PAGE_SIZE, "%s", epp_strings[i]);
> + ret = store_energy_performance_preference(policy, buf, strlen(buf));
> + if (ret < 0)
> + goto out;
> +
> + memset(buf, 0, PAGE_SIZE);
> + ret = show_energy_performance_preference(policy, buf);
> + if (ret < 0)
> + goto out;
> + strreplace(buf, '\n', '\0');
> +
> + if (strcmp(buf, epp_strings[i])) {
> + pr_err("String EPP value mismatch: %s != %s\n", buf, epp_strings[i]);
> + ret = -EINVAL;
> + goto out;
> + }
> + }
> +
> + ret = 0;
> +
> +out:
> + 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;
> +
> + ret2 = amd_pstate_set_mode(orig_mode);
> + if (!ret && ret2)
> + ret = ret2;
> + }
> +
> + return ret;
> +}
> +
> static int amd_pstate_ut_check_driver(u32 index)
> {
> enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE;
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 11fc992ea2da1..e246a1477cf78 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1261,7 +1261,7 @@ static const struct platform_profile_ops amd_pstate_profile_ops = {
> .profile_get = amd_pstate_profile_get,
> };
>
> -static void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy)
> +void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy)
> {
> struct amd_cpudata *cpudata = policy->driver_data;
>
> @@ -1274,6 +1274,7 @@ static void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy)
> kfree(cpudata->profile_name);
> cpudata->dynamic_epp = false;
> }
> +EXPORT_SYMBOL_GPL(amd_pstate_clear_dynamic_epp);
>
> static int amd_pstate_set_dynamic_epp(struct cpufreq_policy *policy)
> {
> @@ -1411,8 +1412,8 @@ static ssize_t show_energy_performance_available_preferences(
> return offset;
> }
>
> -static ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
> - const char *buf, size_t count)
> +ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
> + const char *buf, size_t count)
> {
> struct amd_cpudata *cpudata = policy->driver_data;
> ssize_t ret;
> @@ -1453,8 +1454,9 @@ static ssize_t store_energy_performance_preference(struct cpufreq_policy *policy
>
> return count;
> }
> +EXPORT_SYMBOL_GPL(store_energy_performance_preference);
>
> -static ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *buf)
> +ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *buf)
> {
> struct amd_cpudata *cpudata = policy->driver_data;
> u8 preference, epp;
> @@ -1483,6 +1485,7 @@ static ssize_t show_energy_performance_preference(struct cpufreq_policy *policy,
>
> return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> }
> +EXPORT_SYMBOL_GPL(show_energy_performance_preference);
>
> static ssize_t store_amd_pstate_floor_freq(struct cpufreq_policy *policy,
> const char *buf, size_t count)
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index f7461d1b6bf3c..e4722e54387b0 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -150,6 +150,10 @@ enum amd_pstate_mode {
> const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
> int amd_pstate_get_status(void);
> int amd_pstate_update_status(const char *buf, size_t size);
> +ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
> + const char *buf, size_t count);
> +ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *buf);
> +void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy);
>
> struct freq_attr;
>
> --
> 2.43.0
>