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
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(-)
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
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(+)
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
From: Perry Yuan <[email protected]>
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 <[email protected]>
---
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
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(-)
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
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(+)
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
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(-)
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
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(+)
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
On 1/26/2024 02:08, Perry Yuan wrote:
> 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?
> 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(-)
>
> 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,
> };
On 1/26/2024 02:08, Perry Yuan wrote:
> 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.
>
> 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(+)
>
> 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``
> ===============================================
>
On 1/26/2024 02:08, Perry Yuan wrote:
> 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?
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> 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 */
On 1/26/2024 02:08, Perry Yuan wrote:
> From: Perry Yuan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
> };
>
On 1/26/2024 02:08, Perry Yuan wrote:
> 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
Rather than "Issue" this should be "Closes:", and the tag should come
right after the Reported-by tag.
> 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(-)
>
> 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;
On 1/26/2024 02:08, Perry Yuan wrote:
> 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?
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> 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);
>
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[AMD Official Use Only - General]
Hi Mario,
> -----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
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > 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?
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 <[email protected]>
> > ---
> > 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,
> > };
[AMD Official Use Only - General]
> -----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
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > 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.
>
> >
> > 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(+)
> >
> > 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``
> > ===============================================
> >
[AMD Official Use Only - General]
Hi Mario,
> -----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()
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > 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?
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 <[email protected]>
> > ---
> > 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);
> >
[AMD Official Use Only - General]
Hi Mario,
> -----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
>
> On 1/26/2024 02:08, Perry Yuan wrote:
> > From: Perry Yuan <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> > };
> >
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hello Perry,
On Fri, Jan 26, 2024 at 04:08:05PM +0800, Perry Yuan wrote:
> From: Perry Yuan <[email protected]>
[..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.