AMD Pstate Driver Fixes and Improvements
Subsystems (4)
Perry Yuan
reviewerv3 cover AMD Pstate Driver Fixes and Improvements
Hello everyone,
This patchset addresses critical issues and enhances performance settings for CPUs with heterogeneous core types in the AMD pstate driver.
Specifically, it resolves problems related to calculating the highest performance and frequency on the latest CPUs with preferred cores.
Additionally, the patchset includes documentation improvements in amd-pstate.rst, offering a comprehensive guide covering topics such as recommended reboot requirements during driver switching, debugging procedures for driver loading failures.
Your feedback and suggestions for improvement are highly appreciated. Please review the patches and provide your valuable input.
Thank you.
Best regards,
Perry.
Changes from V2:
* pick review by and ack by flags from Mario and Gautham
* rebase to latest linux-pm bleeding edge branch
* fix driver loading block issue for patch 4, make sure the warning will not abort the driver loading in case there are some new family/model id.
* fix the driver loading sequence issue for patch 10, it allows command line and kernel config option together. command line will override kconfig option.
* add back the AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f to return the highest perf and check others CPU core type in the following codes.
* run some testing on the local system.
* move the amd_core_type to amd-pstate.c because of the amd-pstate.h was removed lately.
Changes from V1:
* drop patch 11 which has been merged in a separate patch. (Mario)
* fix some typos in commit log and tile (Mario)
* fix the patch 11 regression issue of kernel command line (Oleksandr Natalenko)
* pick ack flag for patch 7 (Mario)
* drop patch 4 which is not recommended for user(Mario)
* rebase to linux-pm/bleeding-edge branch
* fix some build warning
* rework the patch 3 for CPU ID matching(Mario)
* address feedback for patch 5 (Mario)
* move the acpi pm profile after got default mode(Mario)
Perry Yuan (10):
cpufreq: amd-pstate: optimize the initial frequency values
verification
cpufreq: amd-pstate: remove unused variable nominal_freq
cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported
cpufreq: amd-pstate: add debug message while CPPC is supported and
disabled by SBIOS
Documentation: PM: amd-pstate: add debugging section for driver
loading failure
Documentation: PM: amd-pstate: add guided mode to the Operation mode
cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() x86/cpufeatures: Add feature bits for AMD heterogeneous processor
cpufreq: amd-pstate: implement heterogeneous core topology for highest
performance initialization
cpufreq: amd-pstate: automatically load pstate driver by default
Documentation/admin-guide/pm/amd-pstate.rst | 15 +- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/processor.h | 2 + arch/x86/kernel/cpu/amd.c | 19 ++ arch/x86/kernel/cpu/scattered.c | 1 + drivers/cpufreq/amd-pstate.c | 201 ++++++++++++++------ 6 files changed, 181 insertions(+), 58 deletions(-)
Signature · 1 line
2.34.1
Perry Yuan
reviewerPATCH v3 1/10 cpufreq: amd-pstate: optimize the initial frequency values verification
To enhance the debugging capability of the driver loading failure for broken CPPC ACPI tables, it can optimize the expression by moving the verification of min_freq, nominal_freq, and other dependency values to the amd_pstate_init_freq() function where they are initialized.
If any of these values are incorrect, the amd-pstate driver will not be registered.
By ensuring that these values are correct before they are used, it will facilitate the debugging process when encountering driver loading failures due to faulty CPPC ACPI tables from BIOS
--- drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-)
drivers/cpufreq/amd-pstate.c +18 -17
@@ -921,6 +921,24 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)WRITE_ONCE (cpudata->nominal_freq, nominal_freq);WRITE_ONCE (cpudata->max_freq, max_freq); +/** +* Below values need to be initialized correctly, otherwise driver will fail to load +* max_freq is calculated according to (nominal_freq * highest_perf)/nominal_perf +* lowest_nonlinear_freq is a value between [min_freq, nominal_freq] +* Check _CPC in ACPI table objects if any values are incorrect +*/ +if (min_freq <=0 || max_freq <=0 || nominal_freq <=0 || min_freq > max_freq) { +pr_err ("min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect\n" , + min_freq, max_freq, nominal_freq *1000 ); +return -EINVAL ; + } + +if (lowest_nonlinear_freq <= min_freq || lowest_nonlinear_freq > nominal_freq *1000 ) { +pr_err ("lowest_nonlinear_freq(%d) value is out of range [min_freq(%d), nominal_freq(%d)]\n" , + lowest_nonlinear_freq, min_freq, nominal_freq *1000 ); +return -EINVAL ; + } +return 0 ; }
@@ -959,15 +977,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) max_freq =READ_ONCE (cpudata->max_freq); nominal_freq =READ_ONCE (cpudata->nominal_freq); -if (min_freq <=0 || max_freq <=0 || - nominal_freq <=0 || min_freq > max_freq) { -dev_err (dev, -"min_freq(%d) or max_freq(%d) or nominal_freq (%d) value is incorrect, check _CPC in ACPI tables\n" , - min_freq, max_freq, nominal_freq); - ret = -EINVAL ; - goto free_cpudata1; - } - policy->cpuinfo.transition_latency =amd_pstate_get_transition_latency (policy->cpu); policy->transition_delay_us =amd_pstate_get_transition_delay_us (policy->cpu);
@@ -1420,14 +1429,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) min_freq =READ_ONCE (cpudata->min_freq); max_freq =READ_ONCE (cpudata->max_freq); nominal_freq =READ_ONCE (cpudata->nominal_freq); -if (min_freq <=0 || max_freq <=0 || - nominal_freq <=0 || min_freq > max_freq) { -dev_err (dev, -"min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect, check _CPC in ACPI tables\n" , - min_freq, max_freq, nominal_freq); - ret = -EINVAL ; - goto free_cpudata1; - } policy->cpuinfo.min_freq = min_freq; policy->cpuinfo.max_freq = max_freq;
Signature · 1 line
2.34.1
Re: [PATCH v3 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification
Quoted 6 lines · Perry Yuan
To enhance the debugging capability of the driver loading failure for broken CPPC ACPI tables, it can optimize the expression by moving the verification of
min_freq,nominal_freq, and other dependency values to theamd_pstate_init_freq()function where they are initialized.
If any of these values are incorrect, theamd-pstatedriver will not be registered.
Quoted 6 lines
By ensuring that these values are correct before they are used, it will facilitate the debugging process when encountering driver loading failures due to faulty CPPC ACPI tables from BIOS
Signed-off-by Perry Yuan <[email protected]>
Quoted 64 lines
--- drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-)drivers/cpufreq/amd-pstate.c +18
-17@@ -921,6 +921,24 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)WRITE_ONCE (cpudata->nominal_freq, nominal_freq);WRITE_ONCE (cpudata->max_freq, max_freq); +/** +* Below values need to be initialized correctly, otherwise driver will fail to load +* max_freq is calculated according to (nominal_freq * highest_perf)/nominal_perf +* lowest_nonlinear_freq is a value between [min_freq, nominal_freq] +* Check _CPC in ACPI table objects if any values are incorrect +*/ +if (min_freq <=0 || max_freq <=0 || nominal_freq <=0 || min_freq > max_freq) { +pr_err ("min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect\n" , + min_freq, max_freq, nominal_freq *1000 ); +return -EINVAL ; + } + +if (lowest_nonlinear_freq <= min_freq || lowest_nonlinear_freq > nominal_freq *1000 ) { +pr_err ("lowest_nonlinear_freq(%d) value is out of range [min_freq(%d), nominal_freq(%d)]\n" , + lowest_nonlinear_freq, min_freq, nominal_freq *1000 ); +return -EINVAL ; + } +return 0 ; }@@ -959,15 +977,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) max_freq =READ_ONCE (cpudata->max_freq); nominal_freq =READ_ONCE (cpudata->nominal_freq); -if (min_freq <=0 || max_freq <=0 || - nominal_freq <=0 || min_freq > max_freq) { -dev_err (dev, -"min_freq(%d) or max_freq(%d) or nominal_freq (%d) value is incorrect, check _CPC in ACPI tables\n" , - min_freq, max_freq, nominal_freq); - ret = -EINVAL ; - goto free_cpudata1; - } - policy->cpuinfo.transition_latency =amd_pstate_get_transition_latency (policy->cpu); policy->transition_delay_us =amd_pstate_get_transition_delay_us (policy->cpu);@@ -1420,14 +1429,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) min_freq =READ_ONCE (cpudata->min_freq); max_freq =READ_ONCE (cpudata->max_freq); nominal_freq =READ_ONCE (cpudata->nominal_freq); -if (min_freq <=0 || max_freq <=0 || - nominal_freq <=0 || min_freq > max_freq) { -dev_err (dev, -"min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect, check _CPC in ACPI tables\n" , - min_freq, max_freq, nominal_freq); - ret = -EINVAL ; - goto free_cpudata1; - } policy->cpuinfo.min_freq = min_freq; policy->cpuinfo.max_freq = max_freq;
Perry Yuan
reviewerPATCH v3 2/10 cpufreq: amd-pstate: remove unused variable nominal_freq
In reply to Perry Yuanremoved the unused variable nominal_freq for build warning. This variable was defined and assigned a value in the previous code, but it was not used in the subsequent code.
--- drivers/cpufreq/amd-pstate.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/cpufreq/amd-pstate.c +2 -4
@@ -944,7 +944,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)static int amd_pstate_cpu_init (struct cpufreq_policy *policy) { -int min_freq, max_freq, nominal_freq, ret; +int min_freq, max_freq, ret;struct device *dev;struct amd_cpudata *cpudata;
@@ -975,7 +975,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) min_freq =READ_ONCE (cpudata->min_freq); max_freq =READ_ONCE (cpudata->max_freq); - nominal_freq =READ_ONCE (cpudata->nominal_freq); policy->cpuinfo.transition_latency =amd_pstate_get_transition_latency (policy->cpu); policy->transition_delay_us =amd_pstate_get_transition_delay_us (policy->cpu);
@@ -1395,7 +1394,7 @@ static bool amd_pstate_acpi_pm_profile_undefined(void)static int amd_pstate_epp_cpu_init (struct cpufreq_policy *policy) { -int min_freq, max_freq, nominal_freq, ret; +int min_freq, max_freq, ret;struct amd_cpudata *cpudata;struct device *dev;u64 value;
@@ -1428,7 +1427,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) min_freq =READ_ONCE (cpudata->min_freq); max_freq =READ_ONCE (cpudata->max_freq); - nominal_freq =READ_ONCE (cpudata->nominal_freq); policy->cpuinfo.min_freq = min_freq; policy->cpuinfo.max_freq = max_freq;
Signature · 1 line
2.34.1
Re: [PATCH v3 02/10] cpufreq: amd-pstate: remove unused variable nominal_freq
Quoted 8 lines · Perry Yuan
removed the unused variable
nominal_freqfor build warning. This variable was defined and assigned a value in the previous code, but it was not used in the subsequent code.Reported-by kernel test robot <[email protected]>Signed-off-by Perry Yuan <[email protected]>Reviewed-by Gautham R. Shenoy <[email protected]>
Quoted 42 lines
--- drivers/cpufreq/amd-pstate.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/cpufreq/amd-pstate.c +2
-4@@ -944,7 +944,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)static int amd_pstate_cpu_init (struct cpufreq_policy *policy) { -int min_freq, max_freq, nominal_freq, ret; +int min_freq, max_freq, ret;struct device *dev;struct amd_cpudata *cpudata;@@ -975,7 +975,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) min_freq =READ_ONCE (cpudata->min_freq); max_freq =READ_ONCE (cpudata->max_freq); - nominal_freq =READ_ONCE (cpudata->nominal_freq); policy->cpuinfo.transition_latency =amd_pstate_get_transition_latency (policy->cpu); policy->transition_delay_us =amd_pstate_get_transition_delay_us (policy->cpu);@@ -1395,7 +1394,7 @@ static bool amd_pstate_acpi_pm_profile_undefined(void)static int amd_pstate_epp_cpu_init (struct cpufreq_policy *policy) { -int min_freq, max_freq, nominal_freq, ret; +int min_freq, max_freq, ret;struct amd_cpudata *cpudata;struct device *dev;u64 value;@@ -1428,7 +1427,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) min_freq =READ_ONCE (cpudata->min_freq); max_freq =READ_ONCE (cpudata->max_freq); - nominal_freq =READ_ONCE (cpudata->nominal_freq); policy->cpuinfo.min_freq = min_freq; policy->cpuinfo.max_freq = max_freq;
Perry Yuan
reviewerPATCH v3 3/10 cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported
In reply to Perry YuanAdd CPU ID checking in case the driver attempt to load on systems where CPPC functionality is unavailable. And the warning message will not be shown if CPPC is not supported.
It will also print debug message if the CPU has no CPPC support that helps to debug the driver loading failure issue.
--- drivers/cpufreq/amd-pstate.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
drivers/cpufreq/amd-pstate.c +19 -0
@@ -1740,6 +1740,20 @@ static int __init amd_pstate_set_driver(int mode_idx)return -EINVAL ; } +/** +* CPPC function is not supported for family ID 17H with model_ID ranging from 0x10 to 0x2F. +* show the debug message that helps to check if the CPU has CPPC support for loading issue. +*/ +static bool amd_cppc_supported (void ) +{ +if ((boot_cpu_data.x86 ==0x17 ) && (boot_cpu_data.x86_model <0x30 )) { +pr_debug_once ("CPPC feature is not supported by the processor\n" ); +return false; + } + +return true; +} +static int __init amd_pstate_init (void ) {struct device *dev_root;
@@ -1748,6 +1762,11 @@ static int __init amd_pstate_init(void)if (boot_cpu_data.x86_vendor !=X86_VENDOR_AMD )return -ENODEV ; +/* show debug message only if CPPC is not supported */ +if (!amd_cppc_supported ()) +return -EOPNOTSUPP ; + +/* show warning message when BIOS broken or ACPI disabled */ if (!acpi_cpc_valid ()) {pr_warn_once ("the _CPC object is not present in SBIOS or ACPI disabled\n" );return -ENODEV ;
Signature · 1 line
2.34.1
Re: [PATCH v3 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported
Quoted 11 lines · Perry Yuan
Add CPU ID checking in case the driver attempt to load on systems where CPPC functionality is unavailable. And the warning message will not be shown if CPPC is not supported.
It will also print debug message if the CPU has no CPPC support that helps to debug the driver loading failure issue.
Quoted 41 lines
--- drivers/cpufreq/amd-pstate.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)drivers/cpufreq/amd-pstate.c +19
-0@@ -1740,6 +1740,20 @@ static int __init amd_pstate_set_driver(int mode_idx)return -EINVAL ; } +/** +* CPPC function is not supported for family ID 17H with model_ID ranging from 0x10 to 0x2F. +* show the debug message that helps to check if the CPU has CPPC support for loading issue. +*/ +static bool amd_cppc_supported (void ) +{ +if ((boot_cpu_data.x86 ==0x17 ) && (boot_cpu_data.x86_model <0x30 )) { +pr_debug_once ("CPPC feature is not supported by the processor\n" ); +return false; + } + +return true; +} +static int __init amd_pstate_init (void ) {struct device *dev_root;@@ -1748,6 +1762,11 @@ static int __init amd_pstate_init(void)if (boot_cpu_data.x86_vendor !=X86_VENDOR_AMD )return -ENODEV ; +/* show debug message only if CPPC is not supported */ +if (!amd_cppc_supported ()) +return -EOPNOTSUPP ; + +/* show warning message when BIOS broken or ACPI disabled */ if (!acpi_cpc_valid ()) {pr_warn_once ("the _CPC object is not present in SBIOS or ACPI disabled\n" );return -ENODEV ;
Perry Yuan
reviewerPATCH v3 4/10 cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
In reply to Perry YuanIf CPPC feature is supported by the CPU however the CPUID flag bit is not set by SBIOS, the amd_pstate will be failed to load while system booting.
So adding one more debug message to inform user to check the SBIOS setting, The change also can help maintainers to debug why amd_pstate driver failed to be loaded at system booting if the processor support CPPC.
--- drivers/cpufreq/amd-pstate.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
drivers/cpufreq/amd-pstate.c +25 -0
@@ -1746,11 +1746,36 @@ static int __init amd_pstate_set_driver(int mode_idx)*/ static bool amd_cppc_supported (void ) { +struct cpuinfo_x86 *c = &cpu_data (0 ); +if ((boot_cpu_data.x86 ==0x17 ) && (boot_cpu_data.x86_model <0x30 )) {pr_debug_once ("CPPC feature is not supported by the processor\n" );return false; } +/* +* If the CPPC feature is disabled in the BIOS for processors that support MSR-based CPPC, +* the AMD Pstate driver may not function correctly. +* Check the CPPC flag and display a warning message if the platform supports CPPC. +* Notice: below checking code will not abort the driver registeration process. +*/ +if (!cpu_feature_enabled (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_ZEN1 ) ||cpu_feature_enabled (X86_FEATURE_ZEN2 )) { +if (c->x86_model >0x60 && c->x86_model <0xaf ) + goto warn; + }else if (cpu_feature_enabled (X86_FEATURE_ZEN3 ) ||cpu_feature_enabled (X86_FEATURE_ZEN4 )) { +if ((c->x86_model >0x10 && c->x86_model <0x1F ) || (c->x86_model >0x40 && c->x86_model <0xaf )) + goto warn; + }else if (cpu_feature_enabled (X86_FEATURE_ZEN5 )) { + goto warn; + } + } + +return true; + +warn: +pr_debug_once ("The CPPC feature is supported but currently disabled by the BIOS.\n" +"Please enable it if your BIOS has the CPPC option.\n" );return true; }
Signature · 1 line
2.34.1
Mario Limonciello
maintainerRe: [PATCH v3 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
Quoted 41 lines · Perry Yuan
If CPPC feature is supported by the CPU however the CPUID flag bit is not set by SBIOS, the
amd_pstatewill be failed to load while system booting.
So adding one more debug message to inform user to check the SBIOS setting, The change also can help maintainers to debug whyamd_pstatedriver failed to be loaded at system booting if the processor support CPPC.Signed-off-by Perry Yuan <[email protected]>--- drivers/cpufreq/amd-pstate.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)drivers/cpufreq/amd-pstate.c +15
-0@@ -1746,11 +1746,36 @@ static int __init amd_pstate_set_driver(int mode_idx)*/ static bool amd_cppc_supported (void ) { +struct cpuinfo_x86 *c = &cpu_data (0 ); +if ((boot_cpu_data.x86 ==0x17 ) && (boot_cpu_data.x86_model <0x30 )) {pr_debug_once ("CPPC feature is not supported by the processor\n" );return false; } +/* +* If the CPPC feature is disabled in the BIOS for processors that support MSR-based CPPC, +* the AMD Pstate driver may not function correctly. +* Check the CPPC flag and display a warning message if the platform supports CPPC. +* Notice: below checking code will not abort the driver registeration process. +*/ +if (!cpu_feature_enabled (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_ZEN1 ) ||cpu_feature_enabled (X86_FEATURE_ZEN2 )) { +if (c->x86_model >0x60 && c->x86_model <0xaf ) + goto warn; + }else if (cpu_feature_enabled (X86_FEATURE_ZEN3 ) ||cpu_feature_enabled (X86_FEATURE_ZEN4 )) { +if ((c->x86_model >0x10 && c->x86_model <0x1F ) || (c->x86_model >0x40 && c->x86_model <0xaf )) + goto warn;
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_set_driver(int mode_idx) · line 1778
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10
-0+ }else if (cpu_feature_enabled (X86_FEATURE_ZEN5 )) { + goto warn; + } + } + +return true; + +warn: +pr_debug_once ("The CPPC feature is supported but currently disabled by the BIOS.\n" +"Please enable it if your BIOS has the CPPC option.\n" );
This should be pr_warn_once() or pr_notice_once() IMO otherwise people won't really be able to know there is a problem.
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_set_driver(int mode_idx) · line 1779
Quoted 1 line
drivers/cpufreq/amd-pstate.c +0
-0return true;
I don't really like the goto and warn label with two identical return statements. Instead how about something like this?
static bool amd_cppc_supported(void)
{
struct cpuinfo_x86 *c = &cpu_data(0);
bool warn = false;
.
.
.
if (checks) {
.
.
warn = TRUE;
}
if (warn)
pr_warn_once();
return TRUE;That would be a better design pattern, especially if we end up having different classes of warnings to show to users in this function in the future.
Re: [PATCH v3 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
In reply to Perry YuanQuoted 9 lines · Perry Yuan
If CPPC feature is supported by the CPU however the CPUID flag bit is not set by SBIOS, the
amd_pstatewill be failed to load while system booting.
So adding one more debug message to inform user to check the SBIOS setting, The change also can help maintainers to debug whyamd_pstatedriver failed to be loaded at system booting if the processor support CPPC.Signed-off-by Perry Yuan <[email protected]>
--
Thanks and Regards
gautham
Perry Yuan
reviewerPATCH v3 5/10 Documentation: PM: amd-pstate: add debugging section for driver loading failure
In reply to Perry YuanTo address issues with the loading of the amd-pstate driver on certain platforms, It needs to enable dynamic debugging to capture debug messages during the driver loading process. By adding "amd_pstate.dyndbg=+p cppc_acpi.dyndbg=+p loglevel=4 debug amd_pstate=active" to the kernel command line, driver debug logging is enabled.
--- Documentation/admin-guide/pm/amd-pstate.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+)
Documentation/admin-guide/pm/amd-pstate.rst +13 -0
@@ -472,6 +472,19 @@ operations for the new ``amd-pstate`` module with this tool. :: Diagnostics and Tuning ======================= +Debugging AMD P-State Driver Loading Issues +------------------------------------------ + +On some platforms, there may be issues with the loading of the amd-pstate driver. +To capture debug messages for issue analysis, users can add below parameter, +"amd_pstate.dyndbg=+p cppc_acpi.dyndbg=+p debug" +to the kernel command line. This will enable dynamic debugging and allow better +analysis and troubleshooting of the driver loading process. + +Please note that adding this parameter will only enable debug logging during the +driver loading phase and may affect system behavior. Use this option with caution +and only for debugging purposes. + Trace Events --------------
Signature · 1 line
2.34.1
Mario Limonciello
maintainerRe: [PATCH v3 05/10] Documentation: PM: amd-pstate: add debugging section for driver loading failure
Quoted 4 lines · Perry Yuan
To address issues with the loading of the amd-pstate driver on certain platforms, It needs to enable dynamic debugging to capture debug messages during the driver loading process. By adding "amd_pstate.dyndbg=+p cppc_acpi.dyndbg=+p loglevel=4 debug amd_pstate=active" to the kernel command line, driver debug logging is enabled.
Here's some suggestion for text.
"
On some platforms if amd-pstate fails to load or operate properly users will need to turn on extra debugging messages. "
However - I would argue that the messages are the wrong level if they need debugging messages turned on when it fails to load.
Something causing a failure to load should be pr_warn() or pr_notice() IMO.
Quoted 11 lines
Signed-off-by Perry Yuan <[email protected]>--- Documentation/admin-guide/pm/amd-pstate.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/admin-guide/pm/amd-pstate.rst +0
-0
Documentation/admin-guide/pm/amd-pstate.rst operations for the new ``amd-pstate`` module with this tool. :: · line 478
Quoted 7 lines
Documentation/admin-guide/pm/amd-pstate.rst +4
-0@@ -472,6 +472,19 @@ operations for the new ``amd-pstate`` module with this tool. :: Diagnostics and Tuning ======================= +Debugging AMD P-State Driver Loading Issues +------------------------------------------ + +On some platforms, there may be issues with the loading of the amd-pstate driver.
This makes it sound like it's by design. Of course it isn't by design, so I would instead say:
"If the amd-pstate driver fails to load, additional debug information may be necessary."
Quoted 12 lines
Documentation/admin-guide/pm/amd-pstate.rst +9
-0+To capture debug messages for issue analysis, users can add below parameter, +"amd_pstate.dyndbg=+p cppc_acpi.dyndbg=+p debug" +to the kernel command line. This will enable dynamic debugging and allow better +analysis and troubleshooting of the driver loading process. + +Please note that adding this parameter will only enable debug logging during the +driver loading phase and may affect system behavior. Use this option with caution +and only for debugging purposes. + Trace Events --------------
Perry Yuan
reviewerPATCH v3 6/10 Documentation: PM: amd-pstate: add guided mode to the Operation mode
In reply to Perry Yuanthe guided mode is also supported, so the operation mode should include that mode as well.
--- Documentation/admin-guide/pm/amd-pstate.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/admin-guide/pm/amd-pstate.rst +1 -1
@@ -406,7 +406,7 @@ control its functionality at the system level. They are located in the ``/sys/devices/system/cpu/amd_pstate/`` directory and affect all CPUs. ``status`` - Operation mode of the driver: "active", "passive" or "disable". + Operation mode of the driver: "active", "passive", "guided" or "disable". "active" The driver is functional and in the ``active mode``
Signature · 1 line
2.34.1
Re: [PATCH v3 06/10] Documentation: PM: amd-pstate: add guided mode to the Operation mode
Quoted 5 lines · Perry Yuan
the guided mode is also supported, so the operation mode should include that mode as well.
Reviewed-by Mario Limonciello <[email protected]>Signed-off-by Perry Yuan <[email protected]>
Thanks for adding this.
Quoted 19 lines
--- Documentation/admin-guide/pm/amd-pstate.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Documentation/admin-guide/pm/amd-pstate.rst +1
-1@@ -406,7 +406,7 @@ control its functionality at the system level. They are located in the ``/sys/devices/system/cpu/amd_pstate/`` directory and affect all CPUs. ``status`` - Operation mode of the driver: "active", "passive" or "disable". + Operation mode of the driver: "active", "passive", "guided" or "disable". "active" The driver is functional and in the ``active mode``Signature · 1 line
2.34.1
Perry Yuan
reviewerPATCH v3 7/10 cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled()
In reply to Perry Yuanreplace the usage of the deprecated boot_cpu_has() function with the modern cpu_feature_enabled() function. The switch to cpu_feature_enabled() ensures compatibility with the latest CPU feature detection mechanisms and improves code maintainability.
--- drivers/cpufreq/amd-pstate.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/cpufreq/amd-pstate.c +12 -12
@@ -157,7 +157,7 @@ static int __init dmi_matched_7k62_bios_bug(const struct dmi_system_id *dmi)* broken BIOS lack of nominal_freq and lowest_freq capabilities * definition in ACPI tables */ -if (boot_cpu_has (X86_FEATURE_ZEN2 )) { +if (cpu_feature_enabled (X86_FEATURE_ZEN2 )) { quirks = dmi->driver_data;pr_info ("Overriding nominal and lowest frequencies for %s\n" , dmi->ident);return 1 ;
@@ -199,7 +199,7 @@ static s16 amd_pstate_get_epp(struct amd_cpudata *cpudata, u64 cppc_req_cached)u64 epp;int ret; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {if (!cppc_req_cached) { epp =rdmsrl_on_cpu (cpudata->cpu,MSR_AMD_CPPC_REQ , &cppc_req_cached);
@@ -252,7 +252,7 @@ static int amd_pstate_set_epp(struct amd_cpudata *cpudata, u32 epp)int ret;struct cppc_perf_ctrls perf_ctrls; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {u64 value =READ_ONCE (cpudata->cppc_req_cached); value &= ~GENMASK_ULL (31 ,24 );
@@ -753,7 +753,7 @@ static int amd_pstate_get_highest_perf(int cpu, u32 *highest_perf) {int ret; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {u64 cap1; ret =rdmsrl_safe_on_cpu (cpu,MSR_AMD_CPPC_CAP1 , &cap1);
@@ -988,7 +988,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)/* It will be updated by governor */ policy->cur = policy->cpuinfo.min_freq; -if (boot_cpu_has (X86_FEATURE_CPPC )) +if (cpu_feature_enabled (X86_FEATURE_CPPC )) policy->fast_switch_possible = true; ret = freq_qos_add_request(&policy->constraints, &cpudata->req[0],
@@ -1221,7 +1221,7 @@ static int amd_pstate_change_mode_without_dvr_change(int mode) cppc_state = mode; -if (boot_cpu_has (X86_FEATURE_CPPC ) || cppc_state ==AMD_PSTATE_ACTIVE ) +if (cpu_feature_enabled (X86_FEATURE_CPPC ) || cppc_state ==AMD_PSTATE_ACTIVE )return 0 ;for_each_present_cpu (cpu) {
@@ -1450,7 +1450,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)else policy->policy =CPUFREQ_POLICY_POWERSAVE ; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) { ret =rdmsrl_on_cpu (cpudata->cpu,MSR_AMD_CPPC_REQ , &value);if (ret)return ret;
@@ -1540,7 +1540,7 @@ static void amd_pstate_epp_update_limit(struct cpufreq_policy *policy) epp =0 ;/* Set initial EPP value */ -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) { value &= ~GENMASK_ULL (31 ,24 ); value |= (u64 )epp <<24 ; }
@@ -1579,7 +1579,7 @@ static void amd_pstate_epp_reenable(struct amd_cpudata *cpudata) value =READ_ONCE (cpudata->cppc_req_cached); max_perf =READ_ONCE (cpudata->highest_perf); -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {wrmsrl_on_cpu (cpudata->cpu,MSR_AMD_CPPC_REQ , value); }else { perf_ctrls.max_perf = max_perf;
@@ -1613,7 +1613,7 @@ static void amd_pstate_epp_offline(struct cpufreq_policy *policy) value =READ_ONCE (cpudata->cppc_req_cached);mutex_lock (&amd_pstate_limits_lock); -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) { cpudata->epp_policy =CPUFREQ_POLICY_UNKNOWN ;/* Set max perf same as min perf */
@@ -1815,7 +1815,7 @@ static int __init amd_pstate_init(void)*/ if (amd_pstate_acpi_pm_profile_undefined () ||amd_pstate_acpi_pm_profile_server () || - !boot_cpu_has (X86_FEATURE_CPPC )) { + !cpu_feature_enabled (X86_FEATURE_CPPC )) {pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ; }
@@ -1834,7 +1834,7 @@ static int __init amd_pstate_init(void) }/* capability check */ -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {pr_debug ("AMD CPPC MSR based functionality is supported\n" );if (cppc_state !=AMD_PSTATE_ACTIVE ) current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
Signature · 1 line
2.34.1
Re: [PATCH v3 07/10] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled()
Quoted 8 lines · Perry Yuan
replace the usage of the deprecated
boot_cpu_has()function with the moderncpu_feature_enabled()function. The switch tocpu_feature_enabled()ensures compatibility with the latest CPU feature detection mechanisms and improves code maintainability.Suggested-by Borislav Petkov (AMD) <[email protected]>Signed-off-by Perry Yuan <[email protected]>
Quoted 118 lines
--- drivers/cpufreq/amd-pstate.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)drivers/cpufreq/amd-pstate.c +12
-12@@ -157,7 +157,7 @@ static int __init dmi_matched_7k62_bios_bug(const struct dmi_system_id *dmi)* broken BIOS lack of nominal_freq and lowest_freq capabilities * definition in ACPI tables */ -if (boot_cpu_has (X86_FEATURE_ZEN2 )) { +if (cpu_feature_enabled (X86_FEATURE_ZEN2 )) { quirks = dmi->driver_data;pr_info ("Overriding nominal and lowest frequencies for %s\n" , dmi->ident);return 1 ;@@ -199,7 +199,7 @@ static s16 amd_pstate_get_epp(struct amd_cpudata *cpudata, u64 cppc_req_cached)u64 epp;int ret; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {if (!cppc_req_cached) { epp =rdmsrl_on_cpu (cpudata->cpu,MSR_AMD_CPPC_REQ , &cppc_req_cached);@@ -252,7 +252,7 @@ static int amd_pstate_set_epp(struct amd_cpudata *cpudata, u32 epp)int ret;struct cppc_perf_ctrls perf_ctrls; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {u64 value =READ_ONCE (cpudata->cppc_req_cached); value &= ~GENMASK_ULL (31 ,24 );@@ -753,7 +753,7 @@ static int amd_pstate_get_highest_perf(int cpu, u32 *highest_perf) {int ret; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {u64 cap1; ret =rdmsrl_safe_on_cpu (cpu,MSR_AMD_CPPC_CAP1 , &cap1);@@ -988,7 +988,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)/* It will be updated by governor */ policy->cur = policy->cpuinfo.min_freq; -if (boot_cpu_has (X86_FEATURE_CPPC )) +if (cpu_feature_enabled (X86_FEATURE_CPPC )) policy->fast_switch_possible = true; ret = freq_qos_add_request(&policy->constraints, &cpudata->req[0],@@ -1221,7 +1221,7 @@ static int amd_pstate_change_mode_without_dvr_change(int mode) cppc_state = mode; -if (boot_cpu_has (X86_FEATURE_CPPC ) || cppc_state ==AMD_PSTATE_ACTIVE ) +if (cpu_feature_enabled (X86_FEATURE_CPPC ) || cppc_state ==AMD_PSTATE_ACTIVE )return 0 ;for_each_present_cpu (cpu) {@@ -1450,7 +1450,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)else policy->policy =CPUFREQ_POLICY_POWERSAVE ; -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) { ret =rdmsrl_on_cpu (cpudata->cpu,MSR_AMD_CPPC_REQ , &value);if (ret)return ret;@@ -1540,7 +1540,7 @@ static void amd_pstate_epp_update_limit(struct cpufreq_policy *policy) epp =0 ;/* Set initial EPP value */ -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) { value &= ~GENMASK_ULL (31 ,24 ); value |= (u64 )epp <<24 ; }@@ -1579,7 +1579,7 @@ static void amd_pstate_epp_reenable(struct amd_cpudata *cpudata) value =READ_ONCE (cpudata->cppc_req_cached); max_perf =READ_ONCE (cpudata->highest_perf); -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {wrmsrl_on_cpu (cpudata->cpu,MSR_AMD_CPPC_REQ , value); }else { perf_ctrls.max_perf = max_perf;@@ -1613,7 +1613,7 @@ static void amd_pstate_epp_offline(struct cpufreq_policy *policy) value =READ_ONCE (cpudata->cppc_req_cached);mutex_lock (&amd_pstate_limits_lock); -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) { cpudata->epp_policy =CPUFREQ_POLICY_UNKNOWN ;/* Set max perf same as min perf */ @@ -1815,7 +1815,7 @@ static int __init amd_pstate_init(void)*/ if (amd_pstate_acpi_pm_profile_undefined () ||amd_pstate_acpi_pm_profile_server () || - !boot_cpu_has (X86_FEATURE_CPPC )) { + !cpu_feature_enabled (X86_FEATURE_CPPC )) {pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ; }@@ -1834,7 +1834,7 @@ static int __init amd_pstate_init(void) }/* capability check */ -if (boot_cpu_has (X86_FEATURE_CPPC )) { +if (cpu_feature_enabled (X86_FEATURE_CPPC )) {pr_debug ("AMD CPPC MSR based functionality is supported\n" );if (cppc_state !=AMD_PSTATE_ACTIVE ) current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;Signature · 1 line
2.34.1
Perry Yuan
reviewerPATCH v3 8/10 x86/cpufeatures: Add feature bits for AMD heterogeneous processor
In reply to Perry YuanCPUID leaf 0x80000026 advertises core types with different efficiency rankings
Bit 30 indicates the heterogeneous core topology feature, if the bit set, it means not all instances at the current hierarchical level have the same core topology.
For better utilization of feature words and help to identify core type, X86_FEATURE_HETERO_CORE_TOPOLOGY is added as a few scattered feature bits.
PDF p274
--- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/kernel/cpu/scattered.c | 1 + 2 files changed, 2 insertions(+)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -470,6 +470,7 @@#define X86_FEATURE_BHI_CTRL (21*32+ 2)/* "" BHI_DIS_S HW control available */ #define X86_FEATURE_CLEAR_BHB_HW (21*32+ 3)/* "" BHI_DIS_S HW control enabled */ #define X86_FEATURE_CLEAR_BHB_LOOP_ON_VMEXIT (21*32+ 4)/* "" Clear branch history at vmexit using SW loop */ +#define X86_FEATURE_HETERO_CORE_TOPOLOGY (21*32+ 5)/* "" Heterogeneous Core Topology */ /* * BUG word(s)
arch/x86/kernel/cpu/scattered.c +1 -0
@@ -51,6 +51,7 @@ static const struct cpuid_bit cpuid_bits[] = { { X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 }, { X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 }, { X86_FEATURE_AMD_LBR_PMC_FREEZE, CPUID_EAX, 2, 0x80000022, 0 }, + { X86_FEATURE_HETERO_CORE_TOPOLOGY, CPUID_EAX, 30, 0x80000026, 0 }, { 0, 0, 0, 0, 0 } };
Signature · 1 line
2.34.1
Borislav Petkov
maintainerRe: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
Quoted 1 line · Perry Yuan
Subject: Re: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
"... Add a heterogeneous core topology feature bit"
not "feature bits"
Quoted 8 lines
CPUID leaf
0x80000026advertises core types with different efficiency rankingsBit 30 indicates the heterogeneous core topology feature, if the bit set, it means not all instances at the current hierarchical level have the same core topology.
For better utilization of feature words and help to identify core type,
X86_FEATURE_HETERO_CORE_TOPOLOGYis added as a few scattered feature bits.
This sentence is not needed.
Quoted 3 lines
PDF p274
This link will become obsolete, you can say instead
Search the web for "architecture programmer's manual volume 2 system programming docID #25493" in order to find the official document.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteMario Limonciello
maintainerRe: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
Quoted 18 lines · Borislav Petkov
Quoted 1 line · Perry Yuan
Subject: Re: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
"... Add a heterogeneous core topology feature bit"
not "feature bits"
Quoted 8 lines
CPUID leaf
0x80000026advertises core types with different efficiency rankingsBit 30 indicates the heterogeneous core topology feature, if the bit set, it means not all instances at the current hierarchical level have the same core topology.
For better utilization of feature words and help to identify core type,
X86_FEATURE_HETERO_CORE_TOPOLOGYis added as a few scattered feature bits.This sentence is not needed.
Quoted 9 lines
Quoted 3 lines
PDF p274
This link will become obsolete, you can say instead
Search the web for "architecture programmer's manual volume 2 system programming docID #25493" in order to find the official document.
Another option is to upload it a non ephemeral location like a kernel Bugzilla and link to that. I do recall there is a bug already opened for this purpose in the past.
Borislav Petkov
maintainerRe: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
Quoted 3 lines · Mario Limonciello
Another option is to upload it a non ephemeral location like a kernel Bugzilla and link to that. I do recall there is a bug already opened for this purpose in the past.
You mean, after what, 30 years of search engine technology they can't do one simple thing of finding a doc on the web after indexing its new location each time the corporate website decides to update to the latest fancy CRM glue?
:-P
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteYuan, Perry
reviewerRE: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Boris,
Quoted 24 lines
-----Original Message-----
From: Borislav Petkov <[email protected]>
Sent: Tuesday, June 11, 2024 10:39 PM
To: Limonciello, Mario <[email protected]>
Cc: Yuan, Perry <[email protected]>; Shenoy, Gautham Ranjal <[email protected]>; [email protected]; [email protected]; Huang, Ray <[email protected]>; Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processorQuoted 3 lines · Mario Limonciello
Another option is to upload it a non ephemeral location like a kernel Bugzilla and link to that. I do recall there is a bug already opened for this purpose in the past.
You mean, after what, 30 years of search engine technology they can't do one simple thing of finding a doc on the web after indexing its new location each time the corporate website decides to update to the latest fancy CRM glue?
Quoted 4 lines
:-P
--
Regards/Gruss,
Will update the PPR reference like below in V4.
See the page 119 of PPR for AMD Family 19h Model 61h B1, docID 56713
Thanks for the feedback.
Quoted 3 lines
Boris.
Borislav Petkov
maintainerRe: [PATCH v3 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
Quoted 1 line · Perry
Link: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/56713-B1_3_05.zip
What about "This link will become obsolete" don't you understand?
Do you need me to explain it in greater detail?
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquettePerry Yuan
reviewerPATCH v3 9/10 cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization
In reply to Perry YuanIntroduces an optimization to the AMD-Pstate driver by implementing a heterogeneous core topology for the initialization of the highest performance value while driver loading.
There are two type cores designed including performance core and Efficiency Core.
Each core type has different highest performance and frequency values configured by the platform. The amd_pstate driver needs to identify the type of core to correctly set an appropriate highest perf value.
X86_FEATURE_HETERO_CORE_TOPOLOGY is used to identify whether the processor support heterogeneous core type by reading CPUID leaf Fn_0x80000026_EAX and bit 30. if the bit is set as one, then amd_pstate driver will check EBX 30:28 bits to get the core type.
PDF p274
--- arch/x86/include/asm/processor.h | 2 + arch/x86/kernel/cpu/amd.c | 19 +++++++++ drivers/cpufreq/amd-pstate.c | 67 ++++++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 16 deletions(-)
arch/x86/include/asm/processor.h +2 -0
@@ -694,10 +694,12 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu)extern u32 amd_get_highest_perf (void );extern void amd_clear_divider (void );extern void amd_check_microcode (void ); +extern int amd_get_this_core_type (void );#else static inline u32 amd_get_highest_perf (void ) {return 0 ; }static inline void amd_clear_divider (void ) { }static inline void amd_check_microcode (void ) { } +static inline int amd_get_this_core_type (void ) {return -1 ; }#endif extern unsigned long arch_align_stack (unsigned long sp);
arch/x86/kernel/cpu/amd.c +19 -0
@@ -1231,3 +1231,22 @@ void noinstr amd_clear_divider(void) :: "a" (0), "d" (0), "r" (1)); }EXPORT_SYMBOL_GPL (amd_clear_divider); + +#define X86_CPU_TYPE_ID_SHIFT 28 + +/** +* amd_get_this_core_type - Get the type of this heterogeneous CPU +* +* Returns the CPU type [31:28] (i.e., performance or efficient) of +* a CPU in the processor. +* If the processor has no core type support, returns -1. +*/ + +int amd_get_this_core_type (void ) +{ +if (!cpu_feature_enabled (X86_FEATURE_HETERO_CORE_TOPOLOGY )) +return -1 ; + +return cpuid_ebx (0x80000026 ) >>X86_CPU_TYPE_ID_SHIFT ; +} +EXPORT_SYMBOL_GPL (amd_get_this_core_type);
drivers/cpufreq/amd-pstate.c +51 -16
106 lines not on this page · raw message
Signature · 1 line
2.34.1
Mario Limonciello
maintainerRe: [PATCH v3 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization
Quoted 5 lines · Perry Yuan
Introduces an optimization to the AMD-Pstate driver by implementing a heterogeneous core topology for the initialization of the highest performance value while driver loading.
There are two type cores designed including performance core and Efficiency Core.
I would say:
The two core types supported are "performance" and "efficiency".
Quoted 4 lines
Each core type has different highest performance and frequency values configured by the platform. The
amd_pstatedriver needs to identify the type of core to correctly set an appropriate highest perf value.
Quoted 8 lines
X86_FEATURE_HETERO_CORE_TOPOLOGYis used to identify whether the processor support heterogeneous core type by reading CPUID leafFn_0x80000026_EAXand bit 30. if the bit is set as one, thenamd_pstatedriver will check EBX 30:28 bits to get the core type.PDF p274
As Boris suggested in the other patch, this URL won't be stable if the CRM system changes again.
I'd use his suggestion of the title, document number and page instead.
Quoted 1 line
Signed-off-by Perry Yuan <[email protected]>
I checked and it fails to apply at this patch at the moment to:
https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/log/?h=bleeding-edge
error: patch failed: arch/x86/include/asm/cpufeatures.h:470
error: arch/x86/include/asm/cpufeatures.h: patch does not apply
Patch failed at 0008 x86/cpufeatures: Add feature bits for AMD heterogeneous processor
Is it dependent upon refs in x86-tip?
Quoted 30 lines
--- arch/x86/include/asm/processor.h | 2 + arch/x86/kernel/cpu/amd.c | 19 +++++++++ drivers/cpufreq/amd-pstate.c | 67 ++++++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 16 deletions(-)arch/x86/include/asm/processor.h +2
-0@@ -694,10 +694,12 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu)extern u32 amd_get_highest_perf (void );extern void amd_clear_divider (void );extern void amd_check_microcode (void ); +extern int amd_get_this_core_type (void );#else static inline u32 amd_get_highest_perf (void ) {return 0 ; }static inline void amd_clear_divider (void ) { }static inline void amd_check_microcode (void ) { } +static inline int amd_get_this_core_type (void ) {return -1 ; }#endif extern unsigned long arch_align_stack (unsigned long sp);arch/x86/kernel/cpu/amd.c +0
-0@@ -1231,3 +1231,22 @@ void noinstr amd_clear_divider(void) :: "a" (0), "d" (0), "r" (1)); }
arch/x86/kernel/cpu/amd.c void noinstr amd_clear_divider(void) · line 1242
Quoted 10 lines
arch/x86/kernel/cpu/amd.c +9
-0EXPORT_SYMBOL_GPL (amd_clear_divider); + +#define X86_CPU_TYPE_ID_SHIFT 28 + +/** +* amd_get_this_core_type - Get the type of this heterogeneous CPU +* +* Returns the CPU type [31:28] (i.e., performance or efficient) of +* a CPU in the processor. +* If the processor has no core type support, returns -1.
Is there any reason to use -1 specifically? It seems like it should be CPU_CORE_TYPE_NO_HETERO_SUP.
arch/x86/kernel/cpu/amd.c void noinstr amd_clear_divider(void) · line 1245
Quoted 3 lines
arch/x86/kernel/cpu/amd.c +3
-0+*/ + +int amd_get_this_core_type (void )
Shouldn't the return be "enum amd_core_type"?
arch/x86/kernel/cpu/amd.c void noinstr amd_clear_divider(void) · line 1248
Quoted 3 lines
arch/x86/kernel/cpu/amd.c +3
-0+{ +if (!cpu_feature_enabled (X86_FEATURE_HETERO_CORE_TOPOLOGY )) +return -1 ;
return CPU_CORE_TYPE_NO_HETERO_SUP;
Quoted 9 lines
arch/x86/kernel/cpu/amd.c +4
-0+ +return cpuid_ebx (0x80000026 ) >>X86_CPU_TYPE_ID_SHIFT ; +} +EXPORT_SYMBOL_GPL (amd_get_this_core_type);drivers/cpufreq/amd-pstate.c +0
-0
drivers/cpufreq/amd-pstate.c line 55
Quoted 7 lines
drivers/cpufreq/amd-pstate.c +2
-2@@ -51,8 +51,9 @@#define AMD_PSTATE_TRANSITION_LATENCY 20000#define AMD_PSTATE_TRANSITION_DELAY 1000 -#define CPPC_HIGHEST_PERF_PERFORMANCE 196 -#define CPPC_HIGHEST_PERF_DEFAULT 166 +#define CPPC_HIGHEST_PERF_EFFICIENT 132 +#define CPPC_HIGHEST_PERF_PERFORMANCE 196
Why did you bump this to two tabs? It can stay at one tab no?
Quoted 21 lines
drivers/cpufreq/amd-pstate.c +9
-0+#define CPPC_HIGHEST_PERF_DEFAULT 166#define AMD_CPPC_EPP_PERFORMANCE 0x00#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80@@ -85,6 +86,14 @@ struct quirk_entry {u32 lowest_freq; }; +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ +enum amd_core_type { +CPU_CORE_TYPE_NO_HETERO_SUP =-1 , +CPU_CORE_TYPE_PERFORMANCE =0 , +CPU_CORE_TYPE_EFFICIENCY =1 , +CPU_CORE_TYPE_UNDEFINED =2 , +}; +/* * TODO: We need more time to fine tune processors with shared memory solution * with community together. @@ -359,9 +368,27 @@ static inline int amd_pstate_enable(bool enable)return static_call (amd_pstate_enable)(enable);
drivers/cpufreq/amd-pstate.c static inline int amd_pstate_enable(bool enable) · line 378
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +8
-0} +static void get_this_core_type (void *data) +{ +int *cpu_type = data; + + *cpu_type =amd_get_this_core_type (); +} + +static int amd_pstate_get_cpu_type (int cpu)
Shouldn't the return be "enum amd_core_type"?
drivers/cpufreq/amd-pstate.c static inline int amd_pstate_enable(bool enable) · line 380
Quoted 2 lines
drivers/cpufreq/amd-pstate.c +2
-0+{ +int cpu_type =0 ;
I think it's confusing to initialize this to CPU_CORE_TYPE_PERFORMANCE. It should be either not initialized or initialized to CPU_CORE_TYPE_NO_HETERO_SUP.
Quoted 71 lines
drivers/cpufreq/amd-pstate.c +30
-14+ +smp_call_function_single (cpu, get_this_core_type, &cpu_type,1 ); + +return cpu_type; +} +static u32 amd_pstate_highest_perf_set (struct amd_cpudata *cpudata) {struct cpuinfo_x86 *c = &cpu_data (0 ); +u32 highest_perf; +int core_type;/* * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f, @@ -371,7 +398,26 @@ static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)if (c->x86 ==0x19 && (c->x86_model >=0x70 && c->x86_model <=0x7f ))return CPPC_HIGHEST_PERF_PERFORMANCE ; -return CPPC_HIGHEST_PERF_DEFAULT ; + core_type =amd_pstate_get_cpu_type (cpudata->cpu); +pr_debug ("core_type %d found\n" , core_type); + +switch (core_type) { +case CPU_CORE_TYPE_NO_HETERO_SUP : + highest_perf =CPPC_HIGHEST_PERF_DEFAULT ; +break ; +case CPU_CORE_TYPE_PERFORMANCE : + highest_perf =CPPC_HIGHEST_PERF_PERFORMANCE ; +break ; +case CPU_CORE_TYPE_EFFICIENCY : + highest_perf =CPPC_HIGHEST_PERF_EFFICIENT ; +break ; +default : + highest_perf =CPPC_HIGHEST_PERF_DEFAULT ; +WARN_ONCE (true,"WARNING: Undefined core type found" ); +break ; + } + +return highest_perf; }static int pstate_init_perf (struct amd_cpudata *cpudata)@@ -384,15 +430,7 @@ static int pstate_init_perf(struct amd_cpudata *cpudata)if (ret)return ret; -/* For platforms that do not support the preferred core feature, the -* highest_pef may be configured with 166 or 255, to avoid max frequency -* calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as -* the default max perf. -*/ -if (cpudata->hw_prefcore) - highest_perf =amd_pstate_highest_perf_set (cpudata); -else - highest_perf =AMD_CPPC_HIGHEST_PERF (cap1); + highest_perf =amd_pstate_highest_perf_set (cpudata);WRITE_ONCE (cpudata->highest_perf, highest_perf);WRITE_ONCE (cpudata->max_limit_perf, highest_perf);@@ -413,10 +451,7 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)if (ret)return ret; -if (cpudata->hw_prefcore) - highest_perf =amd_pstate_highest_perf_set (cpudata); -else - highest_perf = cppc_perf.highest_perf; + highest_perf =amd_pstate_highest_perf_set (cpudata);WRITE_ONCE (cpudata->highest_perf, highest_perf);WRITE_ONCE (cpudata->max_limit_perf, highest_perf);
Re: [PATCH v3 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization
In reply to Perry YuanQuoted 57 lines · Perry Yuan
Introduces an optimization to the AMD-Pstate driver by implementing a heterogeneous core topology for the initialization of the highest performance value while driver loading.
There are two type cores designed including performance core and Efficiency Core.
Each core type has different highest performance and frequency values configured by the platform. Theamd_pstatedriver needs to identify the type of core to correctly set an appropriate highest perf value.
X86_FEATURE_HETERO_CORE_TOPOLOGYis used to identify whether the processor support heterogeneous core type by reading CPUID leafFn_0x80000026_EAXand bit 30. if the bit is set as one, thenamd_pstatedriver will check EBX 30:28 bits to get the core type.PDF p274
Link https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdfSigned-off-by Perry Yuan <[email protected]>--- arch/x86/include/asm/processor.h | 2 + arch/x86/kernel/cpu/amd.c | 19 +++++++++ drivers/cpufreq/amd-pstate.c | 67 ++++++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 16 deletions(-)arch/x86/include/asm/processor.h +2
-0@@ -694,10 +694,12 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu)extern u32 amd_get_highest_perf (void );extern void amd_clear_divider (void );extern void amd_check_microcode (void ); +extern int amd_get_this_core_type (void );#else static inline u32 amd_get_highest_perf (void ) {return 0 ; }static inline void amd_clear_divider (void ) { }static inline void amd_check_microcode (void ) { } +static inline int amd_get_this_core_type (void ) {return -1 ; }#endif extern unsigned long arch_align_stack (unsigned long sp);arch/x86/kernel/cpu/amd.c +8
-0@@ -1231,3 +1231,22 @@ void noinstr amd_clear_divider(void) :: "a" (0), "d" (0), "r" (1)); }EXPORT_SYMBOL_GPL (amd_clear_divider); + +#define X86_CPU_TYPE_ID_SHIFT 28 + +/** +* amd_get_this_core_type - Get the type of this heterogeneous CPU +* +* Returns the CPU type [31:28] (i.e., performance or efficient) of +* a CPU in the processor.
arch/x86/kernel/cpu/amd.c void noinstr amd_clear_divider(void) · line 1251
Quoted 10 lines
arch/x86/kernel/cpu/amd.c +10
-0+* If the processor has no core type support, returns -1. +*/ + +int amd_get_this_core_type (void ) +{ +if (!cpu_feature_enabled (X86_FEATURE_HETERO_CORE_TOPOLOGY )) +return -1 ; + +return cpuid_ebx (0x80000026 ) >>X86_CPU_TYPE_ID_SHIFT ; +}
The 0x80000026 parser doesn't have support for discovering and recording the heterogenous stuff _yet_. Eventually I suppose it will get that and until then amd-pstate will have to discover the core type by itself.
Even this, I find the bitfield way of doing things more cleaner instead of using shifts and masks. Something like...
int amd_get_this_core_type(void)
{
struct {
u32 num_processors :16,
power_efficiency_ranking :8,
native_model_id :4,
core_type :4;
} props;
if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY))
return -1;
cpuid_leaf_reg(0x80000026, CPUID_EBX, &props);
return props->core_type;}
Quoted 92 lines
arch/x86/kernel/cpu/amd.c +1
-0+EXPORT_SYMBOL_GPL (amd_get_this_core_type);drivers/cpufreq/amd-pstate.c +49
-3@@ -51,8 +51,9 @@#define AMD_PSTATE_TRANSITION_LATENCY 20000#define AMD_PSTATE_TRANSITION_DELAY 1000 -#define CPPC_HIGHEST_PERF_PERFORMANCE 196 -#define CPPC_HIGHEST_PERF_DEFAULT 166 +#define CPPC_HIGHEST_PERF_EFFICIENT 132 +#define CPPC_HIGHEST_PERF_PERFORMANCE 196 +#define CPPC_HIGHEST_PERF_DEFAULT 166#define AMD_CPPC_EPP_PERFORMANCE 0x00#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80@@ -85,6 +86,14 @@ struct quirk_entry {u32 lowest_freq; }; +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ +enum amd_core_type { +CPU_CORE_TYPE_NO_HETERO_SUP =-1 , +CPU_CORE_TYPE_PERFORMANCE =0 , +CPU_CORE_TYPE_EFFICIENCY =1 , +CPU_CORE_TYPE_UNDEFINED =2 , +}; +/* * TODO: We need more time to fine tune processors with shared memory solution * with community together. @@ -359,9 +368,27 @@ static inline int amd_pstate_enable(bool enable)return static_call (amd_pstate_enable)(enable); } +static void get_this_core_type (void *data) +{ +int *cpu_type = data; + + *cpu_type =amd_get_this_core_type (); +} + +static int amd_pstate_get_cpu_type (int cpu) +{ +int cpu_type =0 ; + +smp_call_function_single (cpu, get_this_core_type, &cpu_type,1 ); + +return cpu_type; +} +static u32 amd_pstate_highest_perf_set (struct amd_cpudata *cpudata) {struct cpuinfo_x86 *c = &cpu_data (0 ); +u32 highest_perf; +int core_type;/* * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f, @@ -371,7 +398,26 @@ static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)if (c->x86 ==0x19 && (c->x86_model >=0x70 && c->x86_model <=0x7f ))return CPPC_HIGHEST_PERF_PERFORMANCE ; -return CPPC_HIGHEST_PERF_DEFAULT ; + core_type =amd_pstate_get_cpu_type (cpudata->cpu); +pr_debug ("core_type %d found\n" , core_type); + +switch (core_type) { +case CPU_CORE_TYPE_NO_HETERO_SUP : + highest_perf =CPPC_HIGHEST_PERF_DEFAULT ; +break ; +case CPU_CORE_TYPE_PERFORMANCE : + highest_perf =CPPC_HIGHEST_PERF_PERFORMANCE ; +break ; +case CPU_CORE_TYPE_EFFICIENCY : + highest_perf =CPPC_HIGHEST_PERF_EFFICIENT ; +break ; +default : + highest_perf =CPPC_HIGHEST_PERF_DEFAULT ; +WARN_ONCE (true,"WARNING: Undefined core type found" ); +break ; + } + +return highest_perf; }static int pstate_init_perf (struct amd_cpudata *cpudata)@@ -384,15 +430,7 @@ static int pstate_init_perf(struct amd_cpudata *cpudata)if (ret)return ret;
drivers/cpufreq/amd-pstate.c static int pstate_init_perf(struct amd_cpudata *cpudata) · line 433
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +1
-9-/* For platforms that do not support the preferred core feature, the -* highest_pef may be configured with 166 or 255, to avoid max frequency -* calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as -* the default max perf. -*/ -if (cpudata->hw_prefcore) - highest_perf =amd_pstate_highest_perf_set (cpudata); -else - highest_perf =AMD_CPPC_HIGHEST_PERF (cap1); + highest_perf =amd_pstate_highest_perf_set (cpudata);
Without this patch, on systems without hw_prefcore, highest_perf would be derived from MSR_AMD_CPPC_CAP1. The value of the highest_perf could have been 255.
This patch changes the behaviour and sets the default value to CPPC_HIGHEST_PERF_DEFAULT which is only 196. Am I missing something ?
drivers/cpufreq/amd-pstate.c static int cppc_init_perf(struct amd_cpudata *cpudata) · line 454
Quoted 12 lines
drivers/cpufreq/amd-pstate.c +1
-4WRITE_ONCE (cpudata->highest_perf, highest_perf);WRITE_ONCE (cpudata->max_limit_perf, highest_perf);@@ -413,10 +451,7 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)if (ret)return ret; -if (cpudata->hw_prefcore) - highest_perf =amd_pstate_highest_perf_set (cpudata); -else - highest_perf = cppc_perf.highest_perf; + highest_perf =amd_pstate_highest_perf_set (cpudata);
Same comment as above here.
Quoted 5 lines
drivers/cpufreq/amd-pstate.c +0
-0WRITE_ONCE (cpudata->highest_perf, highest_perf);WRITE_ONCE (cpudata->max_limit_perf, highest_perf);Signature · 1 line
2.34.1
Yuan, Perry
reviewerRE: [PATCH v3 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Gautham
Quoted 80 lines
-----Original Message-----
From: Shenoy, Gautham Ranjal <[email protected]>
Sent: Thursday, June 13, 2024 3:14 PM
To: Yuan, Perry <[email protected]>; Limonciello, Mario <[email protected]>
Cc: [email protected]; [email protected]; Huang, Ray <[email protected]>; Petkov, Borislav <[email protected]>; Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH v3 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initializationQuoted 14 lines · Perry Yuan
Introduces an optimization to the AMD-Pstate driver by implementing a heterogeneous core topology for the initialization of the highest performance value while driver loading.
There are two type cores designed including performance core and Efficiency Core.
Each core type has different highest performance and frequency values configured by the platform. Theamd_pstatedriver needs to identify the type of core to correctly set an appropriate highest perf value.
X86_FEATURE_HETERO_CORE_TOPOLOGYis used to identify whether the processor support heterogeneous core type by reading CPUID leafFn_0x80000026_EAXand bit 30. if the bit is set as one, thenamd_pstatedriver will check EBX 30:28 bits to get the core type.Quoted 4 lines
PDF p274
Link:
https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/p
Quoted 45 lines
rogrammer-references/24593.pdf
Signed-off-by Perry Yuan <[email protected]>--- arch/x86/include/asm/processor.h | 2 + arch/x86/kernel/cpu/amd.c | 19 +++++++++ drivers/cpufreq/amd-pstate.c | 67 ++++++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 16 deletions(-)arch/x86/include/asm/processor.h +0
-0@@ -694,10 +694,12 @@ static inline u32 per_cpu_l2c_id(unsigned intcpu) extern u32
amd_get_highest_perf(void); extern voidamd_clear_divider(void); extern voidamd_check_microcode(void);arch/x86/include/asm/processor.h +2
-0+extern int amd_get_this_core_type (void );#else static inline u32 amd_get_highest_perf (void ) {return 0 ; }static inline void amd_clear_divider (void ) { }static inline void amd_check_microcode (void ) { } +static inline int amd_get_this_core_type (void ) {return -1 ; }#endif extern unsigned long arch_align_stack(unsigned long sp); diff --git
a/arch/x86/kernel/cpu/amd.cb/arch/x86/kernel/cpu/amd.cindex44df3f11e731..62a4ef21ef79100644arch/x86/kernel/cpu/amd.c +13
-0@@ -1231,3 +1231,22 @@ void noinstr amd_clear_divider(void) :: "a" (0), "d" (0), "r" (1)); }EXPORT_SYMBOL_GPL (amd_clear_divider); + +#define X86_CPU_TYPE_ID_SHIFT 28 + +/** +* amd_get_this_core_type - Get the type of this heterogeneous CPU +* +* Returns the CPU type [31:28] (i.e., performance or efficient) of +* a CPU in the processor. +* If the processor has no core type support, returns -1. +*/ + +int amd_get_this_core_type (void ) +{
Quoted 8 lines
arch/x86/kernel/cpu/amd.c void noinstr amd_clear_divider(void)
Quoted 4 lines
arch/x86/kernel/cpu/amd.c +4
-0+if (!cpu_feature_enabled (X86_FEATURE_HETERO_CORE_TOPOLOGY )) +return -1 ; + +return cpuid_ebx (0x80000026 ) >>X86_CPU_TYPE_ID_SHIFT ; }The
0x80000026parser doesn't have support for discovering and recording the heterogenous stuff _yet_. Eventually I suppose it will get that and until then amd-pstate will have to discover the core type by itself.
The PPR doc need to be updated to use a new one,
https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/56713-B1_3_05.zip
P119
I have added the cpu feature flag in [PATCH v3 08/10], it will be fine to identify the core type for pstate driver with one line code.
Quoted 131 lines
Even this, I find the bitfield way of doing things more cleaner instead of using shifts and masks. Something like...
int
amd_get_this_core_type(void){ struct { u32 num_processors :16, power_efficiency_ranking :8, native_model_id :4, core_type :4; } props; if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) return -1; cpuid_leaf_reg(0x80000026, CPUID_EBX, &props); return props->core_type;}
Quoted 18 lines
arch/x86/kernel/cpu/amd.c +1
-0+EXPORT_SYMBOL_GPL (amd_get_this_core_type);(quoted diff) +0
-0
b/drivers/cpufreq/amd-pstate.cindexcb59de71b6ee..fa486dfaa7e8100644drivers/cpufreq/amd-pstate.c +3
-2@@ -51,8 +51,9 @@#define AMD_PSTATE_TRANSITION_LATENCY 20000#define AMD_PSTATE_TRANSITION_DELAY 1000 -#define CPPC_HIGHEST_PERF_PERFORMANCE 196 -#define CPPC_HIGHEST_PERF_DEFAULT 166 +#define CPPC_HIGHEST_PERF_EFFICIENT 132 +#define CPPC_HIGHEST_PERF_PERFORMANCE 196 +#define CPPC_HIGHEST_PERF_DEFAULT 166#define AMD_CPPC_EPP_PERFORMANCE 0x00#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80drivers/cpufreq/amd-pstate.c struct quirk_entry { · line 89
Quoted 4 lines
drivers/cpufreq/amd-pstate.c +1
-0@@ -85,6 +86,14 @@ struct quirk_entry {u32 lowest_freq; }; +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ enum
amd_core_typedrivers/cpufreq/amd-pstate.c struct quirk_entry {
Quoted 9 lines
drivers/cpufreq/amd-pstate.c +7
-0+{ +CPU_CORE_TYPE_NO_HETERO_SUP =-1 , +CPU_CORE_TYPE_PERFORMANCE =0 , +CPU_CORE_TYPE_EFFICIENCY =1 , + CPU_CORE_TYPE_UNDEFINED = 2, +}; +/* * TODO: We need more time to fine tune processors with shared memory solution
drivers/cpufreq/amd-pstate.c static u32 amd_pstate_highest_perf_set(struct
Quoted 27 lines
drivers/cpufreq/amd-pstate.c +15
-0* with community together. @@ -359,9 +368,27 @@ static inline int amd_pstate_enable(bool enable)return static_call (amd_pstate_enable)(enable); } +static void get_this_core_type (void *data) { +int *cpu_type = data; + + *cpu_type =amd_get_this_core_type (); } + +static int amd_pstate_get_cpu_type (int cpu) { +int cpu_type =0 ; + +smp_call_function_single (cpu, get_this_core_type, &cpu_type,1 ); + +return cpu_type; +} +static u32 amd_pstate_highest_perf_set (struct amd_cpudata *cpudata) {struct cpuinfo_x86 *c = &cpu_data (0 ); +u32 highest_perf; +int core_type;/* * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f, @@ -371,7 +398,26 @@ static u32 amd_pstate_highest_perf_set(struct
amd_cpudata*cpudata)drivers/cpufreq/amd-pstate.c static u32 amd_pstate_highest_perf_set(struct · line 398
Quoted 1 line
drivers/cpufreq/amd-pstate.c +0
-0if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <=
0x7f))Quoted 22 lines
drivers/cpufreq/amd-pstate.c +19
-1return CPPC_HIGHEST_PERF_PERFORMANCE ; -return CPPC_HIGHEST_PERF_DEFAULT ; + core_type =amd_pstate_get_cpu_type (cpudata->cpu); +pr_debug ("core_type %d found\n" , core_type); + +switch (core_type) { +case CPU_CORE_TYPE_NO_HETERO_SUP : + highest_perf =CPPC_HIGHEST_PERF_DEFAULT ; +break ; +case CPU_CORE_TYPE_PERFORMANCE : + highest_perf =CPPC_HIGHEST_PERF_PERFORMANCE ; +break ; +case CPU_CORE_TYPE_EFFICIENCY : + highest_perf =CPPC_HIGHEST_PERF_EFFICIENT ; +break ; +default : + highest_perf =CPPC_HIGHEST_PERF_DEFAULT ; +WARN_ONCE (true,"WARNING: Undefined core type found" ); +break ; + } +drivers/cpufreq/amd-pstate.c static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +2
-2+return highest_perf; } static int pstate_init_perf(struct amd_cpudata *cpudata) @@ -384,15 +430,7 @@ static int pstate_init_perf(struct amd_cpudata *cpudata) if (ret) return ret; -/* For platforms that do not support the preferred core feature, the -* highest_pef may be configured with 166 or 255, to avoid max frequency
Quoted 1 line
- * calculated wrongly. we take the
AMD_CPPC_HIGHEST_PERF(cap1)value as
drivers/cpufreq/amd-pstate.c static int pstate_init_perf(struct amd_cpudata *cpudata) · line 433
Quoted 7 lines
drivers/cpufreq/amd-pstate.c +1
-6-* the default max perf. -*/ -if (cpudata->hw_prefcore) - highest_perf =amd_pstate_highest_perf_set (cpudata); -else - highest_perf =AMD_CPPC_HIGHEST_PERF (cap1); + highest_perf =amd_pstate_highest_perf_set (cpudata);
Quoted 3 lines
Without this patch, on systems without
hw_prefcore,highest_perfwould be derived fromMSR_AMD_CPPC_CAP1. The value of thehighest_perfcould have been 255.
Thanks to your reminder, highest perf of systems w/o hw_prefcore are 166 or 255 because of the long history. It still needs to use hw_prefcore to identify the CPU otherwise the highest perf will be incorrect on some systems. Let me fix it in v4.
Quoted 3 lines
This patch changes the behaviour and sets the default value to
CPPC_HIGHEST_PERF_DEFAULTwhich is only 196. Am I missing something ?
Some new clients system update the default highest perf value to 196 from 166 from power firmware, so the highest perf need to match the CPU and update the value in pstate driver as well.
Quoted 10 lines
Quoted 5 lines
WRITE_ONCE(cpudata->highest_perf, highest_perf); WRITE_ONCE(cpudata->max_limit_perf, highest_perf); @@ -413,10+451,7
Quoted 4 lines
@@ static int
cppc_init_perf(struct amd_cpudata *cpudata)if (ret) return ret;
Quoted 8 lines
drivers/cpufreq/amd-pstate.c static int cppc_init_perf(struct amd_cpudata *cpudata) · line 454
Quoted 5 lines
drivers/cpufreq/amd-pstate.c +1
-4-if (cpudata->hw_prefcore) - highest_perf =amd_pstate_highest_perf_set (cpudata); -else - highest_perf = cppc_perf.highest_perf; + highest_perf =amd_pstate_highest_perf_set (cpudata);Same comment as above here.
Will get this fixed in v4.
Thanks for your feedback.
Quoted 7 lines
Quoted 7 lines
WRITE_ONCE(cpudata->highest_perf, highest_perf); WRITE_ONCE(cpudata->max_limit_perf, highest_perf);--
2.34.1
Perry Yuan
reviewerPATCH v3 10/10 cpufreq: amd-pstate: automatically load pstate driver by default
In reply to Perry YuanIf the amd-pstate driver is not loaded automatically by default, it is because the kernel command line parameter has not been added. To resolve this issue, it is necessary to call the amd_pstate_set_driver() function to enable the desired mode (passive/active/guided) before registering the driver instance.
This ensures that the driver is loaded correctly without relying on the kernel command line parameter.
Meanwhle, user can add driver mode in command line which will override the kernel config default option.
[ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-v1 xhci-hcd [ 0.982579] amd_pstate: failed to register with return -22
--- drivers/cpufreq/amd-pstate.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
drivers/cpufreq/amd-pstate.c +18 -9
@@ -1841,28 +1841,37 @@ static int __init amd_pstate_init(void)/* check if this machine need CPPC quirks */ dmi_check_system (amd_pstate_quirks_table); -switch (cppc_state) { -case AMD_PSTATE_UNDEFINED : +/* +* get driver mode for loading from command line choice or kernel config +* cppc_state will be AMD_PSTATE_UNDEFINED if no command line input +* command line choice will override the kconfig option +*/ +if (cppc_state ==AMD_PSTATE_UNDEFINED ) { +pr_err ("pyuan cppc_state == AMD_PSTATE_UNDEFINED \n" );/* Disable on the following configs by default: * 1. Undefined platforms * 2. Server platforms * 3. Shared memory designs */ if (amd_pstate_acpi_pm_profile_undefined () || -amd_pstate_acpi_pm_profile_server () || - !cpu_feature_enabled (X86_FEATURE_CPPC )) { +amd_pstate_acpi_pm_profile_server ()) {pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ; } - ret =amd_pstate_set_driver (CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ); -if (ret) -return ret; -break ; +/* get driver mode from kernel config option [1:4] */ + cppc_state =CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ; + } + +switch (cppc_state) {case AMD_PSTATE_DISABLE : +pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ;case AMD_PSTATE_PASSIVE :case AMD_PSTATE_ACTIVE :case AMD_PSTATE_GUIDED : + ret =amd_pstate_set_driver (cppc_state); +if (ret) +return ret;break ;default :return -EINVAL ;
@@ -1883,7 +1892,7 @@ static int __init amd_pstate_init(void)/* enable amd pstate feature */ ret =amd_pstate_enable (true);if (ret) { -pr_err ("failed to enable with return %d\n" , ret); +pr_err ("failed to enable driver mode(%d)\n" , cppc_state);return ret; }
Signature · 1 line
2.34.1
Mario Limonciello
maintainerRe: [PATCH v3 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
Quoted 8 lines · Perry Yuan
If the
amd-pstatedriver is not loaded automatically by default, it is because the kernel command line parameter has not been added. To resolve this issue, it is necessary to call theamd_pstate_set_driver()function to enable the desired mode (passive/active/guided) before registering the driver instance.
This ensures that the driver is loaded correctly without relying on the kernel command line parameter.
Quoted 5 lines
Meanwhle, user can add driver mode in command line which will override the kernel config default option.
[ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-v1 xhci-hcd [ 0.982579] amd_pstate: failed to register with return -22
This is currently "intended" behavior. There is a comment remaining in the driver at the moment not part of this patch:
/*
* TODO: We need more time to fine tune processors with shared memory
solution
* with community together.
*
* There are some performance drops on the CPU benchmarks which reports
from
* Suse. We are co-working with them to fine tune the shared memory
solution. So
* we disable it by default to go acpi-cpufreq on these processors and
add a
* module parameter to be able to enable it manually for debugging. */
Would you say that the performance drops are worked out on the shared memory designs?
* If so; this comment should be dropped too in this patch.
* If not; this patch really shouldn't be done as is.
Quoted 15 lines
Reported-by Andrei Amuraritei <[email protected]>Signed-off-by Perry Yuan <[email protected]>--- drivers/cpufreq/amd-pstate.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)drivers/cpufreq/amd-pstate.c +0
-0@@ -1841,28 +1841,37 @@ static int __init amd_pstate_init(void)/* check if this machine need CPPC quirks */ dmi_check_system (amd_pstate_quirks_table);
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1850
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +7
-2-switch (cppc_state) { -case AMD_PSTATE_UNDEFINED : +/* +* get driver mode for loading from command line choice or kernel config +* cppc_state will be AMD_PSTATE_UNDEFINED if no command line input +* command line choice will override the kconfig option +*/ +if (cppc_state ==AMD_PSTATE_UNDEFINED ) { +pr_err ("pyuan cppc_state == AMD_PSTATE_UNDEFINED \n" );
Looks like a debug line escaped into the patch.
Quoted 42 lines
drivers/cpufreq/amd-pstate.c +11
-7/* Disable on the following configs by default: * 1. Undefined platforms * 2. Server platforms * 3. Shared memory designs */ if (amd_pstate_acpi_pm_profile_undefined () || -amd_pstate_acpi_pm_profile_server () || - !cpu_feature_enabled (X86_FEATURE_CPPC )) { +amd_pstate_acpi_pm_profile_server ()) {pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ; } - ret =amd_pstate_set_driver (CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ); -if (ret) -return ret; -break ; +/* get driver mode from kernel config option [1:4] */ + cppc_state =CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ; + } + +switch (cppc_state) {case AMD_PSTATE_DISABLE : +pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ;case AMD_PSTATE_PASSIVE :case AMD_PSTATE_ACTIVE :case AMD_PSTATE_GUIDED : + ret =amd_pstate_set_driver (cppc_state); +if (ret) +return ret;break ;default :return -EINVAL ;@@ -1883,7 +1892,7 @@ static int __init amd_pstate_init(void)/* enable amd pstate feature */ ret =amd_pstate_enable (true);if (ret) { -pr_err ("failed to enable with return %d\n" , ret); +pr_err ("failed to enable driver mode(%d)\n" , cppc_state);return ret; }
Re: [PATCH v3 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
In reply to Perry YuanHello Perry,
Quoted 28 lines · Perry Yuan
If the
amd-pstatedriver is not loaded automatically by default, it is because the kernel command line parameter has not been added. To resolve this issue, it is necessary to call theamd_pstate_set_driver()function to enable the desired mode (passive/active/guided) before registering the driver instance.
This ensures that the driver is loaded correctly without relying on the kernel command line parameter.Meanwhle, user can add driver mode in command line which will override the kernel config default option.
[ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-v1 xhci-hcd [ 0.982579] amd_pstate: failed to register with return -22Reported-by Andrei Amuraritei <[email protected]>Signed-off-by Perry Yuan <[email protected]>--- drivers/cpufreq/amd-pstate.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)drivers/cpufreq/amd-pstate.c +0
-0@@ -1841,28 +1841,37 @@ static int __init amd_pstate_init(void)/* check if this machine need CPPC quirks */ dmi_check_system (amd_pstate_quirks_table);
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1850
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +7
-2-switch (cppc_state) { -case AMD_PSTATE_UNDEFINED : +/* +* get driver mode for loading from command line choice or kernel config +* cppc_state will be AMD_PSTATE_UNDEFINED if no command line input +* command line choice will override the kconfig option +*/ +if (cppc_state ==AMD_PSTATE_UNDEFINED ) { +pr_err ("pyuan cppc_state == AMD_PSTATE_UNDEFINED \n" );
As Mario pointed out, this needs to be removed :-)
The following review comments are assuming that you want this patch so that amd-pstate is the default driver on shared-memory non-server platforms.
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1854
Quoted 4 lines
drivers/cpufreq/amd-pstate.c +0
-0/* Disable on the following configs by default: * 1. Undefined platforms * 2. Server platforms * 3. Shared memory designs
The comment says the driver needs to be disabled on the shared memory designs by default. But...
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1853
Quoted 4 lines
drivers/cpufreq/amd-pstate.c +0
-2*/ if (amd_pstate_acpi_pm_profile_undefined() || -amd_pstate_acpi_pm_profile_server () || - !cpu_feature_enabled (X86_FEATURE_CPPC )) {
...the check for shared-memory design has been removed. Is this intentional ? So do you want the comment to be fixed so that it is clear that we want amd-pstate to be the default driver on non-server platforms ?
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1862
Quoted 10 lines
drivers/cpufreq/amd-pstate.c +3
-4+ amd_pstate_acpi_pm_profile_server()) {pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ; } - ret =amd_pstate_set_driver (CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ); -if (ret) -return ret; -break ; +/* get driver mode from kernel config option [1:4] */ + cppc_state =CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ;
If someone booted the system with "amd_pstate=disable", the above will overwrite that preference, no ?
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1868
Quoted 6 lines
drivers/cpufreq/amd-pstate.c +4
-0+ } + +switch (cppc_state) {case AMD_PSTATE_DISABLE : +pr_info ("driver load is disabled, boot with specific mode to enable this\n" );return -ENODEV ;
....and this "case" statement will never be reachable in the aforementioned case.
Quoted 20 lines
drivers/cpufreq/amd-pstate.c +4
-1case AMD_PSTATE_PASSIVE :case AMD_PSTATE_ACTIVE :case AMD_PSTATE_GUIDED : + ret =amd_pstate_set_driver (cppc_state); +if (ret) +return ret;break ;default :return -EINVAL ;@@ -1883,7 +1892,7 @@ static int __init amd_pstate_init(void)/* enable amd pstate feature */ ret =amd_pstate_enable (true);if (ret) { -pr_err ("failed to enable with return %d\n" , ret); +pr_err ("failed to enable driver mode(%d)\n" , cppc_state);return ret; }Signature · 1 line
2.34.1
Yuan, Perry
reviewerRE: [PATCH v3 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Gautham
Quoted 54 lines
-----Original Message-----
From: Shenoy, Gautham Ranjal <[email protected]>
Sent: Thursday, June 13, 2024 4:19 PM
To: Yuan, Perry <[email protected]>; Limonciello, Mario <[email protected]>
Cc: [email protected]; [email protected]; Huang, Ray <[email protected]>; Petkov, Borislav <[email protected]>; Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH v3 10/10] cpufreq: amd-pstate: automatically load pstate driver by defaultHello Perry,
Quoted 8 lines · Perry Yuan
If the
amd-pstatedriver is not loaded automatically by default, it is because the kernel command line parameter has not been added. To resolve this issue, it is necessary to call theamd_pstate_set_driver()function to enable the desired mode (passive/active/guided) before registering the driver instance. This ensures that the driver is loaded correctly without relying on the kernel command line parameter.Quoted 4 lines
Meanwhle, user can add driver mode in command line which will override the kernel config default option.
[ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-v1 xhci-hcd
Quoted 14 lines
[ 0.982579] amd_pstate: failed to register with return -22Reported-by Andrei Amuraritei <[email protected]>Signed-off-by Perry Yuan <[email protected]>--- drivers/cpufreq/amd-pstate.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)(quoted diff) +0
-0
b/drivers/cpufreq/amd-pstate.cindexfa486dfaa7e8..6e5c398810bf100644drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1845
Quoted 7 lines
drivers/cpufreq/amd-pstate.c +2
-2@@ -1841,28 +1841,37 @@ static int __init amd_pstate_init(void)/* check if this machine need CPPC quirks */ dmi_check_system (amd_pstate_quirks_table); -switch (cppc_state) { -case AMD_PSTATE_UNDEFINED : +/* +* get driver mode for loading from command line choice or kernel config
Quoted 8 lines
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1846
Quoted 1 line
drivers/cpufreq/amd-pstate.c +1
-0+* cppc_state will be AMD_PSTATE_UNDEFINED if no command line input
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1850
Quoted 4 lines
drivers/cpufreq/amd-pstate.c +4
-0+* command line choice will override the kconfig option +*/ +if (cppc_state ==AMD_PSTATE_UNDEFINED ) { +pr_err ("pyuan cppc_state == AMD_PSTATE_UNDEFINED \n" );As Mario pointed out, this needs to be removed :-)
Yep , I forget to delete it when sending the patches ☹
Quoted 6 lines
The following review comments are assuming that you want this patch so that amd-pstate is the default driver on shared-memory non-server platforms.
Quoted 7 lines
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1854
Quoted 4 lines
drivers/cpufreq/amd-pstate.c +0
-0/* Disable on the following configs by default: * 1. Undefined platforms * 2. Server platforms * 3. Shared memory designs The comment says the driver needs to be disabled on the shared memory designs by default. But...
The comments need to be updated looks like.
Quoted 11 lines
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1853
Quoted 7 lines
drivers/cpufreq/amd-pstate.c +0
-2*/ if (amd_pstate_acpi_pm_profile_undefined() || -amd_pstate_acpi_pm_profile_server () || - !cpu_feature_enabled (X86_FEATURE_CPPC )) {...the check for shared-memory design has been removed. Is this intentional ? So do you want the comment to be fixed so that it is clear that we want amd-pstate to be the default driver on non-server platforms ?
We are going to make the pstate driver as default driver on client CPUs, server CPU will not load pstate drivers when no driver mode command line added.
I believe it will return TRUE on all server CPUs with " amd_pstate_acpi_pm_profile_server() ", so server CPU is not impacted. Even the shared memory type client system, I guess it is OK to load pstate driver since this change.(Just a few generations CPU using shared memory) Some customers reported that TR40 system(shared memory) has better performance than acpi_cpufreq.
Users can disable the pstate driver from kernel config or command line, from the initial kernel config patches made by Mario, the purpose is load EPP driver by default. It just had a few unexpected loading issues fixed by this patch.
Quoted 10 lines
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1858
Quoted 5 lines
drivers/cpufreq/amd-pstate.c +1
-0+ amd_pstate_acpi_pm_profile_server()) { pr_info("driver load is disabled, boot with specificmode to enable this\n");
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void)
Quoted 3 lines
drivers/cpufreq/amd-pstate.c +0
-1return -ENODEV ; } - ret =
amd_pstate_set_driver(CONFIG_X86_AMD_PSTATE_DEFAULT_MODE);
Quoted 8 lines
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1862
Quoted 5 lines
drivers/cpufreq/amd-pstate.c +2
-3-if (ret) -return ret; -break ; +/* get driver mode from kernel config option [1:4] */ + cppc_state =CONFIG_X86_AMD_PSTATE_DEFAULT_MODE ;If someone booted the system with "amd_pstate=disable", the above will overwrite that preference, no ?
Command line will override kernel config anyway. amd_pstate=disable will work or disable from Kconfig.
Quoted 6 lines
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void)
Quoted 6 lines
drivers/cpufreq/amd-pstate.c +4
-0+ } + +switch (cppc_state) {case AMD_PSTATE_DISABLE : + pr_info("driver load is disabled, boot with specific mode to
Quoted 6 lines
enable
drivers/cpufreq/amd-pstate.c static int __init amd_pstate_init(void) · line 1869
Quoted 2 lines
drivers/cpufreq/amd-pstate.c +1
-0+this\n"); return -ENODEV ;....and this "case" statement will never be reachable in the aforementioned case.
It will be called when kernel config set "disable" state from kernel booting.
Quoted 22 lines
Quoted 22 lines
drivers/cpufreq/amd-pstate.c +4
-2case AMD_PSTATE_PASSIVE :case AMD_PSTATE_ACTIVE :case AMD_PSTATE_GUIDED : + ret =amd_pstate_set_driver (cppc_state); +if (ret) +return ret;break ;default :return -EINVAL ;@@ -1883,7 +1892,7 @@ static int __init amd_pstate_init(void)/* enable amd pstate feature */ ret =amd_pstate_enable (true);if (ret) { -pr_err ("failed to enable with return %d\n" , ret); +pr_err ("failed to enable driver mode(%d)\n" , cppc_state);return ret; } --2.34.1