| Message ID | 20230615063225.4029929-1-perry.yuan@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] ACPI: CPPC: Add a symbol to check if the preferred profile is a server | expand |
On Thu, Jun 15, 2023 at 02:32:25PM +0800, Yuan, Perry wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > 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 May I know if the processor is powerful desktop such as threadripper, whether it will be default to 'performance' or 'powersave'? Thanks, Ray > 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 > 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> > --- > drivers/cpufreq/amd-pstate.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index ddd346a239e0..c9d296ebf81e 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -1102,10 +1102,13 @@ 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 (acpi_pm_profile_server() || 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 >
On 6/20/2023 10:06 AM, Huang Rui wrote: > On Tue, Jun 20, 2023 at 11:02:00PM +0800, Limonciello, Mario wrote: >> On 6/20/2023 9:58 AM, Huang Rui wrote: >>> On Thu, Jun 15, 2023 at 02:32:25PM +0800, Yuan, Perry wrote: >>>> From: Mario Limonciello <mario.limonciello@amd.com> >>>> >>>> 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 >>> May I know if the processor is powerful desktop such as threadripper, >>> whether it will be default to 'performance' or 'powersave'? >> It's currently defaulting to 'powersave' for desktops and >> workstations. >> >> Do you think we should adopt performance for these? > Yes, I didn't see any different use cases here between server and > threadripper here. Or I missed anything? Workstations and Desktops usually have to go through energy consumption certifications. Couldn't setting it to performance be inappropriate for those? > Do we have a way to separate them? If Threadripper identified as 3 Workstation I'd agree; but I'd think we're going to lump AM4/AM5 desktops along with Threadripper. So should we still set all those to performance? > > Thanks, > Ray > >>> Thanks, >>> Ray >>> >>>> 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 >>>> 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> >>>> --- >>>> drivers/cpufreq/amd-pstate.c | 7 +++++-- >>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c >>>> index ddd346a239e0..c9d296ebf81e 100644 >>>> --- a/drivers/cpufreq/amd-pstate.c >>>> +++ b/drivers/cpufreq/amd-pstate.c >>>> @@ -1102,10 +1102,13 @@ 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 (acpi_pm_profile_server() || 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 >>>>
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index ddd346a239e0..c9d296ebf81e 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -1102,10 +1102,13 @@ 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 (acpi_pm_profile_server() || 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);