Skip to contentSkip to conversation

AMD Pstate Driver Core Performance Boost

PATCH Superseded Perry Yuan 3 participants 12 messages Mar 18, 2024 – Mar 26, 2024

Subsystems (4)
Documentation x86 CPU frequency scaling framework AMD pstate driver

v6 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Perry Yuan

author reviewer
#0 lore raw

v6 cover AMD Pstate Driver Core Performance Boost

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

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

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

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

1). enable core boost:

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

2). disabble core boost:

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

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

If you would like to test this patchset, it needs to apply the patchset based on below one latest version patchset.
https://lore.kernel.org/lkml/[email protected]/

Perry.

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 <[email protected]>
* 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/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v5: https://lore.kernel.org/lkml/[email protected]/

Perry Yuan (6):
cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h
cpufreq: amd-pstate: initialize new core precision boost state
cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
cpufreq: amd-pstate: fix the MSR highest perf will be reset issue
while cpb boost off
Documentation: cpufreq: amd-pstate: introduce the new cpu boost
control method
cpufreq: amd-pstate-ut: support new cpb boost control interface

 Documentation/admin-guide/pm/amd-pstate.rst |  11 ++
 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                | 143 ++++++++++++++++++--
 include/linux/amd-pstate.h                  |  13 ++
 6 files changed, 160 insertions(+), 13 deletions(-)
Signature · 1 line
2.34.1
author reviewer
#1 lore raw

PATCH v6 1/6 cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h

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 <[email protected]>
Signed-off-by Perry Yuan <[email protected]>
---
 arch/x86/include/asm/msr-index.h | 2 ++
 drivers/cpufreq/acpi-cpufreq.c   | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/msr-index.h +2 -0
index 05956bd8bacf..41503647192e 100644
@@ -775,6 +775,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
drivers/cpufreq/acpi-cpufreq.c +0 -2
index 37f1cdf46d29..2fc82831bddd 100644
@@ -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;
Signature · 1 line
2.34.1
maintainer Acked-by on v6 1/6
#2 lore raw

Re: [PATCH v6 1/6] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h

Quoted 9 lines · Perry Yuan

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 <[email protected]>
Signed-off-by Perry Yuan <[email protected]>

This change is fine with me, so

Acked-by Rafael J. Wysocki <[email protected]>
Quoted 35 lines
---
 arch/x86/include/asm/msr-index.h | 2 ++
 drivers/cpufreq/acpi-cpufreq.c   | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/msr-index.h +2 -0
index 05956bd8bacf..41503647192e 100644
@@ -775,6 +775,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
drivers/cpufreq/acpi-cpufreq.c +0 -3
index 37f1cdf46d29..2fc82831bddd 100644
@@ -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

author reviewer
#3 lore raw

RE: [PATCH v6 1/6] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h

[AMD Official Use Only - General]

Hi Rafael,

Quoted 22 lines

-----Original Message-----
From: Rafael J. Wysocki <[email protected]>
Sent: Monday, March 18, 2024 8:43 PM
To: Yuan, Perry <[email protected]>
Cc: [email protected]; Limonciello, Mario <[email protected]>; [email protected]; Huang, Ray <[email protected]>; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>; Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; [email protected]; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; linux- [email protected]; [email protected]
Subject: Re: [PATCH v6 1/6] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h

Quoted 5 lines · Perry Yuan

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.

Quoted 8 lines
Quoted 4 lines

No intentional functional impact.

Suggested-by Gautham Ranjal Shenoy <[email protected]>
Signed-off-by Perry Yuan <[email protected]>

This change is fine with me, so

Acked-by Rafael J. Wysocki <[email protected]>

Thank you help to ack the change,
Will pick tag in next version.

Perry.

Quoted 40 lines

arch/x86/include/asm/msr-index.h line 775

Quoted 13 lines
---
 arch/x86/include/asm/msr-index.h | 2 ++
 drivers/cpufreq/acpi-cpufreq.c   | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/msr-index.h +0 -0
index 05956bd8bacf..41503647192e 100644
@@ -775,6 +775,8 @@
 #define MSR_K7_HWCR_IRPERF_EN

BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)

arch/x86/include/asm/msr-index.h

Quoted 4 lines
arch/x86/include/asm/msr-index.h +2 -0
 #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)

Quoted 21 lines
arch/x86/include/asm/msr-index.h +0 -0
 /* K6 MSRs */
 #define MSR_K6_WHCR                    0xc0000082
(quoted diff) +0 -0

b/drivers/cpufreq/acpi-cpufreq.c index 37f1cdf46d29..2fc82831bddd 100644

drivers/cpufreq/acpi-cpufreq.c +0 -3
@@ -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

author reviewer
#4 lore raw

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

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

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

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

Reported-by Artem S. Tashkinov" <[email protected]>
Cc Oleksandr Natalenko <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217931
Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 39 +++++++++++++++++++++++++++---------
 include/linux/amd-pstate.h   | 13 ++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)
drivers/cpufreq/amd-pstate.c +29 -10
index 59a2db225d98..81787f83c906 100644
@@ -68,6 +68,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 static bool amd_pstate_prefcore = true;
 static struct quirk_entry *quirks;
+struct amd_pstate_global_params amd_pstate_global_params;
+EXPORT_SYMBOL_GPL(amd_pstate_global_params);
 
 /*
  * AMD Energy Preference Performance (EPP)

@@ -665,18 +667,27 @@ 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_init(struct amd_cpudata *cpudata)
 {
-	u32 highest_perf, nominal_perf;
+	u64 boost_val;
+	int ret;
 
-	highest_perf = READ_ONCE(cpudata->highest_perf);
-	nominal_perf = READ_ONCE(cpudata->nominal_perf);
+	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
+	if (ret) {
+		pr_err_once("failed to read initial CPU boost state!\n");
+		return ret;
+	}
 
-	if (highest_perf <= nominal_perf)
-		return;
+	amd_pstate_global_params.cpb_supported = !(boost_val & MSR_K7_HWCR_CPB_DIS);
+
+	if (amd_pstate_global_params.cpb_supported) {
+		cpudata->boost_supported = true;
+		current_pstate_driver->boost_enabled = true;
+	}
 
-	cpudata->boost_supported = true;
-	current_pstate_driver->boost_enabled = true;
+	amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
+
+	return ret;
 }
 
 static void amd_perf_ctl_reset(unsigned int cpu)

@@ -900,6 +911,11 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	amd_pstate_init_prefcore(cpudata);
 
+	/* initialize cpu cores boot state */
+	ret = amd_pstate_boost_init(cpudata);
+	if (ret)
+		goto free_cpudata1;
+
 	ret = amd_pstate_init_perf(cpudata);
 	if (ret)
 		goto free_cpudata1;

@@ -956,7 +972,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;
 

@@ -1363,6 +1378,11 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	amd_pstate_init_prefcore(cpudata);
 
+	/* initialize cpu cores boot state */
+	ret = amd_pstate_boost_init(cpudata);
+	if (ret)
+		goto free_cpudata1;
+
 	ret = amd_pstate_init_perf(cpudata);
 	if (ret)
 		goto free_cpudata1;

@@ -1417,7 +1437,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 			return ret;
 		WRITE_ONCE(cpudata->cppc_cap1_cached, value);
 	}
-	amd_pstate_boost_init(cpudata);
 
 	return 0;
include/linux/amd-pstate.h +13 -0
index 6b832153a126..c5e41de65f70 100644
@@ -134,4 +134,17 @@ struct quirk_entry {
 	u32 lowest_freq;
 };
 
+/**
+ * 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 */
Signature · 1 line
2.34.1
#5 lore raw

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

Hello Perry,

Quoted 3 lines · Perry Yuan

From: Perry Yuan <[email protected]>

Add gloal global_params to represent current CPU Performance Boost(cpb)

      ^^^^^^
      global ?
Quoted 4 lines

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.

Could we reword this portion along the lines of the following:

"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."

Quoted 28 lines

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

Reported-by Artem S. Tashkinov" <[email protected]>
Cc Oleksandr Natalenko <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217931
Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 39 +++++++++++++++++++++++++++---------
 include/linux/amd-pstate.h   | 13 ++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)
drivers/cpufreq/amd-pstate.c +2 -0
index 59a2db225d98..81787f83c906 100644
@@ -68,6 +68,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 static bool amd_pstate_prefcore = true;
 static struct quirk_entry *quirks;
+struct amd_pstate_global_params amd_pstate_global_params;
+EXPORT_SYMBOL_GPL(amd_pstate_global_params);
 
 /*
  * AMD Energy Preference Performance (EPP)

[..snip..]

Quoted 10 lines
include/linux/amd-pstate.h +2 -0
index 6b832153a126..c5e41de65f70 100644
@@ -134,4 +134,17 @@ struct quirk_entry {
 	u32 lowest_freq;
 };
 
+/**
+ * struct amd_pstate_global_params - Global parameters, mostly tunable via sysfs.

include/linux/amd-pstate.h struct quirk_entry { · line 148

Quoted 10 lines
include/linux/amd-pstate.h +10 -0
+ * @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;

Will this be used in multiple files ? If no, it is better to define this in amd-pstate.c

Otherwise, I have no other issues with the patch.

--
Thanks and Regards
gautham.

author reviewer
#6 lore raw

RE: [PATCH v6 2/6] cpufreq: amd-pstate: initialize new core precision boost state

[AMD Official Use Only - General]

Hi Gautham,

Quoted 16 lines

-----Original Message-----
From: Shenoy, Gautham Ranjal <[email protected]>
Sent: Wednesday, March 20, 2024 7:32 PM
To: Yuan, Perry <[email protected]>
Cc: [email protected]; Limonciello, Mario <[email protected]>; [email protected]; Huang, Ray <[email protected]>; Petkov, Borislav <[email protected]>; Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; [email protected]; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; linux- [email protected]; [email protected]
Subject: Re: [PATCH v6 2/6] cpufreq: amd-pstate: initialize new core precision boost state

Hello Perry,

Quoted 7 lines
Quoted 4 lines · Perry Yuan

From: Perry Yuan <[email protected]>

Add gloal global_params to represent current CPU Performance Boost(cpb)

      ^^^^^^
      global ?

Typo will be fixed in v7.

Quoted 8 lines
Quoted 5 lines

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.

Could we reword this portion along the lines of the following:

Quoted 8 lines

"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."

Sure, will revise the description in v7.

Quoted 43 lines
Quoted 11 lines

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

Reported-by Artem S. Tashkinov" <[email protected]>
Cc Oleksandr Natalenko <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217931
Signed-off-by Perry Yuan <[email protected]>
Quoted 7 lines
---
 drivers/cpufreq/amd-pstate.c | 39 +++++++++++++++++++++++++++---------
 include/linux/amd-pstate.h   | 13 ++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)
(quoted diff) +0 -0

b/drivers/cpufreq/amd-pstate.c index 59a2db225d98..81787f83c906

100644

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +2 -0
@@ -68,6 +68,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;  static bool amd_pstate_prefcore = true;
 static struct quirk_entry *quirks;
+struct amd_pstate_global_params amd_pstate_global_params;
+EXPORT_SYMBOL_GPL(amd_pstate_global_params);
 
 /*
  * AMD Energy Preference Performance (EPP)

[..snip..]

include/linux/amd-pstate.h struct quirk_entry { · line 138

Quoted 10 lines
include/linux/amd-pstate.h +2 -0
index 6b832153a126..c5e41de65f70 100644
@@ -134,4 +134,17 @@ struct quirk_entry {
    u32 lowest_freq;
 };
 
+/**
+ * struct amd_pstate_global_params - Global parameters, mostly tunable via

sysfs.

Quoted 12 lines

include/linux/amd-pstate.h struct quirk_entry { · line 148

Quoted 10 lines
include/linux/amd-pstate.h +10 -0
+ * @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;

Will this be used in multiple files ? If no, it is better to define this in amd-pstate.c

Yes, the var will be used by amd-pstate-ut.c which can detect the CPB state for unit testing. So we need to expose this

Quoted 2 lines

Otherwise, I have no other issues with the patch.

Thanks for the review,

Perry.

Quoted 4 lines

--
Thanks and Regards
gautham.

author reviewer
#7 lore raw

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

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, user can change CPU boost state dynamically under active, 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 <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 99 ++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)
drivers/cpufreq/amd-pstate.c +99 -0
index 81787f83c906..4cd2818045c7 100644
@@ -1291,6 +1291,103 @@ static ssize_t prefcore_show(struct device *dev,
 	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
 }
 
+static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
+	struct cppc_perf_ctrls perf_ctrls;
+	u32 highest_perf, nominal_perf, nominal_freq, max_freq;
+	int ret;
+
+	if (!policy)
+		return -ENODATA;
+
+	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) {
+			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;
+
+	policy->max = policy->cpuinfo.max_freq;
+
+	if (cppc_state == AMD_PSTATE_PASSIVE) {
+		ret = freq_qos_update_request(&cpudata->req[1],
+				      policy->cpuinfo.max_freq);
+	}
+
+	cpufreq_cpu_release(policy);
+
+	return ret;
+}
+
+static ssize_t cpb_boost_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", 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;
+
+	mutex_lock(&amd_pstate_driver_lock);
+	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 -EINVAL;
+
+	amd_pstate_global_params.cpb_boost = !!new_state;
+
+	for_each_present_cpu(cpu) {
+
+		struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+		struct amd_cpudata *cpudata = policy->driver_data;
+
+		if (!cpudata) {
+			pr_err("cpudata is NULL\n");
+			ret = -ENODATA;
+			cpufreq_cpu_put(policy);
+			goto err_exit;
+		}
+
+		amd_cpu_boost_update(cpudata, amd_pstate_global_params.cpb_boost);
+		refresh_frequency_limits(policy);
+		cpufreq_cpu_put(policy);
+	}
+
+err_exit:
+	mutex_unlock(&amd_pstate_driver_lock);
+	return ret < 0 ? ret : count;
+}
+
 cpufreq_freq_attr_ro(amd_pstate_max_freq);
 cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
 

@@ -1301,6 +1398,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,

@@ -1325,6 +1423,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
 };
Signature · 1 line
2.34.1
author reviewer
#8 lore raw

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

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

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

Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/cpufreq/amd-pstate.c +5 -0
index 4cd2818045c7..513f91611cd5 100644
@@ -476,6 +476,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,

@@ -495,6 +496,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);
Signature · 1 line
2.34.1
author reviewer
#9 lore raw

PATCH v6 5/6 Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method

In reply to Perry Yuan

From: Perry Yuan <[email protected]>

Introduce AMD CPU frequency boosting control sysfs entry which 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 <[email protected]>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)
Documentation/admin-guide/pm/amd-pstate.rst +11 -0
index 1e0d101b020a..82fbd01da658 100644
@@ -440,6 +440,17 @@ control its functionality at the system level.  They are located in the
         This attribute is read-only to check the state of preferred core set
         by the kernel parameter.
 
+``cpb_boost``
+        Specifies whether core performance boost is requested to be enabled or disabled
+        If core performance boost is disabled while a core is in a boosted P-state, the
+        core automatically transitions to the highest performance non-boosted P-state.
+        AMD Core Performance Boost(CPB) is controlled by this new attribute file which
+        allow user to change all cores frequency boosting state. It supports both
+        ``active``, ``passive`` and ``guided`` mode control with below value write to it.
+
+        "0" Disable Core Performance Boosting
+        "1" Enable  Core Performance Boosting
+
 ``cpupower`` tool support for ``amd-pstate``
 ===============================================
Signature · 1 line
2.34.1
author reviewer
#10 lore raw

PATCH v6 6/6 cpufreq: amd-pstate-ut: support new cpb boost control interface

In reply to Perry Yuan

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

Signed-off-by Perry Yuan <[email protected]>
---
 drivers/cpufreq/amd-pstate-ut.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/amd-pstate-ut.c +1 -1
index f04ae67dda37..b3601b0e6dd3 100644
@@ -226,7 +226,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;
Signature · 1 line
2.34.1
#11 lore raw
In reply to Perry Yuan

Hello Perry,

Quoted 4 lines · Perry Yuan

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

Quoted 8 lines

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

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

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

1). enable core boost:

You meant disable core boost here.

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

2). disabble core boost:

... and enable core boost here.

Quoted 9 lines
$ 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
Quoted 6 lines

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

If you would like to test this patchset, it needs to apply the patchset based on below one latest version patchset.
https://lore.kernel.org/lkml/[email protected]/

Thanks. Will test this on EPYC platforms.

Quoted 2 lines

Perry.

--
Thanks and Regards
gautham.

Quoted 66 lines

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 <[email protected]>
* 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/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v5: https://lore.kernel.org/lkml/[email protected]/

Perry Yuan (6):
cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h
cpufreq: amd-pstate: initialize new core precision boost state
cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
cpufreq: amd-pstate: fix the MSR highest perf will be reset issue
while cpb boost off
Documentation: cpufreq: amd-pstate: introduce the new cpu boost
control method
cpufreq: amd-pstate-ut: support new cpb boost control interface

 Documentation/admin-guide/pm/amd-pstate.rst |  11 ++
 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                | 143 ++++++++++++++++++--
 include/linux/amd-pstate.h                  |  13 ++
 6 files changed, 160 insertions(+), 13 deletions(-)
Signature · 1 line
2.34.1