Skip to contentSkip to conversation

AMD Pstate Driver Core Performance Boost

PATCH Superseded Perry Yuan 4 participants 21 messages Jan 26, 2024 – Feb 14, 2024

Subsystems (3)
Documentation CPU frequency scaling framework AMD pstate driver

v1 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Perry Yuan

author reviewer
#0 lore raw

cover AMD Pstate Driver Core Performance Boost

The patchset series add core performance boost feature for AMD pstate driver including passisve 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 actrive and passive mode.

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 Yuan (7):
cpufreq: amd-pstate: remove set_boost callback for passive mode
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
cpufreq:amd-pstate: add suspend and resume callback for passive mode
Documentation: cpufreq: amd-pstate: introduce the new cpu boost
control method

 Documentation/admin-guide/pm/amd-pstate.rst |  11 +
 drivers/cpufreq/amd-pstate.c                | 222 ++++++++++++++++----
 include/linux/amd-pstate.h                  |   1 -
 3 files changed, 194 insertions(+), 40 deletions(-)
Signature · 1 line
2.34.1
author reviewer
#1 lore raw

PATCH 1/7 cpufreq: amd-pstate: remove set_boost callback for passive mode

From: Perry Yuan <[email protected]>

The following patches will enable amd-pstate CPU boost control method which will not need this common boost control callback anymore, so we remove the legacy set_boost interface from amd-pstate driver.

Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 26 --------------------------
 1 file changed, 26 deletions(-)
drivers/cpufreq/amd-pstate.c +0 -26
index 9a1e194d5cf8..8f308f56ade6 100644
@@ -632,31 +632,6 @@ 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)
-{
-	struct amd_cpudata *cpudata = policy->driver_data;
-	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)
-		return ret;
-
-	return 0;
-}
-
 static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
 {
 	u32 highest_perf, nominal_perf;

@@ -1391,7 +1366,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,
 	.name		= "amd-pstate",
 	.attr		= amd_pstate_attr,
 };
Signature · 1 line
2.34.1
author reviewer
#4 lore raw

PATCH 2/7 cpufreq: amd-pstate: initialize new core precision boost state

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

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.

Issue: https://bugzilla.kernel.org/show_bug.cgi?id=217931

Reported-by Artem S. Tashkinov" <[email protected]>
Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 46 ++++++++++++++++++++++++++++--------
 include/linux/amd-pstate.h   |  1 -
 2 files changed, 36 insertions(+), 11 deletions(-)
drivers/cpufreq/amd-pstate.c +36 -10
index 8f308f56ade6..0dc9124140d4 100644
@@ -65,6 +65,19 @@ static struct cpufreq_driver amd_pstate_epp_driver;
 static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 
+/**
+ * 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;
+};
+
+static struct global_params global;
+
 /*
  * AMD Energy Preference Performance (EPP)
  * The EPP is used in the CCLK DPM controller to drive

@@ -632,18 +645,27 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
 	return lowest_nonlinear_freq * 1000;
 }
 
-static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
+static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
 {
-	u32 highest_perf, nominal_perf;
+	u64 boost_state, boost_val;
+	int ret;
 
-	highest_perf = READ_ONCE(cpudata->highest_perf);
-	nominal_perf = READ_ONCE(cpudata->nominal_perf);
+	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;
+	}
 
-	if (highest_perf <= nominal_perf)
-		return;
+	boost_state = (boost_val >> 25) & 0x1;
+	if (!boost_state) {
+		global.cpb_supported = true;
+		global.cpb_boost = true;
+	} else {
+		global.cpb_supported = false;
+		global.cpb_boost = false;
+	}
 
-	cpudata->boost_supported = true;
-	current_pstate_driver->boost_enabled = true;
+	return ret;
 }
 
 static void amd_perf_ctl_reset(unsigned int cpu)

@@ -676,6 +698,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);

@@ -725,7 +750,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;
 

@@ -1093,6 +1117,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);

@@ -1143,7 +1170,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;
include/linux/amd-pstate.h +0 -1
index 446394f84606..66d939a344b1 100644
@@ -80,7 +80,6 @@ struct amd_cpudata {
 	struct amd_aperf_mperf prev;
 
 	u64	freq;
-	bool	boost_supported;
 
 	/* EPP feature related attributes*/
 	s16	epp_policy;
Signature · 1 line
2.34.1
author reviewer
#9 lore raw

PATCH 3/7 cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, user can change CPU boost state dynamically under active 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 <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 97 ++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
drivers/cpufreq/amd-pstate.c +97 -0
index 0dc9124140d4..b37bea7440b9 100644
@@ -1036,6 +1036,101 @@ static ssize_t status_store(struct device *a, struct device_attribute *b,
 	return ret < 0 ? ret : count;
 }
 
+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);
 

@@ -1043,6 +1138,7 @@ cpufreq_freq_attr_ro(amd_pstate_highest_perf);
 cpufreq_freq_attr_rw(energy_performance_preference);
 cpufreq_freq_attr_ro(energy_performance_available_preferences);
 static DEVICE_ATTR_RW(status);
+static DEVICE_ATTR_RW(cpb_boost);
 
 static struct freq_attr *amd_pstate_attr[] = {
 	&amd_pstate_max_freq,

@@ -1062,6 +1158,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
 
 static struct attribute *pstate_global_attributes[] = {
 	&dev_attr_status.attr,
+	&dev_attr_cpb_boost.attr,
 	NULL
 };
Signature · 1 line
2.34.1
author reviewer
#12 lore raw

PATCH 4/7 cpufreq: amd-pstate: fix max_perf calculation for amd_get_max_freq()

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

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 <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/cpufreq/amd-pstate.c +4 -0
index b37bea7440b9..3286d72f375e 100644
@@ -599,6 +599,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);
Signature · 1 line
2.34.1
author reviewer
#15 lore raw

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

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

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 <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/cpufreq/amd-pstate.c +5 -0
index 3286d72f375e..5cbbc2999d9a 100644
@@ -443,6 +443,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);
+	u64 nominal_perf = READ_ONCE(cpudata->nominal_perf);
 	u64 value = prev;
 
 	des_perf = clamp_t(unsigned long, des_perf, min_perf, max_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);
Signature · 1 line
2.34.1
author reviewer
#16 lore raw

PATCH 6/7 cpufreq:amd-pstate: add suspend and resume callback for passive mode

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

Add suspend and resume support for passive mode driver which can save the previous CPU Pstate values and restore them while resuming, on some old platforms, the highest perf needs to be restored from driver side, otherwise the highest frequency could be changed during suspend.

Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 48 ++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)
drivers/cpufreq/amd-pstate.c +43 -5
index 5cbbc2999d9a..bba7640d46e0 100644
@@ -785,23 +785,61 @@ static int amd_pstate_cpu_exit(struct cpufreq_policy *policy)
 
 static int amd_pstate_cpu_resume(struct cpufreq_policy *policy)
 {
+	struct cppc_perf_ctrls perf_ctrls;
+	struct amd_cpudata *cpudata = policy->driver_data;
+	u64 value, max_perf;
 	int ret;
 
-	ret = amd_pstate_enable(true);
-	if (ret)
-		pr_err("failed to enable amd-pstate during resume, return %d\n", ret);
+	if (cpudata->suspended) {
+		mutex_lock(&amd_pstate_driver_lock);
 
-	return ret;
+		ret = amd_pstate_enable(true);
+		if (ret) {
+			pr_err("failed to enable amd-pstate during resume, return %d\n", ret);
+			mutex_unlock(&amd_pstate_driver_lock);
+			return ret;
+		}
+
+		value = READ_ONCE(cpudata->cppc_req_cached);
+		max_perf = READ_ONCE(cpudata->highest_perf);
+
+		if (boot_cpu_has(X86_FEATURE_CPPC)) {
+			wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+		} else {
+			perf_ctrls.max_perf = max_perf;
+			cppc_set_perf(cpudata->cpu, &perf_ctrls);
+		}
+
+		cpudata->suspended = false;
+		mutex_unlock(&amd_pstate_driver_lock);
+	}
+
+	return 0;
 }
 
 static int amd_pstate_cpu_suspend(struct cpufreq_policy *policy)
 {
+	struct amd_cpudata *cpudata = policy->driver_data;
 	int ret;
 
+	/* avoid suspending when EPP is not enabled */
+	if (cppc_state != AMD_PSTATE_PASSIVE)
+		return 0;
+
+	mutex_lock(&amd_pstate_driver_lock);
+
+	/* set this flag to avoid calling core offline function
+	 * when system is suspending, use this flag to skip offline function
+	 * called
+	 */
+	cpudata->suspended = true;
+
 	ret = amd_pstate_enable(false);
 	if (ret)
 		pr_err("failed to disable amd-pstate during suspend, return %d\n", ret);
 
+	mutex_unlock(&amd_pstate_driver_lock);
+
 	return ret;
 }
 

@@ -1460,7 +1498,7 @@ static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
 	if (cppc_state != AMD_PSTATE_ACTIVE)
 		return 0;
 
-	/* set this flag to avoid setting core offline*/
+	/* set this flag to avoid setting core offline */
 	cpudata->suspended = true;
 
 	/* disable CPPC in lowlevel firmware */
Signature · 1 line
2.34.1
author reviewer
#18 lore raw

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

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

Introduce AMD CPU frequency boosting control sysfs entry which userd 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 <[email protected]>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)
Documentation/admin-guide/pm/amd-pstate.rst +11 -0
index 1cf40f69278c..d72dc407c4db 100644
@@ -385,6 +385,17 @@ control its functionality at the system level.  They are located in the
         to the operation mode represented by that string - or to be
         unregistered in the "disable" case.
 
+``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 mode`` and ``passive mode`` control with below value write to it.
+
+        "0" Disable Core performance Boosting
+        "1" Enable  Core performance Boosting
+
 ``cpupower`` tool support for ``amd-pstate``
 ===============================================
Signature · 1 line
2.34.1
maintainer
#2 lore raw

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

In reply to Perry Yuan
Quoted 3 lines · Perry Yuan

From: Perry Yuan <[email protected]>

The following patches will enable amd-pstate CPU boost control method

When it's committed it won't be a patch. How about instead "A specific amd-pstate CPU boost control method is to be introduced and the legacy callback doesn't make sense" or something along those lines.

Also; is the ordering correct? In terms of bisectability should this come after the new one is introduced perhaps?

Quoted 52 lines

which will not need this common boost control callback anymore, so we remove the legacy set_boost interface from amd-pstate driver.

Signed-off-by Perry Yuan <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 26 --------------------------
  1 file changed, 26 deletions(-)
drivers/cpufreq/amd-pstate.c +0 -26
index 9a1e194d5cf8..8f308f56ade6 100644
@@ -632,31 +632,6 @@ 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)
-{
-	struct amd_cpudata *cpudata = policy->driver_data;
-	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)
-		return ret;
-
-	return 0;
-}
-
  static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
  {
  	u32 highest_perf, nominal_perf;

@@ -1391,7 +1366,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,
  	.name		= "amd-pstate",
  	.attr		= amd_pstate_attr,
  };
maintainer
#19 lore raw

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

In reply to Perry Yuan
Quoted 4 lines · Perry Yuan

From: Perry Yuan <[email protected]>

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

Typo in this sentence.

Quoted 15 lines

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 <[email protected]>
---
  Documentation/admin-guide/pm/amd-pstate.rst | 11 +++++++++++
  1 file changed, 11 insertions(+)
Documentation/admin-guide/pm/amd-pstate.rst +0 -0
index 1cf40f69278c..d72dc407c4db 100644

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They are located in the · line 394

Quoted 10 lines
Documentation/admin-guide/pm/amd-pstate.rst +7 -0
@@ -385,6 +385,17 @@ control its functionality at the system level.  They are located in the
          to the operation mode represented by that string - or to be
          unregistered in the "disable" case.
 
+``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 mode`` and ``passive mode`` control with below value write to it.

Does it also support guided mode?

Quoted 7 lines
Documentation/admin-guide/pm/amd-pstate.rst +4 -0
+
+        "0" Disable Core performance Boosting
+        "1" Enable  Core performance Boosting
+
  ``cpupower`` tool support for ``amd-pstate``
  ===============================================
maintainer
#17 lore raw

Re: [PATCH 6/7] cpufreq:amd-pstate: add suspend and resume callback for passive mode

In reply to Perry Yuan
Quoted 6 lines · Perry Yuan

From: Perry Yuan <[email protected]>

Add suspend and resume support for passive mode driver which can save the previous CPU Pstate values and restore them while resuming, on some old platforms, the highest perf needs to be restored from driver side, otherwise the highest frequency could be changed during suspend.

So this sounds like a BIOS bug, right? Do you know how far back this problem exists? Should it be a quirked behavior to only run on the broken platforms so we don't need to run the callback on modern ones without it?

Quoted 22 lines
Signed-off-by Perry Yuan <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 48 ++++++++++++++++++++++++++++++++----
  1 file changed, 43 insertions(+), 5 deletions(-)
drivers/cpufreq/amd-pstate.c +3 -3
index 5cbbc2999d9a..bba7640d46e0 100644
@@ -785,23 +785,61 @@ static int amd_pstate_cpu_exit(struct cpufreq_policy *policy)
 
  static int amd_pstate_cpu_resume(struct cpufreq_policy *policy)
  {
+	struct cppc_perf_ctrls perf_ctrls;
+	struct amd_cpudata *cpudata = policy->driver_data;
+	u64 value, max_perf;
  	int ret;
 
-	ret = amd_pstate_enable(true);
-	if (ret)
-		pr_err("failed to enable amd-pstate during resume, return %d\n", ret);

drivers/cpufreq/amd-pstate.c static int amd_pstate_cpu_exit(struct cpufreq_policy *policy) · line 801

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +8 -1
+	if (cpudata->suspended) {
+		mutex_lock(&amd_pstate_driver_lock);
 
-	return ret;
+		ret = amd_pstate_enable(true);
+		if (ret) {
+			pr_err("failed to enable amd-pstate during resume, return %d\n", ret);
+			mutex_unlock(&amd_pstate_driver_lock);
+			return ret;
+		}

This /looks/ like an unintended logic change to me. Previously amd_pstate_enable(true) would be called in all modes, but now it will only be called in passive mode.

Is that right?

Quoted 15 lines
drivers/cpufreq/amd-pstate.c +15 -0
+
+		value = READ_ONCE(cpudata->cppc_req_cached);
+		max_perf = READ_ONCE(cpudata->highest_perf);
+
+		if (boot_cpu_has(X86_FEATURE_CPPC)) {
+			wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+		} else {
+			perf_ctrls.max_perf = max_perf;
+			cppc_set_perf(cpudata->cpu, &perf_ctrls);
+		}
+
+		cpudata->suspended = false;
+		mutex_unlock(&amd_pstate_driver_lock);
+	}
+

drivers/cpufreq/amd-pstate.c static int amd_pstate_cpu_exit(struct cpufreq_policy *policy)

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +3 -0
 
+	return 0;
  }
 
  static int amd_pstate_cpu_suspend(struct cpufreq_policy *policy)
  {
+	struct amd_cpudata *cpudata = policy->driver_data;
  	int ret;
 
+	/* avoid suspending when EPP is not enabled */

The logic seems right, but shouldn't the comment be:

/* only run suspend callbacks for passive mode */

Because this stuff won't run in guided mode or disable mode either

Quoted 29 lines
drivers/cpufreq/amd-pstate.c +14 -1
+	if (cppc_state != AMD_PSTATE_PASSIVE)
+		return 0;
+
+	mutex_lock(&amd_pstate_driver_lock);
+
+	/* set this flag to avoid calling core offline function
+	 * when system is suspending, use this flag to skip offline function
+	 * called
+	 */
+	cpudata->suspended = true;
+
  	ret = amd_pstate_enable(false);
  	if (ret)
  		pr_err("failed to disable amd-pstate during suspend, return %d\n", ret);
 
+	mutex_unlock(&amd_pstate_driver_lock);
+
  	return ret;
  }

@@ -1460,7 +1498,7 @@ static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
  	if (cppc_state != AMD_PSTATE_ACTIVE)
  		return 0;
 
-	/* set this flag to avoid setting core offline*/
+	/* set this flag to avoid setting core offline */
  	cpudata->suspended = true;
 
  	/* disable CPPC in lowlevel firmware */
maintainer
#13 lore raw

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

In reply to Perry Yuan
Quoted 6 lines · Perry Yuan

From: Perry Yuan <[email protected]>

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.

What about the previous cppc_req? Shouldn't it be explicitly updated as a result of this too?

Quoted 21 lines
Signed-off-by Perry Yuan <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 4 ++++
  1 file changed, 4 insertions(+)
drivers/cpufreq/amd-pstate.c +4 -0
index b37bea7440b9..3286d72f375e 100644
@@ -599,6 +599,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);
maintainer
#10 lore raw

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

In reply to Perry Yuan
Quoted 4 lines · Perry Yuan

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, user can change CPU boost state dynamically under active and passive modes.

What about guided mode?

Quoted 35 lines

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 <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 97 ++++++++++++++++++++++++++++++++++++
  1 file changed, 97 insertions(+)
drivers/cpufreq/amd-pstate.c +7 -0
index 0dc9124140d4..b37bea7440b9 100644
@@ -1036,6 +1036,101 @@ static ssize_t status_store(struct device *a, struct device_attribute *b,
  	return ret < 0 ? ret : count;
  }
 
+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;
+

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a, struct device_attribute *b, · line 1055

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+	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);

I think it's better to use the macro AMD_CPPC_MAX_PERF in this case. The less magic masks in the code, the easier it is to follow.

Quoted 97 lines
drivers/cpufreq/amd-pstate.c +80 -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);
 

@@ -1043,6 +1138,7 @@ cpufreq_freq_attr_ro(amd_pstate_highest_perf);
  cpufreq_freq_attr_rw(energy_performance_preference);
  cpufreq_freq_attr_ro(energy_performance_available_preferences);
  static DEVICE_ATTR_RW(status);
+static DEVICE_ATTR_RW(cpb_boost);
 
  static struct freq_attr *amd_pstate_attr[] = {
  	&amd_pstate_max_freq,

@@ -1062,6 +1158,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
 
  static struct attribute *pstate_global_attributes[] = {
  	&dev_attr_status.attr,
+	&dev_attr_cpb_boost.attr,
  	NULL
  };
maintainer
#5 lore raw

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

In reply to Perry Yuan
Quoted 8 lines · Perry Yuan

From: Perry Yuan <[email protected]>

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.

Quoted 5 lines

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.

Issue: https://bugzilla.kernel.org/show_bug.cgi?id=217931

Rather than "Issue" this should be "Closes:", and the tag should come right after the Reported-by tag.

Quoted 50 lines
Reported-by Artem S. Tashkinov" <[email protected]>
Signed-off-by Perry Yuan <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 46 ++++++++++++++++++++++++++++--------
  include/linux/amd-pstate.h   |  1 -
  2 files changed, 36 insertions(+), 11 deletions(-)
drivers/cpufreq/amd-pstate.c +21 -4
index 8f308f56ade6..0dc9124140d4 100644
@@ -65,6 +65,19 @@ static struct cpufreq_driver amd_pstate_epp_driver;
  static int cppc_state = AMD_PSTATE_UNDEFINED;
  static bool cppc_enabled;
 
+/**
+ * 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;
+};
+
+static struct global_params global;
+
  /*
   * AMD Energy Preference Performance (EPP)
   * The EPP is used in the CCLK DPM controller to drive

@@ -632,18 +645,27 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
  	return lowest_nonlinear_freq * 1000;
  }
 
-static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
+static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
  {
-	u32 highest_perf, nominal_perf;
+	u64 boost_state, boost_val;
+	int ret;
 
-	highest_perf = READ_ONCE(cpudata->highest_perf);
-	nominal_perf = READ_ONCE(cpudata->nominal_perf);
+	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;
+	}

drivers/cpufreq/amd-pstate.c static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata) · line 666

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +8 -2
-	if (highest_perf <= nominal_perf)
-		return;
+	boost_state = (boost_val >> 25) & 0x1;
+	if (!boost_state) {
+		global.cpb_supported = true;
+		global.cpb_boost = true;
+	} else {
+		global.cpb_supported = false;
+		global.cpb_boost = false;
+	}

Seems like a lot of lines and an extra variable that could be simplified down to just:

global.cpb_supported = (boost_val >> 25) & 0x1;
global.cpb_boost = global.cpb_supported;

Quoted 55 lines
drivers/cpufreq/amd-pstate.c +7 -4
-	cpudata->boost_supported = true;
-	current_pstate_driver->boost_enabled = true;
+	return ret;
  }
 
  static void amd_perf_ctl_reset(unsigned int cpu)

@@ -676,6 +698,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);

@@ -725,7 +750,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;
 

@@ -1093,6 +1117,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);

@@ -1143,7 +1170,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;
include/linux/amd-pstate.h +0 -1
index 446394f84606..66d939a344b1 100644
@@ -80,7 +80,6 @@ struct amd_cpudata {
  	struct amd_aperf_mperf prev;
 
  	u64	freq;
-	bool	boost_supported;
 
  	/* EPP feature related attributes*/
  	s16	epp_policy;
build bot
#6 lore raw

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

In reply to Perry Yuan
kernel test robot noticed the following build errors:

Hi Perry,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/acpi-bus]
[also build test ERROR on v6.8-rc1 next-20240125]
[cannot apply to rafael-pm/linux-next linus/master rafael-pm/devprop] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Perry-Yuan/cpufreq-amd-pstate-remove-set_boost-callback-for-passive-mode/20240126-171412
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi-bus
patch link:    https://lore.kernel.org/r/0409d40c500eeb8d4d84ecb028b73f2eee147822.1706255676.git.perry.yuan%40amd.com
patch subject: [PATCH 2/7] cpufreq: amd-pstate: initialize new core precision boost state
config: i386-randconfig-141-20240127 (https://download.01.org/0day-ci/archive/20240128/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240128/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/cpufreq/amd-pstate-ut.c: In function 'amd_pstate_ut_check_freq':

>> drivers/cpufreq/amd-pstate-ut.c:229:28: error: 'struct amd_cpudata' has no member named 'boost_supported'
     229 |                 if (cpudata->boost_supported) {
         |                            ^~
   In file included from drivers/cpufreq/amd-pstate-ut.c:29:
   include/linux/amd-pstate.h: At top level:
   include/linux/amd-pstate.h:104:27: warning: 'amd_pstate_mode_string' defined but not used [-Wunused-const-variable=]
     104 | static const char * const amd_pstate_mode_string[] = {
         |                           ^~~~~~~~~~~~~~~~~~~~~~

vim +229 drivers/cpufreq/amd-pstate-ut.c

14eb1c96e3a3fd Meng Li        2022-08-17  193  
14eb1c96e3a3fd Meng Li        2022-08-17  194  /*
14eb1c96e3a3fd Meng Li        2022-08-17  195   * Check if frequency values are reasonable.
14eb1c96e3a3fd Meng Li        2022-08-17  196   * max_freq >= nominal_freq > lowest_nonlinear_freq > min_freq > 0
14eb1c96e3a3fd Meng Li        2022-08-17  197   * check max freq when set support boost mode.
14eb1c96e3a3fd Meng Li        2022-08-17  198   */
14eb1c96e3a3fd Meng Li        2022-08-17  199  static void amd_pstate_ut_check_freq(u32 index)
14eb1c96e3a3fd Meng Li        2022-08-17  200  {
14eb1c96e3a3fd Meng Li        2022-08-17  201  	int cpu = 0;
14eb1c96e3a3fd Meng Li        2022-08-17  202  	struct cpufreq_policy *policy = NULL;
14eb1c96e3a3fd Meng Li        2022-08-17  203  	struct amd_cpudata *cpudata = NULL;
14eb1c96e3a3fd Meng Li        2022-08-17  204  
14eb1c96e3a3fd Meng Li        2022-08-17  205  	for_each_possible_cpu(cpu) {
14eb1c96e3a3fd Meng Li        2022-08-17  206  		policy = cpufreq_cpu_get(cpu);
14eb1c96e3a3fd Meng Li        2022-08-17  207  		if (!policy)
14eb1c96e3a3fd Meng Li        2022-08-17  208  			break;
14eb1c96e3a3fd Meng Li        2022-08-17  209  		cpudata = policy->driver_data;
14eb1c96e3a3fd Meng Li        2022-08-17  210  
14eb1c96e3a3fd Meng Li        2022-08-17  211  		if (!((cpudata->max_freq >= cpudata->nominal_freq) &&
14eb1c96e3a3fd Meng Li        2022-08-17  212  			(cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
14eb1c96e3a3fd Meng Li        2022-08-17  213  			(cpudata->lowest_nonlinear_freq > cpudata->min_freq) &&
14eb1c96e3a3fd Meng Li        2022-08-17  214  			(cpudata->min_freq > 0))) {
14eb1c96e3a3fd Meng Li        2022-08-17  215  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  216  			pr_err("%s cpu%d max=%d >= nominal=%d > lowest_nonlinear=%d > min=%d > 0, the formula is incorrect!\n",
14eb1c96e3a3fd Meng Li        2022-08-17  217  				__func__, cpu, cpudata->max_freq, cpudata->nominal_freq,
14eb1c96e3a3fd Meng Li        2022-08-17  218  				cpudata->lowest_nonlinear_freq, cpudata->min_freq);
60dd283804479c Swapnil Sapkal 2023-08-18  219  			goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  220  		}
14eb1c96e3a3fd Meng Li        2022-08-17  221  
14eb1c96e3a3fd Meng Li        2022-08-17  222  		if (cpudata->min_freq != policy->min) {
14eb1c96e3a3fd Meng Li        2022-08-17  223  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  224  			pr_err("%s cpu%d cpudata_min_freq=%d policy_min=%d, they should be equal!\n",
14eb1c96e3a3fd Meng Li        2022-08-17  225  				__func__, cpu, cpudata->min_freq, policy->min);
60dd283804479c Swapnil Sapkal 2023-08-18  226  			goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  227  		}
14eb1c96e3a3fd Meng Li        2022-08-17  228  
14eb1c96e3a3fd Meng Li        2022-08-17 @229  		if (cpudata->boost_supported) {
14eb1c96e3a3fd Meng Li        2022-08-17  230  			if ((policy->max == cpudata->max_freq) ||
14eb1c96e3a3fd Meng Li        2022-08-17  231  					(policy->max == cpudata->nominal_freq))
14eb1c96e3a3fd Meng Li        2022-08-17  232  				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
14eb1c96e3a3fd Meng Li        2022-08-17  233  			else {
14eb1c96e3a3fd Meng Li        2022-08-17  234  				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  235  				pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n",
14eb1c96e3a3fd Meng Li        2022-08-17  236  					__func__, cpu, policy->max, cpudata->max_freq,
14eb1c96e3a3fd Meng Li        2022-08-17  237  					cpudata->nominal_freq);
60dd283804479c Swapnil Sapkal 2023-08-18  238  				goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  239  			}
14eb1c96e3a3fd Meng Li        2022-08-17  240  		} else {
14eb1c96e3a3fd Meng Li        2022-08-17  241  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  242  			pr_err("%s cpu%d must support boost!\n", __func__, cpu);
60dd283804479c Swapnil Sapkal 2023-08-18  243  			goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  244  		}
60dd283804479c Swapnil Sapkal 2023-08-18  245  		cpufreq_cpu_put(policy);
14eb1c96e3a3fd Meng Li        2022-08-17  246  	}
14eb1c96e3a3fd Meng Li        2022-08-17  247  
14eb1c96e3a3fd Meng Li        2022-08-17  248  	amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
60dd283804479c Swapnil Sapkal 2023-08-18  249  	return;
60dd283804479c Swapnil Sapkal 2023-08-18  250  skip_test:
60dd283804479c Swapnil Sapkal 2023-08-18  251  	cpufreq_cpu_put(policy);
14eb1c96e3a3fd Meng Li        2022-08-17  252  }
14eb1c96e3a3fd Meng Li        2022-08-17  253
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
author reviewer
#3 lore raw

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

In reply to Mario Limonciello

[AMD Official Use Only - General]

Hi Mario,

Quoted 17 lines

-----Original Message-----
From: Limonciello, Mario <[email protected]>
Sent: Friday, January 26, 2024 11:45 PM
To: Yuan, Perry <[email protected]>; [email protected]; [email protected]; Huang, Ray <[email protected]>; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>
Cc: Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH 1/7] cpufreq: amd-pstate: remove set_boost callback for passive mode

Quoted 2 lines · Perry Yuan

From: Perry Yuan <[email protected]>

Quoted 8 lines
Quoted 2 lines

The following patches will enable amd-pstate CPU boost control method

When it's committed it won't be a patch. How about instead "A specific amd- pstate CPU boost control method is to be introduced and the legacy callback doesn't make sense" or something along those lines.

Also; is the ordering correct? In terms of bisectability should this come after the new one is introduced perhaps?

I move the patch to the end of the series and update the commit info like you suggested. Thank you for the feedback.

Regards.
Perry

Quoted 57 lines
Quoted 11 lines

which will not need this common boost control callback anymore, so we remove the legacy set_boost interface from amd-pstate driver.

Signed-off-by Perry Yuan <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 26 --------------------------
  1 file changed, 26 deletions(-)
(quoted diff) +0 -0

b/drivers/cpufreq/amd-pstate.c index 9a1e194d5cf8..8f308f56ade6

100644

drivers/cpufreq/amd-pstate.c static int amd_get_lowest_nonlinear_freq(struct

Quoted 3 lines
drivers/cpufreq/amd-pstate.c +0 -0
@@ -632,31 +632,6 @@ static int amd_get_lowest_nonlinear_freq(struct

amd_cpudata *cpudata)

drivers/cpufreq/amd-pstate.c static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +0 -6
    return lowest_nonlinear_freq * 1000;
  }
 
-static int amd_pstate_set_boost(struct cpufreq_policy *policy, int
 state) -{
-   struct amd_cpudata *cpudata = policy->driver_data;
-   int ret;
-
-   if (!cpudata->boost_supported) {
-           pr_err("Boost mode is not supported by this processor or

SBIOS\n");

drivers/cpufreq/amd-pstate.c static struct cpufreq_driver amd_pstate_driver =

Quoted 22 lines
drivers/cpufreq/amd-pstate.c +0 -18
-           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)
-           return ret;
-
-   return 0;
-}
-
  static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
  {
    u32 highest_perf, nominal_perf;

@@ -1391,7 +1366,6 @@ static struct cpufreq_driver amd_pstate_driver =

{

Quoted 7 lines
drivers/cpufreq/amd-pstate.c +0 -1
    .exit           = amd_pstate_cpu_exit,
    .suspend        = amd_pstate_cpu_suspend,
    .resume         = amd_pstate_cpu_resume,
-   .set_boost      = amd_pstate_set_boost,
    .name           = "amd-pstate",
    .attr           = amd_pstate_attr,
  };
author reviewer
#20 lore raw

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

In reply to Mario Limonciello

[AMD Official Use Only - General]

Quoted 14 lines

-----Original Message-----
From: Limonciello, Mario <[email protected]>
Sent: Friday, January 26, 2024 11:47 PM
To: Yuan, Perry <[email protected]>; [email protected]; [email protected]; Huang, Ray <[email protected]>; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>
Cc: Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH 7/7] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method

Quoted 7 lines
Quoted 4 lines · Perry Yuan

From: Perry Yuan <[email protected]>

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

Typo in this sentence.

Fixed by V2.

Quoted 29 lines
Quoted 5 lines

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.

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They

Quoted 12 lines
Signed-off-by Perry Yuan <[email protected]>
---
  Documentation/admin-guide/pm/amd-pstate.rst | 11 +++++++++++
  1 file changed, 11 insertions(+)
Documentation/admin-guide/pm/amd-pstate.rst +0 -0
index 1cf40f69278c..d72dc407c4db 100644
@@ -385,6 +385,17 @@ control its functionality at the system level.  They

are located in the

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They are located in the

Quoted 5 lines
Documentation/admin-guide/pm/amd-pstate.rst +2 -0
          to the operation mode represented by that string - or to be
          unregistered in the "disable" case.
 
+``cpb_boost``
+        Specifies whether core performance boost is requested to be enabled

or disabled

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They · line 390

Quoted 1 line
Documentation/admin-guide/pm/amd-pstate.rst +1 -0
+        If core performance boost is disabled while a core is in a boosted P-

state, the

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They · line 391

Quoted 1 line
Documentation/admin-guide/pm/amd-pstate.rst +1 -0
+        core automatically transitions to the highest performance non-boosted

P-state.

Quoted 8 lines

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They · line 392

Quoted 1 line
Documentation/admin-guide/pm/amd-pstate.rst +1 -0
+        AMD Core Performance Boost(CPB) is controlled by this new attribute

file which

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They · line 393

Quoted 1 line
Documentation/admin-guide/pm/amd-pstate.rst +1 -0
+        allow user to change all cores frequency boosting state. It supports

both

Documentation/admin-guide/pm/amd-pstate.rst control its functionality at the system level. They · line 394

Quoted 1 line
Documentation/admin-guide/pm/amd-pstate.rst +1 -0
+        ``active mode`` and ``passive mode`` control with below value write to

it.

Does it also support guided mode?

Yes, guide mode is also supported and I tested it. Looks like it is better to add the guide mode support info Into the doc update. Will update in V2.

# cat /sys/devices/system/cpu/amd_pstate/status
guided

check on and off lscpu output.

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 2983.578
  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

CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ

  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
Quoted 8 lines
Quoted 8 lines
Documentation/admin-guide/pm/amd-pstate.rst +4 -0
+
+        "0" Disable Core performance Boosting
+        "1" Enable  Core performance Boosting
+
  ``cpupower`` tool support for ``amd-pstate``
  ===============================================
author reviewer
#14 lore raw

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

In reply to Mario Limonciello

[AMD Official Use Only - General]

Hi Mario,

Quoted 17 lines

-----Original Message-----
From: Limonciello, Mario <[email protected]>
Sent: Friday, January 26, 2024 11:54 PM
To: Yuan, Perry <[email protected]>; [email protected]; [email protected]; Huang, Ray <[email protected]>; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>
Cc: Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH 4/7] cpufreq: amd-pstate: fix max_perf calculation for amd_get_max_freq()

Quoted 2 lines · Perry Yuan

From: Perry Yuan <[email protected]>

Quoted 7 lines
Quoted 4 lines

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.

What about the previous cppc_req? Shouldn't it be explicitly updated as a result of this too?

The CPPC REQ value has been updated in this function. When boost state changed, the MSR will be updated firstly.

+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);

Quoted 24 lines
Quoted 9 lines
Signed-off-by Perry Yuan <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 4 ++++
  1 file changed, 4 insertions(+)
(quoted diff) +0 -0

b/drivers/cpufreq/amd-pstate.c index b37bea7440b9..3286d72f375e

100644

drivers/cpufreq/amd-pstate.c static int amd_get_max_freq(struct amd_cpudata

Quoted 3 lines
drivers/cpufreq/amd-pstate.c +0 -0
@@ -599,6 +599,10 @@ static int amd_get_max_freq(struct amd_cpudata

*cpudata)

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +4 -0
    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);
author reviewer
#11 lore raw

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

In reply to Mario Limonciello

[AMD Official Use Only - General]

Hi Mario,

Quoted 14 lines

-----Original Message-----
From: Limonciello, Mario <[email protected]>
Sent: Friday, January 26, 2024 11:57 PM
To: Yuan, Perry <[email protected]>; [email protected]; [email protected]; Huang, Ray <[email protected]>; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>
Cc: Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH 3/7] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control

Quoted 8 lines
Quoted 4 lines · Perry Yuan

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, user can change CPU boost state dynamically under active and passive modes.

What about guided mode?

Guided mode is supported as well.

Quoted 43 lines
Quoted 12 lines

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"

Quoted 6 lines
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 <[email protected]>
---
  drivers/cpufreq/amd-pstate.c | 97

++++++++++++++++++++++++++++++++++++

Quoted 4 lines
  1 file changed, 97 insertions(+)
(quoted diff) +0 -0

b/drivers/cpufreq/amd-pstate.c index 0dc9124140d4..b37bea7440b9

100644

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a,

Quoted 3 lines
drivers/cpufreq/amd-pstate.c +0 -0
@@ -1036,6 +1036,101 @@ static ssize_t status_store(struct device *a,

struct device_attribute *b,

Quoted 15 lines
drivers/cpufreq/amd-pstate.c +12 -0
    return ret < 0 ? ret : count;
  }
 
+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);
Quoted 8 lines

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a, struct device_attribute *b, · line 1055

Quoted 5 lines
drivers/cpufreq/amd-pstate.c +5 -0
+
+   if (boot_cpu_has(X86_FEATURE_CPPC)) {
+           u64 value = READ_ONCE(cpudata->cppc_req_cached);
+
+           value &= ~GENMASK_ULL(7, 0);

I think it's better to use the macro AMD_CPPC_MAX_PERF in this case. The less magic masks in the code, the easier it is to follow.

Here the highest_perf and nominal_perf will reuse the perf values where they should be correctly initialized. In this function, we would like to focus on to use highest perf or nominal perf for boost control.

The AMD_CPPC_MAX_PERF should be used when driver loading to set actual highest_perf value as required. Here I would like avoid to make the highest perf value changed.

Boost control only switch the two perf value highest_perf vs nominal_perf as user request.

Perry.

Quoted 101 lines

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a, struct device_attribute *b,

Quoted 5 lines
drivers/cpufreq/amd-pstate.c +4 -0
+           value |= on ? highest_perf : nominal_perf;
+           WRITE_ONCE(cpudata->cppc_req_cached, value);
+
+           wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,

value);

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a, struct device_attribute *b,

Quoted 6 lines
drivers/cpufreq/amd-pstate.c +6 -0
+
+   } 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);

Quoted 16 lines
drivers/cpufreq/amd-pstate.c +16 -0
+                   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);
+   }
+

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a, struct device_attribute *b,

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+   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,

drivers/cpufreq/amd-pstate.c static ssize_t status_store(struct device *a, struct device_attribute *b,

Quoted 8 lines
drivers/cpufreq/amd-pstate.c +8 -0
+                       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");

drivers/cpufreq/amd-pstate.c

Quoted 35 lines
drivers/cpufreq/amd-pstate.c +31 -0
+           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);
 

@@ -1043,6 +1138,7 @@

cpufreq_freq_attr_ro(amd_pstate_highest_perf);

Quoted 16 lines
drivers/cpufreq/amd-pstate.c +2 -0
  cpufreq_freq_attr_rw(energy_performance_preference);
  cpufreq_freq_attr_ro(energy_performance_available_preferences);
  static DEVICE_ATTR_RW(status);
+static DEVICE_ATTR_RW(cpb_boost);
 
  static struct freq_attr *amd_pstate_attr[] = {
    &amd_pstate_max_freq,

@@ -1062,6 +1158,7 @@ static struct freq_attr *amd_pstate_epp_attr[] =
 {
 
  static struct attribute *pstate_global_attributes[] = {
    &dev_attr_status.attr,
+   &dev_attr_cpb_boost.attr,
    NULL
  };
build bot
#7 lore raw

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

In reply to Perry Yuan
kernel test robot noticed the following build errors:

Hi Perry,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/acpi-bus]
[also build test ERROR on v6.8-rc3]
[cannot apply to rafael-pm/linux-next linus/master rafael-pm/devprop next-20240209] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Perry-Yuan/cpufreq-amd-pstate-remove-set_boost-callback-for-passive-mode/20240126-171412
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi-bus
patch link:    https://lore.kernel.org/r/0409d40c500eeb8d4d84ecb028b73f2eee147822.1706255676.git.perry.yuan%40amd.com
patch subject: [PATCH 2/7] cpufreq: amd-pstate: initialize new core precision boost state
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240212/[email protected]/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240212/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/cpufreq/amd-pstate-ut.c:229:16: error: no member named 'boost_supported' in 'struct amd_cpudata'
     229 |                 if (cpudata->boost_supported) {
         |                     ~~~~~~~  ^

1 error generated.

vim +229 drivers/cpufreq/amd-pstate-ut.c

14eb1c96e3a3fd Meng Li        2022-08-17  193  
14eb1c96e3a3fd Meng Li        2022-08-17  194  /*
14eb1c96e3a3fd Meng Li        2022-08-17  195   * Check if frequency values are reasonable.
14eb1c96e3a3fd Meng Li        2022-08-17  196   * max_freq >= nominal_freq > lowest_nonlinear_freq > min_freq > 0
14eb1c96e3a3fd Meng Li        2022-08-17  197   * check max freq when set support boost mode.
14eb1c96e3a3fd Meng Li        2022-08-17  198   */
14eb1c96e3a3fd Meng Li        2022-08-17  199  static void amd_pstate_ut_check_freq(u32 index)
14eb1c96e3a3fd Meng Li        2022-08-17  200  {
14eb1c96e3a3fd Meng Li        2022-08-17  201  	int cpu = 0;
14eb1c96e3a3fd Meng Li        2022-08-17  202  	struct cpufreq_policy *policy = NULL;
14eb1c96e3a3fd Meng Li        2022-08-17  203  	struct amd_cpudata *cpudata = NULL;
14eb1c96e3a3fd Meng Li        2022-08-17  204  
14eb1c96e3a3fd Meng Li        2022-08-17  205  	for_each_possible_cpu(cpu) {
14eb1c96e3a3fd Meng Li        2022-08-17  206  		policy = cpufreq_cpu_get(cpu);
14eb1c96e3a3fd Meng Li        2022-08-17  207  		if (!policy)
14eb1c96e3a3fd Meng Li        2022-08-17  208  			break;
14eb1c96e3a3fd Meng Li        2022-08-17  209  		cpudata = policy->driver_data;
14eb1c96e3a3fd Meng Li        2022-08-17  210  
14eb1c96e3a3fd Meng Li        2022-08-17  211  		if (!((cpudata->max_freq >= cpudata->nominal_freq) &&
14eb1c96e3a3fd Meng Li        2022-08-17  212  			(cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
14eb1c96e3a3fd Meng Li        2022-08-17  213  			(cpudata->lowest_nonlinear_freq > cpudata->min_freq) &&
14eb1c96e3a3fd Meng Li        2022-08-17  214  			(cpudata->min_freq > 0))) {
14eb1c96e3a3fd Meng Li        2022-08-17  215  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  216  			pr_err("%s cpu%d max=%d >= nominal=%d > lowest_nonlinear=%d > min=%d > 0, the formula is incorrect!\n",
14eb1c96e3a3fd Meng Li        2022-08-17  217  				__func__, cpu, cpudata->max_freq, cpudata->nominal_freq,
14eb1c96e3a3fd Meng Li        2022-08-17  218  				cpudata->lowest_nonlinear_freq, cpudata->min_freq);
60dd283804479c Swapnil Sapkal 2023-08-18  219  			goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  220  		}
14eb1c96e3a3fd Meng Li        2022-08-17  221  
14eb1c96e3a3fd Meng Li        2022-08-17  222  		if (cpudata->min_freq != policy->min) {
14eb1c96e3a3fd Meng Li        2022-08-17  223  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  224  			pr_err("%s cpu%d cpudata_min_freq=%d policy_min=%d, they should be equal!\n",
14eb1c96e3a3fd Meng Li        2022-08-17  225  				__func__, cpu, cpudata->min_freq, policy->min);
60dd283804479c Swapnil Sapkal 2023-08-18  226  			goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  227  		}
14eb1c96e3a3fd Meng Li        2022-08-17  228  
14eb1c96e3a3fd Meng Li        2022-08-17 @229  		if (cpudata->boost_supported) {
14eb1c96e3a3fd Meng Li        2022-08-17  230  			if ((policy->max == cpudata->max_freq) ||
14eb1c96e3a3fd Meng Li        2022-08-17  231  					(policy->max == cpudata->nominal_freq))
14eb1c96e3a3fd Meng Li        2022-08-17  232  				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
14eb1c96e3a3fd Meng Li        2022-08-17  233  			else {
14eb1c96e3a3fd Meng Li        2022-08-17  234  				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  235  				pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n",
14eb1c96e3a3fd Meng Li        2022-08-17  236  					__func__, cpu, policy->max, cpudata->max_freq,
14eb1c96e3a3fd Meng Li        2022-08-17  237  					cpudata->nominal_freq);
60dd283804479c Swapnil Sapkal 2023-08-18  238  				goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  239  			}
14eb1c96e3a3fd Meng Li        2022-08-17  240  		} else {
14eb1c96e3a3fd Meng Li        2022-08-17  241  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
14eb1c96e3a3fd Meng Li        2022-08-17  242  			pr_err("%s cpu%d must support boost!\n", __func__, cpu);
60dd283804479c Swapnil Sapkal 2023-08-18  243  			goto skip_test;
14eb1c96e3a3fd Meng Li        2022-08-17  244  		}
60dd283804479c Swapnil Sapkal 2023-08-18  245  		cpufreq_cpu_put(policy);
14eb1c96e3a3fd Meng Li        2022-08-17  246  	}
14eb1c96e3a3fd Meng Li        2022-08-17  247  
14eb1c96e3a3fd Meng Li        2022-08-17  248  	amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
60dd283804479c Swapnil Sapkal 2023-08-18  249  	return;
60dd283804479c Swapnil Sapkal 2023-08-18  250  skip_test:
60dd283804479c Swapnil Sapkal 2023-08-18  251  	cpufreq_cpu_put(policy);
14eb1c96e3a3fd Meng Li        2022-08-17  252  }
14eb1c96e3a3fd Meng Li        2022-08-17  253
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#8 lore raw

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

In reply to Perry Yuan

Hello Perry,

Quoted 1 line · Perry Yuan

From: Perry Yuan <[email protected]>

[..snip..]

Quoted 6 lines
drivers/cpufreq/amd-pstate.c +1 -1
@@ -632,18 +645,27 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
 	return lowest_nonlinear_freq * 1000;
 }
 
-static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
+static int amd_pstate_boost_init(struct amd_cpudata *cpudata)

drivers/cpufreq/amd-pstate.c static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata) · line 655

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +3 -3
 {
-	u32 highest_perf, nominal_perf;
+	u64 boost_state, boost_val;
 
 
+	int ret;
 
-	highest_perf = READ_ONCE(cpudata->highest_perf);
-	nominal_perf = READ_ONCE(cpudata->nominal_perf);
+	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);

Use hwcr_val instead of boost_val ?

drivers/cpufreq/amd-pstate.c static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata) · line 659

Quoted 8 lines
drivers/cpufreq/amd-pstate.c +5 -2
+	if (ret) {
+		pr_err_once("failed to read initial CPU boost state!\n");
+		return ret;
+	}
 
-	if (highest_perf <= nominal_perf)
-		return;
+	boost_state = (boost_val >> 25) & 0x1;

How about the following ?

        boost_disabled = (hwcr_val & MSR_K7_HWCR_CPB_DIS);

drivers/cpufreq/amd-pstate.c static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata) · line 660

Quoted 1 line
drivers/cpufreq/amd-pstate.c +1 -0
+	if (!boost_state) {

        if (!boost_disabled) {
Quoted 64 lines
drivers/cpufreq/amd-pstate.c +7 -2
+		global.cpb_supported = true;
+		global.cpb_boost = true;
+	} else {
+		global.cpb_supported = false;
+		global.cpb_boost = false;
+	}
 
-	cpudata->boost_supported = true;
-	current_pstate_driver->boost_enabled = true;
+	return ret;
 }

static void amd_perf_ctl_reset(unsigned int cpu)

drivers/cpufreq/amd-pstate.c +6 -2
@@ -676,6 +698,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);

@@ -725,7 +750,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;
 

@@ -1093,6 +1117,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);

@@ -1143,7 +1170,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;
include/linux/amd-pstate.h +0 -1
index 446394f84606..66d939a344b1 100644
@@ -80,7 +80,6 @@ struct amd_cpudata {
 	struct amd_aperf_mperf prev;
 
 	u64	freq;
-	bool	boost_supported;
 
 	/* EPP feature related attributes*/
 	s16	epp_policy;
Signature · 1 line
2.34.1

Otherwise looks good to me.
--
Thanks and Regards
gautham.