[PATCH v3 0/3] Enable amd-pstate active mode by default

[PATCH v3 0/3] Enable amd-pstate active mode by default

[Mario Limonciello]

Active mode for amd-pstate has shown enough success now that it makes sense
to enable it by default on client systems.

This series introduces a new kernel configuration option to set the default
policy for amd-pstate modes for a kernel.

Server systems will by identified by the PM preferred profile and still be
set as disabled by default for now.

v2->v3:
 * Drop patch 4; Intel intentionally doesn't want intel-pstate on SOHO
   server
 * Move symbols from patch 1 into patch 2
 * Add tags
Mario Limonciello (3):
  ACPI: CPPC: Add definition for undefined FADT preferred PM profile
    value
  cpufreq: amd-pstate: Set a fallback policy based on preferred_profile
  cpufreq: amd-pstate: Add a kernel config option to set default mode

 drivers/cpufreq/Kconfig.x86  |  17 ++++++
 drivers/cpufreq/amd-pstate.c | 101 +++++++++++++++++++++++++----------
 include/acpi/actbl.h         |   3 +-
 include/linux/amd-pstate.h   |   4 +-
 4 files changed, 96 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] ACPI: CPPC: Add definition for undefined FADT preferred PM profile value

[Mario Limonciello]

In the event a new preferred PM profile value is introduced it's best for
code to be able to defensively guard against it so that the wrong settings
don't get applied on a new system that uses this profile but ancient
kernels.

Acked-by: Huang Rui <ray.huang@amd.com>
Suggested-by: Gautham Ranjal Shenoy <gautham.shenoy@amd.com>
Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#fixed-acpi-description-table-fadt
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2->v3:
 * Drop new symbols (they fold into patch 2)
 * Update commit message
 * Add Ray's tag
---
 include/acpi/actbl.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/acpi/actbl.h b/include/acpi/actbl.h
index e5dfb6f4de52..451f6276da49 100644
--- a/include/acpi/actbl.h
+++ b/include/acpi/actbl.h
@@ -307,7 +307,8 @@ enum acpi_preferred_pm_profiles {
 	PM_SOHO_SERVER = 5,
 	PM_APPLIANCE_PC = 6,
 	PM_PERFORMANCE_SERVER = 7,
-	PM_TABLET = 8
+	PM_TABLET = 8,
+	NR_PM_PROFILES = 9
 };
 
 /* Values for sleep_status and sleep_control registers (V5+ FADT) */
-- 
2.34.1

RE: [PATCH v3 1/3] ACPI: CPPC: Add definition for undefined FADT preferred PM profile value

[Yuan, Perry]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Wednesday, June 21, 2023 1:25 AM
> To: Rafael J . Wysocki <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>; Huang, Ray <Ray.Huang@amd.com>;
> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> pm@vger.kernel.org; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> Yuan, Perry <Perry.Yuan@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: [PATCH v3 1/3] ACPI: CPPC: Add definition for undefined FADT
> preferred PM profile value
>
> In the event a new preferred PM profile value is introduced it's best for code
> to be able to defensively guard against it so that the wrong settings don't get
> applied on a new system that uses this profile but ancient kernels.
>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Suggested-by: Gautham Ranjal Shenoy <gautham.shenoy@amd.com>
> Link:
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Progr
> amming_Model/ACPI_Software_Programming_Model.html#fixed-acpi-
> description-table-fadt
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2->v3:
>  * Drop new symbols (they fold into patch 2)
>  * Update commit message
>  * Add Ray's tag
> ---
>  include/acpi/actbl.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/acpi/actbl.h b/include/acpi/actbl.h index
> e5dfb6f4de52..451f6276da49 100644
> --- a/include/acpi/actbl.h
> +++ b/include/acpi/actbl.h
> @@ -307,7 +307,8 @@ enum acpi_preferred_pm_profiles {
>       PM_SOHO_SERVER = 5,
>       PM_APPLIANCE_PC = 6,
>       PM_PERFORMANCE_SERVER = 7,
> -     PM_TABLET = 8
> +     PM_TABLET = 8,
> +     NR_PM_PROFILES = 9
>  };
>
>  /* Values for sleep_status and sleep_control registers (V5+ FADT) */
> --
> 2.34.1

LGTM,
Reviewed-by: Perry Yuan <Perry.Yuan@amd.com>

[PATCH v3 2/3] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile

[Mario Limonciello]

If a user's configuration doesn't explicitly specify the cpufreq
scaling governor then the code currently explicitly falls back to
'powersave'. This default is fine for notebooks and desktops, but
servers and undefined machines should default to 'performance'.

Look at the 'preferred_profile' field from the FADT to set this
policy accordingly.

Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#fixed-acpi-description-table-fadt
Acked-by: Huang Rui <ray.huang@amd.com>
Suggested-by: Wyes Karny <Wyes.Karny@amd.com>
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2->v3:
 * Move new symbols out of patch 1 into this patch
 * Add Ray's tag
---
 drivers/cpufreq/amd-pstate.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index d8269994322e..3546d7db614d 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1071,6 +1071,26 @@ static const struct attribute_group amd_pstate_global_attr_group = {
 	.attrs = pstate_global_attributes,
 };
 
+static bool amd_pstate_acpi_pm_profile_server(void)
+{
+	switch (acpi_gbl_FADT.preferred_profile) {
+	case PM_ENTERPRISE_SERVER:
+	case PM_SOHO_SERVER:
+	case PM_PERFORMANCE_SERVER:
+		return true;
+	}
+	return false;
+}
+
+static bool amd_pstate_acpi_pm_profile_undefined(void)
+{
+	if (acpi_gbl_FADT.preferred_profile == PM_UNSPECIFIED)
+		return true;
+	if (acpi_gbl_FADT.preferred_profile >= NR_PM_PROFILES)
+		return true;
+	return false;
+}
+
 static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 {
 	int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
@@ -1128,10 +1148,14 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 	policy->max = policy->cpuinfo.max_freq;
 
 	/*
-	 * Set the policy to powersave to provide a valid fallback value in case
+	 * Set the policy to provide a valid fallback value in case
 	 * the default cpufreq governor is neither powersave nor performance.
 	 */
-	policy->policy = CPUFREQ_POLICY_POWERSAVE;
+	if (amd_pstate_acpi_pm_profile_server() ||
+	    amd_pstate_acpi_pm_profile_undefined())
+		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
+	else
+		policy->policy = CPUFREQ_POLICY_POWERSAVE;
 
 	if (boot_cpu_has(X86_FEATURE_CPPC)) {
 		ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &value);
-- 
2.34.1

RE: [PATCH v3 2/3] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile

[Yuan, Perry]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Wednesday, June 21, 2023 1:25 AM
> To: Rafael J . Wysocki <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>; Huang, Ray <Ray.Huang@amd.com>;
> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> pm@vger.kernel.org; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> Yuan, Perry <Perry.Yuan@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: [PATCH v3 2/3] cpufreq: amd-pstate: Set a fallback policy based on
> preferred_profile
>
> If a user's configuration doesn't explicitly specify the cpufreq scaling governor
> then the code currently explicitly falls back to 'powersave'. This default is fine
> for notebooks and desktops, but servers and undefined machines should
> default to 'performance'.
>
> Look at the 'preferred_profile' field from the FADT to set this policy
> accordingly.
>
> Link:
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Progr
> amming_Model/ACPI_Software_Programming_Model.html#fixed-acpi-
> description-table-fadt
> Acked-by: Huang Rui <ray.huang@amd.com>
> Suggested-by: Wyes Karny <Wyes.Karny@amd.com>
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2->v3:
>  * Move new symbols out of patch 1 into this patch
>  * Add Ray's tag
> ---
>  drivers/cpufreq/amd-pstate.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index d8269994322e..3546d7db614d 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1071,6 +1071,26 @@ static const struct attribute_group
> amd_pstate_global_attr_group = {
>       .attrs = pstate_global_attributes,
>  };
>
> +static bool amd_pstate_acpi_pm_profile_server(void)
> +{
> +     switch (acpi_gbl_FADT.preferred_profile) {
> +     case PM_ENTERPRISE_SERVER:
> +     case PM_SOHO_SERVER:
> +     case PM_PERFORMANCE_SERVER:
> +             return true;
> +     }
> +     return false;
> +}
> +
> +static bool amd_pstate_acpi_pm_profile_undefined(void)
> +{
> +     if (acpi_gbl_FADT.preferred_profile == PM_UNSPECIFIED)
> +             return true;
> +     if (acpi_gbl_FADT.preferred_profile >= NR_PM_PROFILES)
> +             return true;
> +     return false;
> +}
> +
>  static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)  {
>       int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> @@ -1128,10 +1148,14 @@ static int amd_pstate_epp_cpu_init(struct
> cpufreq_policy *policy)
>       policy->max = policy->cpuinfo.max_freq;
>
>       /*
> -      * Set the policy to powersave to provide a valid fallback value in case
> +      * Set the policy to provide a valid fallback value in case
>        * the default cpufreq governor is neither powersave nor
> performance.
>        */
> -     policy->policy = CPUFREQ_POLICY_POWERSAVE;
> +     if (amd_pstate_acpi_pm_profile_server() ||
> +         amd_pstate_acpi_pm_profile_undefined())
> +             policy->policy = CPUFREQ_POLICY_PERFORMANCE;
> +     else
> +             policy->policy = CPUFREQ_POLICY_POWERSAVE;
>
>       if (boot_cpu_has(X86_FEATURE_CPPC)) {
>               ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,
> &value);
> --
> 2.34.1
Reviewed-by: Perry Yuan <Perry.Yuan@amd.com>

[PATCH v3 3/3] cpufreq: amd-pstate: Add a kernel config option to set default mode

[Mario Limonciello]

Users are having more success with amd-pstate since the introduction
of EPP and Guided modes.  To expose the driver to more users by default
introduce a kernel configuration option for setting the default mode.

Users can use an integer to map out which default mode they want to use
in lieu of a kernel command line option.

This will default to EPP, but only if:
1) The CPU supports an MSR.
2) The system profile is identified
3) The system profile is identified as a non-server by the FADT.

Link: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/121
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Co-developed-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2->v3:
 * Use new symbol names from being folded into this patch
 * Add Ray's tag
---
 drivers/cpufreq/Kconfig.x86  | 17 +++++++++
 drivers/cpufreq/amd-pstate.c | 73 ++++++++++++++++++++++++------------
 include/linux/amd-pstate.h   |  4 +-
 3 files changed, 68 insertions(+), 26 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 00476e94db90..438c9e75a04d 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -51,6 +51,23 @@ config X86_AMD_PSTATE
 
 	  If in doubt, say N.
 
+config X86_AMD_PSTATE_DEFAULT_MODE
+	int "AMD Processor P-State default mode"
+	depends on X86_AMD_PSTATE
+	default 3 if X86_AMD_PSTATE
+	range 1 4
+	help
+	  Select the default mode the amd-pstate driver will use on
+	  supported hardware.
+	  The value set has the following meanings:
+		1 -> Disabled
+		2 -> Passive
+		3 -> Active (EPP)
+		4 -> Guided
+
+	  For details, take a look at:
+	  <file:Documentation/admin-guide/pm/amd-pstate.rst>.
+
 config X86_AMD_PSTATE_UT
 	tristate "selftest for AMD Processor P-State driver"
 	depends on X86 && ACPI_PROCESSOR
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 3546d7db614d..81fba0dcbee9 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -62,7 +62,7 @@
 static struct cpufreq_driver *current_pstate_driver;
 static struct cpufreq_driver amd_pstate_driver;
 static struct cpufreq_driver amd_pstate_epp_driver;
-static int cppc_state = AMD_PSTATE_DISABLE;
+static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 
 /*
@@ -1410,6 +1410,25 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
 	.attr		= amd_pstate_epp_attr,
 };
 
+static int __init amd_pstate_set_driver(int mode_idx)
+{
+	if (mode_idx >= AMD_PSTATE_DISABLE && mode_idx < AMD_PSTATE_MAX) {
+		cppc_state = mode_idx;
+		if (cppc_state == AMD_PSTATE_DISABLE)
+			pr_info("driver is explicitly disabled\n");
+
+		if (cppc_state == AMD_PSTATE_ACTIVE)
+			current_pstate_driver = &amd_pstate_epp_driver;
+
+		if (cppc_state == AMD_PSTATE_PASSIVE || cppc_state == AMD_PSTATE_GUIDED)
+			current_pstate_driver = &amd_pstate_driver;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int __init amd_pstate_init(void)
 {
 	struct device *dev_root;
@@ -1417,15 +1436,6 @@ 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 or other modes in kernel command line
-	 */
-	if (cppc_state == AMD_PSTATE_DISABLE) {
-		pr_info("driver load is disabled, boot with specific mode 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");
@@ -1436,6 +1446,33 @@ static int __init amd_pstate_init(void)
 	if (cpufreq_get_current_driver())
 		return -EEXIST;
 
+	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() ||
+		    !boot_cpu_has(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:
+		return -ENODEV;
+	case AMD_PSTATE_PASSIVE:
+	case AMD_PSTATE_ACTIVE:
+	case AMD_PSTATE_GUIDED:
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	/* capability check */
 	if (boot_cpu_has(X86_FEATURE_CPPC)) {
 		pr_debug("AMD CPPC MSR based functionality is supported\n");
@@ -1488,21 +1525,7 @@ static int __init amd_pstate_param(char *str)
 	size = strlen(str);
 	mode_idx = get_mode_idx_from_str(str, size);
 
-	if (mode_idx >= AMD_PSTATE_DISABLE && mode_idx < AMD_PSTATE_MAX) {
-		cppc_state = mode_idx;
-		if (cppc_state == AMD_PSTATE_DISABLE)
-			pr_info("driver is explicitly disabled\n");
-
-		if (cppc_state == AMD_PSTATE_ACTIVE)
-			current_pstate_driver = &amd_pstate_epp_driver;
-
-		if (cppc_state == AMD_PSTATE_PASSIVE || cppc_state == AMD_PSTATE_GUIDED)
-			current_pstate_driver = &amd_pstate_driver;
-
-		return 0;
-	}
-
-	return -EINVAL;
+	return amd_pstate_set_driver(mode_idx);
 }
 early_param("amd_pstate", amd_pstate_param);
 
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index c10ebf8c42e6..446394f84606 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -94,7 +94,8 @@ struct amd_cpudata {
  * enum amd_pstate_mode - driver working mode of amd pstate
  */
 enum amd_pstate_mode {
-	AMD_PSTATE_DISABLE = 0,
+	AMD_PSTATE_UNDEFINED = 0,
+	AMD_PSTATE_DISABLE,
 	AMD_PSTATE_PASSIVE,
 	AMD_PSTATE_ACTIVE,
 	AMD_PSTATE_GUIDED,
@@ -102,6 +103,7 @@ enum amd_pstate_mode {
 };
 
 static const char * const amd_pstate_mode_string[] = {
+	[AMD_PSTATE_UNDEFINED]   = "undefined",
 	[AMD_PSTATE_DISABLE]     = "disable",
 	[AMD_PSTATE_PASSIVE]     = "passive",
 	[AMD_PSTATE_ACTIVE]      = "active",
-- 
2.34.1

Re: [PATCH v3 0/3] Enable amd-pstate active mode by default

[Rafael J. Wysocki]

On Tue, Jun 20, 2023 at 7:27 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Active mode for amd-pstate has shown enough success now that it makes sense
> to enable it by default on client systems.
>
> This series introduces a new kernel configuration option to set the default
> policy for amd-pstate modes for a kernel.
>
> Server systems will by identified by the PM preferred profile and still be
> set as disabled by default for now.
>
> v2->v3:
>  * Drop patch 4; Intel intentionally doesn't want intel-pstate on SOHO
>    server
>  * Move symbols from patch 1 into patch 2
>  * Add tags
> Mario Limonciello (3):
>   ACPI: CPPC: Add definition for undefined FADT preferred PM profile
>     value
>   cpufreq: amd-pstate: Set a fallback policy based on preferred_profile
>   cpufreq: amd-pstate: Add a kernel config option to set default mode
>
>  drivers/cpufreq/Kconfig.x86  |  17 ++++++
>  drivers/cpufreq/amd-pstate.c | 101 +++++++++++++++++++++++++----------
>  include/acpi/actbl.h         |   3 +-
>  include/linux/amd-pstate.h   |   4 +-
>  4 files changed, 96 insertions(+), 29 deletions(-)
>
> --

All applied as 6.5 material, thanks!