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

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

[Perry Yuan]

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(-)

-- 
2.34.1

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

[Perry Yuan]

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

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 <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 9a1e194d5cf8..8f308f56ade6 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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,
 };
-- 
2.34.1

[PATCH 2/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.

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

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8f308f56ade6..0dc9124140d4 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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;
 
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index 446394f84606..66d939a344b1 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -80,7 +80,6 @@ struct amd_cpudata {
 	struct amd_aperf_mperf prev;
 
 	u64	freq;
-	bool	boost_supported;
 
 	/* EPP feature related attributes*/
 	s16	epp_policy;
-- 
2.34.1

[PATCH 3/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` 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 0dc9124140d4..b37bea7440b9 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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
 };
 
-- 
2.34.1

[PATCH 4/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 b37bea7440b9..3286d72f375e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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);
 
-- 
2.34.1

[PATCH 5/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 3286d72f375e..5cbbc2999d9a 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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);
 
-- 
2.34.1

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

[Perry Yuan]

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

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 <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 48 ++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 5cbbc2999d9a..bba7640d46e0 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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 */
-- 
2.34.1

[PATCH 7/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 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 <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 1cf40f69278c..d72dc407c4db 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -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``
 ===============================================
 
-- 
2.34.1

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

[Mario Limonciello]

On 1/26/2024 02:08, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> 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?

> 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 <Perry.Yuan@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 26 --------------------------
>   1 file changed, 26 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9a1e194d5cf8..8f308f56ade6 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -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,
>   };

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

[Mario Limonciello]

On 1/26/2024 02:08, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> Introduce AMD CPU frequency boosting control sysfs entry which userd for
> switching boost on and boost off.

Typo in this sentence.

> 
> 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 1cf40f69278c..d72dc407c4db 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -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?

> +
> +        "0" Disable Core performance Boosting
> +        "1" Enable  Core performance Boosting
> +
>   ``cpupower`` tool support for ``amd-pstate``
>   ===============================================
>   

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

[Mario Limonciello]

On 1/26/2024 02:08, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> 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?

> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 48 ++++++++++++++++++++++++++++++++----
>   1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5cbbc2999d9a..bba7640d46e0 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -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;
> +		}

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?

> +
> +		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 */
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

> +	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 */

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

[Mario Limonciello]

On 1/26/2024 02:08, Perry Yuan wrote:
> 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.

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

> 
> 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 b37bea7440b9..3286d72f375e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -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);
>   

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

[Mario Limonciello]

On 1/26/2024 02:08, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> With this new sysfs entry `cpb_boost`created, user can change CPU boost
> state dynamically under `active` and `passive` modes.

What about guided mode?

> 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 0dc9124140d4..b37bea7440b9 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -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);

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.

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

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

[Mario Limonciello]

On 1/26/2024 02:08, 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.
> 
> 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.

> Reported-by: Artem S. Tashkinov" <aros@gmx.com>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 46 ++++++++++++++++++++++++++++--------
>   include/linux/amd-pstate.h   |  1 -
>   2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 8f308f56ade6..0dc9124140d4 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -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;
> +	}

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;

>   
> -	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;
>   
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index 446394f84606..66d939a344b1 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -80,7 +80,6 @@ struct amd_cpudata {
>   	struct amd_aperf_mperf prev;
>   
>   	u64	freq;
> -	bool	boost_supported;
>   
>   	/* EPP feature related attributes*/
>   	s16	epp_policy;

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

[kernel test robot]

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/202401280215.WvGTuIEq-lkp@intel.com/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/202401280215.WvGTuIEq-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401280215.WvGTuIEq-lkp@intel.com/

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  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Mario,

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, January 26, 2024 11:45 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Shenoy,
> Gautham Ranjal <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@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 1/7] cpufreq: amd-pstate: remove set_boost callback for
> passive mode
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > 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

>
> > 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 <Perry.Yuan@amd.com>
> > ---
> >   drivers/cpufreq/amd-pstate.c | 26 --------------------------
> >   1 file changed, 26 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 9a1e194d5cf8..8f308f56ade6
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -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,
> >   };

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

[Yuan, Perry]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, January 26, 2024 11:47 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Shenoy,
> Gautham Ranjal <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@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 7/7] Documentation: cpufreq: amd-pstate: introduce the
> new cpu boost control method
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > Introduce AMD CPU frequency boosting control sysfs entry which userd
> > for switching boost on and boost off.
>
> Typo in this sentence.

Fixed by V2.

>
> >
> > 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 1cf40f69278c..d72dc407c4db 100644
> > --- a/Documentation/admin-guide/pm/amd-pstate.rst
> > +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> > @@ -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?

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


>
> > +
> > +        "0" Disable Core performance Boosting
> > +        "1" Enable  Core performance Boosting
> > +
> >   ``cpupower`` tool support for ``amd-pstate``
> >   ===============================================
> >

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

[Yuan, Perry]

[AMD Official Use Only - General]

Hi  Mario,

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, January 26, 2024 11:54 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Shenoy,
> Gautham Ranjal <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@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 4/7] cpufreq: amd-pstate: fix max_perf calculation for
> amd_get_max_freq()
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > 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.
>
> 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);




>
> >
> > 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 b37bea7440b9..3286d72f375e
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -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);
> >

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

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Mario,

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, January 26, 2024 11:57 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Shenoy,
> Gautham Ranjal <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@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 3/7] cpufreq: amd-pstate: implement cpb_boost sysfs
> entry for boost control
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > With this new sysfs entry `cpb_boost`created, user can change CPU
> > boost state dynamically under `active` and `passive` modes.
>
> What about guided mode?
>

Guided mode is supported as well.


> > 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 0dc9124140d4..b37bea7440b9
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -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);
>
> 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.

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

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

[kernel test robot]

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/202402120216.mjdQyGCs-lkp@intel.com/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/202402120216.mjdQyGCs-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402120216.mjdQyGCs-lkp@intel.com/

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  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Gautham R. Shenoy]

Hello Perry,

On Fri, Jan 26, 2024 at 04:08:05PM +0800, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>

[..snip..]

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

Use hwcr_val instead of 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;

How about the following ?

        boost_disabled = (hwcr_val & MSR_K7_HWCR_CPB_DIS);



> +	if (!boost_state) {

        if (!boost_disabled) {

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

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