LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements
@ 2024-05-13  2:07 Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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 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                | 209 +++++++++++++-------
 include/linux/amd-pstate.h                  |   8 +
 7 files changed, 181 insertions(+), 74 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-14  4:50   ` Dhananjay Ugwekar
  2024-05-28  5:29   ` Gautham R. Shenoy
  2024-05-13  2:07 ` [PATCH v2 02/10] cpufreq: amd-pstate: remove unused variable nominal_freq Perry Yuan
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 6a342b0c0140..614f6fac0764 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -889,6 +889,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);
+		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;
 }
 
@@ -927,15 +945,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);
 
@@ -1388,14 +1397,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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 02/10] cpufreq: amd-pstate: remove unused  variable nominal_freq
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-17  9:03   ` Gautham R. Shenoy
  2024-05-13  2:07 ` [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported Perry Yuan
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

Closes: https://lore.kernel.org/oe-kbuild-all/202405080431.BPU6Yg9s-lkp@intel.com/
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 614f6fac0764..9ca22e94f3f3 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -912,7 +912,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;
 
@@ -943,7 +943,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);
@@ -1363,7 +1362,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;
@@ -1396,7 +1395,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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 02/10] cpufreq: amd-pstate: remove unused variable nominal_freq Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-06-06 14:26   ` Gautham R. Shenoy
  2024-05-13  2:07 ` [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Perry Yuan
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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.

Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Closes: https://lore.kernel.org/linux-pm/CYYPR12MB8655D32EA18574C9497E888A9C122@CYYPR12MB8655.namprd12.prod.outlook.com/T/#t
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 9ca22e94f3f3..3109b46fb02e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1701,6 +1701,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;
@@ -1709,6 +1723,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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (2 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13 17:21   ` kernel test robot
  2024-05-13  2:07 ` [PATCH v2 05/10] Documentation: PM: amd-pstate: add debugging section for driver loading failure Perry Yuan
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218686
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 3109b46fb02e..8b624c79c287 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1707,12 +1707,37 @@ 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 flag is disabled in the BIOS for processors that support MSR-based CPPC
+	 * the AMD Pstate driver may not function correctly.
+	 */
+	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 > 0x00 && c->x86_model < 0x0F) || (c->x86_model > 0x2f && c->x86_model < 0xaf) ||
+				(c->x86_model > 0x1f && c->x86_model < 0x1f)) {
+				goto warn;
+			}
+		} else {
+			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 supports the CPPC option.\n");
+	return false;
 }
 
 static int __init amd_pstate_init(void)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 05/10] Documentation: PM: amd-pstate: add debugging section for driver loading failure
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (3 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 06/10] Documentation: PM: amd-pstate: add guided mode to the Operation mode Perry Yuan
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 1e0d101b020a..de83e742738e 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -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
 --------------
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 06/10] Documentation: PM: amd-pstate: add guided mode to the Operation mode
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (4 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 05/10] Documentation: PM: amd-pstate: add debugging section for driver loading failure Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 07/10] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() Perry Yuan
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

the guided mode is also supported, so the operation mode should include
that mode as well.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index de83e742738e..7eb35ad21c7d 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -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``
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 07/10] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled()
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (5 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 06/10] Documentation: PM: amd-pstate: add guided mode to the Operation mode Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor Perry Yuan
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

Acked-by: Mario Limonciello <mario.limonciello@amd.com>
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8b624c79c287..8b8cae1b678d 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -125,7 +125,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;
@@ -167,7 +167,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);
@@ -220,7 +220,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);
@@ -721,7 +721,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);
@@ -956,7 +956,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],
@@ -1189,7 +1189,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) {
@@ -1418,7 +1418,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;
@@ -1501,7 +1501,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;
 	}
@@ -1540,7 +1540,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;
@@ -1574,7 +1574,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 */
@@ -1776,7 +1776,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;
 		}
@@ -1795,7 +1795,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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (6 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 07/10] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default Perry Yuan
  9 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

Link: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
PDF p274
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/scattered.c    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 3c7434329661..39a92338c015 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -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)
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index af5aa2c754c2..9e237a3daf7e 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -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 }
 };
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (7 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13  2:07 ` [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default Perry Yuan
  9 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

Link: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
PDF p274
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 arch/x86/include/asm/processor.h |  2 +
 arch/x86/kernel/cpu/amd.c        | 19 +++++++++
 drivers/cpufreq/amd-pstate.c     | 66 ++++++++++++++++++++------------
 include/linux/amd-pstate.h       |  8 ++++
 4 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 78e51b0d6433..04d30dd35d0a 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -682,10 +682,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);
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 307302af0aee..67966bdcde65 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1219,3 +1219,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);
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8b8cae1b678d..dce901a403c9 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -50,8 +50,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
 
 /*
  * TODO: We need more time to fine tune processors with shared memory solution
@@ -327,19 +328,47 @@ 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,
-	 * the highest performance level is set to 196.
-	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
-	 */
-	if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= 0x7f))
-		return CPPC_HIGHEST_PERF_PERFORMANCE;
+	core_type = amd_pstate_get_cpu_type(cpudata->cpu);
+	pr_debug("core_type %d found\n", core_type);
 
-	return CPPC_HIGHEST_PERF_DEFAULT;
+	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)
@@ -352,15 +381,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);
@@ -381,10 +402,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);
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index d58fc022ec46..869d916003f1 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -134,4 +134,12 @@ 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,
+};
+
 #endif /* _LINUX_AMD_PSTATE_H */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
  2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
                   ` (8 preceding siblings ...)
  2024-05-13  2:07 ` [PATCH v2 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization Perry Yuan
@ 2024-05-13  2:07 ` Perry Yuan
  2024-05-13  9:36   ` Dhananjay Ugwekar
  9 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2024-05-13  2:07 UTC (permalink / raw
  To: Mario.Limonciello, gautham.shenoy, Ray.Huang, Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

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

[    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

Reported-by: Andrei Amuraritei <andamu@posteo.net>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index dce901a403c9..03342fef7d94 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1785,28 +1785,30 @@ 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 */
+	if (cppc_state == AMD_PSTATE_UNDEFINED)
+		cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE;
+
+	/* Disable on the following configs by default:
+	 * 1. Undefined platforms
+	 * 2. Server platforms
+	 */
+	if (amd_pstate_acpi_pm_profile_undefined() ||
+		amd_pstate_acpi_pm_profile_server()) {
+		pr_info("driver load is disabled for server or undefined platform\n");
+		return -ENODEV;
+	}
+
 	switch (cppc_state) {
-	case AMD_PSTATE_UNDEFINED:
-		/* 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)) {
-			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;
 	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;
@@ -1827,7 +1829,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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
  2024-05-13  2:07 ` [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default Perry Yuan
@ 2024-05-13  9:36   ` Dhananjay Ugwekar
  2024-05-14  4:54     ` Yuan, Perry
  0 siblings, 1 reply; 18+ messages in thread
From: Dhananjay Ugwekar @ 2024-05-13  9:36 UTC (permalink / raw
  To: Perry Yuan, Mario.Limonciello, gautham.shenoy, Ray.Huang,
	Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

Hello Perry,

On 5/13/2024 7:37 AM, Perry Yuan wrote:
> If 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.
> 
> [    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
> 
> Reported-by: Andrei Amuraritei <andamu@posteo.net>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index dce901a403c9..03342fef7d94 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1785,28 +1785,30 @@ 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 */
> +	if (cppc_state == AMD_PSTATE_UNDEFINED)
> +		cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE;
> +
> +	/* Disable on the following configs by default:
> +	 * 1. Undefined platforms
> +	 * 2. Server platforms
> +	 */
> +	if (amd_pstate_acpi_pm_profile_undefined() ||
> +		amd_pstate_acpi_pm_profile_server()) {
> +		pr_info("driver load is disabled for server or undefined platform\n");
> +		return -ENODEV;
> +	}
> +

Wont this change make it impossible to use amd-pstate on server platforms? It wont work
even if we pass "amd-pstate=active" in cmdline, because this check is before the switch
case.

Thanks,
Dhananjay

>  	switch (cppc_state) {
> -	case AMD_PSTATE_UNDEFINED:
> -		/* 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)) {
> -			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;
>  	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;
> @@ -1827,7 +1829,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;
>  	}
>  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
  2024-05-13  2:07 ` [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Perry Yuan
@ 2024-05-13 17:21   ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2024-05-13 17:21 UTC (permalink / raw
  To: Perry Yuan, Mario.Limonciello, gautham.shenoy, Ray.Huang,
	Borislav.Petkov
  Cc: llvm, oe-kbuild-all, rafael.j.wysocki, Alexander.Deucher,
	Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel

Hi Perry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge next-20240513]
[cannot apply to tip/x86/core linus/master v6.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Perry-Yuan/cpufreq-amd-pstate-optimize-the-initial-frequency-values-verification/20240513-101217
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/94ad4bca2d8e44a5e53082959220a184c0222e1e.1715356532.git.perry.yuan%40amd.com
patch subject: [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
config: i386-randconfig-013-20240513 (https://download.01.org/0day-ci/archive/20240514/202405140103.onvXHkqE-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240514/202405140103.onvXHkqE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405140103.onvXHkqE-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/cpufreq/amd-pstate.c:1727:26: warning: overlapping comparisons always evaluate to false [-Wtautological-overlap-compare]
    1727 |                                 (c->x86_model > 0x1f && c->x86_model < 0x1f)) {
         |                                  ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +1727 drivers/cpufreq/amd-pstate.c

  1703	
  1704	/**
  1705	 * CPPC function is not supported for family ID 17H with model_ID ranging from 0x10 to 0x2F.
  1706	 * show the debug message that helps to check if the CPU has CPPC support for loading issue.
  1707	 */
  1708	static bool amd_cppc_supported(void)
  1709	{
  1710		struct cpuinfo_x86 *c = &cpu_data(0);
  1711	
  1712		if ((boot_cpu_data.x86 == 0x17) && (boot_cpu_data.x86_model < 0x30)) {
  1713			pr_debug_once("CPPC feature is not supported by the processor\n");
  1714			return false;
  1715		}
  1716	
  1717		/*
  1718		 * If the CPPC flag is disabled in the BIOS for processors that support MSR-based CPPC
  1719		 * the AMD Pstate driver may not function correctly.
  1720		 */
  1721		if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
  1722			if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) {
  1723				if (c->x86_model > 0x60 && c->x86_model < 0xaf)
  1724					goto warn;
  1725			} else if (cpu_feature_enabled(X86_FEATURE_ZEN3) || cpu_feature_enabled(X86_FEATURE_ZEN4)) {
  1726				if ((c->x86_model > 0x00 && c->x86_model < 0x0F) || (c->x86_model > 0x2f && c->x86_model < 0xaf) ||
> 1727					(c->x86_model > 0x1f && c->x86_model < 0x1f)) {
  1728					goto warn;
  1729				}
  1730			} else {
  1731				goto warn;
  1732			}
  1733		}
  1734	
  1735		return true;
  1736	
  1737	warn:
  1738		pr_debug_once("The CPPC feature is supported but currently disabled by the BIOS.\n"
  1739						"Please enable it if your BIOS supports the CPPC option.\n");
  1740		return false;
  1741	}
  1742	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification
  2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
@ 2024-05-14  4:50   ` Dhananjay Ugwekar
  2024-05-28  5:29   ` Gautham R. Shenoy
  1 sibling, 0 replies; 18+ messages in thread
From: Dhananjay Ugwekar @ 2024-05-14  4:50 UTC (permalink / raw
  To: Perry Yuan, Mario.Limonciello, gautham.shenoy, Ray.Huang,
	Borislav.Petkov
  Cc: rafael.j.wysocki, Alexander.Deucher, Xinmei.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel

Hello Perry,

On 5/13/2024 7:37 AM, Perry Yuan wrote:
> 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
> 
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 6a342b0c0140..614f6fac0764 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -889,6 +889,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);
> +		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);

A reminder, we should fix the below code section (due to only nominal freq being in MHz),

685 static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)                                                        
686 {                                                                                                                                
687         struct amd_cpudata *cpudata = policy->driver_data;                                                                       
688         int ret;                                                                                                                 
689                                                                                                                                  
690         if (!cpudata->boost_supported) {                                                                                         
691                 pr_err("Boost mode is not supported by this processor or SBIOS\n");                                              
692                 return -EINVAL;                                                                                                  
693         }                                                                                                                        
694                                                                                                                                  
695         if (state)                                                                                                               
696                 policy->cpuinfo.max_freq = cpudata->max_freq;                                                                    
697         else                                                                                                                     
698                 policy->cpuinfo.max_freq = cpudata->nominal_freq;      <--- mismatch in left and right hand side units  


To avoid below situation(from a Zen4 AMD EPYC system with boost disabled),

/sys/devices/system/cpu/cpu0/cpufreq# cat scaling_max_freq                                                             
2151                                                             	<--- MHz                                                                    
/sys/devices/system/cpu/cpu0/cpufreq# cat amd_pstate_max_freq                                                          
2287000									<--- KHz

Thanks,
Dhananjay


> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -927,15 +945,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);
>  
> @@ -1388,14 +1397,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;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
  2024-05-13  9:36   ` Dhananjay Ugwekar
@ 2024-05-14  4:54     ` Yuan, Perry
  0 siblings, 0 replies; 18+ messages in thread
From: Yuan, Perry @ 2024-05-14  4:54 UTC (permalink / raw
  To: Ugwekar, Dhananjay, Limonciello, Mario, Shenoy, Gautham Ranjal,
	Huang, Ray, Petkov, Borislav
  Cc: rafael.j.wysocki@intel.com, Deucher, Alexander, Huang, Shimmer,
	Du, Xiaojian, Meng, Li (Jassmine), linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Dhananjay

> -----Original Message-----
> From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@amd.com>
> Sent: Monday, May 13, 2024 5:37 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Huang, Ray <Ray.Huang@amd.com>; Petkov,
> Borislav <Borislav.Petkov@amd.com>
> Cc: rafael.j.wysocki@intel.com; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate
> driver by default
>
> Hello Perry,
>
> On 5/13/2024 7:37 AM, Perry Yuan wrote:
> > If 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.
> >
> > [    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
> >
> > Reported-by: Andrei Amuraritei <andamu@posteo.net>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705
> > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 36
> > +++++++++++++++++++-----------------
> >  1 file changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index dce901a403c9..03342fef7d94
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -1785,28 +1785,30 @@ 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 */
> > +   if (cppc_state == AMD_PSTATE_UNDEFINED)
> > +           cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE;
> > +
> > +   /* Disable on the following configs by default:
> > +    * 1. Undefined platforms
> > +    * 2. Server platforms
> > +    */
> > +   if (amd_pstate_acpi_pm_profile_undefined() ||
> > +           amd_pstate_acpi_pm_profile_server()) {
> > +           pr_info("driver load is disabled for server or undefined
> platform\n");
> > +           return -ENODEV;
> > +   }
> > +
>
> Wont this change make it impossible to use amd-pstate on server platforms? It
> wont work even if we pass "amd-pstate=active" in cmdline, because this check
> is before the switch case.
>
> Thanks,
> Dhananjay
>

Good catch,  the original checking is disabling server platform by default, it should not block the server platform when using "amd-pstate=active" in cmdline,  will fix it in v3 to allow parameter input for server platfrm from command line.


Regards.
Perry

> >     switch (cppc_state) {
> > -   case AMD_PSTATE_UNDEFINED:
> > -           /* 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)) {
> > -                   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;
> >     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;
> > @@ -1827,7 +1829,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;
> >     }
> >


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 02/10] cpufreq: amd-pstate: remove unused  variable nominal_freq
  2024-05-13  2:07 ` [PATCH v2 02/10] cpufreq: amd-pstate: remove unused variable nominal_freq Perry Yuan
@ 2024-05-17  9:03   ` Gautham R. Shenoy
  0 siblings, 0 replies; 18+ messages in thread
From: Gautham R. Shenoy @ 2024-05-17  9:03 UTC (permalink / raw
  To: Perry Yuan
  Cc: Mario.Limonciello, Ray.Huang, Borislav.Petkov, rafael.j.wysocki,
	Alexander.Deucher, Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm,
	linux-kernel

On Mon, May 13, 2024 at 10:07:20AM +0800, Perry Yuan wrote:
> removed 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.
> 
> Closes: https://lore.kernel.org/oe-kbuild-all/202405080431.BPU6Yg9s-lkp@intel.com/
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

> ---
>  drivers/cpufreq/amd-pstate.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 614f6fac0764..9ca22e94f3f3 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -912,7 +912,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;
>  
> @@ -943,7 +943,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);
> @@ -1363,7 +1362,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;
> @@ -1396,7 +1395,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;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification
  2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
  2024-05-14  4:50   ` Dhananjay Ugwekar
@ 2024-05-28  5:29   ` Gautham R. Shenoy
  1 sibling, 0 replies; 18+ messages in thread
From: Gautham R. Shenoy @ 2024-05-28  5:29 UTC (permalink / raw
  To: Perry Yuan
  Cc: Mario.Limonciello, Ray.Huang, Borislav.Petkov, rafael.j.wysocki,
	Alexander.Deucher, Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm,
	linux-kernel

On Mon, May 13, 2024 at 10:07:19AM +0800, Perry Yuan wrote:
> 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
> 
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>

Looks good to me.

Acked-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

> ---
>  drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 6a342b0c0140..614f6fac0764 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -889,6 +889,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);
> +		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;
>  }
>  
> @@ -927,15 +945,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);
>  
> @@ -1388,14 +1397,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;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported
  2024-05-13  2:07 ` [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported Perry Yuan
@ 2024-06-06 14:26   ` Gautham R. Shenoy
  0 siblings, 0 replies; 18+ messages in thread
From: Gautham R. Shenoy @ 2024-06-06 14:26 UTC (permalink / raw
  To: Perry Yuan
  Cc: Mario.Limonciello, Ray.Huang, Borislav.Petkov, rafael.j.wysocki,
	Alexander.Deucher, Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm,
	linux-kernel

On Mon, May 13, 2024 at 10:07:21AM +0800, Perry Yuan wrote:
> 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.
> 
> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Closes: https://lore.kernel.org/linux-pm/CYYPR12MB8655D32EA18574C9497E888A9C122@CYYPR12MB8655.namprd12.prod.outlook.com/T/#t
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

> ---
>  drivers/cpufreq/amd-pstate.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9ca22e94f3f3..3109b46fb02e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1701,6 +1701,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;
> @@ -1709,6 +1723,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;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-06-06 14:26 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
2024-05-14  4:50   ` Dhananjay Ugwekar
2024-05-28  5:29   ` Gautham R. Shenoy
2024-05-13  2:07 ` [PATCH v2 02/10] cpufreq: amd-pstate: remove unused variable nominal_freq Perry Yuan
2024-05-17  9:03   ` Gautham R. Shenoy
2024-05-13  2:07 ` [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported Perry Yuan
2024-06-06 14:26   ` Gautham R. Shenoy
2024-05-13  2:07 ` [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Perry Yuan
2024-05-13 17:21   ` kernel test robot
2024-05-13  2:07 ` [PATCH v2 05/10] Documentation: PM: amd-pstate: add debugging section for driver loading failure Perry Yuan
2024-05-13  2:07 ` [PATCH v2 06/10] Documentation: PM: amd-pstate: add guided mode to the Operation mode Perry Yuan
2024-05-13  2:07 ` [PATCH v2 07/10] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() Perry Yuan
2024-05-13  2:07 ` [PATCH v2 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor Perry Yuan
2024-05-13  2:07 ` [PATCH v2 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization Perry Yuan
2024-05-13  2:07 ` [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default Perry Yuan
2024-05-13  9:36   ` Dhananjay Ugwekar
2024-05-14  4:54     ` Yuan, Perry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).