diff mbox series

[v4,5/5] cpufreq/amd-pstate-ut: Add a unit test for raw EPP

Message ID 20250321022858.1538173-6-superm1@kernel.org (mailing list archive)
State New
Delegated to: Mario Limonciello
Headers show
Series amd-pstate Dynamic EPP and raw EPP | expand

Commit Message

Mario Limonciello March 21, 2025, 2:28 a.m. UTC 
From: Mario Limonciello <mario.limonciello@amd.com>

Ensure that all supported raw EPP values work properly.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/cpufreq/amd-pstate-ut.c | 58 +++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Dhananjay Ugwekar March 24, 2025, 11:25 a.m. UTC | #1 
On 3/21/2025 7:58 AM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> Ensure that all supported raw EPP values work properly.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/cpufreq/amd-pstate-ut.c | 58 +++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index e671bc7d15508..d0c5c0aa3cc94 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -26,6 +26,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>
>  
> @@ -33,6 +34,7 @@
>  
>  #include "amd-pstate.h"
>  
> +DEFINE_FREE(free_page, void *, if (_T) free_page((unsigned long)_T))
>  
>  struct amd_pstate_ut_struct {
>  	const char *name;
> @@ -46,6 +48,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 struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
> @@ -53,6 +56,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     }
>  };
>  
> @@ -239,6 +243,60 @@ 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;
> +	void *buf __free(free_page) = NULL;
> +	struct amd_cpudata *cpudata;
> +	int ret, cpu = 0;
> +	u16 epp;
> +
> +	policy = cpufreq_cpu_get(cpu);
> +	if (!policy)
> +		return -ENODEV;
> +
> +	cpudata = policy->driver_data;
> +
> +	/* 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 = (void *)__get_free_page(GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = amd_pstate_set_mode(AMD_PSTATE_ACTIVE);

This causes a deadlock, as we are holding cpufreq_policy reference and calling set_mode(), which 
needs to unregister the cpufreq subsystem (i.e. needs all cpufreq_policy references freed). 

We might need to drop and retake the reference around set_mode(). Also make cpudata variable NULL 
to avoid anyone using a stale cpudata pointer later on by mistake.
OR 
Directly call set_epp() instead of store_energy_performance_preference() and eliminate the need 
of policy ref after set_mode().

> +	if (ret)
> +		return ret;
> +
> +	for (epp = 0; epp <= U8_MAX; epp++) {
> +		u8 val;
> +
> +		/* write all EPP values */
> +		memset(buf, 0, sizeof(*buf));
> +		snprintf(buf, PAGE_SIZE, "%d", epp);
> +		ret = store_energy_performance_preference(policy, buf, sizeof(*buf));
> +		if (ret < 0)
> +			return ret;
> +
> +		/* check if the EPP value reads back correctly for raw numbers */
> +		memset(buf, 0, sizeof(*buf));
> +		ret = show_energy_performance_preference(policy, buf);
> +		if (ret < 0)
> +			return ret;
> +		strreplace(buf, '\n', '\0');
> +		ret = kstrtou8(buf, 0, &val);
> +		if (!ret && epp != val) {
> +			pr_err("Raw EPP value mismatch: %d != %d\n", epp, val);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int amd_pstate_ut_check_driver(u32 index)
>  {
>  	enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE;
diff mbox series

Patch

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index e671bc7d15508..d0c5c0aa3cc94 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -26,6 +26,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>
 
@@ -33,6 +34,7 @@ 
 
 #include "amd-pstate.h"
 
+DEFINE_FREE(free_page, void *, if (_T) free_page((unsigned long)_T))
 
 struct amd_pstate_ut_struct {
 	const char *name;
@@ -46,6 +48,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 struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
@@ -53,6 +56,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     }
 };
 
@@ -239,6 +243,60 @@  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;
+	void *buf __free(free_page) = NULL;
+	struct amd_cpudata *cpudata;
+	int ret, cpu = 0;
+	u16 epp;
+
+	policy = cpufreq_cpu_get(cpu);
+	if (!policy)
+		return -ENODEV;
+
+	cpudata = policy->driver_data;
+
+	/* 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 = (void *)__get_free_page(GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = amd_pstate_set_mode(AMD_PSTATE_ACTIVE);
+	if (ret)
+		return ret;
+
+	for (epp = 0; epp <= U8_MAX; epp++) {
+		u8 val;
+
+		/* write all EPP values */
+		memset(buf, 0, sizeof(*buf));
+		snprintf(buf, PAGE_SIZE, "%d", epp);
+		ret = store_energy_performance_preference(policy, buf, sizeof(*buf));
+		if (ret < 0)
+			return ret;
+
+		/* check if the EPP value reads back correctly for raw numbers */
+		memset(buf, 0, sizeof(*buf));
+		ret = show_energy_performance_preference(policy, buf);
+		if (ret < 0)
+			return ret;
+		strreplace(buf, '\n', '\0');
+		ret = kstrtou8(buf, 0, &val);
+		if (!ret && epp != val) {
+			pr_err("Raw EPP value mismatch: %d != %d\n", epp, val);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int amd_pstate_ut_check_driver(u32 index)
 {
 	enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE;