linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: move turbo_disabled() out of intel_set_max_freq_ratio
@ 2020-04-29  1:50 Li RongQing
  2020-04-29  8:15 ` Giovanni Gherdovich
  0 siblings, 1 reply; 3+ messages in thread
From: Li RongQing @ 2020-04-29  1:50 UTC (permalink / raw)
  To: tglx, mingo, bp, hpa, x86, peterz, ggherdovich, linux-kernel,
	rafael.j.wysocki

move the turbo_disabled before intel_set_max_freq_ratio,
when turbo is disabled, the max frequency ratio is a const
value, it is unnecessary to read MSR_TURBO_RATIO* msr to
compute

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 arch/x86/kernel/smpboot.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index fe3ab9632f3b..8979c459df2f 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1987,7 +1987,7 @@ static bool intel_set_max_freq_ratio(void)
 out:
 	arch_turbo_freq_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE,
 					base_freq);
-	arch_set_max_freq_ratio(turbo_disabled());
+	arch_set_max_freq_ratio(false);
 	return true;
 }
 
@@ -2009,6 +2009,9 @@ static void init_freq_invariance(void)
 	if (smp_processor_id() != 0 || !boot_cpu_has(X86_FEATURE_APERFMPERF))
 		return;
 
+	if (turbo_disabled())
+		return;
+
 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
 		ret = intel_set_max_freq_ratio();
 
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86: move turbo_disabled() out of intel_set_max_freq_ratio
  2020-04-29  1:50 [PATCH] x86: move turbo_disabled() out of intel_set_max_freq_ratio Li RongQing
@ 2020-04-29  8:15 ` Giovanni Gherdovich
  2020-04-29  8:53   ` 答复: " Li,Rongqing
  0 siblings, 1 reply; 3+ messages in thread
From: Giovanni Gherdovich @ 2020-04-29  8:15 UTC (permalink / raw)
  To: Li RongQing, tglx, mingo, bp, hpa, x86, peterz, linux-kernel,
	rafael.j.wysocki

On Wed, 2020-04-29 at 09:50 +0800, Li RongQing wrote:
> move the turbo_disabled before intel_set_max_freq_ratio,
> when turbo is disabled, the max frequency ratio is a const
> value, it is unnecessary to read MSR_TURBO_RATIO* msr to
> compute
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  arch/x86/kernel/smpboot.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index fe3ab9632f3b..8979c459df2f 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1987,7 +1987,7 @@ static bool intel_set_max_freq_ratio(void)
>  out:
>  	arch_turbo_freq_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE,
>  					base_freq);
> -	arch_set_max_freq_ratio(turbo_disabled());
> +	arch_set_max_freq_ratio(false);
>  	return true;
>  }
>  
> @@ -2009,6 +2009,9 @@ static void init_freq_invariance(void)
>  	if (smp_processor_id() != 0 || !boot_cpu_has(X86_FEATURE_APERFMPERF))
>  		return;
>  
> +	if (turbo_disabled())
> +		return;
> +
>  	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
>  		ret = intel_set_max_freq_ratio();
>

Hello,

the problem is that turbo can be enabled/disabled by the firmware at runtime,
after the machine has booted.

This happens for example with the Dell XPS 13, where turbo gets disabled by the
firmware if the machine is disconnected from AC power and runs on battery. The
laptop could boot on battery (turbo disabled), then after some time the user
connects the AC power supply, turbo gets enabled, and with your patch we
wouldn't know what is the turbo_freq/base_freq ratio to do frequency
invariance (we skipped reading MSR_TURBO_RATIO_LIMIT at boot because turbo was
disabled at that timed).

This behavior was requested by reviewers in this thread:
https://lore.kernel.org/lkml/1906426.HDqaVa71mF@kreacher/
and implemented with 918229cdd5ab ("x86/intel_pstate: Handle runtime turbo
disablement/enablement in frequency invariance").


Thanks,
Giovanni Gherdovich

^ permalink raw reply	[flat|nested] 3+ messages in thread
* 答复: [PATCH] x86: move turbo_disabled() out of intel_set_max_freq_ratio
  2020-04-29  8:15 ` Giovanni Gherdovich
@ 2020-04-29  8:53   ` Li,Rongqing
  0 siblings, 0 replies; 3+ messages in thread
From: Li,Rongqing @ 2020-04-29  8:53 UTC (permalink / raw)
  To: Giovanni Gherdovich, tglx, mingo, bp, hpa, x86, peterz,
	linux-kernel, rafael.j.wysocki


> Hello,
> 
> the problem is that turbo can be enabled/disabled by the firmware at runtime,
> after the machine has booted.
> 
> This happens for example with the Dell XPS 13, where turbo gets disabled by
> the firmware if the machine is disconnected from AC power and runs on battery.
> The laptop could boot on battery (turbo disabled), then after some time the
> user connects the AC power supply, turbo gets enabled, and with your patch
> we wouldn't know what is the turbo_freq/base_freq ratio to do frequency
> invariance (we skipped reading MSR_TURBO_RATIO_LIMIT at boot because
> turbo was disabled at that timed).
> 
> This behavior was requested by reviewers in this thread:
> https://lore.kernel.org/lkml/1906426.HDqaVa71mF@kreacher/
> and implemented with 918229cdd5ab ("x86/intel_pstate: Handle runtime turbo
> disablement/enablement in frequency invariance").
> 


Thanks for you explanation

Sorry for noise

-Li

> 
> Thanks,
> Giovanni Gherdovich

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-29  9:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29  1:50 [PATCH] x86: move turbo_disabled() out of intel_set_max_freq_ratio Li RongQing
2020-04-29  8:15 ` Giovanni Gherdovich
2020-04-29  8:53   ` 答复: " Li,Rongqing

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).