[PATCH 0/2] amd-pstate: Make amd-pstate the default driver on server platforms

[PATCH 0/2] amd-pstate: Make amd-pstate the default driver on server platforms

[Gautham R. Shenoy]

Hello,

This patchset contains two patches to

* Prevent frequency throttling on power-limited systems with
  amd-pstate active mode with performance governor.

* Make amd_pstate default on EPYC Family 1A+. Based on tests, the
  amd-pstate driver performs well enough on EPYC.

These patches are based on the "linux-next" branch of
https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git

Gautham R. Shenoy (1):
  amd-pstate: Set min_perf to nominal_perf for active mode performance
    gov

Swapnil Sapkal (1):
  amd-pstate: Switch to amd-pstate by default on some Server platforms

 drivers/cpufreq/amd-pstate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/2] amd-pstate: Set min_perf to nominal_perf for active mode performance gov

[Gautham R. Shenoy]

The amd-pstate driver sets CPPC_REQ.min_perf to CPPC_REQ.max_perf when
in active mode with performance governor. Typically CPPC_REQ.max_perf
is set to CPPC.highest_perf. This causes frequency throttling on
power-limited platforms which causes performance regressions on
certain classes of workloads.

Hence, set the CPPC_REQ.min_perf to the CPPC.nominal_perf or
CPPC_REQ.max_perf, whichever is lower of the two.

Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 6f6d961879cc..23349f50c83e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1535,7 +1535,7 @@ static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy)
 	value = READ_ONCE(cpudata->cppc_req_cached);
 
 	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
-		min_perf = max_perf;
+		min_perf = min(cpudata->nominal_perf, max_perf);
 
 	/* Initial min/max values for CPPC Performance Controls Register */
 	value &= ~AMD_CPPC_MIN_PERF(~0L);
-- 
2.34.1

[PATCH 2/2] amd-pstate: Switch to amd-pstate by default on some Server platforms

[Gautham R. Shenoy]

From: Swapnil Sapkal <swapnil.sapkal@amd.com>

Currently the default cpufreq driver for all the AMD EPYC servers is
acpi-cpufreq. Going forward, switch to amd-pstate as the default
driver on the AMD EPYC server platforms with CPU family 0x1A or
higher. The default mode will be active mode.

Testing shows that amd-pstate with active mode and performance
governor provides comparable or better performance per-watt against
acpi-cpufreq + performance governor.

Likewise, amd-pstate with active mode and powersave governor with the
energy_performance_preference=power (EPP=255) provides comparable or
better performance per-watt against acpi-cpufreq + schedutil governor
for a wide range of workloads.

Users can still revert to using acpi-cpufreq driver on these platforms
with the "amd_pstate=disable" kernel commandline parameter.

Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 23349f50c83e..3c14962adeb2 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1832,10 +1832,10 @@ static int __init amd_pstate_init(void)
 	if (cppc_state == AMD_PSTATE_UNDEFINED) {
 		/* Disable on the following configs by default:
 		 * 1. Undefined platforms
-		 * 2. Server platforms
+		 * 2. Server platforms with CPUs older than Family 0x1A.
 		 */
 		if (amd_pstate_acpi_pm_profile_undefined() ||
-		    amd_pstate_acpi_pm_profile_server()) {
+		    (amd_pstate_acpi_pm_profile_server() && boot_cpu_data.x86 < 0x1A)) {
 			pr_info("driver load is disabled, boot with specific mode to enable this\n");
 			return -ENODEV;
 		}
-- 
2.34.1

Re: [PATCH 0/2] amd-pstate: Make amd-pstate the default driver on server platforms

[Mario Limonciello]

On 10/21/2024 05:18, Gautham R. Shenoy wrote:
> Hello,
> 
> This patchset contains two patches to
> 
> * Prevent frequency throttling on power-limited systems with
>    amd-pstate active mode with performance governor.
> 
> * Make amd_pstate default on EPYC Family 1A+. Based on tests, the
>    amd-pstate driver performs well enough on EPYC.
> 
> These patches are based on the "linux-next" branch of
> https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git
> 
> Gautham R. Shenoy (1):
>    amd-pstate: Set min_perf to nominal_perf for active mode performance
>      gov
> 
> Swapnil Sapkal (1):
>    amd-pstate: Switch to amd-pstate by default on some Server platforms
> 
>   drivers/cpufreq/amd-pstate.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

I'll queue this up for 6.13.

There were previously two differences for server and non-server:
  * amd-pstate not enabled by default
  * amd-pstate cpufreq policy starting in performance mode vs power-save?

This series adjusts the first for at least the newer parts, but I would 
like to ask does it make sense to also evaluating changing the default 
policy to powersave as a follow up, or should this policy delta remain?

Re: [PATCH 0/2] amd-pstate: Make amd-pstate the default driver on server platforms

[Gautham R. Shenoy]

Hello Mario,

On Mon, Oct 21, 2024 at 08:05:17AM -0500, Mario Limonciello wrote:
> On 10/21/2024 05:18, Gautham R. Shenoy wrote:
> > Hello,
> > 
> > This patchset contains two patches to
> > 
> > * Prevent frequency throttling on power-limited systems with
> >    amd-pstate active mode with performance governor.
> > 
> > * Make amd_pstate default on EPYC Family 1A+. Based on tests, the
> >    amd-pstate driver performs well enough on EPYC.
> > 
> > These patches are based on the "linux-next" branch of
> > https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git
> > 
> > Gautham R. Shenoy (1):
> >    amd-pstate: Set min_perf to nominal_perf for active mode performance
> >      gov
> > 
> > Swapnil Sapkal (1):
> >    amd-pstate: Switch to amd-pstate by default on some Server platforms
> > 
> >   drivers/cpufreq/amd-pstate.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>

Thank you!

> I'll queue this up for 6.13.
> 
> There were previously two differences for server and non-server:
>  * amd-pstate not enabled by default
>  * amd-pstate cpufreq policy starting in performance mode vs power-save?
> 
> This series adjusts the first for at least the newer parts, but I would like
> to ask does it make sense to also evaluating changing the default policy to
> powersave as a follow up, or should this policy delta remain?

We would like to retain the policy delta for now and retain
CPUFREQ_POLICY_PERFORMANCE as the default on servers.

--
Thanks and Regards
gautham.

Re: [PATCH 0/2] amd-pstate: Make amd-pstate the default driver on server platforms

[Mario Limonciello]

On 10/22/2024 00:11, Gautham R. Shenoy wrote:
> Hello Mario,
> 
> On Mon, Oct 21, 2024 at 08:05:17AM -0500, Mario Limonciello wrote:
>> On 10/21/2024 05:18, Gautham R. Shenoy wrote:
>>> Hello,
>>>
>>> This patchset contains two patches to
>>>
>>> * Prevent frequency throttling on power-limited systems with
>>>     amd-pstate active mode with performance governor.
>>>
>>> * Make amd_pstate default on EPYC Family 1A+. Based on tests, the
>>>     amd-pstate driver performs well enough on EPYC.
>>>
>>> These patches are based on the "linux-next" branch of
>>> https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git
>>>
>>> Gautham R. Shenoy (1):
>>>     amd-pstate: Set min_perf to nominal_perf for active mode performance
>>>       gov
>>>
>>> Swapnil Sapkal (1):
>>>     amd-pstate: Switch to amd-pstate by default on some Server platforms
>>>
>>>    drivers/cpufreq/amd-pstate.c | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>>
> 
> Thank you!
> 
>> I'll queue this up for 6.13.
>>
>> There were previously two differences for server and non-server:
>>   * amd-pstate not enabled by default
>>   * amd-pstate cpufreq policy starting in performance mode vs power-save?
>>
>> This series adjusts the first for at least the newer parts, but I would like
>> to ask does it make sense to also evaluating changing the default policy to
>> powersave as a follow up, or should this policy delta remain?
> 
> We would like to retain the policy delta for now and retain
> CPUFREQ_POLICY_PERFORMANCE as the default on servers.
> 

Ack, thanks.