* [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost
@ 2024-06-19 9:16 Perry Yuan
2024-06-19 9:16 ` [PATCH v12 1/9] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h Perry Yuan
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Hi all,
This patch series implements the Core Performance Boost (CPB) feature for
the AMD pstate driver, including support for passive, guided, and active modes.
Users can change the global core frequency boost control with a new sysfs entry:
/sys/devices/system/cpu/amd_pstate/cpb_boost
It also supports updating the individual CPU boost state in the sysfs boost file:
/sys/devices/system/cpu/cpuX/cpufreq/boost
By default, the global control will override the individual CPU boost state.
1) disable core boost globally:
$ 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) enable core boost globally:
$ 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 V9 patches add per CPU boost control, user can enable/disable CPUs boost
as the below command tested on a laptop system.
# before
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ
0 0 0 0 0:0:0:0 yes 4208.0000 400.0000 1666.7740
1 0 0 0 0:0:0:0 yes 4208.0000 400.0000 400.0000
2 0 0 1 1:1:1:0 yes 4208.0000 400.0000 3386.1260
3 0 0 1 1:1:1:0 yes 4208.0000 400.0000 400.0000
$ sudo rdmsr 0xc00102b3 -p 0
10a6
$ sudo bash -c "echo 1 > /sys/devices/system/cpu/cpu0/cpufreq/boost"
# after
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ
0 0 0 0 0:0:0:0 yes 3501.0000 400.0000 400.0000
1 0 0 0 0:0:0:0 yes 4208.0000 400.0000 1391.0690
2 0 0 1 1:1:1:0 yes 4208.0000 400.0000 3654.4541
3 0 0 1 1:1:1:0 yes 4208.0000 400.0000 400.0000
$ sudo rdmsr 0xc00102b3 -p 0
108a
rebasd to keep syncing to Mario kernel tree: bleeding-edge
https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git
Tested result:
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ
0 0 0 0 0:0:0:0 yes 4354.0000 400.0000 1110.7140
1 0 0 0 0:0:0:0 yes 4354.0000 400.0000 1110.7140
2 0 0 1 1:1:1:0 yes 4354.0000 400.0000 1110.7140
3 0 0 1 1:1:1:0 yes 4354.0000 400.0000 3110.0000
4 0 0 2 2:2:2:0 yes 4354.0000 400.0000 2732.3569
5 0 0 2 2:2:2:0 yes 4354.0000 400.0000 1110.7140
6 0 0 3 3:3:3:0 yes 4354.0000 400.0000 1110.7140
7 0 0 3 3:3:3:0 yes 4354.0000 400.0000 1110.7140
8 0 0 4 4:4:4:0 yes 4354.0000 400.0000 2312.2109
9 0 0 4 4:4:4:0 yes 4354.0000 400.0000 1110.7140
10 0 0 5 5:5:5:0 yes 4354.0000 400.0000 2310.1011
11 0 0 5 5:5:5:0 yes 4354.0000 400.0000 1110.7140
sudo bash -c "echo 0 > /sys/devices/system/cpu/cpu11/cpufreq/boost"
sudo bash -c "echo 0 > /sys/devices/system/cpu/cpu10/cpufreq/boost"
sudo bash -c "echo 0 > /sys/devices/system/cpu/cpu9/cpufreq/boost"
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ
0 0 0 0 0:0:0:0 yes 4354.0000 400.0000 1242.7240
1 0 0 0 0:0:0:0 yes 4354.0000 400.0000 1110.7140
2 0 0 1 1:1:1:0 yes 4354.0000 400.0000 2754.5710
3 0 0 1 1:1:1:0 yes 4354.0000 400.0000 2659.8159
4 0 0 2 2:2:2:0 yes 4354.0000 400.0000 2308.9929
5 0 0 2 2:2:2:0 yes 4354.0000 400.0000 1110.7140
6 0 0 3 3:3:3:0 yes 4354.0000 400.0000 1110.7140
7 0 0 3 3:3:3:0 yes 4354.0000 400.0000 1110.7140
8 0 0 4 4:4:4:0 yes 4354.0000 400.0000 1110.7140
9 0 0 4 4:4:4:0 yes 2801.0000 400.0000 1110.7140
10 0 0 5 5:5:5:0 yes 2801.0000 400.0000 1110.7140
11 0 0 5 5:5:5:0 yes 2801.0000 400.0000 1110.7140
sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ
0 0 0 0 0:0:0:0 yes 2801.0000 400.0000 1233.8630
1 0 0 0 0:0:0:0 yes 2801.0000 400.0000 1110.7140
2 0 0 1 1:1:1:0 yes 2801.0000 400.0000 2714.4851
3 0 0 1 1:1:1:0 yes 2801.0000 400.0000 2732.3569
4 0 0 2 2:2:2:0 yes 2801.0000 400.0000 2564.2639
5 0 0 2 2:2:2:0 yes 2801.0000 400.0000 1110.7140
6 0 0 3 3:3:3:0 yes 2801.0000 400.0000 2732.3569
7 0 0 3 3:3:3:0 yes 2801.0000 400.0000 1110.7140
8 0 0 4 4:4:4:0 yes 2801.0000 400.0000 1233.8660
9 0 0 4 4:4:4:0 yes 2801.0000 400.0000 1110.7140
10 0 0 5 5:5:5:0 yes 2801.0000 400.0000 1233.6630
11 0 0 5 5:5:5:0 yes 2801.0000 400.0000 1233.5050
sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ MHZ
0 0 0 0 0:0:0:0 yes 4354.0000 400.0000 1234.0200
1 0 0 0 0:0:0:0 yes 4354.0000 400.0000 1110.7140
2 0 0 1 1:1:1:0 yes 4354.0000 400.0000 1110.7140
3 0 0 1 1:1:1:0 yes 4354.0000 400.0000 1110.7140
4 0 0 2 2:2:2:0 yes 4354.0000 400.0000 1110.7140
5 0 0 2 2:2:2:0 yes 4354.0000 400.0000 1110.7140
6 0 0 3 3:3:3:0 yes 4354.0000 400.0000 2278.8491
7 0 0 3 3:3:3:0 yes 4354.0000 400.0000 2732.3569
8 0 0 4 4:4:4:0 yes 4354.0000 400.0000 1110.7140
9 0 0 4 4:4:4:0 yes 4354.0000 400.0000 1110.7140
10 0 0 5 5:5:5:0 yes 4354.0000 400.0000 2353.0449
11 0 0 5 5:5:5:0 yes 4354.0000 400.0000 1110.7140
Perry.
Changes from v11:
* add “goto out_free_policy" for error handling and update error
mesasge for init_boost in cpufreq.c (Mario)
* drop dead code policy check in amd_pstate_cpu_boost_update (Mario)
* pick RB flags and Ack flags from Mario
* rebased to Mario bleeding edge kerne branch
Changes from v10:
* rework the boost interface with cpufreq core boost control, align the sysfs file
created from cpufreq.c and allow indivial CPU boost control (Mario)
* fix the pr_warn code format with %zd (Oleksandr Natalenko)
* replace sscanf with kstrtobool for cpufreq.c (new)
* drop the boost sysfs file creation from amd pstate patch #6
* add init_boost for cpufreq.c to unify the boost file creation(Mario)
* add set_boost callback for EPP driver mode
* fix syncronization issue for indivial boost control and global CPB control, now the
two control way will keep syncronization after anyone CPU boost state changed.
* rebased to Mario kernel tree: bleeding-edge
* run testing on local system, no regression issue found so far.
Changes from v9:
* change per CPU boost sysfs file name to `boost` (Mario)
* rebased to latest linux-pm/bleeding-edge
Changes from v8:
* pick RB flag for patch 4 (Mario)
* change boot_cpu_has to cpu_feature_enabled for patch 2 (Boris)
* merge patch 6 into patch 3 (Mario)
* add two patch for per CPU boost control patch 6 & 7(Mario)
* rebased to latest linux-pm/bleeding-edge
Changes from v7:
* fix the mutext locking issue in the sysfs file update(Ray, Mario)
* pick ack flag from Ray
* use X86_FEATURE_CPB to verify the CPB function in Patch #2(Ray)
* rerun the testing to check function works well
* rebased to linux-pm/bleeding-edge latest
Changes from v6:
* reword patch 2 commit log (Gautham)
* update cover letter description(Gautham)
* rebase to kernel v6.9-rc5
Changes from v4:
* drop the legacy boost remove patch, let us keep the legacy interface
in case some applications break.
* rebase to linux-pm/bleeding-edge branch
* rework the patchset base on [PATCH v8 0/8] AMD Pstate Fixes And
Enhancements which has some intial work done there.
Changes from v4:
* move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h
* pick RB flag from Gautham R. Shenoy
* add Cc Oleksandr Natalenko <oleksandr@natalenko.name>
* rebase to latest linux-pm/bleeding-edge branch
* rebase the patch set on top of [PATCH v7 0/6] AMD Pstate Fixes And Enhancements
* update [PATCH v7 2/6] to use MSR_K7_HWCR_CPB_DIS_BIT
Changes from v3:
* rebased to linux-pm/bleeding-edge v6.8
* rename global to amd_pstate_global_params(Oleksandr Natalenko)
* remove comments for boot_supported in amd_pstate.h
* fix the compiler warning for amd-pstate-ut.ko
* use for_each_online_cpu in cpb_boost_store which fix the null pointer
error during testing
* fix the max frequency value to be KHz when cpb boost disabled(Gautham R. Shenoy)
Changes from v2:
* move global struct to amd-pstate.h
* fix the amd-pstate-ut with new cpb control interface
Changes from v1:
* drop suspend/resume fix patch 6/7 because of the fix should be in
another fix series instead of CPB feature
* move the set_boost remove patch to the last(Mario)
* Fix commit info with "Closes:" (Mario)
* simplified global.cpb_supported initialization(Mario)
* Add guide mode support for CPB control
* Fixed some Doc typos and add guide mode info to Doc as well.
v1: https://lore.kernel.org/all/cover.1706255676.git.perry.yuan@amd.com/
v2: https://lore.kernel.org/lkml/cover.1707047943.git.perry.yuan@amd.com/
v3: https://lore.kernel.org/lkml/cover.1707297581.git.perry.yuan@amd.com/
v4: https://lore.kernel.org/lkml/cover.1710322310.git.perry.yuan@amd.com/
v5: https://lore.kernel.org/lkml/cover.1710473712.git.perry.yuan@amd.com/
v6: https://lore.kernel.org/lkml/cover.1710754236.git.perry.yuan@amd.com/
v7: https://lore.kernel.org/lkml/cover.1713861200.git.perry.yuan@amd.com/
v8: https://lore.kernel.org/lkml/cover.1714112854.git.perry.yuan@amd.com/
v9: https://lore.kernel.org/lkml/cover.1714989803.git.perry.yuan@amd.com/
v10: https://lore.kernel.org/lkml/cover.1715152592.git.perry.yuan@amd.com/
v11: https://lore.kernel.org/lkml/cover.1718262992.git.perry.yuan@amd.com/
Perry Yuan (9):
cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h
cpufreq: simplify boolean parsing with kstrtobool in store function
cpufreq: introduce init_boost callback to initialize boost state for
pstate drivers
cpufreq: amd-pstate: initialize new core precision boost state
cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
cpufreq: amd-pstate: Add set_boost callback for active mode
cpufreq: amd-pstate: fix the MSR highest perf will be reset issue
while cpb boost off
Documentation: cpufreq: amd-pstate: introduce the new cpu boost
control method
Documentation: cpufreq: amd-pstate: update doc for Per CPU boost
control method
Documentation/admin-guide/pm/amd-pstate.rst | 30 +++
arch/x86/include/asm/msr-index.h | 2 +
drivers/cpufreq/acpi-cpufreq.c | 2 -
drivers/cpufreq/amd-pstate-ut.c | 2 +-
drivers/cpufreq/amd-pstate.c | 192 +++++++++++++++++---
drivers/cpufreq/amd-pstate.h | 14 ++
drivers/cpufreq/cpufreq.c | 24 ++-
include/linux/cpufreq.h | 2 +
8 files changed, 233 insertions(+), 35 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v12 1/9] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 9:16 ` [PATCH v12 2/9] cpufreq: simplify boolean parsing with kstrtobool in store function Perry Yuan
` (7 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
There are some other drivers also need to use the
MSR_K7_HWCR_CPB_DIS_BIT for CPB control bit, so it makes sense to move
the definition to a common header file to allow other driver to use it.
No intentional functional impact.
Suggested-by: Gautham Ranjal Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
Acked-by: Huang Rui <ray.huang@amd.com>
---
arch/x86/include/asm/msr-index.h | 2 ++
drivers/cpufreq/acpi-cpufreq.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e022e6eb766c..384739d592af 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -781,6 +781,8 @@
#define MSR_K7_HWCR_IRPERF_EN BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)
#define MSR_K7_FID_VID_CTL 0xc0010041
#define MSR_K7_FID_VID_STATUS 0xc0010042
+#define MSR_K7_HWCR_CPB_DIS_BIT 25
+#define MSR_K7_HWCR_CPB_DIS BIT_ULL(MSR_K7_HWCR_CPB_DIS_BIT)
/* K6 MSRs */
#define MSR_K6_WHCR 0xc0000082
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 37f1cdf46d29..2fc82831bddd 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -50,8 +50,6 @@ enum {
#define AMD_MSR_RANGE (0x7)
#define HYGON_MSR_RANGE (0x7)
-#define MSR_K7_HWCR_CPB_DIS (1ULL << 25)
-
struct acpi_cpufreq_data {
unsigned int resume;
unsigned int cpu_feature;
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 2/9] cpufreq: simplify boolean parsing with kstrtobool in store function
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
2024-06-19 9:16 ` [PATCH v12 1/9] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-21 4:30 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers Perry Yuan
` (6 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Update the cpufreq store function to use kstrtobool for parsing boolean
values. This simplifies the code and improves readability by using a
standard kernel function for boolean string conversion.
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/cpufreq/cpufreq.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a45aac17c20f..1fdabb660231 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -614,10 +614,9 @@ static ssize_t show_boost(struct kobject *kobj,
static ssize_t store_boost(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
- int ret, enable;
+ bool enable;
- ret = sscanf(buf, "%d", &enable);
- if (ret != 1 || enable < 0 || enable > 1)
+ if (kstrtobool(buf, &enable))
return -EINVAL;
if (cpufreq_boost_trigger_state(enable)) {
@@ -641,10 +640,10 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)
static ssize_t store_local_boost(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- int ret, enable;
+ int ret;
+ bool enable;
- ret = kstrtoint(buf, 10, &enable);
- if (ret || enable < 0 || enable > 1)
+ if (kstrtobool(buf, &enable))
return -EINVAL;
if (!cpufreq_driver->boost_enabled)
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
2024-06-19 9:16 ` [PATCH v12 1/9] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h Perry Yuan
2024-06-19 9:16 ` [PATCH v12 2/9] cpufreq: simplify boolean parsing with kstrtobool in store function Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 17:37 ` Mario Limonciello
2024-06-19 9:16 ` [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state Perry Yuan
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Introduce a new init_boost callback in cpufreq to initialize the boost
state for specific pstate drivers. This initialization is required before
calling the set_boost interface for each CPU.
The init_boost callback will set up and synchronize each CPU's current
boost state before invoking the set_boost function. Without this step,
the boost state may be inconsistent across CPUs.
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
drivers/cpufreq/cpufreq.c | 13 +++++++++++--
include/linux/cpufreq.h | 2 ++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1fdabb660231..64c6770e24c8 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1429,8 +1429,17 @@ static int cpufreq_online(unsigned int cpu)
goto out_free_policy;
}
- /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
- policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
+ /* init boost state to prepare set_boost callback for each CPU */
+ if (cpufreq_driver->init_boost) {
+ ret = cpufreq_driver->init_boost(policy);
+ if (ret)
+ pr_debug("%s: %d: boost initialization failed\n", __func__,
+ __LINE__);
+ goto out_offline_policy;
+ } else {
+ /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
+ policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
+ }
/*
* The initialization has succeeded and the policy is online.
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 20f7e98ee8af..0698c0292d8f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -409,6 +409,8 @@ struct cpufreq_driver {
bool boost_enabled;
int (*set_boost)(struct cpufreq_policy *policy, int state);
+ /* initialize boost state to be consistent before calling set_boost */
+ int (*init_boost)(struct cpufreq_policy *policy);
/*
* Set by drivers that want to register with the energy model after the
* policy is properly initialized, but before the governor is started.
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
` (2 preceding siblings ...)
2024-06-19 9:16 ` [PATCH v12 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 19:32 ` Mario Limonciello
2024-06-21 5:23 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control Perry Yuan
` (4 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
Add one global `global_params` to represent 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.
The active, guided and passive modes of the amd-pstate driver can
support frequency boost control when the "Core Performance Boost"
(CPB) feature is enabled in the BIOS. When enabled in BIOS, the user
has an option at runtime to allow/disallow the cores from operating in
the boost frequency range.
Add an amd_pstate_global_params object to record whether CPB is
enabled in BIOS, and if it has been activated by the user
Reported-by: Artem S. Tashkinov" <aros@gmx.com>
Cc: Oleksandr Natalenko <oleksandr@natalenko.name>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 59 +++++++++++++++++++++++++++++-------
drivers/cpufreq/amd-pstate.h | 13 ++++++++
2 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 5bdcdd3ea163..0c50b8ba16b6 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -102,6 +102,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
static bool cppc_enabled;
static bool amd_pstate_prefcore = true;
static struct quirk_entry *quirks;
+struct amd_pstate_global_params amd_pstate_global_params;
+EXPORT_SYMBOL_GPL(amd_pstate_global_params);
/*
* AMD Energy Preference Performance (EPP)
@@ -694,7 +696,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
if (!cpudata->boost_supported) {
pr_err("Boost mode is not supported by this processor or SBIOS\n");
- return -EINVAL;
+ return -ENOTSUPP;
}
if (state)
@@ -712,18 +714,38 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
return 0;
}
-static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
+static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
{
- u32 highest_perf, nominal_perf;
+ u64 boost_val;
+ int ret = -1;
- highest_perf = READ_ONCE(cpudata->highest_perf);
- nominal_perf = READ_ONCE(cpudata->nominal_perf);
+ if (!cpu_feature_enabled(X86_FEATURE_CPB)) {
+ pr_debug_once("Boost CPB capabilities not present in the processor\n");
+ ret = -EOPNOTSUPP;
+ goto exit_err;
+ }
- if (highest_perf <= nominal_perf)
- return;
+ ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
+ if (ret) {
+ pr_err_once("failed to read initial CPU boost state!\n");
+ ret = -EIO;
+ goto exit_err;
+ }
+
+ amd_pstate_global_params.cpb_supported = !(boost_val & MSR_K7_HWCR_CPB_DIS);
+ if (amd_pstate_global_params.cpb_supported) {
+ current_pstate_driver->boost_enabled = true;
+ WRITE_ONCE(cpudata->boost_supported, true);
+ }
- cpudata->boost_supported = true;
- current_pstate_driver->boost_enabled = true;
+ amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
+ return 0;
+
+exit_err:
+ WRITE_ONCE(cpudata->boost_supported, false);
+ current_pstate_driver->boost_enabled = false;
+ amd_pstate_global_params.cpb_boost = false;
+ return ret;
}
static void amd_perf_ctl_reset(unsigned int cpu)
@@ -1005,7 +1027,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;
@@ -1387,6 +1408,21 @@ static bool amd_pstate_acpi_pm_profile_undefined(void)
return false;
}
+static int amd_pstate_init_boost(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+ int ret;
+
+ /* initialize cpu cores boot state */
+ ret = amd_pstate_boost_set(cpudata);
+ if (ret)
+ return ret;
+
+ policy->boost_enabled = READ_ONCE(cpudata->boost_supported);
+
+ return 0;
+}
+
static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
{
int min_freq, max_freq, nominal_freq, ret;
@@ -1465,7 +1501,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;
@@ -1704,6 +1739,7 @@ static struct cpufreq_driver amd_pstate_driver = {
.exit = amd_pstate_cpu_exit,
.suspend = amd_pstate_cpu_suspend,
.resume = amd_pstate_cpu_resume,
+ .init_boost = amd_pstate_init_boost,
.set_boost = amd_pstate_set_boost,
.update_limits = amd_pstate_update_limits,
.name = "amd-pstate",
@@ -1721,6 +1757,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.suspend = amd_pstate_epp_suspend,
.resume = amd_pstate_epp_resume,
.update_limits = amd_pstate_update_limits,
+ .init_boost = amd_pstate_init_boost,
.name = "amd-pstate-epp",
.attr = amd_pstate_epp_attr,
};
diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
index f80b33fa5d43..133042370a8f 100644
--- a/drivers/cpufreq/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -102,4 +102,17 @@ struct amd_cpudata {
s16 epp_default;
};
+/**
+ * struct amd_pstate_global_params - Global parameters, mostly tunable via sysfs.
+ * @cpb_boost: Whether or not to use boost CPU P-states.
+ * @cpb_supported: Whether or not CPU boost P-states are available
+ * based on the MSR_K7_HWCR bit[25] state
+ */
+struct amd_pstate_global_params {
+ bool cpb_boost;
+ bool cpb_supported;
+};
+
+extern struct amd_pstate_global_params amd_pstate_global_params;
+
#endif /* _LINUX_AMD_PSTATE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
` (3 preceding siblings ...)
2024-06-19 9:16 ` [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 19:39 ` Mario Limonciello
2024-06-19 9:16 ` [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode Perry Yuan
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
With this new sysfs entry `cpb_boost`created, user can change CPU boost
state dynamically under `active`, `guided` and `passive` modes.
And the highest perf and frequency will also be updated as the boost
state changing.
0): check current boost state
cat /sys/devices/system/cpu/amd_pstate/cpb_boost
1): disable CPU boost
sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
2): enable CPU boost
sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217931
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217618
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/cpufreq/amd-pstate-ut.c | 2 +-
drivers/cpufreq/amd-pstate.c | 112 +++++++++++++++++++++++++++++++-
drivers/cpufreq/amd-pstate.h | 1 +
3 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index fc275d41d51e..b528f198f4c3 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -227,7 +227,7 @@ static void amd_pstate_ut_check_freq(u32 index)
goto skip_test;
}
- if (cpudata->boost_supported) {
+ if (amd_pstate_global_params.cpb_boost) {
if ((policy->max == cpudata->max_freq) ||
(policy->max == cpudata->nominal_freq))
amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 0c50b8ba16b6..1c2320808ae1 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -104,6 +104,7 @@ static bool amd_pstate_prefcore = true;
static struct quirk_entry *quirks;
struct amd_pstate_global_params amd_pstate_global_params;
EXPORT_SYMBOL_GPL(amd_pstate_global_params);
+static int amd_pstate_cpu_boost(int cpu, bool state);
/*
* AMD Energy Preference Performance (EPP)
@@ -736,6 +737,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
if (amd_pstate_global_params.cpb_supported) {
current_pstate_driver->boost_enabled = true;
WRITE_ONCE(cpudata->boost_supported, true);
+ WRITE_ONCE(cpudata->boost_state, true);
}
amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
@@ -743,6 +745,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
exit_err:
WRITE_ONCE(cpudata->boost_supported, false);
+ WRITE_ONCE(cpudata->boost_state, false);
current_pstate_driver->boost_enabled = false;
amd_pstate_global_params.cpb_boost = false;
return ret;
@@ -1346,6 +1349,111 @@ static ssize_t prefcore_show(struct device *dev,
return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
}
+static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+ struct cppc_perf_ctrls perf_ctrls;
+ u32 highest_perf, nominal_perf, nominal_freq, max_freq;
+ int ret;
+
+ highest_perf = READ_ONCE(cpudata->highest_perf);
+ nominal_perf = READ_ONCE(cpudata->nominal_perf);
+ nominal_freq = READ_ONCE(cpudata->nominal_freq);
+ max_freq = READ_ONCE(cpudata->max_freq);
+
+ 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) {
+ cpufreq_cpu_release(policy);
+ pr_debug("failed to set energy perf value (%d)\n", ret);
+ return ret;
+ }
+ }
+
+ if (on)
+ policy->cpuinfo.max_freq = max_freq;
+ else
+ policy->cpuinfo.max_freq = nominal_freq * 1000;
+
+ policy->max = policy->cpuinfo.max_freq;
+
+ if (cppc_state == AMD_PSTATE_PASSIVE) {
+ ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
+ if (ret < 0)
+ pr_debug("Failed to update freq constraint: CPU%d\n", cpudata->cpu);
+ }
+
+ return ret < 0 ? ret : 0;
+}
+
+static int amd_pstate_cpu_boost(int cpu, bool state)
+{
+ int ret;
+ struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+ struct amd_cpudata *cpudata = policy->driver_data;
+
+ if (!policy) {
+ pr_err("policy is NULL\n");
+ ret = -ENODATA;
+ goto err_exit;
+ }
+
+ ret = amd_pstate_cpu_boost_update(policy, state);
+ refresh_frequency_limits(policy);
+ WRITE_ONCE(cpudata->boost_state, state);
+ policy->boost_enabled = state;
+
+err_exit:
+ cpufreq_cpu_put(policy);
+ return ret < 0 ? ret : 0;
+}
+
+static ssize_t cpb_boost_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%u\n", amd_pstate_global_params.cpb_boost);
+}
+
+static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b,
+ const char *buf, size_t count)
+{
+ bool new_state;
+ ssize_t ret;
+ int cpu;
+
+ if (!amd_pstate_global_params.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 ret;
+
+ mutex_lock(&amd_pstate_driver_lock);
+ for_each_present_cpu(cpu) {
+ ret = amd_pstate_cpu_boost(cpu, new_state);
+ if (ret < 0) {
+ pr_warn("failed to update cpu boost for CPU%d (%zd)\n", cpu, ret);
+ goto err_exit;
+ }
+ }
+ amd_pstate_global_params.cpb_boost = !!new_state;
+
+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);
@@ -1356,6 +1464,7 @@ cpufreq_freq_attr_rw(energy_performance_preference);
cpufreq_freq_attr_ro(energy_performance_available_preferences);
static DEVICE_ATTR_RW(status);
static DEVICE_ATTR_RO(prefcore);
+static DEVICE_ATTR_RW(cpb_boost);
static struct freq_attr *amd_pstate_attr[] = {
&amd_pstate_max_freq,
@@ -1380,6 +1489,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
static struct attribute *pstate_global_attributes[] = {
&dev_attr_status.attr,
&dev_attr_prefcore.attr,
+ &dev_attr_cpb_boost.attr,
NULL
};
@@ -1418,7 +1528,7 @@ static int amd_pstate_init_boost(struct cpufreq_policy *policy)
if (ret)
return ret;
- policy->boost_enabled = READ_ONCE(cpudata->boost_supported);
+ policy->boost_enabled = READ_ONCE(cpudata->boost_state);
return 0;
}
diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
index 133042370a8f..fb240a870289 100644
--- a/drivers/cpufreq/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -100,6 +100,7 @@ struct amd_cpudata {
u64 cppc_cap1_cached;
bool suspended;
s16 epp_default;
+ bool boost_state;
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
` (4 preceding siblings ...)
2024-06-19 9:16 ` [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 19:40 ` Mario Limonciello
2024-06-21 5:39 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off Perry Yuan
` (2 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Add support for the set_boost callback in the active mode driver to
enable boost control via the cpufreq core. This ensures a consistent
boost control interface across all pstate modes, including passive
mode, guided mode, and active mode.
With this addition, all three pstate modes can support the same boost
control interface with unique interface and global CPB control. Each
CPU also supports individual boost control, allowing global CPB to
change all cores' boost states simultaneously. Specific CPUs can
update their boost states separately, ensuring all cores' boost
states are synchronized.
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 1c2320808ae1..299e52d4b17e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -699,20 +699,11 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
pr_err("Boost mode is not supported by this processor or SBIOS\n");
return -ENOTSUPP;
}
+ mutex_lock(&amd_pstate_driver_lock);
+ ret = amd_pstate_cpu_boost(policy->cpu, state);
+ mutex_unlock(&amd_pstate_driver_lock);
- if (state)
- policy->cpuinfo.max_freq = cpudata->max_freq;
- else
- policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
-
- 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;
+ return ret < 0 ? ret : 0;
}
static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
@@ -1868,6 +1859,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.resume = amd_pstate_epp_resume,
.update_limits = amd_pstate_update_limits,
.init_boost = amd_pstate_init_boost,
+ .set_boost = amd_pstate_set_boost,
.name = "amd-pstate-epp",
.attr = amd_pstate_epp_attr,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
` (5 preceding siblings ...)
2024-06-19 9:16 ` [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-21 5:46 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 8/9] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method Perry Yuan
2024-06-19 9:16 ` [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU " Perry Yuan
8 siblings, 1 reply; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
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 it needs 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>
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@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 299e52d4b17e..f2ccef089acc 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -524,6 +524,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
u32 des_perf, u32 max_perf, bool fast_switch, int gov_flags)
{
u64 prev = READ_ONCE(cpudata->cppc_req_cached);
+ u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
u64 value = prev;
min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
@@ -543,6 +544,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 (!amd_pstate_global_params.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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 8/9] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
` (6 preceding siblings ...)
2024-06-19 9:16 ` [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 19:40 ` Mario Limonciello
2024-06-19 9:16 ` [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU " Perry Yuan
8 siblings, 1 reply; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
Introduce AMD CPU frequency boosting control sysfs entry which used for
switching boost on and boost off.
If core performance boost is disabled while a core is in a boosted P-state,
the core automatically transitions to the highest performance non-boosted P-state
The highest perf and frequency will be limited by the setting value.
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
Documentation/admin-guide/pm/amd-pstate.rst | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 1e0d101b020a..bcc0d9404c18 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -440,6 +440,16 @@ control its functionality at the system level. They are located in the
This attribute is read-only to check the state of preferred core set
by the kernel parameter.
+``cpb_boost``
+ Specifies whether core performance boost is requested to be enabled or disabled
+ If core performance boost is disabled while a core is in a boosted P-state, the
+ core automatically transitions to the highest performance non-boosted P-state.
+ AMD Core Performance Boost(CPB) is controlled by this attribute file which allows
+ user to change all cores frequency boosting state. It supports all amd-pstate modes.
+
+ "0" Disable Core Performance Boosting
+ "1" Enable Core Performance Boosting
+
``cpupower`` tool support for ``amd-pstate``
===============================================
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU boost control method
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
` (7 preceding siblings ...)
2024-06-19 9:16 ` [PATCH v12 8/9] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method Perry Yuan
@ 2024-06-19 9:16 ` Perry Yuan
2024-06-19 19:45 ` Mario Limonciello
8 siblings, 1 reply; 24+ messages in thread
From: Perry Yuan @ 2024-06-19 9:16 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Updates the documentation in `amd-pstate.rst` to include information about the
per CPU boost control feature. Users can now enable or disable the Core Performance
Boost (CPB) feature on individual CPUs using the `boost` sysfs attribute.
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
Documentation/admin-guide/pm/amd-pstate.rst | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index bcc0d9404c18..98e43c53c387 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -281,6 +281,26 @@ integer values defined between 0 to 255 when EPP feature is enabled by platform
firmware, if EPP feature is disabled, driver will ignore the written value
This attribute is read-write.
+``boost``
+The `boost` sysfs attribute provides control over the CPU core
+performance boost, allowing users to manage the maximum frequency limitation
+of the CPU. This attribute can be used to enable or disable the boost feature
+on individual CPUs.
+
+When the boost feature is enabled, the CPU can dynamically increase its frequency
+beyond the base frequency, providing enhanced performance for demanding workloads.
+On the other hand, disabling the boost feature restricts the CPU to operate at the
+base frequency, which may be desirable in certain scenarios to prioritize power
+efficiency or manage temperature.
+
+To manipulate the `boost` attribute, users can write a value of `0` to disable the
+boost or `1` to enable it, for the respective CPU using the sysfs path
+`/sys/devices/system/cpu/cpuX/cpufreq/boost`, where `X` represents the CPU number.
+
+It is important to note that modifying the global variable
+`amd_pstate_global_params.cpb_boost` will override the individual CPU settings.
+
+
Other performance and frequency values can be read back from
``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v12 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers
2024-06-19 9:16 ` [PATCH v12 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers Perry Yuan
@ 2024-06-19 17:37 ` Mario Limonciello
0 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2024-06-19 17:37 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 6/19/2024 04:16, Perry Yuan wrote:
> Introduce a new init_boost callback in cpufreq to initialize the boost
> state for specific pstate drivers. This initialization is required before
> calling the set_boost interface for each CPU.
>
> The init_boost callback will set up and synchronize each CPU's current
> boost state before invoking the set_boost function. Without this step,
> the boost state may be inconsistent across CPUs.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
> drivers/cpufreq/cpufreq.c | 13 +++++++++++--
> include/linux/cpufreq.h | 2 ++
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1fdabb660231..64c6770e24c8 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1429,8 +1429,17 @@ static int cpufreq_online(unsigned int cpu)
> goto out_free_policy;
> }
>
> - /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> - policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> + /* init boost state to prepare set_boost callback for each CPU */
> + if (cpufreq_driver->init_boost) {
> + ret = cpufreq_driver->init_boost(policy);
> + if (ret)
> + pr_debug("%s: %d: boost initialization failed\n", __func__,
> + __LINE__);
> + goto out_offline_policy;
You've got a mistake here that you're missing curly braces around the
"if (ret)" block.
So what's going to happen with this code is that no matter if
init_boost() succeeds then you'll still offline the policy.
> + } else {
> + /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> + policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> + }
>
> /*
> * The initialization has succeeded and the policy is online.
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 20f7e98ee8af..0698c0292d8f 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -409,6 +409,8 @@ struct cpufreq_driver {
> bool boost_enabled;
> int (*set_boost)(struct cpufreq_policy *policy, int state);
>
> + /* initialize boost state to be consistent before calling set_boost */
> + int (*init_boost)(struct cpufreq_policy *policy);
> /*
> * Set by drivers that want to register with the energy model after the
> * policy is properly initialized, but before the governor is started.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state
2024-06-19 9:16 ` [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state Perry Yuan
@ 2024-06-19 19:32 ` Mario Limonciello
2024-06-21 5:23 ` Gautham R.Shenoy
1 sibling, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2024-06-19 19:32 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 6/19/2024 04:16, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> Add one global `global_params` to represent 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.
>
> The active, guided and passive modes of the amd-pstate driver can
> support frequency boost control when the "Core Performance Boost"
> (CPB) feature is enabled in the BIOS. When enabled in BIOS, the user
> has an option at runtime to allow/disallow the cores from operating in
> the boost frequency range.
>
> Add an amd_pstate_global_params object to record whether CPB is
> enabled in BIOS, and if it has been activated by the user
>
> Reported-by: Artem S. Tashkinov" <aros@gmx.com>
> Cc: Oleksandr Natalenko <oleksandr@natalenko.name>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 59 +++++++++++++++++++++++++++++-------
> drivers/cpufreq/amd-pstate.h | 13 ++++++++
> 2 files changed, 61 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5bdcdd3ea163..0c50b8ba16b6 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -102,6 +102,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
> static bool cppc_enabled;
> static bool amd_pstate_prefcore = true;
> static struct quirk_entry *quirks;
> +struct amd_pstate_global_params amd_pstate_global_params;
> +EXPORT_SYMBOL_GPL(amd_pstate_global_params);
>
> /*
> * AMD Energy Preference Performance (EPP)
> @@ -694,7 +696,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
>
> if (!cpudata->boost_supported) {
> pr_err("Boost mode is not supported by this processor or SBIOS\n");
> - return -EINVAL;
> + return -ENOTSUPP;
You're using -ENOTSUPP here and -EOPNOTSUPP in amd_pstate_boost_set().
You should be consistent with these error codes. That being said I
think the other error flow needs to go. More on that below.
> }
>
> if (state)
> @@ -712,18 +714,38 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> return 0;
> }
>
> -static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
> +static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
> {
> - u32 highest_perf, nominal_perf;
> + u64 boost_val;
> + int ret = -1;
>
> - highest_perf = READ_ONCE(cpudata->highest_perf);
> - nominal_perf = READ_ONCE(cpudata->nominal_perf);
> + if (!cpu_feature_enabled(X86_FEATURE_CPB)) {
> + pr_debug_once("Boost CPB capabilities not present in the processor\n");
> + ret = -EOPNOTSUPP;
> + goto exit_err;
> + }
This needs a little bit of unwinding to think about whether it's correct
return code.
This is the call path:
cpufreq_driver->init_boost();
-> amd_pstate_init_boost()
->-> amd_pstate_boost_set()
So if we end up with a platform that supports CPPC but doesn't support
boost we might end up nuking the amd-pstate support "because" of the
lack of boost.
For this reason I think it's better to set the return code to 0 in this
specific place. The others in this function look correct to me and if
fail should cause a critical problem with setting up CPPC.
>
> - if (highest_perf <= nominal_perf)
> - return;
> + ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
> + if (ret) {
> + pr_err_once("failed to read initial CPU boost state!\n");
> + ret = -EIO;
> + goto exit_err;
> + }
> +
> + amd_pstate_global_params.cpb_supported = !(boost_val & MSR_K7_HWCR_CPB_DIS);
> + if (amd_pstate_global_params.cpb_supported) {
> + current_pstate_driver->boost_enabled = true;
> + WRITE_ONCE(cpudata->boost_supported, true);
> + }
>
> - cpudata->boost_supported = true;
> - current_pstate_driver->boost_enabled = true;
> + amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
> + return 0;
> +
> +exit_err:
> + WRITE_ONCE(cpudata->boost_supported, false);
> + current_pstate_driver->boost_enabled = false;
> + amd_pstate_global_params.cpb_boost = false;
> + return ret;
> }
>
> static void amd_perf_ctl_reset(unsigned int cpu)
> @@ -1005,7 +1027,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;
>
> @@ -1387,6 +1408,21 @@ static bool amd_pstate_acpi_pm_profile_undefined(void)
> return false;
> }
>
> +static int amd_pstate_init_boost(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> + int ret;
> +
> + /* initialize cpu cores boot state */
> + ret = amd_pstate_boost_set(cpudata);
> + if (ret)
> + return ret;
> +
> + policy->boost_enabled = READ_ONCE(cpudata->boost_supported);
> +
> + return 0;
> +}
> +
> static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> {
> int min_freq, max_freq, nominal_freq, ret;
> @@ -1465,7 +1501,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;
>
> @@ -1704,6 +1739,7 @@ static struct cpufreq_driver amd_pstate_driver = {
> .exit = amd_pstate_cpu_exit,
> .suspend = amd_pstate_cpu_suspend,
> .resume = amd_pstate_cpu_resume,
> + .init_boost = amd_pstate_init_boost,
> .set_boost = amd_pstate_set_boost,
> .update_limits = amd_pstate_update_limits,
> .name = "amd-pstate",
> @@ -1721,6 +1757,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> .suspend = amd_pstate_epp_suspend,
> .resume = amd_pstate_epp_resume,
> .update_limits = amd_pstate_update_limits,
> + .init_boost = amd_pstate_init_boost,
> .name = "amd-pstate-epp",
> .attr = amd_pstate_epp_attr,
> };
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index f80b33fa5d43..133042370a8f 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -102,4 +102,17 @@ struct amd_cpudata {
> s16 epp_default;
> };
>
> +/**
> + * struct amd_pstate_global_params - Global parameters, mostly tunable via sysfs.
> + * @cpb_boost: Whether or not to use boost CPU P-states.
> + * @cpb_supported: Whether or not CPU boost P-states are available
> + * based on the MSR_K7_HWCR bit[25] state
> + */
> +struct amd_pstate_global_params {
> + bool cpb_boost;
> + bool cpb_supported;
> +};
> +
> +extern struct amd_pstate_global_params amd_pstate_global_params;
> +
> #endif /* _LINUX_AMD_PSTATE_H */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
2024-06-19 9:16 ` [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control Perry Yuan
@ 2024-06-19 19:39 ` Mario Limonciello
2024-06-21 5:36 ` Gautham R.Shenoy
0 siblings, 1 reply; 24+ messages in thread
From: Mario Limonciello @ 2024-06-19 19:39 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 6/19/2024 04:16, 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`, `guided` and `passive` modes.
> And the highest perf and frequency will also be updated as the boost
> state changing.
s/changing/changes/
>
> 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-ut.c | 2 +-
> drivers/cpufreq/amd-pstate.c | 112 +++++++++++++++++++++++++++++++-
> drivers/cpufreq/amd-pstate.h | 1 +
> 3 files changed, 113 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index fc275d41d51e..b528f198f4c3 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -227,7 +227,7 @@ static void amd_pstate_ut_check_freq(u32 index)
> goto skip_test;
> }
>
> - if (cpudata->boost_supported) {
> + if (amd_pstate_global_params.cpb_boost) {
> if ((policy->max == cpudata->max_freq) ||
> (policy->max == cpudata->nominal_freq))
> amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 0c50b8ba16b6..1c2320808ae1 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -104,6 +104,7 @@ static bool amd_pstate_prefcore = true;
> static struct quirk_entry *quirks;
> struct amd_pstate_global_params amd_pstate_global_params;
> EXPORT_SYMBOL_GPL(amd_pstate_global_params);
> +static int amd_pstate_cpu_boost(int cpu, bool state);
>
> /*
> * AMD Energy Preference Performance (EPP)
> @@ -736,6 +737,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
> if (amd_pstate_global_params.cpb_supported) {
> current_pstate_driver->boost_enabled = true;
> WRITE_ONCE(cpudata->boost_supported, true);
> + WRITE_ONCE(cpudata->boost_state, true);
> }
>
> amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
> @@ -743,6 +745,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
>
> exit_err:
> WRITE_ONCE(cpudata->boost_supported, false);
> + WRITE_ONCE(cpudata->boost_state, false);
> current_pstate_driver->boost_enabled = false;
> amd_pstate_global_params.cpb_boost = false;
> return ret;
> @@ -1346,6 +1349,111 @@ static ssize_t prefcore_show(struct device *dev,
> return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
> }
>
> +static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> + struct cppc_perf_ctrls perf_ctrls;
> + u32 highest_perf, nominal_perf, nominal_freq, max_freq;
> + int ret;
> +
> + highest_perf = READ_ONCE(cpudata->highest_perf);
> + nominal_perf = READ_ONCE(cpudata->nominal_perf);
> + nominal_freq = READ_ONCE(cpudata->nominal_freq);
> + max_freq = READ_ONCE(cpudata->max_freq);
> +
> + 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) {
> + cpufreq_cpu_release(policy);
> + pr_debug("failed to set energy perf value (%d)\n", ret);
> + return ret;
> + }
> + }
> +
> + if (on)
> + policy->cpuinfo.max_freq = max_freq;
> + else
> + policy->cpuinfo.max_freq = nominal_freq * 1000;
> +
> + policy->max = policy->cpuinfo.max_freq;
> +
> + if (cppc_state == AMD_PSTATE_PASSIVE) {
> + ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
> + if (ret < 0)
> + pr_debug("Failed to update freq constraint: CPU%d\n", cpudata->cpu);
> + }
> +
> + return ret < 0 ? ret : 0;
> +}
> +
> +static int amd_pstate_cpu_boost(int cpu, bool state)
> +{
> + int ret;
> + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> + struct amd_cpudata *cpudata = policy->driver_data;
> +
> + if (!policy) {
> + pr_err("policy is NULL\n");
> + ret = -ENODATA;
> + goto err_exit;
> + }
> +
> + ret = amd_pstate_cpu_boost_update(policy, state);
> + refresh_frequency_limits(policy);
> + WRITE_ONCE(cpudata->boost_state, state);
> + policy->boost_enabled = state;
> +
> +err_exit:
> + cpufreq_cpu_put(policy);
> + return ret < 0 ? ret : 0;
> +}
> +
> +static ssize_t cpb_boost_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%u\n", amd_pstate_global_params.cpb_boost);
> +}
It is incongruent that this returns a 0/1 but prefcore returns
enabled/disabled using str_enabled_disabled().
Can we make this consistent and alos use str_enabled_disabled() please?
> +
> +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;
> +
> + if (!amd_pstate_global_params.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 ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + for_each_present_cpu(cpu) {
> + ret = amd_pstate_cpu_boost(cpu, new_state);
> + if (ret < 0) {
> + pr_warn("failed to update cpu boost for CPU%d (%zd)\n", cpu, ret);
> + goto err_exit;
> + }
> + }
> + amd_pstate_global_params.cpb_boost = !!new_state;
> +
> +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);
>
> @@ -1356,6 +1464,7 @@ cpufreq_freq_attr_rw(energy_performance_preference);
> cpufreq_freq_attr_ro(energy_performance_available_preferences);
> static DEVICE_ATTR_RW(status);
> static DEVICE_ATTR_RO(prefcore);
> +static DEVICE_ATTR_RW(cpb_boost);
>
> static struct freq_attr *amd_pstate_attr[] = {
> &amd_pstate_max_freq,
> @@ -1380,6 +1489,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> static struct attribute *pstate_global_attributes[] = {
> &dev_attr_status.attr,
> &dev_attr_prefcore.attr,
> + &dev_attr_cpb_boost.attr,
> NULL
> };
>
> @@ -1418,7 +1528,7 @@ static int amd_pstate_init_boost(struct cpufreq_policy *policy)
> if (ret)
> return ret;
>
> - policy->boost_enabled = READ_ONCE(cpudata->boost_supported);
> + policy->boost_enabled = READ_ONCE(cpudata->boost_state);
>
> return 0;
> }
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index 133042370a8f..fb240a870289 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -100,6 +100,7 @@ struct amd_cpudata {
> u64 cppc_cap1_cached;
> bool suspended;
> s16 epp_default;
> + bool boost_state;
> };
>
> /**
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode
2024-06-19 9:16 ` [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode Perry Yuan
@ 2024-06-19 19:40 ` Mario Limonciello
2024-06-21 5:39 ` Gautham R.Shenoy
1 sibling, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2024-06-19 19:40 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 6/19/2024 04:16, Perry Yuan wrote:
> Add support for the set_boost callback in the active mode driver to
> enable boost control via the cpufreq core. This ensures a consistent
> boost control interface across all pstate modes, including passive
> mode, guided mode, and active mode.
>
> With this addition, all three pstate modes can support the same boost
> control interface with unique interface and global CPB control. Each
> CPU also supports individual boost control, allowing global CPB to
> change all cores' boost states simultaneously. Specific CPUs can
> update their boost states separately, ensuring all cores' boost
> states are synchronized.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 18 +++++-------------
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1c2320808ae1..299e52d4b17e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -699,20 +699,11 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> pr_err("Boost mode is not supported by this processor or SBIOS\n");
> return -ENOTSUPP;
> }
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_cpu_boost(policy->cpu, state);
> + mutex_unlock(&amd_pstate_driver_lock);
>
> - if (state)
> - policy->cpuinfo.max_freq = cpudata->max_freq;
> - else
> - policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
> -
> - 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;
> + return ret < 0 ? ret : 0;
> }
>
> static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
> @@ -1868,6 +1859,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> .resume = amd_pstate_epp_resume,
> .update_limits = amd_pstate_update_limits,
> .init_boost = amd_pstate_init_boost,
> + .set_boost = amd_pstate_set_boost,
> .name = "amd-pstate-epp",
> .attr = amd_pstate_epp_attr,
> };
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 8/9] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method
2024-06-19 9:16 ` [PATCH v12 8/9] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method Perry Yuan
@ 2024-06-19 19:40 ` Mario Limonciello
0 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2024-06-19 19:40 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 6/19/2024 04:16, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> Introduce AMD CPU frequency boosting control sysfs entry which used for
> switching boost on and boost off.
>
> If core performance boost is disabled while a core is in a boosted P-state,
> the core automatically transitions to the highest performance non-boosted P-state
> The highest perf and frequency will be limited by the setting value.
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Documentation/admin-guide/pm/amd-pstate.rst | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 1e0d101b020a..bcc0d9404c18 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -440,6 +440,16 @@ control its functionality at the system level. They are located in the
> This attribute is read-only to check the state of preferred core set
> by the kernel parameter.
>
> +``cpb_boost``
> + Specifies whether core performance boost is requested to be enabled or disabled
> + If core performance boost is disabled while a core is in a boosted P-state, the
> + core automatically transitions to the highest performance non-boosted P-state.
> + AMD Core Performance Boost(CPB) is controlled by this attribute file which allows
> + user to change all cores frequency boosting state. It supports all amd-pstate modes.
> +
> + "0" Disable Core Performance Boosting
> + "1" Enable Core Performance Boosting
> +
Please switch to disabled/enabled as suggested in patch 5.
> ``cpupower`` tool support for ``amd-pstate``
> ===============================================
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU boost control method
2024-06-19 9:16 ` [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU " Perry Yuan
@ 2024-06-19 19:45 ` Mario Limonciello
2024-06-21 6:42 ` Gautham R.Shenoy
0 siblings, 1 reply; 24+ messages in thread
From: Mario Limonciello @ 2024-06-19 19:45 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, viresh.kumar, gautham.shenoy
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 6/19/2024 04:16, Perry Yuan wrote:
> Updates the documentation in `amd-pstate.rst` to include information about the
> per CPU boost control feature. Users can now enable or disable the Core Performance
> Boost (CPB) feature on individual CPUs using the `boost` sysfs attribute.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Documentation/admin-guide/pm/amd-pstate.rst | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index bcc0d9404c18..98e43c53c387 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -281,6 +281,26 @@ integer values defined between 0 to 255 when EPP feature is enabled by platform
> firmware, if EPP feature is disabled, driver will ignore the written value
> This attribute is read-write.
>
> +``boost``
> +The `boost` sysfs attribute provides control over the CPU core
> +performance boost, allowing users to manage the maximum frequency limitation
> +of the CPU. This attribute can be used to enable or disable the boost feature
> +on individual CPUs.
> +
> +When the boost feature is enabled, the CPU can dynamically increase its frequency
> +beyond the base frequency, providing enhanced performance for demanding workloads.
> +On the other hand, disabling the boost feature restricts the CPU to operate at the
> +base frequency, which may be desirable in certain scenarios to prioritize power
> +efficiency or manage temperature.
> +
> +To manipulate the `boost` attribute, users can write a value of `0` to disable the
> +boost or `1` to enable it, for the respective CPU using the sysfs path
> +`/sys/devices/system/cpu/cpuX/cpufreq/boost`, where `X` represents the CPU number.
> +
> +It is important to note that modifying the global variable
> +`amd_pstate_global_params.cpb_boost` will override the individual CPU settings.
The whole point of the global cpb_boost is an easy way for software to
turn on/off for the whole package.
This makes me wonder if we want a "custom" string for the global param
if one of the CPUs has been individually modified separately from the
global flag. Then software can very easily tell when it's been toggled
for a given CPU or via the global flag.
* Basically if userspace wrote to a single CPU store a bit in the global
structure to say cpb_custom = true if it's not already set.
* If writing to cpb_boost from sysfs then "cpb_custom = false"
* If reading cpb_boost from sysfs and "cpb_custom = true" then double
check all CPUs.
- If they don't have the same value return "custom".
- If they all have the same values this means software has updated
them all, set "cpb_custom = false" and then return enabled/disabled.
What do you think?
> +
> +
> Other performance and frequency values can be read back from
> ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/9] cpufreq: simplify boolean parsing with kstrtobool in store function
2024-06-19 9:16 ` [PATCH v12 2/9] cpufreq: simplify boolean parsing with kstrtobool in store function Perry Yuan
@ 2024-06-21 4:30 ` Gautham R.Shenoy
0 siblings, 0 replies; 24+ messages in thread
From: Gautham R.Shenoy @ 2024-06-21 4:30 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, viresh.kumar
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Perry Yuan <perry.yuan@amd.com> writes:
> Update the cpufreq store function to use kstrtobool for parsing boolean
> values. This simplifies the code and improves readability by using a
> standard kernel function for boolean string conversion.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> ---
> drivers/cpufreq/cpufreq.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index a45aac17c20f..1fdabb660231 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -614,10 +614,9 @@ static ssize_t show_boost(struct kobject *kobj,
> static ssize_t store_boost(struct kobject *kobj, struct kobj_attribute *attr,
> const char *buf, size_t count)
> {
> - int ret, enable;
> + bool enable;
>
> - ret = sscanf(buf, "%d", &enable);
> - if (ret != 1 || enable < 0 || enable > 1)
> + if (kstrtobool(buf, &enable))
> return -EINVAL;
>
> if (cpufreq_boost_trigger_state(enable)) {
> @@ -641,10 +640,10 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)
> static ssize_t store_local_boost(struct cpufreq_policy *policy,
> const char *buf, size_t count)
> {
> - int ret, enable;
> + int ret;
> + bool enable;
>
> - ret = kstrtoint(buf, 10, &enable);
> - if (ret || enable < 0 || enable > 1)
> + if (kstrtobool(buf, &enable))
> return -EINVAL;
>
> if (!cpufreq_driver->boost_enabled)
> --
> 2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state
2024-06-19 9:16 ` [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state Perry Yuan
2024-06-19 19:32 ` Mario Limonciello
@ 2024-06-21 5:23 ` Gautham R.Shenoy
1 sibling, 0 replies; 24+ messages in thread
From: Gautham R.Shenoy @ 2024-06-21 5:23 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, viresh.kumar
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Perry Yuan <perry.yuan@amd.com> writes:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> Add one global `global_params` to represent 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.
>
> The active, guided and passive modes of the amd-pstate driver can
> support frequency boost control when the "Core Performance Boost"
> (CPB) feature is enabled in the BIOS. When enabled in BIOS, the user
> has an option at runtime to allow/disallow the cores from operating in
> the boost frequency range.
>
> Add an amd_pstate_global_params object to record whether CPB is
> enabled in BIOS, and if it has been activated by the user
Can we rephrase this as :
"The "Core Performance Boost (CPB) feature, when enabled in the BIOS,
allows the OS to control the highest performance for each individual
core. The active, passive and the guided modes of the amd-pstate driver
do support controlling the core frequency boost when this BIOS feature
is enabled. Additionally, the amd-pstate driver provides a sysfs
interface allowing the user to activate/deactive this core performance
boost featur at runtime.
Add an amd_pstate_global_params object to record whether CPB is enabled
in the BIOS, and if it has been activated by the user."
>
> Reported-by: Artem S. Tashkinov" <aros@gmx.com>
> Cc: Oleksandr Natalenko <oleksandr@natalenko.name>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217931
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 59 +++++++++++++++++++++++++++++-------
> drivers/cpufreq/amd-pstate.h | 13 ++++++++
> 2 files changed, 61 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5bdcdd3ea163..0c50b8ba16b6 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -102,6 +102,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
> static bool cppc_enabled;
> static bool amd_pstate_prefcore = true;
> static struct quirk_entry *quirks;
> +struct amd_pstate_global_params amd_pstate_global_params;
> +EXPORT_SYMBOL_GPL(amd_pstate_global_params);
>
> /*
> * AMD Energy Preference Performance (EPP)
> @@ -694,7 +696,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
>
> if (!cpudata->boost_supported) {
> pr_err("Boost mode is not supported by this processor or SBIOS\n");
> - return -EINVAL;
> + return -ENOTSUPP;
> }
>
> if (state)
> @@ -712,18 +714,38 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> return 0;
> }
>
> -static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
> +static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
There is already an amd_pstate_set_boost(). The new name you are
providing is amd_pstate_boost_set(). Makes it hard to read the code.
Can we rename the existing amd_pstate_set_boost() to
amd_pstate_update_boost() and keep amd_pstate_boost_set() for this
function ?
> {
> - u32 highest_perf, nominal_perf;
> + u64 boost_val;
> + int ret = -1;
>
> - highest_perf = READ_ONCE(cpudata->highest_perf);
> - nominal_perf = READ_ONCE(cpudata->nominal_perf);
> + if (!cpu_feature_enabled(X86_FEATURE_CPB)) {
> + pr_debug_once("Boost CPB capabilities not present in the processor\n");
> + ret = -EOPNOTSUPP;
> + goto exit_err;
> + }
>
> - if (highest_perf <= nominal_perf)
> - return;
> + ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
> + if (ret) {
> + pr_err_once("failed to read initial CPU boost state!\n");
> + ret = -EIO;
> + goto exit_err;
> + }
> +
> + amd_pstate_global_params.cpb_supported = !(boost_val & MSR_K7_HWCR_CPB_DIS);
"amd_pstate_global_params.cpb_supported" will always contain the
MSR_K7_HWCR[CPB_DIS] of the last CPU that calls amd_pstate_boost_set()
since the code overwrites the value each time amd_pstate_boost_set() is
called for cpudata. So would it be correct to assume the
MSR_K7_HWCR[CPB_DIS] is going to be the same for every CPU ?
--
Thanks and Regards
gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
2024-06-19 19:39 ` Mario Limonciello
@ 2024-06-21 5:36 ` Gautham R.Shenoy
0 siblings, 0 replies; 24+ messages in thread
From: Gautham R.Shenoy @ 2024-06-21 5:36 UTC (permalink / raw)
To: Mario Limonciello, Perry Yuan, rafael.j.wysocki, viresh.kumar
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Mario Limonciello <mario.limonciello@amd.com> writes:
> On 6/19/2024 04:16, 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`, `guided` and `passive` modes.
>> And the highest perf and frequency will also be updated as the boost
>> state changing.
>
> s/changing/changes/
>
>>
>> 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"
From the point of view of the functionality this new interface is no
different from
/sys/devices/system/cpu/cpufreq/boost, which is currently suppported for
the passive mode and the guided mode.
Is there anything that prevents us from enabling this interface for the
active mode as well?
It is easier if we use the common interface because tools such as
cpupower will then not need to fiddle with several different sysfs files
to enable/disable boost.
--
Thanks and Regards
gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode
2024-06-19 9:16 ` [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode Perry Yuan
2024-06-19 19:40 ` Mario Limonciello
@ 2024-06-21 5:39 ` Gautham R.Shenoy
2024-06-21 5:54 ` Yuan, Perry
1 sibling, 1 reply; 24+ messages in thread
From: Gautham R.Shenoy @ 2024-06-21 5:39 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, viresh.kumar
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Perry Yuan <perry.yuan@amd.com> writes:
> Add support for the set_boost callback in the active mode driver to
> enable boost control via the cpufreq core. This ensures a consistent
> boost control interface across all pstate modes, including passive
> mode, guided mode, and active mode.
>
> With this addition, all three pstate modes can support the same boost
> control interface with unique interface and global CPB control. Each
> CPU also supports individual boost control, allowing global CPB to
> change all cores' boost states simultaneously. Specific CPUs can
> update their boost states separately, ensuring all cores' boost
> states are synchronized.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Acked-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
With this patch, is there any specific reason why we still need
/sys/devices/system/cpu/amd_pstate/boost_enable ?
> ---
> drivers/cpufreq/amd-pstate.c | 18 +++++-------------
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1c2320808ae1..299e52d4b17e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -699,20 +699,11 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> pr_err("Boost mode is not supported by this processor or SBIOS\n");
> return -ENOTSUPP;
> }
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_cpu_boost(policy->cpu, state);
> + mutex_unlock(&amd_pstate_driver_lock);
>
> - if (state)
> - policy->cpuinfo.max_freq = cpudata->max_freq;
> - else
> - policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
> -
> - 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;
> + return ret < 0 ? ret : 0;
> }
>
> static int amd_pstate_boost_set(struct amd_cpudata *cpudata)
> @@ -1868,6 +1859,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> .resume = amd_pstate_epp_resume,
> .update_limits = amd_pstate_update_limits,
> .init_boost = amd_pstate_init_boost,
> + .set_boost = amd_pstate_set_boost,
> .name = "amd-pstate-epp",
> .attr = amd_pstate_epp_attr,
> };
> --
> 2.34.1
--
Thanks and Regards
gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off
2024-06-19 9:16 ` [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off Perry Yuan
@ 2024-06-21 5:46 ` Gautham R.Shenoy
2024-06-21 5:56 ` Yuan, Perry
0 siblings, 1 reply; 24+ messages in thread
From: Gautham R.Shenoy @ 2024-06-21 5:46 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, viresh.kumar
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Perry Yuan <perry.yuan@amd.com> writes:
> 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 it needs 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.
Perhaps we could rephrase this as
"When Core Performance Boost is disabled by the user, the
CPPC_REQ.max_perf should not exceed the nominal_perf since by definition
the frequencies between nominal_perf and the highest_perf are in the
boost range. Fix this in amd_pstate_update()"
Also change the subject to
"cpufreq:amd-pstate: Cap the CPPC.max_perf to nominal_perf if CPB is off"
?
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Other than that,
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@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 299e52d4b17e..f2ccef089acc 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -524,6 +524,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
> u32 des_perf, u32 max_perf, bool fast_switch, int gov_flags)
> {
> u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> + u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
> u64 value = prev;
>
> min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
> @@ -543,6 +544,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 (!amd_pstate_global_params.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
--
Thanks and Regards
gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode
2024-06-21 5:39 ` Gautham R.Shenoy
@ 2024-06-21 5:54 ` Yuan, Perry
0 siblings, 0 replies; 24+ messages in thread
From: Yuan, Perry @ 2024-06-21 5:54 UTC (permalink / raw)
To: Shenoy, Gautham Ranjal, rafael.j.wysocki@intel.com,
Limonciello, Mario, viresh.kumar@linaro.org
Cc: Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Gautham.
> -----Original Message-----
> From: Shenoy, Gautham Ranjal <gautham.shenoy@amd.com>
> Sent: Friday, June 21, 2024 1:39 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org
> Cc: 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 v12 6/9] cpufreq: amd-pstate: Add set_boost callback for
> active mode
>
> Perry Yuan <perry.yuan@amd.com> writes:
>
> > Add support for the set_boost callback in the active mode driver to
> > enable boost control via the cpufreq core. This ensures a consistent
> > boost control interface across all pstate modes, including passive
> > mode, guided mode, and active mode.
> >
> > With this addition, all three pstate modes can support the same boost
> > control interface with unique interface and global CPB control. Each
> > CPU also supports individual boost control, allowing global CPB to
> > change all cores' boost states simultaneously. Specific CPUs can
> > update their boost states separately, ensuring all cores' boost states
> > are synchronized.
> >
> > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
>
>
> Acked-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
>
> With this patch, is there any specific reason why we still need
> /sys/devices/system/cpu/amd_pstate/boost_enable ?
Mario suggested to use global and PER CPU control in the previous thread.
So now we have one global "/sys/devices/system/cpu/amd_pstate/cpb_boost" and
Per CPU control "sys/devices/system/cpu/cpuX/cpufreq/boost "
You can see more details in the cover letter.
Perry.
>
>
> > ---
> > drivers/cpufreq/amd-pstate.c | 18 +++++-------------
> > 1 file changed, 5 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 1c2320808ae1..299e52d4b17e
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -699,20 +699,11 @@ static int amd_pstate_set_boost(struct
> cpufreq_policy *policy, int state)
> > pr_err("Boost mode is not supported by this processor or
> SBIOS\n");
> > return -ENOTSUPP;
> > }
> > + mutex_lock(&amd_pstate_driver_lock);
> > + ret = amd_pstate_cpu_boost(policy->cpu, state);
> > + mutex_unlock(&amd_pstate_driver_lock);
> >
> > - if (state)
> > - policy->cpuinfo.max_freq = cpudata->max_freq;
> > - else
> > - policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
> > -
> > - 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;
> > + return ret < 0 ? ret : 0;
> > }
> >
> > static int amd_pstate_boost_set(struct amd_cpudata *cpudata) @@
> > -1868,6 +1859,7 @@ static struct cpufreq_driver amd_pstate_epp_driver =
> {
> > .resume = amd_pstate_epp_resume,
> > .update_limits = amd_pstate_update_limits,
> > .init_boost = amd_pstate_init_boost,
> > + .set_boost = amd_pstate_set_boost,
> > .name = "amd-pstate-epp",
> > .attr = amd_pstate_epp_attr,
> > };
> > --
> > 2.34.1
>
> --
> Thanks and Regards
> gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off
2024-06-21 5:46 ` Gautham R.Shenoy
@ 2024-06-21 5:56 ` Yuan, Perry
0 siblings, 0 replies; 24+ messages in thread
From: Yuan, Perry @ 2024-06-21 5:56 UTC (permalink / raw)
To: Shenoy, Gautham Ranjal, rafael.j.wysocki@intel.com,
Limonciello, Mario, viresh.kumar@linaro.org
Cc: Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Shenoy, Gautham Ranjal <gautham.shenoy@amd.com>
> Sent: Friday, June 21, 2024 1:46 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org
> Cc: 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 v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf
> will be reset issue while cpb boost off
>
> Perry Yuan <perry.yuan@amd.com> writes:
>
> > 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 it needs 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.
>
> Perhaps we could rephrase this as
>
> "When Core Performance Boost is disabled by the user, the
> CPPC_REQ.max_perf should not exceed the nominal_perf since by definition
> the frequencies between nominal_perf and the highest_perf are in the boost
> range. Fix this in amd_pstate_update()"
>
> Also change the subject to
>
> "cpufreq:amd-pstate: Cap the CPPC.max_perf to nominal_perf if CPB is off"
Sure, will update the tile in v13, will send out the V13 today to catch the merge window.
Thanks for the review.
Perry.
>
> ?
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > Acked-by: Huang Rui <ray.huang@amd.com>
> > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>
> Other than that,
>
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@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 299e52d4b17e..f2ccef089acc 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -524,6 +524,7 @@ static void amd_pstate_update(struct amd_cpudata
> *cpudata, u32 min_perf,
> > u32 des_perf, u32 max_perf, bool fast_switch, int
> gov_flags)
> > {
> > u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> > + u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
> > u64 value = prev;
> >
> > min_perf = clamp_t(unsigned long, min_perf, cpudata-
> >min_limit_perf,
> > @@ -543,6 +544,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 (!amd_pstate_global_params.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
>
> --
> Thanks and Regards
> gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU boost control method
2024-06-19 19:45 ` Mario Limonciello
@ 2024-06-21 6:42 ` Gautham R.Shenoy
0 siblings, 0 replies; 24+ messages in thread
From: Gautham R.Shenoy @ 2024-06-21 6:42 UTC (permalink / raw)
To: Mario Limonciello, Perry Yuan, rafael.j.wysocki, viresh.kumar
Cc: Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Mario Limonciello <mario.limonciello@amd.com> writes:
> On 6/19/2024 04:16, Perry Yuan wrote:
>> Updates the documentation in `amd-pstate.rst` to include information about the
>> per CPU boost control feature. Users can now enable or disable the Core Performance
>> Boost (CPB) feature on individual CPUs using the `boost` sysfs attribute.
>>
>> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> Documentation/admin-guide/pm/amd-pstate.rst | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
>> index bcc0d9404c18..98e43c53c387 100644
>> --- a/Documentation/admin-guide/pm/amd-pstate.rst
>> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
>> @@ -281,6 +281,26 @@ integer values defined between 0 to 255 when EPP feature is enabled by platform
>> firmware, if EPP feature is disabled, driver will ignore the written value
>> This attribute is read-write.
>>
>> +``boost``
>> +The `boost` sysfs attribute provides control over the CPU core
>> +performance boost, allowing users to manage the maximum frequency limitation
>> +of the CPU. This attribute can be used to enable or disable the boost feature
>> +on individual CPUs.
>> +
>> +When the boost feature is enabled, the CPU can dynamically increase its frequency
>> +beyond the base frequency, providing enhanced performance for demanding workloads.
>> +On the other hand, disabling the boost feature restricts the CPU to operate at the
>> +base frequency, which may be desirable in certain scenarios to prioritize power
>> +efficiency or manage temperature.
>> +
>> +To manipulate the `boost` attribute, users can write a value of `0` to disable the
>> +boost or `1` to enable it, for the respective CPU using the sysfs path
>> +`/sys/devices/system/cpu/cpuX/cpufreq/boost`, where `X` represents the CPU number.
>> +
>> +It is important to note that modifying the global variable
>> +`amd_pstate_global_params.cpb_boost` will override the individual CPU settings.
>
> The whole point of the global cpb_boost is an easy way for software to
> turn on/off for the whole package.
>
> This makes me wonder if we want a "custom" string for the global param
> if one of the CPUs has been individually modified separately from the
> global flag. Then software can very easily tell when it's been toggled
> for a given CPU or via the global flag.
Yeah, this is one usecase which may justify the need for a separate
amd_pstate/cpb_boost interface. i.e., if there are users who do want to
reset the boost state on all the CPUs if individual CPUs have
independently toggled the boost states.
>
> * Basically if userspace wrote to a single CPU store a bit in the global
> structure to say cpb_custom = true if it's not already set.
> * If writing to cpb_boost from sysfs then "cpb_custom = false"
> * If reading cpb_boost from sysfs and "cpb_custom = true" then double
> check all CPUs.
Actually, we can maintain a counter which is set to nr_online_cpus when
the global cpb_boost is enabled and set to 0 when global cpb_boost is
disabled. When each individual CPU disables cpb , we decrement the value
of the counter. We increment the counter value when some individual CPU
enables cpb.
So global counter returns "enabled" if counter == nr_policies, returns
"disabled" if counter == 0 and "custom" for all other values of the
counter.
> - If they don't have the same value return "custom".
> - If they all have the same values this means software has updated
> them all, set "cpb_custom = false" and then return enabled/disabled.
>
> What do you think?
Could be done. But again, are there users who want to see the "custom"
value ?
--
Thanks and Regards
gautham.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2024-06-21 6:43 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-19 9:16 [PATCH v12 0/9] AMD Pstate Driver Core Performance Boost Perry Yuan
2024-06-19 9:16 ` [PATCH v12 1/9] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h Perry Yuan
2024-06-19 9:16 ` [PATCH v12 2/9] cpufreq: simplify boolean parsing with kstrtobool in store function Perry Yuan
2024-06-21 4:30 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers Perry Yuan
2024-06-19 17:37 ` Mario Limonciello
2024-06-19 9:16 ` [PATCH v12 4/9] cpufreq: amd-pstate: initialize new core precision boost state Perry Yuan
2024-06-19 19:32 ` Mario Limonciello
2024-06-21 5:23 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 5/9] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control Perry Yuan
2024-06-19 19:39 ` Mario Limonciello
2024-06-21 5:36 ` Gautham R.Shenoy
2024-06-19 9:16 ` [PATCH v12 6/9] cpufreq: amd-pstate: Add set_boost callback for active mode Perry Yuan
2024-06-19 19:40 ` Mario Limonciello
2024-06-21 5:39 ` Gautham R.Shenoy
2024-06-21 5:54 ` Yuan, Perry
2024-06-19 9:16 ` [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off Perry Yuan
2024-06-21 5:46 ` Gautham R.Shenoy
2024-06-21 5:56 ` Yuan, Perry
2024-06-19 9:16 ` [PATCH v12 8/9] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method Perry Yuan
2024-06-19 19:40 ` Mario Limonciello
2024-06-19 9:16 ` [PATCH v12 9/9] Documentation: cpufreq: amd-pstate: update doc for Per CPU " Perry Yuan
2024-06-19 19:45 ` Mario Limonciello
2024-06-21 6:42 ` Gautham R.Shenoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).