[PATCH v3 0/7] AMD Pstate Driver Core Performance Boost

[PATCH v3 0/7] AMD Pstate Driver Core Performance Boost

[Perry Yuan]

Hi all,
The patchset series add core performance boost feature for AMD pstate
driver including passisve ,guide and active mode support.

User can change core frequency boost control with a new sysfs entry:

"/sys/devices/system/cpu/amd_pstate/cpb_boost"

The legancy boost interface has been removed due to the function
conflict with new cpb_boost which can support all modes.

1). enable core boost:
$ sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
$ lscpu -ae
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE    MAXMHZ   MINMHZ      MHZ
  0    0      0    0 0:0:0:0          yes 4201.0000 400.0000 2983.578
  1    0      0    1 1:1:1:0          yes 4201.0000 400.0000 2983.578
  2    0      0    2 2:2:2:0          yes 4201.0000 400.0000 2583.855
  3    0      0    3 3:3:3:0          yes 4201.0000 400.0000 2983.578
  4    0      0    4 4:4:4:0          yes 4201.0000 400.0000 2983.578

2). disabble core boost:
$ sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
$ lscpu -ae
   0    0      0    0 0:0:0:0          yes 5759.0000 400.0000 2983.578
  1    0      0    1 1:1:1:0          yes 5759.0000 400.0000 2983.578
  2    0      0    2 2:2:2:0          yes 5759.0000 400.0000 2983.578
  3    0      0    3 3:3:3:0          yes 5759.0000 400.0000 2983.578
  4    0      0    4 4:4:4:0          yes 5759.0000 400.0000 2983.578


The patches have been tested with the AMD 7950X processor and many users
would like to get core boost control enabled for power saving.

Perry.

Changes from v2:
 * move global struct to amd-pstate.h
 * fix the amd-pstate-ut with new cpb control interface

Changes from v1:
 * drop suspend/resume fix patch 6/7 because of the fix should be in
   another fix series instead of CPB feature
 * move the set_boost remove patch to the last(Mario)
 * Fix commit info with "Closes:" (Mario)
 * simplified global.cpb_supported initialization(Mario)
 * Add guide mode support for CPB control
 * Fixed some Doc typos and add guide mode info to Doc as well.

v1: https://lore.kernel.org/all/cover.1706255676.git.perry.yuan@amd.com/
v2: https://lore.kernel.org/lkml/cover.1707047943.git.perry.yuan@amd.com/

Perry Yuan (7):
  cpufreq: amd-pstate: initialize new core precision boost state
  cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
  cpufreq: amd-pstate: fix max_perf calculation for amd_get_max_freq()
  cpufreq: amd-pstate: fix the MSR highest perf will be reset issue
    while cpb boost off
  Documentation: cpufreq: amd-pstate: introduce the new cpu boost
    control method
  cpufreq: amd-pstate: remove legacy set_boost callback for passive mode
  cpufreq: amd-pstate-ut: support new cpb boost control interface

 Documentation/admin-guide/pm/amd-pstate.rst |  11 ++
 drivers/cpufreq/amd-pstate-ut.c             |   2 +-
 drivers/cpufreq/amd-pstate.c                | 156 +++++++++++++++-----
 include/linux/amd-pstate.h                  |  15 +-
 4 files changed, 148 insertions(+), 36 deletions(-)

-- 
2.34.1

[PATCH v3 1/7] cpufreq: amd-pstate: initialize new core precision boost state

[Perry Yuan]

From: Perry Yuan <Perry.Yuan@amd.com>

Add gloal global_params to represent current CPU Performance Boost(cpb)
state for cpu frequency scaling, both active and passive modes all can
support CPU cores frequency boosting control which is based on the BIOS
setting, while BIOS turn on the "Core Performance Boost", it will
allow OS control each core highest perf limitation from OS side.

If core performance boost is disabled while a core is in a boosted P-state,
the core transitions to the highest performance non-boosted P-state,
that is the same as the nominal frequency limit.

Reported-by: Artem S. Tashkinov" <aros@gmx.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 49 ++++++++++++------------------------
 include/linux/amd-pstate.h   | 14 +++++++++++
 2 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 08e112444c27..3772f71f525f 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -67,6 +67,7 @@ static struct cpufreq_driver amd_pstate_epp_driver;
 static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 static bool amd_pstate_prefcore = true;
+struct global_params global;
 
 /*
  * AMD Energy Preference Performance (EPP)
@@ -669,43 +670,21 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
 	return lowest_nonlinear_freq * 1000;
 }
 
-static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
+static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
 {
-	struct amd_cpudata *cpudata = policy->driver_data;
+	u64 boost_val;
 	int ret;
 
-	if (!cpudata->boost_supported) {
-		pr_err("Boost mode is not supported by this processor or SBIOS\n");
-		return -EINVAL;
-	}
-
-	if (state)
-		policy->cpuinfo.max_freq = cpudata->max_freq;
-	else
-		policy->cpuinfo.max_freq = cpudata->nominal_freq;
-
-	policy->max = policy->cpuinfo.max_freq;
-
-	ret = freq_qos_update_request(&cpudata->req[1],
-				      policy->cpuinfo.max_freq);
-	if (ret < 0)
+	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
+	if (ret) {
+		pr_err_once("failed to read initial CPU boost state!\n");
 		return ret;
+	}
 
-	return 0;
-}
-
-static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
-{
-	u32 highest_perf, nominal_perf;
-
-	highest_perf = READ_ONCE(cpudata->highest_perf);
-	nominal_perf = READ_ONCE(cpudata->nominal_perf);
-
-	if (highest_perf <= nominal_perf)
-		return;
+	global.cpb_supported = !((boost_val >> 25) & 0x1);
+	global.cpb_boost = global.cpb_supported;
 
-	cpudata->boost_supported = true;
-	current_pstate_driver->boost_enabled = true;
+	return ret;
 }
 
 static void amd_perf_ctl_reset(unsigned int cpu)
@@ -848,6 +827,9 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 	if (ret)
 		goto free_cpudata1;
 
+	/* initialize cpu cores boot state */
+	amd_pstate_boost_init(cpudata);
+
 	min_freq = amd_get_min_freq(cpudata);
 	max_freq = amd_get_max_freq(cpudata);
 	nominal_freq = amd_get_nominal_freq(cpudata);
@@ -899,7 +881,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	policy->driver_data = cpudata;
 
-	amd_pstate_boost_init(cpudata);
 	if (!current_pstate_driver->adjust_perf)
 		current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
 
@@ -1310,6 +1291,9 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 	if (ret)
 		goto free_cpudata1;
 
+	/* initialize cpu cores boot state */
+	amd_pstate_boost_init(cpudata);
+
 	min_freq = amd_get_min_freq(cpudata);
 	max_freq = amd_get_max_freq(cpudata);
 	nominal_freq = amd_get_nominal_freq(cpudata);
@@ -1360,7 +1344,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 			return ret;
 		WRITE_ONCE(cpudata->cppc_cap1_cached, value);
 	}
-	amd_pstate_boost_init(cpudata);
 
 	return 0;
 
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index d21838835abd..465e9295a60c 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -124,4 +124,18 @@ static const char * const amd_pstate_mode_string[] = {
 	[AMD_PSTATE_GUIDED]      = "guided",
 	NULL,
 };
+
+/**
+ * struct global_params - Global parameters, mostly tunable via sysfs.
+ * @cpb_boost:		Whether or not to use boost CPU P-states.
+ * @cpb_supported:	Whether or not CPU boost P-states are available
+ *			based on the MSR_K7_HWCR bit[25] state
+ */
+struct global_params {
+	bool cpb_boost;
+	bool cpb_supported;
+};
+
+extern struct global_params global;
+
 #endif /* _LINUX_AMD_PSTATE_H */
-- 
2.34.1

[PATCH v3 2/7] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control

[Perry Yuan]

From: Perry Yuan <Perry.Yuan@amd.com>

With this new sysfs entry `cpb_boost`created, user can change CPU boost
state dynamically under `active`, `guided` and `passive` modes.
And the highest perf and frequency will also be updated as the boost
state changing.

0): check current boost state
cat /sys/devices/system/cpu/amd_pstate/cpb_boost

1): disable CPU boost
sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost"

2): enable CPU boost
sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost"

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217931
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217618
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 97 ++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 3772f71f525f..82b43bb2ab83 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1200,6 +1200,101 @@ static ssize_t prefcore_show(struct device *dev,
 	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
 }
 
+static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
+	struct cppc_perf_ctrls perf_ctrls;
+	u32 highest_perf, nominal_perf;
+	int ret;
+
+	if (!policy)
+		return -ENODATA;
+
+	highest_perf = READ_ONCE(cpudata->highest_perf);
+	nominal_perf = READ_ONCE(cpudata->nominal_perf);
+
+	if (boot_cpu_has(X86_FEATURE_CPPC)) {
+		u64 value = READ_ONCE(cpudata->cppc_req_cached);
+
+		value &= ~GENMASK_ULL(7, 0);
+		value |= on ? highest_perf : nominal_perf;
+		WRITE_ONCE(cpudata->cppc_req_cached, value);
+
+		wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+
+	} else {
+		perf_ctrls.max_perf = on ? highest_perf : nominal_perf;
+		ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1);
+		if (ret) {
+			pr_debug("failed to set energy perf value (%d)\n", ret);
+			return ret;
+		}
+	}
+
+	if (on)
+		policy->cpuinfo.max_freq = cpudata->max_freq;
+	else
+		policy->cpuinfo.max_freq = cpudata->nominal_freq;
+
+	policy->max = policy->cpuinfo.max_freq;
+
+	if (cppc_state == AMD_PSTATE_PASSIVE) {
+		ret = freq_qos_update_request(&cpudata->req[1],
+				      policy->cpuinfo.max_freq);
+	}
+
+	cpufreq_cpu_release(policy);
+
+	return ret;
+}
+
+static ssize_t cpb_boost_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", global.cpb_boost);
+}
+
+static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b,
+			    const char *buf, size_t count)
+{
+	bool new_state;
+	ssize_t ret;
+	int cpu;
+
+	mutex_lock(&amd_pstate_driver_lock);
+	if (!global.cpb_supported) {
+		pr_err("Boost mode is not supported by this processor or SBIOS\n");
+		return -EINVAL;
+	}
+
+	ret = kstrtobool(buf, &new_state);
+	if (ret)
+		return -EINVAL;
+
+	global.cpb_boost = !!new_state;
+
+	for_each_possible_cpu(cpu) {
+
+		struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+		struct amd_cpudata *cpudata = policy->driver_data;
+
+		if (!cpudata) {
+			pr_err("cpudata is NULL\n");
+			ret = -ENODATA;
+			cpufreq_cpu_put(policy);
+			goto err_exit;
+		}
+
+		amd_cpu_boost_update(cpudata, global.cpb_boost);
+		refresh_frequency_limits(policy);
+		cpufreq_cpu_put(policy);
+	}
+
+err_exit:
+	mutex_unlock(&amd_pstate_driver_lock);
+	return ret < 0 ? ret : count;
+}
+
 cpufreq_freq_attr_ro(amd_pstate_max_freq);
 cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
 
@@ -1210,6 +1305,7 @@ cpufreq_freq_attr_rw(energy_performance_preference);
 cpufreq_freq_attr_ro(energy_performance_available_preferences);
 static DEVICE_ATTR_RW(status);
 static DEVICE_ATTR_RO(prefcore);
+static DEVICE_ATTR_RW(cpb_boost);
 
 static struct freq_attr *amd_pstate_attr[] = {
 	&amd_pstate_max_freq,
@@ -1234,6 +1330,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
 static struct attribute *pstate_global_attributes[] = {
 	&dev_attr_status.attr,
 	&dev_attr_prefcore.attr,
+	&dev_attr_cpb_boost.attr,
 	NULL
 };
 
-- 
2.34.1

[PATCH v3 3/7] cpufreq: amd-pstate: fix max_perf calculation for amd_get_max_freq()

[Perry Yuan]

From: Perry Yuan <Perry.Yuan@amd.com>

When CPU core Precision Boost state changed, the max frequency will also
need to be updated according to the current boost state, if boost is
disabled now, the max perf will be limited to nominal perf values.
otherwise the max frequency will be showed wrongly.

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 82b43bb2ab83..1f86ce920d6c 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -624,6 +624,10 @@ static int amd_get_max_freq(struct amd_cpudata *cpudata)
 	nominal_perf = READ_ONCE(cpudata->nominal_perf);
 	max_perf = READ_ONCE(cpudata->highest_perf);
 
+	/* when boost is off, the highest perf will be limited to nominal_perf */
+	if (!global.cpb_boost)
+		max_perf = nominal_perf;
+
 	boost_ratio = div_u64(max_perf << SCHED_CAPACITY_SHIFT,
 			      nominal_perf);
 
-- 
2.34.1

[PATCH v3 4/7] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off

[Perry Yuan]

From: Perry Yuan <Perry.Yuan@amd.com>

Select the min perf to fix the highest perf value while update pstate
CPPC request MSR register, here we need to limit the max perf value when
CPU boost is disabled in case of that highest perf value in the MSR will be
reset to original highest perf value which cause the BOOST control
failed.

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 1f86ce920d6c..35791efc6e88 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -439,6 +439,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 			      u32 des_perf, u32 max_perf, bool fast_switch, int gov_flags)
 {
 	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
+	u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
 	u64 value = prev;
 
 	min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
@@ -458,6 +459,10 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 	value &= ~AMD_CPPC_DES_PERF(~0L);
 	value |= AMD_CPPC_DES_PERF(des_perf);
 
+	/* limit the max perf when core performance boost feature is disabled */
+	if (!global.cpb_boost)
+		max_perf = min_t(unsigned long, nominal_perf, max_perf);
+
 	value &= ~AMD_CPPC_MAX_PERF(~0L);
 	value |= AMD_CPPC_MAX_PERF(max_perf);
 
-- 
2.34.1

[PATCH v3 5/7] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method

[Perry Yuan]

From: Perry Yuan <Perry.Yuan@amd.com>

Introduce AMD CPU frequency boosting control sysfs entry which used for
switching boost on and boost off.

If core performance boost is disabled while a core is in a boosted P-state,
the core automatically transitions to the highest performance non-boosted P-state
The highest perf and frequency will be limited by the setting value.

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 0a3aa6b8ffd5..fcf5b75a5341 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -440,6 +440,17 @@ control its functionality at the system level.  They are located in the
         This attribute is read-only to check the state of preferred core set
         by the kernel parameter.
 
+``cpb_boost``
+        Specifies whether core performance boost is requested to be enabled or disabled
+        If core performance boost is disabled while a core is in a boosted P-state, the
+        core automatically transitions to the highest performance non-boosted P-state.
+        AMD Core Performance Boost(CPB) is controlled by this new attribute file which
+        allow user to change all cores frequency boosting state. It supports both
+        ``active``, ``passive`` and ``guided`` mode control with below value write to it.
+
+        "0" Disable Core Performance Boosting
+        "1" Enable  Core Performance Boosting
+
 ``cpupower`` tool support for ``amd-pstate``
 ===============================================
 
-- 
2.34.1

[PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost callback for passive mode

[Perry Yuan]

With new freqency boost interface supported, legacy boost control
doesn't make sense any more which only support passive mode.
so it can remove the legacy set_boost interface from amd-pstate driver
in case of there is conflict with new boost control logic.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 1 -
 include/linux/amd-pstate.h   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 35791efc6e88..1dd523db3871 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1675,7 +1675,6 @@ static struct cpufreq_driver amd_pstate_driver = {
 	.exit		= amd_pstate_cpu_exit,
 	.suspend	= amd_pstate_cpu_suspend,
 	.resume		= amd_pstate_cpu_resume,
-	.set_boost	= amd_pstate_set_boost,
 	.update_limits	= amd_pstate_update_limits,
 	.name		= "amd-pstate",
 	.attr		= amd_pstate_attr,
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index 465e9295a60c..ab7ca26974da 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -93,7 +93,6 @@ struct amd_cpudata {
 	struct amd_aperf_mperf prev;
 
 	u64	freq;
-	bool	boost_supported;
 	bool	hw_prefcore;
 
 	/* EPP feature related attributes*/
-- 
2.34.1

[PATCH v3 7/7] cpufreq: amd-pstate-ut: support new cpb boost control interface

[Perry Yuan]

As the new CPB boost control is enabled, pstate unit test needs to remove
legacy `boost_supported` check and start to use new CPB boost control
interface `global.cpb_boost`.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate-ut.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index f04ae67dda37..91780ec42712 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -226,7 +226,7 @@ static void amd_pstate_ut_check_freq(u32 index)
 			goto skip_test;
 		}
 
-		if (cpudata->boost_supported) {
+		if (global.cpb_supported) {
 			if ((policy->max == cpudata->max_freq) ||
 					(policy->max == cpudata->nominal_freq))
 				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
-- 
2.34.1

Re: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost callback for passive mode

[Oleksandr Natalenko]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

Hello.

On středa 7. února 2024 10:21:57 CET Perry Yuan wrote:
> With new freqency boost interface supported, legacy boost control
> doesn't make sense any more which only support passive mode.
> so it can remove the legacy set_boost interface from amd-pstate driver
> in case of there is conflict with new boost control logic.
> 
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 1 -
>  include/linux/amd-pstate.h   | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 35791efc6e88..1dd523db3871 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1675,7 +1675,6 @@ static struct cpufreq_driver amd_pstate_driver = {
>  	.exit		= amd_pstate_cpu_exit,
>  	.suspend	= amd_pstate_cpu_suspend,
>  	.resume		= amd_pstate_cpu_resume,
> -	.set_boost	= amd_pstate_set_boost,
>  	.update_limits	= amd_pstate_update_limits,
>  	.name		= "amd-pstate",
>  	.attr		= amd_pstate_attr,
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index 465e9295a60c..ab7ca26974da 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -93,7 +93,6 @@ struct amd_cpudata {
>  	struct amd_aperf_mperf prev;
>  
>  	u64	freq;
> -	bool	boost_supported;

As a result of this removal the kernel-doc for this struct should be amended too because even after this patch is applied the `boost_supported` field remains documented.

>  	bool	hw_prefcore;
>  
>  	/* EPP feature related attributes*/
> 


-- 
Oleksandr Natalenko (post-factum)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost callback for passive mode

[Yuan, Perry]

[AMD Official Use Only - General]

Hi,

> -----Original Message-----
> From: Oleksandr Natalenko <oleksandr@natalenko.name>
> Sent: Wednesday, February 7, 2024 6:11 PM
> To: rafael.j.wysocki@intel.com; Limonciello, Mario
> <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray
> <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost
> callback for passive mode
>
> Hello.
>
> On středa 7. února 2024 10:21:57 CET Perry Yuan wrote:
> > With new freqency boost interface supported, legacy boost control
> > doesn't make sense any more which only support passive mode.
> > so it can remove the legacy set_boost interface from amd-pstate driver
> > in case of there is conflict with new boost control logic.
> >
> > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 1 -
> >  include/linux/amd-pstate.h   | 1 -
> >  2 files changed, 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 35791efc6e88..1dd523db3871
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -1675,7 +1675,6 @@ static struct cpufreq_driver amd_pstate_driver =
> {
> >     .exit           = amd_pstate_cpu_exit,
> >     .suspend        = amd_pstate_cpu_suspend,
> >     .resume         = amd_pstate_cpu_resume,
> > -   .set_boost      = amd_pstate_set_boost,
> >     .update_limits  = amd_pstate_update_limits,
> >     .name           = "amd-pstate",
> >     .attr           = amd_pstate_attr,
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index 465e9295a60c..ab7ca26974da 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -93,7 +93,6 @@ struct amd_cpudata {
> >     struct amd_aperf_mperf prev;
> >
> >     u64     freq;
> > -   bool    boost_supported;
>
> As a result of this removal the kernel-doc for this struct should be amended
> too because even after this patch is applied the `boost_supported` field
> remains documented.

The set_boost is callback used by cpufreq.c, the boost_supported is not documented before the patch.
We just need to add new sysfs "cpb_boost" introduction in the "/Documentation/admin-guide/pm/amd-pstate.rst"
The legacy boost is not supported any more, we have no need to document it again as it is.

Perry.

>
> >     bool    hw_prefcore;
> >
> >     /* EPP feature related attributes*/
> >
>
>
> --
> Oleksandr Natalenko (post-factum)

Re: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost callback for passive mode

[Oleksandr Natalenko]

[-- Attachment #1: Type: text/plain, Size: 3939 bytes --]

On středa 7. února 2024 14:17:06 CET Yuan, Perry wrote:
> [AMD Official Use Only - General]
> 
> Hi,
> 
> > -----Original Message-----
> > From: Oleksandr Natalenko <oleksandr@natalenko.name>
> > Sent: Wednesday, February 7, 2024 6:11 PM
> > To: rafael.j.wysocki@intel.com; Limonciello, Mario
> > <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray
> > <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal
> > <gautham.shenoy@amd.com>; Petkov, Borislav
> > <Borislav.Petkov@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer
> > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> > Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost
> > callback for passive mode
> >
> > Hello.
> >
> > On středa 7. února 2024 10:21:57 CET Perry Yuan wrote:
> > > With new freqency boost interface supported, legacy boost control
> > > doesn't make sense any more which only support passive mode.
> > > so it can remove the legacy set_boost interface from amd-pstate driver
> > > in case of there is conflict with new boost control logic.
> > >
> > > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > > ---
> > >  drivers/cpufreq/amd-pstate.c | 1 -
> > >  include/linux/amd-pstate.h   | 1 -
> > >  2 files changed, 2 deletions(-)
> > >
> > > diff --git a/drivers/cpufreq/amd-pstate.c
> > > b/drivers/cpufreq/amd-pstate.c index 35791efc6e88..1dd523db3871
> > 100644
> > > --- a/drivers/cpufreq/amd-pstate.c
> > > +++ b/drivers/cpufreq/amd-pstate.c
> > > @@ -1675,7 +1675,6 @@ static struct cpufreq_driver amd_pstate_driver =
> > {
> > >     .exit           = amd_pstate_cpu_exit,
> > >     .suspend        = amd_pstate_cpu_suspend,
> > >     .resume         = amd_pstate_cpu_resume,
> > > -   .set_boost      = amd_pstate_set_boost,
> > >     .update_limits  = amd_pstate_update_limits,
> > >     .name           = "amd-pstate",
> > >     .attr           = amd_pstate_attr,
> > > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > > index 465e9295a60c..ab7ca26974da 100644
> > > --- a/include/linux/amd-pstate.h
> > > +++ b/include/linux/amd-pstate.h
> > > @@ -93,7 +93,6 @@ struct amd_cpudata {
> > >     struct amd_aperf_mperf prev;
> > >
> > >     u64     freq;
> > > -   bool    boost_supported;
> >
> > As a result of this removal the kernel-doc for this struct should be amended
> > too because even after this patch is applied the `boost_supported` field
> > remains documented.
> 
> The set_boost is callback used by cpufreq.c, the boost_supported is not documented before the patch.
> We just need to add new sysfs "cpb_boost" introduction in the "/Documentation/admin-guide/pm/amd-pstate.rst"
> The legacy boost is not supported any more, we have no need to document it again as it is.

`boost_supported` as a field of `struct amd_cpudata` is documented in the kernel-doc right before the structure, and this bit should be removed like shown below:

```
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -56,7 +56,6 @@ struct amd_aperf_mperf {
  * @cur: Difference of Aperf/Mperf/tsc count between last and current sample
  * @prev: Last Aperf/Mperf/tsc count value read from register
  * @freq: current cpu frequency value
- * @boost_supported: check whether the Processor or SBIOS supports boost mode
  * @hw_prefcore: check whether HW supports preferred core featue.
  * 		  Only when hw_prefcore and early prefcore param are true,
  * 		  AMD P-State driver supports preferred core featue.

```

> Perry.
> 
> >
> > >     bool    hw_prefcore;
> > >
> > >     /* EPP feature related attributes*/
> > >
> >
> >
> > --
> > Oleksandr Natalenko (post-factum)
> 


-- 
Oleksandr Natalenko (post-factum)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost callback for passive mode

[Yuan, Perry]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Oleksandr Natalenko <oleksandr@natalenko.name>
> Sent: Wednesday, February 7, 2024 9:27 PM
> To: rafael.j.wysocki@intel.com; Limonciello, Mario
> <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray
> <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy set_boost
> callback for passive mode
>
> On středa 7. února 2024 14:17:06 CET Yuan, Perry wrote:
> > [AMD Official Use Only - General]
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Oleksandr Natalenko <oleksandr@natalenko.name>
> > > Sent: Wednesday, February 7, 2024 6:11 PM
> > > To: rafael.j.wysocki@intel.com; Limonciello, Mario
> > > <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray
> > > <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal
> > > <gautham.shenoy@amd.com>; Petkov, Borislav
> > > <Borislav.Petkov@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>
> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang,
> Shimmer
> > > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>;
> Meng,
> > > Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> > > kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 6/7] cpufreq: amd-pstate: remove legacy
> > > set_boost callback for passive mode
> > >
> > > Hello.
> > >
> > > On středa 7. února 2024 10:21:57 CET Perry Yuan wrote:
> > > > With new freqency boost interface supported, legacy boost control
> > > > doesn't make sense any more which only support passive mode.
> > > > so it can remove the legacy set_boost interface from amd-pstate
> > > > driver in case of there is conflict with new boost control logic.
> > > >
> > > > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > > > ---
> > > >  drivers/cpufreq/amd-pstate.c | 1 -
> > > >  include/linux/amd-pstate.h   | 1 -
> > > >  2 files changed, 2 deletions(-)
> > > >
> > > > diff --git a/drivers/cpufreq/amd-pstate.c
> > > > b/drivers/cpufreq/amd-pstate.c index 35791efc6e88..1dd523db3871
> > > 100644
> > > > --- a/drivers/cpufreq/amd-pstate.c
> > > > +++ b/drivers/cpufreq/amd-pstate.c
> > > > @@ -1675,7 +1675,6 @@ static struct cpufreq_driver
> > > > amd_pstate_driver =
> > > {
> > > >     .exit           = amd_pstate_cpu_exit,
> > > >     .suspend        = amd_pstate_cpu_suspend,
> > > >     .resume         = amd_pstate_cpu_resume,
> > > > -   .set_boost      = amd_pstate_set_boost,
> > > >     .update_limits  = amd_pstate_update_limits,
> > > >     .name           = "amd-pstate",
> > > >     .attr           = amd_pstate_attr,
> > > > diff --git a/include/linux/amd-pstate.h
> > > > b/include/linux/amd-pstate.h index 465e9295a60c..ab7ca26974da
> > > > 100644
> > > > --- a/include/linux/amd-pstate.h
> > > > +++ b/include/linux/amd-pstate.h
> > > > @@ -93,7 +93,6 @@ struct amd_cpudata {
> > > >     struct amd_aperf_mperf prev;
> > > >
> > > >     u64     freq;
> > > > -   bool    boost_supported;
> > >
> > > As a result of this removal the kernel-doc for this struct should be
> > > amended too because even after this patch is applied the
> > > `boost_supported` field remains documented.
> >
> > The set_boost is callback used by cpufreq.c, the boost_supported is not
> documented before the patch.
> > We just need to add new sysfs "cpb_boost" introduction in the
> "/Documentation/admin-guide/pm/amd-pstate.rst"
> > The legacy boost is not supported any more, we have no need to
> document it again as it is.
>
> `boost_supported` as a field of `struct amd_cpudata` is documented in the
> kernel-doc right before the structure, and this bit should be removed like
> shown below:

You mean the comment in the amd-pstate.h, right?
I misunderstood that you are referring to amd-pstate.rst Doc.
The comment is removed from v4, will send it out after other feedback collected.
Thanks for the review!

Perry.

- * @boost_supported: check whether the Processor or SBIOS supports
> boost mode

>
> ```
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -56,7 +56,6 @@ struct amd_aperf_mperf {
>   * @cur: Difference of Aperf/Mperf/tsc count between last and current
> sample
>   * @prev: Last Aperf/Mperf/tsc count value read from register
>   * @freq: current cpu frequency value
> - * @boost_supported: check whether the Processor or SBIOS supports
> boost mode
>   * @hw_prefcore: check whether HW supports preferred core featue.
>   *             Only when hw_prefcore and early prefcore param are true,
>   *             AMD P-State driver supports preferred core featue.
>
> ```
>
> > Perry.
> >
> > >
> > > >     bool    hw_prefcore;
> > > >
> > > >     /* EPP feature related attributes*/
> > > >
> > >
> > >
> > > --
> > > Oleksandr Natalenko (post-factum)
> >
>
>
> --
> Oleksandr Natalenko (post-factum)

Re: [PATCH v3 1/7] cpufreq: amd-pstate: initialize new core precision boost state

[Oleksandr Natalenko]

[-- Attachment #1: Type: text/plain, Size: 5515 bytes --]

On středa 7. února 2024 10:21:52 CET Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> Add gloal global_params to represent current CPU Performance Boost(cpb)
> state for cpu frequency scaling, both active and passive modes all can
> support CPU cores frequency boosting control which is based on the BIOS
> setting, while BIOS turn on the "Core Performance Boost", it will
> allow OS control each core highest perf limitation from OS side.
> 
> If core performance boost is disabled while a core is in a boosted P-state,
> the core transitions to the highest performance non-boosted P-state,
> that is the same as the nominal frequency limit.
> 
> Reported-by: Artem S. Tashkinov" <aros@gmx.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 49 ++++++++++++------------------------
>  include/linux/amd-pstate.h   | 14 +++++++++++
>  2 files changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 08e112444c27..3772f71f525f 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -67,6 +67,7 @@ static struct cpufreq_driver amd_pstate_epp_driver;
>  static int cppc_state = AMD_PSTATE_UNDEFINED;
>  static bool cppc_enabled;
>  static bool amd_pstate_prefcore = true;
> +struct global_params global;

Doesn't this require `EXPORT_SYMBOL_GPL(global)` then? Otherwise with v6.8 I get this:

```
ERROR: modpost: "global" [drivers/cpufreq/amd-pstate-ut.ko] undefined!
```

If so, I'd pretty mush rename this to be less generic, say, `amd_pstate_global_params_struct` for the struct name itself and `amd_pstate_global_params` for the actual variable name.

>  
>  /*
>   * AMD Energy Preference Performance (EPP)
> @@ -669,43 +670,21 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
>  	return lowest_nonlinear_freq * 1000;
>  }
>  
> -static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> +static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
>  {
> -	struct amd_cpudata *cpudata = policy->driver_data;
> +	u64 boost_val;
>  	int ret;
>  
> -	if (!cpudata->boost_supported) {
> -		pr_err("Boost mode is not supported by this processor or SBIOS\n");
> -		return -EINVAL;
> -	}
> -
> -	if (state)
> -		policy->cpuinfo.max_freq = cpudata->max_freq;
> -	else
> -		policy->cpuinfo.max_freq = cpudata->nominal_freq;
> -
> -	policy->max = policy->cpuinfo.max_freq;
> -
> -	ret = freq_qos_update_request(&cpudata->req[1],
> -				      policy->cpuinfo.max_freq);
> -	if (ret < 0)
> +	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
> +	if (ret) {
> +		pr_err_once("failed to read initial CPU boost state!\n");
>  		return ret;
> +	}
>  
> -	return 0;
> -}
> -
> -static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
> -{
> -	u32 highest_perf, nominal_perf;
> -
> -	highest_perf = READ_ONCE(cpudata->highest_perf);
> -	nominal_perf = READ_ONCE(cpudata->nominal_perf);
> -
> -	if (highest_perf <= nominal_perf)
> -		return;
> +	global.cpb_supported = !((boost_val >> 25) & 0x1);
> +	global.cpb_boost = global.cpb_supported;
>  
> -	cpudata->boost_supported = true;
> -	current_pstate_driver->boost_enabled = true;
> +	return ret;
>  }
>  
>  static void amd_perf_ctl_reset(unsigned int cpu)
> @@ -848,6 +827,9 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>  	if (ret)
>  		goto free_cpudata1;
>  
> +	/* initialize cpu cores boot state */
> +	amd_pstate_boost_init(cpudata);
> +
>  	min_freq = amd_get_min_freq(cpudata);
>  	max_freq = amd_get_max_freq(cpudata);
>  	nominal_freq = amd_get_nominal_freq(cpudata);
> @@ -899,7 +881,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>  
>  	policy->driver_data = cpudata;
>  
> -	amd_pstate_boost_init(cpudata);
>  	if (!current_pstate_driver->adjust_perf)
>  		current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
>  
> @@ -1310,6 +1291,9 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  	if (ret)
>  		goto free_cpudata1;
>  
> +	/* initialize cpu cores boot state */
> +	amd_pstate_boost_init(cpudata);
> +
>  	min_freq = amd_get_min_freq(cpudata);
>  	max_freq = amd_get_max_freq(cpudata);
>  	nominal_freq = amd_get_nominal_freq(cpudata);
> @@ -1360,7 +1344,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  			return ret;
>  		WRITE_ONCE(cpudata->cppc_cap1_cached, value);
>  	}
> -	amd_pstate_boost_init(cpudata);
>  
>  	return 0;
>  
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index d21838835abd..465e9295a60c 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -124,4 +124,18 @@ static const char * const amd_pstate_mode_string[] = {
>  	[AMD_PSTATE_GUIDED]      = "guided",
>  	NULL,
>  };
> +
> +/**
> + * struct global_params - Global parameters, mostly tunable via sysfs.
> + * @cpb_boost:		Whether or not to use boost CPU P-states.
> + * @cpb_supported:	Whether or not CPU boost P-states are available
> + *			based on the MSR_K7_HWCR bit[25] state
> + */
> +struct global_params {
> +	bool cpb_boost;
> +	bool cpb_supported;
> +};
> +
> +extern struct global_params global;
> +
>  #endif /* _LINUX_AMD_PSTATE_H */
> 


-- 
Oleksandr Natalenko (post-factum)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/7] cpufreq: amd-pstate: initialize new core precision boost state

[Oleksandr Natalenko]

[-- Attachment #1: Type: text/plain, Size: 10369 bytes --]

On středa 7. února 2024 15:45:20 CET Oleksandr Natalenko wrote:
> On středa 7. února 2024 10:21:52 CET Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> > 
> > Add gloal global_params to represent current CPU Performance Boost(cpb)
> > state for cpu frequency scaling, both active and passive modes all can
> > support CPU cores frequency boosting control which is based on the BIOS
> > setting, while BIOS turn on the "Core Performance Boost", it will
> > allow OS control each core highest perf limitation from OS side.
> > 
> > If core performance boost is disabled while a core is in a boosted P-state,
> > the core transitions to the highest performance non-boosted P-state,
> > that is the same as the nominal frequency limit.
> > 
> > Reported-by: Artem S. Tashkinov" <aros@gmx.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 49 ++++++++++++------------------------
> >  include/linux/amd-pstate.h   | 14 +++++++++++
> >  2 files changed, 30 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 08e112444c27..3772f71f525f 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -67,6 +67,7 @@ static struct cpufreq_driver amd_pstate_epp_driver;
> >  static int cppc_state = AMD_PSTATE_UNDEFINED;
> >  static bool cppc_enabled;
> >  static bool amd_pstate_prefcore = true;
> > +struct global_params global;
> 
> Doesn't this require `EXPORT_SYMBOL_GPL(global)` then? Otherwise with v6.8 I get this:
> 
> ```
> ERROR: modpost: "global" [drivers/cpufreq/amd-pstate-ut.ko] undefined!
> ```
> 
> If so, I'd pretty mush rename this to be less generic, say, `amd_pstate_global_params_struct` for the struct name itself and `amd_pstate_global_params` for the actual variable name.

Something like this should do the trick (compile-tested only):

```
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 91780ec42712..c5e2ca02f5ea 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -226,7 +226,7 @@ static void amd_pstate_ut_check_freq(u32 index)
 			goto skip_test;
 		}
 
-		if (global.cpb_supported) {
+		if (amd_pstate_global_params.cpb_supported) {
 			if ((policy->max == cpudata->max_freq) ||
 					(policy->max == cpudata->nominal_freq))
 				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index aebecd4e2e73..3e23d114c73f 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -68,7 +68,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 static bool amd_pstate_prefcore = true;
 static struct quirk_entry *quirks;
-struct global_params global;
+struct amd_pstate_global_params amd_pstate_global_params;
+EXPORT_SYMBOL_GPL(amd_pstate_global_params);
 
 /*
  * AMD Energy Preference Performance (EPP)
@@ -496,7 +497,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 	value |= AMD_CPPC_DES_PERF(des_perf);
 
 	/* limit the max perf when core performance boost feature is disabled */
-	if (!global.cpb_boost)
+	if (!amd_pstate_global_params.cpb_boost)
 		max_perf = min_t(unsigned long, nominal_perf, max_perf);
 
 	value &= ~AMD_CPPC_MAX_PERF(~0L);
@@ -672,7 +673,7 @@ static int amd_get_max_freq(struct amd_cpudata *cpudata)
 	max_perf = READ_ONCE(cpudata->highest_perf);
 
 	/* when boost is off, the highest perf will be limited to nominal_perf */
-	if (!global.cpb_boost)
+	if (!amd_pstate_global_params.cpb_boost)
 		max_perf = nominal_perf;
 
 	boost_ratio = div_u64(max_perf << SCHED_CAPACITY_SHIFT,
@@ -737,8 +738,8 @@ static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
 		return ret;
 	}
 
-	global.cpb_supported = !((boost_val >> 25) & 0x1);
-	global.cpb_boost = global.cpb_supported;
+	amd_pstate_global_params.cpb_supported = !((boost_val >> 25) & 0x1);
+	amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
 
 	return ret;
 }
@@ -1337,7 +1338,7 @@ static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
 static ssize_t cpb_boost_show(struct device *dev,
 			   struct device_attribute *attr, char *buf)
 {
-	return sysfs_emit(buf, "%u\n", global.cpb_boost);
+	return sysfs_emit(buf, "%u\n", amd_pstate_global_params.cpb_boost);
 }
 
 static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b,
@@ -1348,7 +1349,7 @@ static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b,
 	int cpu;
 
 	mutex_lock(&amd_pstate_driver_lock);
-	if (!global.cpb_supported) {
+	if (!amd_pstate_global_params.cpb_supported) {
 		pr_err("Boost mode is not supported by this processor or SBIOS\n");
 		return -EINVAL;
 	}
@@ -1357,7 +1358,7 @@ static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b,
 	if (ret)
 		return -EINVAL;
 
-	global.cpb_boost = !!new_state;
+	amd_pstate_global_params.cpb_boost = !!new_state;
 
 	for_each_possible_cpu(cpu) {
 
@@ -1371,7 +1372,7 @@ static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b,
 			goto err_exit;
 		}
 
-		amd_cpu_boost_update(cpudata, global.cpb_boost);
+		amd_cpu_boost_update(cpudata, amd_pstate_global_params.cpb_boost);
 		refresh_frequency_limits(policy);
 		cpufreq_cpu_put(policy);
 	}
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index b0db335f3883..f6e2c9825700 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -129,16 +129,16 @@ struct quirk_entry {
 };
 
 /**
- * struct global_params - Global parameters, mostly tunable via sysfs.
+ * struct amd_pstate_global_params - Global parameters, mostly tunable via sysfs.
  * @cpb_boost:		Whether or not to use boost CPU P-states.
  * @cpb_supported:	Whether or not CPU boost P-states are available
  *			based on the MSR_K7_HWCR bit[25] state
  */
-struct global_params {
+struct amd_pstate_global_params {
 	bool cpb_boost;
 	bool cpb_supported;
 };
 
-extern struct global_params global;
+extern struct amd_pstate_global_params amd_pstate_global_params;
 
 #endif /* _LINUX_AMD_PSTATE_H */
```

> >  
> >  /*
> >   * AMD Energy Preference Performance (EPP)
> > @@ -669,43 +670,21 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
> >  	return lowest_nonlinear_freq * 1000;
> >  }
> >  
> > -static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> > +static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
> >  {
> > -	struct amd_cpudata *cpudata = policy->driver_data;
> > +	u64 boost_val;
> >  	int ret;
> >  
> > -	if (!cpudata->boost_supported) {
> > -		pr_err("Boost mode is not supported by this processor or SBIOS\n");
> > -		return -EINVAL;
> > -	}
> > -
> > -	if (state)
> > -		policy->cpuinfo.max_freq = cpudata->max_freq;
> > -	else
> > -		policy->cpuinfo.max_freq = cpudata->nominal_freq;
> > -
> > -	policy->max = policy->cpuinfo.max_freq;
> > -
> > -	ret = freq_qos_update_request(&cpudata->req[1],
> > -				      policy->cpuinfo.max_freq);
> > -	if (ret < 0)
> > +	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
> > +	if (ret) {
> > +		pr_err_once("failed to read initial CPU boost state!\n");
> >  		return ret;
> > +	}
> >  
> > -	return 0;
> > -}
> > -
> > -static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
> > -{
> > -	u32 highest_perf, nominal_perf;
> > -
> > -	highest_perf = READ_ONCE(cpudata->highest_perf);
> > -	nominal_perf = READ_ONCE(cpudata->nominal_perf);
> > -
> > -	if (highest_perf <= nominal_perf)
> > -		return;
> > +	global.cpb_supported = !((boost_val >> 25) & 0x1);
> > +	global.cpb_boost = global.cpb_supported;
> >  
> > -	cpudata->boost_supported = true;
> > -	current_pstate_driver->boost_enabled = true;
> > +	return ret;
> >  }
> >  
> >  static void amd_perf_ctl_reset(unsigned int cpu)
> > @@ -848,6 +827,9 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> >  	if (ret)
> >  		goto free_cpudata1;
> >  
> > +	/* initialize cpu cores boot state */
> > +	amd_pstate_boost_init(cpudata);
> > +
> >  	min_freq = amd_get_min_freq(cpudata);
> >  	max_freq = amd_get_max_freq(cpudata);
> >  	nominal_freq = amd_get_nominal_freq(cpudata);
> > @@ -899,7 +881,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> >  
> >  	policy->driver_data = cpudata;
> >  
> > -	amd_pstate_boost_init(cpudata);
> >  	if (!current_pstate_driver->adjust_perf)
> >  		current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
> >  
> > @@ -1310,6 +1291,9 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> >  	if (ret)
> >  		goto free_cpudata1;
> >  
> > +	/* initialize cpu cores boot state */
> > +	amd_pstate_boost_init(cpudata);
> > +
> >  	min_freq = amd_get_min_freq(cpudata);
> >  	max_freq = amd_get_max_freq(cpudata);
> >  	nominal_freq = amd_get_nominal_freq(cpudata);
> > @@ -1360,7 +1344,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> >  			return ret;
> >  		WRITE_ONCE(cpudata->cppc_cap1_cached, value);
> >  	}
> > -	amd_pstate_boost_init(cpudata);
> >  
> >  	return 0;
> >  
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index d21838835abd..465e9295a60c 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -124,4 +124,18 @@ static const char * const amd_pstate_mode_string[] = {
> >  	[AMD_PSTATE_GUIDED]      = "guided",
> >  	NULL,
> >  };
> > +
> > +/**
> > + * struct global_params - Global parameters, mostly tunable via sysfs.
> > + * @cpb_boost:		Whether or not to use boost CPU P-states.
> > + * @cpb_supported:	Whether or not CPU boost P-states are available
> > + *			based on the MSR_K7_HWCR bit[25] state
> > + */
> > +struct global_params {
> > +	bool cpb_boost;
> > +	bool cpb_supported;
> > +};
> > +
> > +extern struct global_params global;
> > +
> >  #endif /* _LINUX_AMD_PSTATE_H */
> > 
> 
> 
> 


-- 
Oleksandr Natalenko (post-factum)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 1/7] cpufreq: amd-pstate: initialize new core precision boost state

[Yuan, Perry]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Oleksandr Natalenko <oleksandr@natalenko.name>
> Sent: Wednesday, February 7, 2024 10:45 PM
> To: rafael.j.wysocki@intel.com; Limonciello, Mario
> <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray
> <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v3 1/7] cpufreq: amd-pstate: initialize new core precision
> boost state
>
> On středa 7. února 2024 10:21:52 CET Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > Add gloal global_params to represent current CPU Performance
> > Boost(cpb) state for cpu frequency scaling, both active and passive
> > modes all can support CPU cores frequency boosting control which is
> > based on the BIOS setting, while BIOS turn on the "Core Performance
> > Boost", it will allow OS control each core highest perf limitation from OS
> side.
> >
> > If core performance boost is disabled while a core is in a boosted
> > P-state, the core transitions to the highest performance non-boosted
> > P-state, that is the same as the nominal frequency limit.
> >
> > Reported-by: Artem S. Tashkinov" <aros@gmx.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 49 ++++++++++++------------------------
> >  include/linux/amd-pstate.h   | 14 +++++++++++
> >  2 files changed, 30 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 08e112444c27..3772f71f525f 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -67,6 +67,7 @@ static struct cpufreq_driver amd_pstate_epp_driver;
> > static int cppc_state = AMD_PSTATE_UNDEFINED;  static bool
> > cppc_enabled;  static bool amd_pstate_prefcore = true;
> > +struct global_params global;
>
> Doesn't this require `EXPORT_SYMBOL_GPL(global)` then? Otherwise with
> v6.8 I get this:
>

Will check this with v6.8 as well.

> ```
> ERROR: modpost: "global" [drivers/cpufreq/amd-pstate-ut.ko] undefined!
> ```
>
> If so, I'd pretty mush rename this to be less generic, say,
> `amd_pstate_global_params_struct` for the struct name itself and
> `amd_pstate_global_params` for the actual variable name.

Let me optimize and rename the struct definition and var name.

Thanks for the feedback.
Perry.

>
> >
> >  /*
> >   * AMD Energy Preference Performance (EPP) @@ -669,43 +670,21 @@
> > static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
> >     return lowest_nonlinear_freq * 1000;  }
> >
> > -static int amd_pstate_set_boost(struct cpufreq_policy *policy, int
> > state)
> > +static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
> >  {
> > -   struct amd_cpudata *cpudata = policy->driver_data;
> > +   u64 boost_val;
> >     int ret;
> >
> > -   if (!cpudata->boost_supported) {
> > -           pr_err("Boost mode is not supported by this processor or
> SBIOS\n");
> > -           return -EINVAL;
> > -   }
> > -
> > -   if (state)
> > -           policy->cpuinfo.max_freq = cpudata->max_freq;
> > -   else
> > -           policy->cpuinfo.max_freq = cpudata->nominal_freq;
> > -
> > -   policy->max = policy->cpuinfo.max_freq;
> > -
> > -   ret = freq_qos_update_request(&cpudata->req[1],
> > -                                 policy->cpuinfo.max_freq);
> > -   if (ret < 0)
> > +   ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
> > +   if (ret) {
> > +           pr_err_once("failed to read initial CPU boost state!\n");
> >             return ret;
> > +   }
> >
> > -   return 0;
> > -}
> > -
> > -static void amd_pstate_boost_init(struct amd_cpudata *cpudata) -{
> > -   u32 highest_perf, nominal_perf;
> > -
> > -   highest_perf = READ_ONCE(cpudata->highest_perf);
> > -   nominal_perf = READ_ONCE(cpudata->nominal_perf);
> > -
> > -   if (highest_perf <= nominal_perf)
> > -           return;
> > +   global.cpb_supported = !((boost_val >> 25) & 0x1);
> > +   global.cpb_boost = global.cpb_supported;
> >
> > -   cpudata->boost_supported = true;
> > -   current_pstate_driver->boost_enabled = true;
> > +   return ret;
> >  }
> >
> >  static void amd_perf_ctl_reset(unsigned int cpu) @@ -848,6 +827,9 @@
> > static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> >     if (ret)
> >             goto free_cpudata1;
> >
> > +   /* initialize cpu cores boot state */
> > +   amd_pstate_boost_init(cpudata);
> > +
> >     min_freq = amd_get_min_freq(cpudata);
> >     max_freq = amd_get_max_freq(cpudata);
> >     nominal_freq = amd_get_nominal_freq(cpudata); @@ -899,7 +881,6
> @@
> > static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> >
> >     policy->driver_data = cpudata;
> >
> > -   amd_pstate_boost_init(cpudata);
> >     if (!current_pstate_driver->adjust_perf)
> >             current_pstate_driver->adjust_perf =
> amd_pstate_adjust_perf;
> >
> > @@ -1310,6 +1291,9 @@ static int amd_pstate_epp_cpu_init(struct
> cpufreq_policy *policy)
> >     if (ret)
> >             goto free_cpudata1;
> >
> > +   /* initialize cpu cores boot state */
> > +   amd_pstate_boost_init(cpudata);
> > +
> >     min_freq = amd_get_min_freq(cpudata);
> >     max_freq = amd_get_max_freq(cpudata);
> >     nominal_freq = amd_get_nominal_freq(cpudata); @@ -1360,7
> +1344,6 @@
> > static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> >                     return ret;
> >             WRITE_ONCE(cpudata->cppc_cap1_cached, value);
> >     }
> > -   amd_pstate_boost_init(cpudata);
> >
> >     return 0;
> >
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index d21838835abd..465e9295a60c 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -124,4 +124,18 @@ static const char * const
> amd_pstate_mode_string[] = {
> >     [AMD_PSTATE_GUIDED]      = "guided",
> >     NULL,
> >  };
> > +
> > +/**
> > + * struct global_params - Global parameters, mostly tunable via sysfs.
> > + * @cpb_boost:             Whether or not to use boost CPU P-states.
> > + * @cpb_supported: Whether or not CPU boost P-states are
> available
> > + *                 based on the MSR_K7_HWCR bit[25] state
> > + */
> > +struct global_params {
> > +   bool cpb_boost;
> > +   bool cpb_supported;
> > +};
> > +
> > +extern struct global_params global;
> > +
> >  #endif /* _LINUX_AMD_PSTATE_H */
> >
>
>
> --
> Oleksandr Natalenko (post-factum)