Re: [PATCH v1 2/6] cpufreq: intel_pstate: Do not update global.turbo_disabled after initialization
From: srinivas pandruvada
Date: Wed Jun 05 2024 - 08:50:40 EST
On Wed, 2024-06-05 at 13:21 +0800, Xi Ruoyao wrote:
> On Tue, 2024-06-04 at 09:56 -0700, srinivas pandruvada wrote:
> > > > With such a delay, I am not sure how this even worked before.
>
> It didn't work out of box but it worked after manually writing 0 to
> no_turbo after 20 seconds, see
> https://bugzilla.kernel.org/show_bug.cgi?id=218702.
That make sense. So it never worked out of box. The store_no_turbo()
has additional read for turbo flag before, which is removed now. I
think adding that back will will restore old behavior.
diff --git a/drivers/cpufreq/intel_pstate.c
b/drivers/cpufreq/intel_pstate.c
index 4b986c044741..0d5330e5b96b 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1301,6 +1301,8 @@ static ssize_t store_no_turbo(struct kobject *a,
struct kobj_attribute *b,
no_turbo = !!clamp_t(int, input, 0, 1);
+ global.turbo_disabled = turbo_is_disabled();
+
if (no_turbo == global.no_turbo)
goto unlock_driver;
Need to adjust the mutex around it also.
Regarding the bugzilla, I disabled turbo and no_turbo shows 1. Not sure
how it shows "0" to you.
Thanks,
Srinivas
>
> > > > Can you revert the patch in question and use kernel dynamic
> > > > debug
> > > > dyndbg="file intel_pstate.c +p" kernel command line and collect
> > > > log
> > > > for
> > > > 30 seconds?
>
> Attached. The related part seems:
>
> [ 0.553606] intel_pstate: Intel P-state driver initializing
> [ 0.553630] intel_pstate: controlling: cpu 0
> [ 0.553640] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.553642] intel_pstate: cpu:0 min_policy_perf:4
> max_policy_perf:31
> [ 0.553643] intel_pstate: cpu:0 global_min:0 global_max:44
> [ 0.553644] intel_pstate: cpu:0 max_perf_ratio:31 min_perf_ratio:4
> [ 0.553680] intel_pstate: controlling: cpu 1
> [ 0.553702] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.553703] intel_pstate: cpu:1 min_policy_perf:4
> max_policy_perf:31
> [ 0.553704] intel_pstate: cpu:1 global_min:0 global_max:44
> [ 0.553705] intel_pstate: cpu:1 max_perf_ratio:31 min_perf_ratio:4
> [ 0.553742] intel_pstate: controlling: cpu 2
> [ 0.553763] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.553764] intel_pstate: cpu:2 min_policy_perf:4
> max_policy_perf:31
> [ 0.553765] intel_pstate: cpu:2 global_min:0 global_max:44
> [ 0.553766] intel_pstate: cpu:2 max_perf_ratio:31 min_perf_ratio:4
> [ 0.553809] intel_pstate: controlling: cpu 3
> [ 0.553830] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.553831] intel_pstate: cpu:3 min_policy_perf:4
> max_policy_perf:31
> [ 0.553831] intel_pstate: cpu:3 global_min:0 global_max:44
> [ 0.553832] intel_pstate: cpu:3 max_perf_ratio:31 min_perf_ratio:4
> [ 0.553868] intel_pstate: controlling: cpu 4
> [ 0.553892] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.553893] intel_pstate: cpu:4 min_policy_perf:4
> max_policy_perf:31
> [ 0.553894] intel_pstate: cpu:4 global_min:0 global_max:44
> [ 0.553895] intel_pstate: cpu:4 max_perf_ratio:31 min_perf_ratio:4
> [ 0.553943] intel_pstate: controlling: cpu 5
> [ 0.553967] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.553968] intel_pstate: cpu:5 min_policy_perf:4
> max_policy_perf:31
> [ 0.553968] intel_pstate: cpu:5 global_min:0 global_max:44
> [ 0.553969] intel_pstate: cpu:5 max_perf_ratio:31 min_perf_ratio:4
> [ 0.554009] intel_pstate: controlling: cpu 6
> [ 0.554034] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.554035] intel_pstate: cpu:6 min_policy_perf:4
> max_policy_perf:31
> [ 0.554036] intel_pstate: cpu:6 global_min:0 global_max:44
> [ 0.554037] intel_pstate: cpu:6 max_perf_ratio:31 min_perf_ratio:4
> [ 0.554077] intel_pstate: controlling: cpu 7
> [ 0.554098] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 0.554099] intel_pstate: cpu:7 min_policy_perf:4
> max_policy_perf:31
> [ 0.554099] intel_pstate: cpu:7 global_min:0 global_max:44
> [ 0.554100] intel_pstate: cpu:7 max_perf_ratio:31 min_perf_ratio:4
> [ 0.554104] intel_pstate: HWP enabled
> [ 2.183669] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183675] intel_pstate: cpu:6 min_policy_perf:4
> max_policy_perf:31
> [ 2.183677] intel_pstate: cpu:6 global_min:4 global_max:44
> [ 2.183679] intel_pstate: cpu:6 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183710] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183711] intel_pstate: cpu:4 min_policy_perf:4
> max_policy_perf:31
> [ 2.183713] intel_pstate: cpu:4 global_min:4 global_max:44
> [ 2.183715] intel_pstate: cpu:4 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183742] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183744] intel_pstate: cpu:2 min_policy_perf:4
> max_policy_perf:31
> [ 2.183745] intel_pstate: cpu:2 global_min:4 global_max:44
> [ 2.183747] intel_pstate: cpu:2 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183773] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183775] intel_pstate: cpu:0 min_policy_perf:4
> max_policy_perf:31
> [ 2.183776] intel_pstate: cpu:0 global_min:4 global_max:44
> [ 2.183777] intel_pstate: cpu:0 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183803] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183805] intel_pstate: cpu:7 min_policy_perf:4
> max_policy_perf:31
> [ 2.183806] intel_pstate: cpu:7 global_min:4 global_max:44
> [ 2.183807] intel_pstate: cpu:7 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183831] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183833] intel_pstate: cpu:5 min_policy_perf:4
> max_policy_perf:31
> [ 2.183834] intel_pstate: cpu:5 global_min:4 global_max:44
> [ 2.183836] intel_pstate: cpu:5 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183862] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183864] intel_pstate: cpu:3 min_policy_perf:4
> max_policy_perf:31
> [ 2.183865] intel_pstate: cpu:3 global_min:4 global_max:44
> [ 2.183867] intel_pstate: cpu:3 max_perf_ratio:31 min_perf_ratio:4
> [ 2.183893] intel_pstate: set_policy cpuinfo.max 3100000 policy-
> >max 3100000
> [ 2.183895] intel_pstate: cpu:1 min_policy_perf:4
> max_policy_perf:31
> [ 2.183896] intel_pstate: cpu:1 global_min:4 global_max:44
> [ 2.183898] intel_pstate: cpu:1 max_perf_ratio:31 min_perf_ratio:4
>
> > > I think that it worked because the MSR was read every time
> > > intel_pstate ran, so it got updated at one point and stayed that
> > > way.
> >
> > But here HWP in active mode is getting used. So it should have
> > fewer
> > request calls to set accept via cpufreq set_policy()
> >
> > callback or with some HWP interrupt.
>