From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755257AbcJEX66 (ORCPT ); Wed, 5 Oct 2016 19:58:58 -0400 Received: from cmta17.telus.net ([209.171.16.90]:40218 "EHLO cmta17.telus.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753098AbcJEX6z (ORCPT ); Wed, 5 Oct 2016 19:58:55 -0400 X-Authority-Analysis: v=2.2 cv=XtgVARN9 c=1 sm=1 tr=0 a=zJWegnE7BH9C0Gl4FFgQyA==:117 a=zJWegnE7BH9C0Gl4FFgQyA==:17 a=Pyq9K9CWowscuQLKlpiwfMBGOR0=:19 a=IkcTkHD0fZMA:10 a=QyXUC8HyAAAA:8 a=fH9wVI_4bevhOUNGQFYA:9 a=QEXdDO2ut3YA:10 a=avl4LiGQNoF5OB0DmCJ7:22 From: "Doug Smythies" To: "'Rafael J. Wysocki'" Cc: "'Linux Kernel Mailing List'" , "'Srinivas Pandruvada'" , "'Linux PM list'" References: <1565019.BFYQPSZAnN@vostro.rjw.lan> rlwTbczQYEcWqrlwVb93qM In-Reply-To: rlwTbczQYEcWqrlwVb93qM Subject: RE: [RFC/RFT][PATCH v2] cpufreq: intel_pstate: Proportional algorithm for Atom Date: Wed, 5 Oct 2016 16:58:40 -0700 Message-ID: <002501d21f64$67356970$35a03c50$@net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Thread-Index: AdIfCaWPRqN3SzGTQaWoTjQ1+61mTgAWE30Q Content-Language: en-ca X-CMAE-Envelope: MS4wfCnUPxN3oI25v7tnlYs1bIO3DKRZ0bVUqNFiGklZN4E+aut3Ll6pxqQGTGWlILhq5+wmLMAOiM6yUnvbRWLfY84a7Syzx4dPHML+rHrlnWI99NsUd2fY zc3tTw3AA9k3yhYjSa4bbuXT3hGi1g3/MaPi0a+mFoel9JvFlefWO0YQ2+D1wri31oITyGlTOUG2g6m9bSeuoH9VvgqGwTPIubzO2jtUcdaL3rclj038tccD F6eDJYBSHW1VZgB8JWZsnHMJ/E66JK5M7dOqZ/9ULpIT+UZweEqs22OriT4AxySk Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rafael, It doesn't compile for me. See further down. On 2016.10.05 06:15 Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > The PID algorithm used by the intel_pstate driver tends to drive > performance to the minimum for workloads with utilization below the > setpoint, which is undesirable, so replace it with a modified > "proportional" algorithm on Atom. > > The new algorithm will set the new P-state to be 1.25 times the > available maximum times the (frequency-invariant) utilization during > the previous sampling period except when the target P-state computed > this way is lower than the average P-state during the previous > sampling period. In the latter case, it will increase the target by > 50% of the difference between it and the average P-state to prevent > performance from dropping down too fast in some cases. > > Signed-off-by: Rafael J. Wysocki > --- > > It is better to compare the average P-state to the target (than to > compare the average perf ratio to the utilization), because that takes > turbo into account more accurately. > > Plus if the target is below the min, it is better to compare the min > to the average instead of comparing the target to it. > > --- > drivers/cpufreq/intel_pstate.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > Index: linux-pm/drivers/cpufreq/intel_pstate.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c > +++ linux-pm/drivers/cpufreq/intel_pstate.c > @@ -1232,6 +1232,7 @@ static inline int32_t get_target_pstate_ > { > struct sample *sample = &cpu->sample; > int32_t busy_frac, boost; > + int target, avg_pstate; > > busy_frac = div_fp(sample->mperf, sample->tsc); > > @@ -1242,7 +1243,26 @@ static inline int32_t get_target_pstate_ > busy_frac = boost; > > sample->busy_scaled = busy_frac * 100; > - return get_avg_pstate(cpu) - pid_calc(&cpu->pid, sample->busy_scaled); > + > + target = limits->no_turbo || limits->turbo_disabled : ^ ^^^ For proper conditional expression syntax, shouldn't that be a "?" ? I can not get this patch to compile, but if I change that ":" to a "?" (as in the previous version of this patch) it compiles. > + cpu->pstate.max_pstate : cpu->pstate.turbo_pstate; > + target += target >> 2; > + target = mul_fp(target, busy_frac); > + if (target < cpu->pstate.min_pstate) > + target = cpu->pstate.min_pstate; > + > + /* > + * If the average P-state during the previous cycle was higher than the > + * current target, add 50% of the difference to the target to reduce > + * possible performance oscillations and offset possible performance > + * loss related to moving the workload from one CPU to another within > + * a package/module. > + */ > + avg_pstate = get_avg_pstate(cpu); > + if (avg_pstate > target) > + target += (avg_pstate - target) >> 1; > + > + return target; > } > > static inline int32_t get_target_pstate_use_performance(struct cpudata *cpu) > > --