diff mbox series

[v5,8/8] cpufreq: amd-pstate: auto-load pstate driver by default

Message ID 83301c4cea4f92fb19e14b23f2bac7facfd8bdbb.1718811234.git.perry.yuan@amd.com (mailing list archive)
State Mainlined, archived
Headers show
Series AMD Pstate Driver Fixes and Improvements | expand

Commit Message

Yuan, Perry June 19, 2024, 3:40 p.m. UTC
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.

When there is no parameter added to command line, Kernel config will
provide the default mode to load.

Meanwhile, user can add driver mode in command line which will override
the kernel config default option.

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>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

Comments

Gautham R. Shenoy June 21, 2024, 8:41 a.m. UTC | #1
Perry Yuan <perry.yuan@amd.com> writes:

> 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.
>
> When there is no parameter added to command line, Kernel config will
> provide the default mode to load.
>
> Meanwhile, user can add driver mode in command line which will override
> the kernel config default option.
>
> 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>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

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

> ---
>  drivers/cpufreq/amd-pstate.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 0f8ffbc0dc2a..a96ad7d10a4d 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1801,8 +1801,13 @@ 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:
> +	/*
> +	* determine the driver mode from the command line or kernel config.
> +	* If no command line input is provided, cppc_state will be AMD_PSTATE_UNDEFINED.
> +	* command line options will override the kernel config settings.
> +	*/
> +
> +	if (cppc_state == AMD_PSTATE_UNDEFINED) {
>  		/* Disable on the following configs by default:
>  		 * 1. Undefined platforms
>  		 * 2. Server platforms
> @@ -1812,15 +1817,20 @@ static int __init amd_pstate_init(void)
>  			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;
> @@ -1841,7 +1851,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
diff mbox series

Patch

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 0f8ffbc0dc2a..a96ad7d10a4d 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1801,8 +1801,13 @@  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:
+	/*
+	* determine the driver mode from the command line or kernel config.
+	* If no command line input is provided, cppc_state will be AMD_PSTATE_UNDEFINED.
+	* command line options will override the kernel config settings.
+	*/
+
+	if (cppc_state == AMD_PSTATE_UNDEFINED) {
 		/* Disable on the following configs by default:
 		 * 1. Undefined platforms
 		 * 2. Server platforms
@@ -1812,15 +1817,20 @@  static int __init amd_pstate_init(void)
 			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;
@@ -1841,7 +1851,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;
 	}