diff mbox series

[3/5] cpufreq: amd-pstate: add amd-pstate driver parameter for mode selection

Message ID 20221117024955.3319484-4-Perry.Yuan@amd.com
State New
Headers
Series AMD Pstate driver Urgent Change |

Commit Message

Yuan, Perry Nov. 17, 2022, 2:49 a.m. UTC 
  When the amd_pstate driver is built-in users still need a method to be
able enable or disable it depending upon their circumstance.
Add support for an early parameter to do this.

There is some performance degradation on a number of ASICs in the
passive mode. This performance issue was originally discovered in
shared memory systems but it has been proven that certain workloads
on MSR systems also suffer performance issues.
Set the amd-pstate driver as disabled by default to temporarily
mitigate the performance problem.

1) with `amd_pstate=disable`, pstate driver will be disabled to load at
kernel booting.

2) with `amd_pstate=passive`, pstate driver will be enabled and loaded as
non-autonomous working mode supported in the low-level power management
firmware.

3) If neither parameter is specified, the driver will be disabled by
default to avoid triggering performance regressions in certain ASICs

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

Comments

Gautham R. Shenoy Nov. 17, 2022, 4:13 a.m. UTC | #1 
On Thu, Nov 17, 2022 at 10:49:53AM +0800, Perry Yuan wrote:
> When the amd_pstate driver is built-in users still need a method to be
> able enable or disable it depending upon their circumstance.
> Add support for an early parameter to do this.
> 
> There is some performance degradation on a number of ASICs in the
> passive mode. This performance issue was originally discovered in
> shared memory systems but it has been proven that certain workloads
> on MSR systems also suffer performance issues.
> Set the amd-pstate driver as disabled by default to temporarily
> mitigate the performance problem.
> 
> 1) with `amd_pstate=disable`, pstate driver will be disabled to load at
> kernel booting.
> 
> 2) with `amd_pstate=passive`, pstate driver will be enabled and loaded as
> non-autonomous working mode supported in the low-level power management
> firmware.
> 
> 3) If neither parameter is specified, the driver will be disabled by
> default to avoid triggering performance regressions in certain ASICs
> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>

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

> ---
>  drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 701f49d6d240..204e39006dda 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -59,12 +59,8 @@
>   * 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.
>   */
> -static bool shared_mem = false;
> -module_param(shared_mem, bool, 0444);
> -MODULE_PARM_DESC(shared_mem,
> -		 "enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
> -
>  static struct cpufreq_driver amd_pstate_driver;
> +static int cppc_load __initdata;
>  
>  static inline int pstate_enable(bool enable)
>  {
> @@ -626,6 +622,15 @@ static int __init amd_pstate_init(void)
>  
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
>  		return -ENODEV;
> +	/*
> +	 * by default the pstate driver is disabled to load
> +	 * enable the amd_pstate passive mode driver explicitly
> +	 * with amd_pstate=passive in kernel command line
> +	 */
> +	if (!cppc_load) {
> +		pr_debug("driver load is disabled, boot with amd_pstate=passive to enable this\n");
> +		return -ENODEV;
> +	}
>  
>  	if (!acpi_cpc_valid()) {
>  		pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
> @@ -640,13 +645,11 @@ static int __init amd_pstate_init(void)
>  	if (boot_cpu_has(X86_FEATURE_CPPC)) {
>  		pr_debug("AMD CPPC MSR based functionality is supported\n");
>  		amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> -	} else if (shared_mem) {
> +	} else {
> +		pr_debug("AMD CPPC shared memory based functionality is supported\n");
>  		static_call_update(amd_pstate_enable, cppc_enable);
>  		static_call_update(amd_pstate_init_perf, cppc_init_perf);
>  		static_call_update(amd_pstate_update_perf, cppc_update_perf);
> -	} else {
> -		pr_info("This processor supports shared memory solution, you can enable it with amd_pstate.shared_mem=1\n");
> -		return -ENODEV;
>  	}
>  
>  	/* enable amd pstate feature */
> @@ -665,6 +668,21 @@ static int __init amd_pstate_init(void)
>  }
>  device_initcall(amd_pstate_init);
>  
> +static int __init amd_pstate_param(char *str)
> +{
> +	if (!str)
> +		return -EINVAL;
> +
> +	if (!strcmp(str, "disable")) {
> +		cppc_load = 0;
> +		pr_info("driver is explicitly disabled\n");
> +	} else if (!strcmp(str, "passive"))
> +		cppc_load = 1;
> +
> +	return 0;
> +}
> +early_param("amd_pstate", amd_pstate_param);
> +
>  MODULE_AUTHOR("Huang Rui <ray.huang@amd.com>");
>  MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
>  MODULE_LICENSE("GPL");
> -- 
> 2.25.1
>
Wyes Karny Nov. 17, 2022, 5:06 a.m. UTC | #2 
On 11/17/2022 8:19 AM, Perry Yuan wrote:
> When the amd_pstate driver is built-in users still need a method to be
> able enable or disable it depending upon their circumstance.
> Add support for an early parameter to do this.
> 
> There is some performance degradation on a number of ASICs in the
> passive mode. This performance issue was originally discovered in
> shared memory systems but it has been proven that certain workloads
> on MSR systems also suffer performance issues.
> Set the amd-pstate driver as disabled by default to temporarily
> mitigate the performance problem.
> 
> 1) with `amd_pstate=disable`, pstate driver will be disabled to load at
> kernel booting.
> 
> 2) with `amd_pstate=passive`, pstate driver will be enabled and loaded as
> non-autonomous working mode supported in the low-level power management
> firmware.
> 
> 3) If neither parameter is specified, the driver will be disabled by
> default to avoid triggering performance regressions in certain ASICs
> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>

Tested-by: Wyes Karny <wyes.karny@amd.com>

> ---
>  drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 701f49d6d240..204e39006dda 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -59,12 +59,8 @@
>   * 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.
>   */
> -static bool shared_mem = false;
> -module_param(shared_mem, bool, 0444);
> -MODULE_PARM_DESC(shared_mem,
> -		 "enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
> -
>  static struct cpufreq_driver amd_pstate_driver;
> +static int cppc_load __initdata;
>  
>  static inline int pstate_enable(bool enable)
>  {
> @@ -626,6 +622,15 @@ static int __init amd_pstate_init(void)
>  
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
>  		return -ENODEV;
> +	/*
> +	 * by default the pstate driver is disabled to load
> +	 * enable the amd_pstate passive mode driver explicitly
> +	 * with amd_pstate=passive in kernel command line
> +	 */
> +	if (!cppc_load) {
> +		pr_debug("driver load is disabled, boot with amd_pstate=passive to enable this\n");
> +		return -ENODEV;
> +	}
>  
>  	if (!acpi_cpc_valid()) {
>  		pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
> @@ -640,13 +645,11 @@ static int __init amd_pstate_init(void)
>  	if (boot_cpu_has(X86_FEATURE_CPPC)) {
>  		pr_debug("AMD CPPC MSR based functionality is supported\n");
>  		amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> -	} else if (shared_mem) {
> +	} else {
> +		pr_debug("AMD CPPC shared memory based functionality is supported\n");
>  		static_call_update(amd_pstate_enable, cppc_enable);
>  		static_call_update(amd_pstate_init_perf, cppc_init_perf);
>  		static_call_update(amd_pstate_update_perf, cppc_update_perf);
> -	} else {
> -		pr_info("This processor supports shared memory solution, you can enable it with amd_pstate.shared_mem=1\n");
> -		return -ENODEV;
>  	}
>  
>  	/* enable amd pstate feature */
> @@ -665,6 +668,21 @@ static int __init amd_pstate_init(void)
>  }
>  device_initcall(amd_pstate_init);
>  
> +static int __init amd_pstate_param(char *str)
> +{
> +	if (!str)
> +		return -EINVAL;
> +
> +	if (!strcmp(str, "disable")) {
> +		cppc_load = 0;
> +		pr_info("driver is explicitly disabled\n");
> +	} else if (!strcmp(str, "passive"))
> +		cppc_load = 1;
> +
> +	return 0;
> +}
> +early_param("amd_pstate", amd_pstate_param);
> +
>  MODULE_AUTHOR("Huang Rui <ray.huang@amd.com>");
>  MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
>  MODULE_LICENSE("GPL");
diff mbox series

Patch

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 701f49d6d240..204e39006dda 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -59,12 +59,8 @@ 
  * 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.
  */
-static bool shared_mem = false;
-module_param(shared_mem, bool, 0444);
-MODULE_PARM_DESC(shared_mem,
-		 "enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
-
 static struct cpufreq_driver amd_pstate_driver;
+static int cppc_load __initdata;
 
 static inline int pstate_enable(bool enable)
 {
@@ -626,6 +622,15 @@  static int __init amd_pstate_init(void)
 
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return -ENODEV;
+	/*
+	 * by default the pstate driver is disabled to load
+	 * enable the amd_pstate passive mode driver explicitly
+	 * with amd_pstate=passive in kernel command line
+	 */
+	if (!cppc_load) {
+		pr_debug("driver load is disabled, boot with amd_pstate=passive to enable this\n");
+		return -ENODEV;
+	}
 
 	if (!acpi_cpc_valid()) {
 		pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
@@ -640,13 +645,11 @@  static int __init amd_pstate_init(void)
 	if (boot_cpu_has(X86_FEATURE_CPPC)) {
 		pr_debug("AMD CPPC MSR based functionality is supported\n");
 		amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
-	} else if (shared_mem) {
+	} else {
+		pr_debug("AMD CPPC shared memory based functionality is supported\n");
 		static_call_update(amd_pstate_enable, cppc_enable);
 		static_call_update(amd_pstate_init_perf, cppc_init_perf);
 		static_call_update(amd_pstate_update_perf, cppc_update_perf);
-	} else {
-		pr_info("This processor supports shared memory solution, you can enable it with amd_pstate.shared_mem=1\n");
-		return -ENODEV;
 	}
 
 	/* enable amd pstate feature */
@@ -665,6 +668,21 @@  static int __init amd_pstate_init(void)
 }
 device_initcall(amd_pstate_init);
 
+static int __init amd_pstate_param(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "disable")) {
+		cppc_load = 0;
+		pr_info("driver is explicitly disabled\n");
+	} else if (!strcmp(str, "passive"))
+		cppc_load = 1;
+
+	return 0;
+}
+early_param("amd_pstate", amd_pstate_param);
+
 MODULE_AUTHOR("Huang Rui <ray.huang@amd.com>");
 MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
 MODULE_LICENSE("GPL");