* [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
@ 2020-09-01 3:02 Francisco Jerez
2020-09-01 15:48 ` Srinivas Pandruvada
0 siblings, 1 reply; 6+ messages in thread
From: Francisco Jerez @ 2020-09-01 3:02 UTC (permalink / raw)
To: linux-pm; +Cc: Srinivas Pandruvada, Rafael J . Wysocki, Caleb Callaway
This fixes the behavior of the scaling_max_freq and scaling_min_freq
sysfs files in systems which had turbo disabled by the BIOS.
Caleb noticed that the HWP is programmed to operate in the wrong
P-state range on his system when the CPUFREQ policy min/max frequency
is set via sysfs. This seems to be because in his system
intel_pstate_get_hwp_max() is returning the maximum turbo P-state even
though turbo was disabled by the BIOS, which causes intel_pstate to
scale kHz frequencies incorrectly e.g. setting the maximum turbo
frequency whenever the maximum guaranteed frequency is requested via
sysfs.
Tested-by: Caleb Callaway <caleb.callaway@intel.com>
Signed-off-by: Francisco Jerez <currojerez@riseup.net>
---
drivers/cpufreq/intel_pstate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index e0220a6fbc69..7eb7b62bd5c4 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -825,7 +825,7 @@ static void intel_pstate_get_hwp_max(unsigned int cpu, int *phy_max,
rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
- if (global.no_turbo)
+ if (global.no_turbo || global.turbo_disabled)
*current_max = HWP_GUARANTEED_PERF(cap);
else
*current_max = HWP_HIGHEST_PERF(cap);
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
2020-09-01 3:02 [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases Francisco Jerez
@ 2020-09-01 15:48 ` Srinivas Pandruvada
2020-09-01 17:59 ` Rafael J. Wysocki
2020-09-01 18:19 ` Francisco Jerez
0 siblings, 2 replies; 6+ messages in thread
From: Srinivas Pandruvada @ 2020-09-01 15:48 UTC (permalink / raw)
To: Francisco Jerez, linux-pm; +Cc: Rafael J . Wysocki, Caleb Callaway
On Mon, 2020-08-31 at 20:02 -0700, Francisco Jerez wrote:
> This fixes the behavior of the scaling_max_freq and scaling_min_freq
> sysfs files in systems which had turbo disabled by the BIOS.
>
> Caleb noticed that the HWP is programmed to operate in the wrong
> P-state range on his system when the CPUFREQ policy min/max frequency
> is set via sysfs. This seems to be because in his system
> intel_pstate_get_hwp_max() is returning the maximum turbo P-state
> even
> though turbo was disabled by the BIOS, which causes intel_pstate to
> scale kHz frequencies incorrectly e.g. setting the maximum turbo
> frequency whenever the maximum guaranteed frequency is requested via
> sysfs.
When turbo is disabled via MSR_IA32_MISC_ENABLE_TURBO_DISABLE (From
BIOS), then no matter what we write to HWP. max, the hardware will clip
to HWP_GUARANTEED_PERF.
But it looks like this is some issue on properly disabling turbo from
BIOS, since you observe turbo frequencies (via aperf, mperf) not just
sysfs display issue.
>
> Tested-by: Caleb Callaway <caleb.callaway@intel.com>
> Signed-off-by: Francisco Jerez <currojerez@riseup.net>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index e0220a6fbc69..7eb7b62bd5c4 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -825,7 +825,7 @@ static void intel_pstate_get_hwp_max(unsigned int
> cpu, int *phy_max,
>
> rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
> WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
> - if (global.no_turbo)
> + if (global.no_turbo || global.turbo_disabled)
> *current_max = HWP_GUARANTEED_PERF(cap);
> else
> *current_max = HWP_HIGHEST_PERF(cap);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
2020-09-01 15:48 ` Srinivas Pandruvada
@ 2020-09-01 17:59 ` Rafael J. Wysocki
2020-09-01 18:19 ` Francisco Jerez
1 sibling, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2020-09-01 17:59 UTC (permalink / raw)
To: Srinivas Pandruvada, Francisco Jerez
Cc: Linux PM, Rafael J . Wysocki, Caleb Callaway
On Tue, Sep 1, 2020 at 5:48 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Mon, 2020-08-31 at 20:02 -0700, Francisco Jerez wrote:
> > This fixes the behavior of the scaling_max_freq and scaling_min_freq
> > sysfs files in systems which had turbo disabled by the BIOS.
> >
> > Caleb noticed that the HWP is programmed to operate in the wrong
> > P-state range on his system when the CPUFREQ policy min/max frequency
> > is set via sysfs. This seems to be because in his system
> > intel_pstate_get_hwp_max() is returning the maximum turbo P-state
> > even
> > though turbo was disabled by the BIOS, which causes intel_pstate to
> > scale kHz frequencies incorrectly e.g. setting the maximum turbo
> > frequency whenever the maximum guaranteed frequency is requested via
> > sysfs.
>
> When turbo is disabled via MSR_IA32_MISC_ENABLE_TURBO_DISABLE (From
> BIOS), then no matter what we write to HWP. max, the hardware will clip
> to HWP_GUARANTEED_PERF.
>
> But it looks like this is some issue on properly disabling turbo from
> BIOS, since you observe turbo frequencies (via aperf, mperf) not just
> sysfs display issue.
>
>
>
> >
> > Tested-by: Caleb Callaway <caleb.callaway@intel.com>
> > Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied as 5.9-rc material with minor edits in the subject.
Thanks!
> > ---
> > drivers/cpufreq/intel_pstate.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/intel_pstate.c
> > b/drivers/cpufreq/intel_pstate.c
> > index e0220a6fbc69..7eb7b62bd5c4 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -825,7 +825,7 @@ static void intel_pstate_get_hwp_max(unsigned int
> > cpu, int *phy_max,
> >
> > rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
> > WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
> > - if (global.no_turbo)
> > + if (global.no_turbo || global.turbo_disabled)
> > *current_max = HWP_GUARANTEED_PERF(cap);
> > else
> > *current_max = HWP_HIGHEST_PERF(cap);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
2020-09-01 15:48 ` Srinivas Pandruvada
2020-09-01 17:59 ` Rafael J. Wysocki
@ 2020-09-01 18:19 ` Francisco Jerez
2020-09-01 18:56 ` Callaway, Caleb
1 sibling, 1 reply; 6+ messages in thread
From: Francisco Jerez @ 2020-09-01 18:19 UTC (permalink / raw)
To: Srinivas Pandruvada, linux-pm; +Cc: Rafael J . Wysocki, Caleb Callaway
[-- Attachment #1.1: Type: text/plain, Size: 2520 bytes --]
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> writes:
> On Mon, 2020-08-31 at 20:02 -0700, Francisco Jerez wrote:
>> This fixes the behavior of the scaling_max_freq and scaling_min_freq
>> sysfs files in systems which had turbo disabled by the BIOS.
>>
>> Caleb noticed that the HWP is programmed to operate in the wrong
>> P-state range on his system when the CPUFREQ policy min/max frequency
>> is set via sysfs. This seems to be because in his system
>> intel_pstate_get_hwp_max() is returning the maximum turbo P-state
>> even
>> though turbo was disabled by the BIOS, which causes intel_pstate to
>> scale kHz frequencies incorrectly e.g. setting the maximum turbo
>> frequency whenever the maximum guaranteed frequency is requested via
>> sysfs.
>
> When turbo is disabled via MSR_IA32_MISC_ENABLE_TURBO_DISABLE (From
> BIOS), then no matter what we write to HWP. max, the hardware will clip
> to HWP_GUARANTEED_PERF.
>
> But it looks like this is some issue on properly disabling turbo from
> BIOS, since you observe turbo frequencies (via aperf, mperf) not just
> sysfs display issue.
>
Caleb should be able to confirm that, but my understanding is that his
system was still clocking up to the max turbo frequency despite turbo
being disabled in the BIOS and the maximum guaranteed frequency having
been specified in scaling_max_freq.
However there is a bug even in systems where the clipping you describe
is working correctly, the conversion from kHz frequency to P-state uses
a bogus scaling factor without this patch applied.
>
>
>>
>> Tested-by: Caleb Callaway <caleb.callaway@intel.com>
>> Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>
Thanks!
>> ---
>> drivers/cpufreq/intel_pstate.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/intel_pstate.c
>> b/drivers/cpufreq/intel_pstate.c
>> index e0220a6fbc69..7eb7b62bd5c4 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -825,7 +825,7 @@ static void intel_pstate_get_hwp_max(unsigned int
>> cpu, int *phy_max,
>>
>> rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
>> WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
>> - if (global.no_turbo)
>> + if (global.no_turbo || global.turbo_disabled)
>> *current_max = HWP_GUARANTEED_PERF(cap);
>> else
>> *current_max = HWP_HIGHEST_PERF(cap);
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
2020-09-01 18:19 ` Francisco Jerez
@ 2020-09-01 18:56 ` Callaway, Caleb
2020-09-01 19:30 ` Srinivas Pandruvada
0 siblings, 1 reply; 6+ messages in thread
From: Callaway, Caleb @ 2020-09-01 18:56 UTC (permalink / raw)
To: Francisco Jerez, Srinivas Pandruvada, linux-pm; +Cc: Wysocki, Rafael J
Hi folks,
Thanks for working on this! It's possible my system is still clocking up to max turbo, but I didn't explicitly test this. My goal was to fix the CPU frequency at 2.8 GHz on a CFL-S platform using the following script:
_cpufreq=<frequency in Khz>
cpu_sysfs="/sys/devices/system/cpu/cpufreq"
for cpu in $cpu_sysfs/*; do
echo "Setting frequency for $cpu..."
echo "performance" > $cpu/scaling_governor
echo $_cpufreq > $cpu/scaling_max_freq
echo $_cpufreq > $cpu/scaling_min_freq
done
To get the desired fixed frequency, I had to set _cpufreq==2100000 when Turbo was disabled in the BIOS; with Turbo enabled, I had to use the value 2800000. I observed the result with:
watch "cat /proc/cpuinfo | grep 'cpu MHz'"
With Francisco's patch, I could use the value 2800000 for both Turbo enabled and Turbo disabled.
I'm not well-acquainted with the moving parts here, but if there's an additional supporting experiment I can run, just let me know.
HTH,
-Caleb
-----Original Message-----
From: Francisco Jerez <currojerez@riseup.net>
Sent: Tuesday, September 1, 2020 11:19 AM
To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>; linux-pm@vger.kernel.org
Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Callaway, Caleb <caleb.callaway@intel.com>
Subject: Re: [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> writes:
> On Mon, 2020-08-31 at 20:02 -0700, Francisco Jerez wrote:
>> This fixes the behavior of the scaling_max_freq and scaling_min_freq
>> sysfs files in systems which had turbo disabled by the BIOS.
>>
>> Caleb noticed that the HWP is programmed to operate in the wrong
>> P-state range on his system when the CPUFREQ policy min/max frequency
>> is set via sysfs. This seems to be because in his system
>> intel_pstate_get_hwp_max() is returning the maximum turbo P-state
>> even though turbo was disabled by the BIOS, which causes intel_pstate
>> to scale kHz frequencies incorrectly e.g. setting the maximum turbo
>> frequency whenever the maximum guaranteed frequency is requested via
>> sysfs.
>
> When turbo is disabled via MSR_IA32_MISC_ENABLE_TURBO_DISABLE (From
> BIOS), then no matter what we write to HWP. max, the hardware will
> clip to HWP_GUARANTEED_PERF.
>
> But it looks like this is some issue on properly disabling turbo from
> BIOS, since you observe turbo frequencies (via aperf, mperf) not just
> sysfs display issue.
>
Caleb should be able to confirm that, but my understanding is that his system was still clocking up to the max turbo frequency despite turbo being disabled in the BIOS and the maximum guaranteed frequency having been specified in scaling_max_freq.
However there is a bug even in systems where the clipping you describe is working correctly, the conversion from kHz frequency to P-state uses a bogus scaling factor without this patch applied.
>
>
>>
>> Tested-by: Caleb Callaway <caleb.callaway@intel.com>
>> Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>
Thanks!
>> ---
>> drivers/cpufreq/intel_pstate.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/intel_pstate.c
>> b/drivers/cpufreq/intel_pstate.c index e0220a6fbc69..7eb7b62bd5c4
>> 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -825,7 +825,7 @@ static void intel_pstate_get_hwp_max(unsigned int
>> cpu, int *phy_max,
>>
>> rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
>> WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
>> - if (global.no_turbo)
>> + if (global.no_turbo || global.turbo_disabled)
>> *current_max = HWP_GUARANTEED_PERF(cap);
>> else
>> *current_max = HWP_HIGHEST_PERF(cap);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases.
2020-09-01 18:56 ` Callaway, Caleb
@ 2020-09-01 19:30 ` Srinivas Pandruvada
0 siblings, 0 replies; 6+ messages in thread
From: Srinivas Pandruvada @ 2020-09-01 19:30 UTC (permalink / raw)
To: Callaway, Caleb, Francisco Jerez, linux-pm; +Cc: Wysocki, Rafael J
On Tue, 2020-09-01 at 18:56 +0000, Callaway, Caleb wrote:
> Hi folks,
>
> Thanks for working on this! It's possible my system is still clocking
> up to max turbo, but I didn't explicitly test this.
This is unlikely that will clock to turbo. Better to test that.
But since the frequency limits can't be set correctly, the patch is
still valid.
Thanks,
Srinivas
> My goal was to fix the CPU frequency at 2.8 GHz on a CFL-S platform
> using the following script:
>
> _cpufreq=<frequency in Khz>
> cpu_sysfs="/sys/devices/system/cpu/cpufreq"
> for cpu in $cpu_sysfs/*; do
> echo "Setting frequency for $cpu..."
> echo "performance" > $cpu/scaling_governor
> echo $_cpufreq > $cpu/scaling_max_freq
> echo $_cpufreq > $cpu/scaling_min_freq
> done
>
> To get the desired fixed frequency, I had to set _cpufreq==2100000
> when Turbo was disabled in the BIOS; with Turbo enabled, I had to use
> the value 2800000. I observed the result with:
>
> watch "cat /proc/cpuinfo | grep 'cpu MHz'"
>
> With Francisco's patch, I could use the value 2800000 for both Turbo
> enabled and Turbo disabled.
>
> I'm not well-acquainted with the moving parts here, but if there's an
> additional supporting experiment I can run, just let me know.
>
> HTH,
> -Caleb
>
> -----Original Message-----
> From: Francisco Jerez <currojerez@riseup.net>
> Sent: Tuesday, September 1, 2020 11:19 AM
> To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>;
> linux-pm@vger.kernel.org
> Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Callaway, Caleb <
> caleb.callaway@intel.com>
> Subject: Re: [PATCH] cpufreq: intel_pstate: Fix
> intel_pstate_get_hwp_max() for turbo disabled cases.
>
> Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> writes:
>
> > On Mon, 2020-08-31 at 20:02 -0700, Francisco Jerez wrote:
> > > This fixes the behavior of the scaling_max_freq and
> > > scaling_min_freq
> > > sysfs files in systems which had turbo disabled by the BIOS.
> > >
> > > Caleb noticed that the HWP is programmed to operate in the wrong
> > > P-state range on his system when the CPUFREQ policy min/max
> > > frequency
> > > is set via sysfs. This seems to be because in his system
> > > intel_pstate_get_hwp_max() is returning the maximum turbo P-
> > > state
> > > even though turbo was disabled by the BIOS, which causes
> > > intel_pstate
> > > to scale kHz frequencies incorrectly e.g. setting the maximum
> > > turbo
> > > frequency whenever the maximum guaranteed frequency is requested
> > > via
> > > sysfs.
> >
> > When turbo is disabled via MSR_IA32_MISC_ENABLE_TURBO_DISABLE
> > (From
> > BIOS), then no matter what we write to HWP. max, the hardware will
> > clip to HWP_GUARANTEED_PERF.
> >
> > But it looks like this is some issue on properly disabling turbo
> > from
> > BIOS, since you observe turbo frequencies (via aperf, mperf) not
> > just
> > sysfs display issue.
> >
>
> Caleb should be able to confirm that, but my understanding is that
> his system was still clocking up to the max turbo frequency despite
> turbo being disabled in the BIOS and the maximum guaranteed frequency
> having been specified in scaling_max_freq.
>
> However there is a bug even in systems where the clipping you
> describe is working correctly, the conversion from kHz frequency to
> P-state uses a bogus scaling factor without this patch applied.
>
> >
> > > Tested-by: Caleb Callaway <caleb.callaway@intel.com>
> > > Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> >
>
> Thanks!
>
> > > ---
> > > drivers/cpufreq/intel_pstate.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/cpufreq/intel_pstate.c
> > > b/drivers/cpufreq/intel_pstate.c index
> > > e0220a6fbc69..7eb7b62bd5c4
> > > 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -825,7 +825,7 @@ static void intel_pstate_get_hwp_max(unsigned
> > > int
> > > cpu, int *phy_max,
> > >
> > > rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
> > > WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
> > > - if (global.no_turbo)
> > > + if (global.no_turbo || global.turbo_disabled)
> > > *current_max = HWP_GUARANTEED_PERF(cap);
> > > else
> > > *current_max = HWP_HIGHEST_PERF(cap);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-01 19:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 3:02 [PATCH] cpufreq: intel_pstate: Fix intel_pstate_get_hwp_max() for turbo disabled cases Francisco Jerez
2020-09-01 15:48 ` Srinivas Pandruvada
2020-09-01 17:59 ` Rafael J. Wysocki
2020-09-01 18:19 ` Francisco Jerez
2020-09-01 18:56 ` Callaway, Caleb
2020-09-01 19:30 ` Srinivas Pandruvada
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).