* [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile @ 2023-06-15 6:32 Perry Yuan 2023-06-20 14:58 ` Huang Rui 0 siblings, 1 reply; 6+ messages in thread From: Perry Yuan @ 2023-06-15 6:32 UTC (permalink / raw) To: rafael.j.wysocki, viresh.kumar, Ray.Huang, Mario.Limonciello Cc: Deepak.Sharma, Wyes.Karny, gautham.shenoy, Sunpeng.Li, Xinmei.Huang, Xiaojian.Du, Li.Meng, linux-pm, linux-kernel 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 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile 2023-06-15 6:32 [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile Perry Yuan @ 2023-06-20 14:58 ` Huang Rui 2023-06-20 15:02 ` Limonciello, Mario 0 siblings, 1 reply; 6+ messages in thread From: Huang Rui @ 2023-06-20 14:58 UTC (permalink / raw) To: Yuan, Perry Cc: rafael.j.wysocki@intel.com, viresh.kumar@linaro.org, Limonciello, Mario, Sharma, Deepak, Karny, Wyes, Shenoy, Gautham Ranjal, Li, Sun peng (Leo), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine), linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile 2023-06-20 14:58 ` Huang Rui @ 2023-06-20 15:02 ` Limonciello, Mario 2023-06-20 15:06 ` Huang Rui 0 siblings, 1 reply; 6+ messages in thread From: Limonciello, Mario @ 2023-06-20 15:02 UTC (permalink / raw) To: Huang Rui, Yuan, Perry Cc: rafael.j.wysocki@intel.com, viresh.kumar@linaro.org, Sharma, Deepak, Karny, Wyes, Shenoy, Gautham Ranjal, Li, Sun peng (Leo), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine), linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org 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? > > 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 >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile 2023-06-20 15:02 ` Limonciello, Mario @ 2023-06-20 15:06 ` Huang Rui 2023-06-20 15:18 ` Limonciello, Mario 0 siblings, 1 reply; 6+ messages in thread From: Huang Rui @ 2023-06-20 15:06 UTC (permalink / raw) To: Limonciello, Mario Cc: Yuan, Perry, rafael.j.wysocki@intel.com, viresh.kumar@linaro.org, Sharma, Deepak, Karny, Wyes, Shenoy, Gautham Ranjal, Li, Sun peng (Leo), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine), linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org 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? Do we have a way to separate them? 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 > >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile 2023-06-20 15:06 ` Huang Rui @ 2023-06-20 15:18 ` Limonciello, Mario 2023-06-20 15:36 ` Huang Rui 0 siblings, 1 reply; 6+ messages in thread From: Limonciello, Mario @ 2023-06-20 15:18 UTC (permalink / raw) To: Huang Rui Cc: Yuan, Perry, rafael.j.wysocki@intel.com, viresh.kumar@linaro.org, Sharma, Deepak, Karny, Wyes, Shenoy, Gautham Ranjal, Li, Sun peng (Leo), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine), linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org 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 >>>> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile 2023-06-20 15:18 ` Limonciello, Mario @ 2023-06-20 15:36 ` Huang Rui 0 siblings, 0 replies; 6+ messages in thread From: Huang Rui @ 2023-06-20 15:36 UTC (permalink / raw) To: Limonciello, Mario Cc: Yuan, Perry, rafael.j.wysocki@intel.com, viresh.kumar@linaro.org, Sharma, Deepak, Karny, Wyes, Shenoy, Gautham Ranjal, Li, Sun peng (Leo), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine), linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org On Tue, Jun 20, 2023 at 11:18:30PM +0800, Limonciello, Mario wrote: > > 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? Hmm, that makes sense. Energy consumption certification is sufficient reason. > > 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? > If we don't have good way to separate them, we can set them to powersave at this moment with your original patches. But I think we would better dig out a method in future. Please add my Acks for these series in next version. Acked-by: Huang Rui <ray.huang@amd.com> > > > > 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 > >>>> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-20 15:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-15 6:32 [PATCH v2 2/4] cpufreq: amd-pstate: Set a fallback policy based on preferred_profile Perry Yuan 2023-06-20 14:58 ` Huang Rui 2023-06-20 15:02 ` Limonciello, Mario 2023-06-20 15:06 ` Huang Rui 2023-06-20 15:18 ` Limonciello, Mario 2023-06-20 15:36 ` Huang Rui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).