LinuxLists.cc - [PATCH v10 0/7] AMD Pstate Driver Core Performance Boost

2024-05-08 07:24:45

Subject: [PATCH v10 0/7] 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"

1) globally disable 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) globally enable 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 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

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

Perry.

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 <[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]/
v6: https://lore.kernel.org/lkml/[email protected]/
v7: https://lore.kernel.org/lkml/[email protected]/
v8: https://lore.kernel.org/lkml/[email protected]/
v9: https://lore.kernel.org/lkml/[email protected]/

Perry Yuan (7):
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: introduce per CPU frequency boost control
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 | 189 ++++++++++++++++++--
include/linux/amd-pstate.h | 14 ++
6 files changed, 225 insertions(+), 14 deletions(-)

--
2.34.1

2024-05-08 07:25:15

by Yuan, Perry

[permalink] [raw]

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

From: Perry Yuan <[email protected]>

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" <[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 | 45 +++++++++++++++++++++++++++---------
include/linux/amd-pstate.h | 13 +++++++++++
2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 2db095867d03..f7dab0f7b452 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -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)
@@ -647,7 +649,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)
@@ -665,18 +667,31 @@ 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);
+ if (!cpu_feature_enabled(X86_FEATURE_CPB)) {
+ cpudata->boost_supported = false;
+ current_pstate_driver->boost_enabled = false;
+ pr_debug_once("Boost CPB capabilities not present in the processor\n");
+ return -ENOTSUPP;
+ }

- 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");
+ return ret;
+ }

- cpudata->boost_supported = true;
- current_pstate_driver->boost_enabled = true;
+ 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;
+
+ amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
+
+ return ret;
}

static void amd_perf_ctl_reset(unsigned int cpu)
@@ -899,6 +914,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;
@@ -954,7 +974,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;

@@ -1361,6 +1380,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;
@@ -1414,7 +1438,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
return ret;
WRITE_ONCE(cpudata->cppc_cap1_cached, value);
}
- amd_pstate_boost_init(cpudata);

return 0;

diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index d58fc022ec46..8ba5dd4d3405 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -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 */
--
2.34.1

2024-05-08 08:27:54

by Borislav Petkov

On 5/8/2024 16:31, Oleksandr Natalenko wrote:
> On středa 8. května 2024 21:21:39, SELČ Oleksandr Natalenko wrote:
>> On středa 8. května 2024 21:13:40, SELČ Oleksandr Natalenko wrote:
>>> On středa 8. května 2024 17:11:42, SELČ Oleksandr Natalenko wrote:
>>>> Hello.
>>>>
>>>> On středa 8. května 2024 9:21:05, SELČ Perry Yuan wrote:
>>>>> 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"
>>>>>
>>>>>
>>>>> 1) globally disable 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) globally enable 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 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
>>>>>
>>>>>
>>>>> The patches have been tested with the AMD 7950X processor and many users
>>>>> would like to get core boost control enabled for power saving.
>>>>>
>>>>> Perry.
>>>>>
>>>>>
>>>>> 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 <[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]/
>>>>> v6: https://lore.kernel.org/lkml/[email protected]/
>>>>> v7: https://lore.kernel.org/lkml/[email protected]/
>>>>> v8: https://lore.kernel.org/lkml/[email protected]/
>>>>> v9: https://lore.kernel.org/lkml/[email protected]/
>>>>>
>>>>> Perry Yuan (7):
>>>>> 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: introduce per CPU frequency boost control
>>>>> 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 | 189 ++++++++++++++++++--
>>>>> include/linux/amd-pstate.h | 14 ++
>>>>> 6 files changed, 225 insertions(+), 14 deletions(-)
>>>>
>>>> I've applied this series along with fixes and improvements [1], and I cannot get guided mode to work with my CPU any more.
>>>>
>>>> The CPU is:
>>>>
>>>> ```
>>>> Vendor ID: AuthenticAMD
>>>> Model name: AMD Ryzen 9 5950X 16-Core Processor
>>>> CPU family: 25
>>>> Model: 33
>>>> Thread(s) per core: 2
>>>> Core(s) per socket: 16
>>>> Socket(s): 1
>>>> Stepping: 2
>>>> ```
>>>>
>>>> I've got `amd_pstate=guided` set in the kernel cmdline, but `amd-pstate-epp` gets loaded anyway.
>>>
>>> OK, this part is solved like below:
>>>
>>> ```
>>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>>> index aafa4466e5ced..5aee7d2b8cfd7 100644
>>> --- a/drivers/cpufreq/amd-pstate.c
>>> +++ b/drivers/cpufreq/amd-pstate.c
>>> @@ -66,7 +66,7 @@
>>> static struct cpufreq_driver *current_pstate_driver;
>>> static struct cpufreq_driver amd_pstate_driver;
>>> static struct cpufreq_driver amd_pstate_epp_driver;
>>> -static int cppc_state;
>>> +static int cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE;
>>> static bool cppc_enabled;
>>> static bool amd_pstate_prefcore = true;
>>> static struct quirk_entry *quirks;
>>> @@ -1958,10 +1958,6 @@ static int __init amd_pstate_init(void)
>>> /* check if this machine need CPPC quirks */
>>> dmi_check_system(amd_pstate_quirks_table);
>>>
>>> - /* get default driver mode for loading*/
>>> - cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE;
>>> - pr_debug("cppc working state set to mode:%d\n", cppc_state);
>>> -
>>> switch (cppc_state) {
>>> case AMD_PSTATE_DISABLE:
>>> pr_info("driver load is disabled, boot with specific mode to enable this\n");
>>> ```
>>>
>>> as we have discussed here [1].
>>>
>>> [1] https://lore.kernel.org/lkml/CYYPR12MB865554562BE018D46FF0108C9CE52@CYYPR12MB8655.namprd12.prod.outlook.com/
>>
>> Ah no, scratch it, it's not solved. With `amd_pstate=guided` the driver fails to register during the boottime with the same `sysfs` error:
>>
>> ```
>> kernel: sysfs: cannot create duplicate filename '/devices/system/cpu/cpufreq/policy0/boost'
>> kernel: Hardware name: ASUS System Product Name/Pro WS X570-ACE, BIOS 4805 03/18/2024
>> kernel: Call Trace:
>> kernel: <TASK>
>> kernel: dump_stack_lvl+0x47/0x60
>> kernel: sysfs_warn_dup+0x5a/0x70
>> kernel: sysfs_create_file_ns+0x196/0x1b0
>> kernel: cpufreq_online+0x244/0xde0
>> kernel: cpufreq_add_dev+0x7b/0x90
>> kernel: subsys_interface_register+0x19e/0x1d0
>> kernel: cpufreq_register_driver+0x177/0x2f0
>> kernel: amd_pstate_init+0x1b8/0x2c0
>> kernel: do_one_initcall+0x5b/0x320
>> kernel: kernel_init_freeable+0x1dc/0x380
>> kernel: kernel_init+0x1a/0x1c0
>> kernel: ret_from_fork+0x34/0x50
>> kernel: ret_from_fork_asm+0x1b/0x30
>> kernel: </TASK>
>> ```
>>
>> and things revert to `acpi_cpufreq` instead.
>>
>> What's wrong?
>
> This happens with both `amd_pstate=guided` and `amd_pstate=passive`, while with `amd_pstate=active` it works. Also note I've got:
>
> ```
> CONFIG_X86_AMD_PSTATE=y
> CONFIG_X86_AMD_PSTATE_DEFAULT_MODE=3
> ```
>
> aka "active" by default.
>
> It seems I miss to understand something in the init sequence.
>

I think what's going on is that by reusing the same name as acpi_cppc
does for sysfs when amd pstate is changing modes the sysfs file from
amd-pstate isn't cleared and so acpi_cppc tries to make it.

Don't get me wrong - I do think that we should keep the same sysfs name
for both, it's easier for users.

But if we're going to keep mode switching from amd-pstate at runtime we
really need to make sure that amd-pstate-ut explicitly tests all these
combinations to catch these types of problems.

>>
>>>
>>> But this part:
>>>
>>>> When I try to set `guided` manually via `echo guided | sudo tee /sys/devices/system/cpu/amd_pstate/status`, the status gets dropped to `disable`, `tee` errors out with `-ENODEV`, and there's this in the kernel log:
>>>>
>>>> ```
>>>> $ jctl -kb | grep sysfs: | cut -d ' ' -f 5-
>>>> kernel: sysfs: cannot create duplicate filename '/devices/system/cpu/cpufreq/policy0/boost'
>>>> …
>>>> kernel: sysfs: cannot create duplicate filename '/devices/system/cpu/cpufreq/policy31/boost'
>>>> ```
>>>
>>> is not. I've successfully booted with `amd_pstate=guided`, then did this:
>>>
>>> ```
>>> $ echo active | sudo tee /sys/devices/system/cpu/amd_pstate/status
>>> ```
>>>
>>> just for the sake of test, and got this:
>>>
>>> ```
>>> tee: /sys/devices/system/cpu/amd_pstate/status: File exists
>>> ```
>>>
>>> and this:
>>>
>>> ```
>>> kernel: WARNING: CPU: 9 PID: 8528 at drivers/cpufreq/cpufreq.c:2961 cpufreq_unregister_driver+0x1a/0xc0
>>> ```
>>>
>>> which corresponds to:
>>>
>>> ```
>>> 2957 void cpufreq_unregister_driver(struct cpufreq_driver *driver)
>>> 2958 {
>>> 2959 unsigned long flags;
>>> 2960
>>> 2961 if (WARN_ON(!cpufreq_driver || (driver != cpufreq_driver)))
>>> 2962 return;
>>> ```
>>>
>>> I haven't conducted this test before, so I don't know whether this behaviour is new, or it was present in older iterations. I also don't know if this belongs to the "boost" series or the "fixes", and just letting you know so that you can test the runtime switching yourself and see if it is reproducible in your environment as well or not.
>>>
>>>> The following is applied on top of v6.9-rc7:
>>>>
>>>> ```
>>>> cpufreq: amd-pstate: automatically load pstate driver by default
>>>> cpufreq: amd-pstate: fix the highest frequency issue which limit performance
>>>> cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization
>>>> x86/cpufeatures: Add feature bits for AMD heterogeneous processor
>>>> cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled()
>>>> Documentation: PM: amd-pstate: add guide mode to the Operation mode
>>>> Documentation: PM: amd-pstate: add debugging section for driver loading failure
>>>> Documentation: PM: amd-pstate: introducing recommended reboot requirement during driver switch
>>>> cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
>>>> cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported
>>>> cpufreq: amd-pstate: optimiza the initial frequency values verification
>>>> Documentation: cpufreq: amd-pstate: update doc for Per CPU boost control method
>>>> cpufreq: amd-pstate: introduce per CPU frequency boost control
>>>> Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method
>>>> cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off
>>>> cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
>>>> cpufreq: amd-pstate: initialize new core precision boost state
>>>> cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h
>>>> cpufreq: amd-pstate: remove unused variable lowest_nonlinear_freq
>>>> cpufreq: amd-pstate: fix code format problems
>>>> cpufreq: amd-pstate: Add quirk for the pstate CPPC capabilities missing
>>>> cpufreq: amd-pstate: get transition delay and latency value from ACPI tables
>>>> cpufreq: amd-pstate: Bail out if min/max/nominal_freq is 0
>>>> cpufreq: amd-pstate: Remove amd_get_{min,max,nominal,lowest_nonlinear}_freq()
>>>> cpufreq: amd-pstate: Unify computation of {max,min,nominal,lowest_nonlinear}_freq
>>>> cpufreq: amd-pstate: Document the units for freq variables in amd_cpudata
>>>> cpufreq: amd-pstate: Document *_limit_* fields in struct amd_cpudata
>>>> ```
>>>>
>>>> Previously, with your submissions, it was possible to use `guided` mode with my Zen 3.
>>>>
>>>> [1] https://lore.kernel.org/lkml/[email protected]/
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>
>

2024-05-09 12:01:57

On May 9, 2024 6:01:09 PM GMT+02:00, "Yuan, Perry" <[email protected]> wrote:
>I understand your point, the patch set has taken very long-time review since version 1,
>I would like to get this done as soon as possible with reviewers support, then we can deliver the feature to users who have been waiting long enough.

Spamming maintainers with it is going to help with that how exactly?

--
Sent from a small device: formatting sucks and brevity is inevitable.