linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] intel_pstate: Avoid pointless FRAC_BITS shifts under div_fp()
@ 2016-03-31 13:46 Rafael J. Wysocki
  2016-04-01  7:44 ` Philippe Longepe
  0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2016-03-31 13:46 UTC (permalink / raw)
  To: Linux PM list, Srinivas Pandruvada; +Cc: Linux Kernel Mailing List

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are multiple places in intel_pstate where int_tofp() is applied
to both arguments of div_fp(), but this is pointless, because int_tofp()
simply shifts its argument to the left by FRAC_BITS which mathematically
is equivalent to multuplication by 2^FRAC_BITS, so if this is done
to both arguments of a division, the extra factors will cancel each
other during that operation anyway.

Drop the pointless int_tofp() applied to div_fp() arguments throughout
the driver.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/intel_pstate.c |   27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -206,17 +206,17 @@ static inline void pid_reset(struct _pid
 
 static inline void pid_p_gain_set(struct _pid *pid, int percent)
 {
-	pid->p_gain = div_fp(int_tofp(percent), int_tofp(100));
+	pid->p_gain = div_fp(percent, 100);
 }
 
 static inline void pid_i_gain_set(struct _pid *pid, int percent)
 {
-	pid->i_gain = div_fp(int_tofp(percent), int_tofp(100));
+	pid->i_gain = div_fp(percent, 100);
 }
 
 static inline void pid_d_gain_set(struct _pid *pid, int percent)
 {
-	pid->d_gain = div_fp(int_tofp(percent), int_tofp(100));
+	pid->d_gain = div_fp(percent, 100);
 }
 
 static signed int pid_calc(struct _pid *pid, int32_t busy)
@@ -394,7 +394,7 @@ static ssize_t show_turbo_pct(struct kob
 
 	total = cpu->pstate.turbo_pstate - cpu->pstate.min_pstate + 1;
 	no_turbo = cpu->pstate.max_pstate - cpu->pstate.min_pstate + 1;
-	turbo_fp = div_fp(int_tofp(no_turbo), int_tofp(total));
+	turbo_fp = div_fp(no_turbo, total);
 	turbo_pct = 100 - fp_toint(mul_fp(turbo_fp, int_tofp(100)));
 	return sprintf(buf, "%u\n", turbo_pct);
 }
@@ -465,8 +465,7 @@ static ssize_t store_max_perf_pct(struct
 				   limits->max_perf_pct);
 	limits->max_perf_pct = max(limits->min_perf_pct,
 				   limits->max_perf_pct);
-	limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
-				  int_tofp(100));
+	limits->max_perf = div_fp(limits->max_perf_pct, 100);
 
 	if (hwp_active)
 		intel_pstate_hwp_set_online_cpus();
@@ -490,8 +489,7 @@ static ssize_t store_min_perf_pct(struct
 				   limits->min_perf_pct);
 	limits->min_perf_pct = min(limits->max_perf_pct,
 				   limits->min_perf_pct);
-	limits->min_perf = div_fp(int_tofp(limits->min_perf_pct),
-				  int_tofp(100));
+	limits->min_perf = div_fp(limits->min_perf_pct, 100);
 
 	if (hwp_active)
 		intel_pstate_hwp_set_online_cpus();
@@ -973,8 +971,8 @@ static inline int32_t get_target_pstate_
 	 * specified pstate.
 	 */
 	core_busy = cpu->sample.core_pct_busy;
-	max_pstate = int_tofp(cpu->pstate.max_pstate_physical);
-	current_pstate = int_tofp(cpu->pstate.current_pstate);
+	max_pstate = cpu->pstate.max_pstate_physical;
+	current_pstate = cpu->pstate.current_pstate;
 	core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
 
 	/*
@@ -986,8 +984,7 @@ static inline int32_t get_target_pstate_
 	duration_ns = cpu->sample.time - cpu->last_sample_time;
 	if ((s64)duration_ns > pid_params.sample_rate_ns * 3
 	    && cpu->last_sample_time > 0) {
-		sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
-				      int_tofp(duration_ns));
+		sample_ratio = div_fp(pid_params.sample_rate_ns, duration_ns);
 		core_busy = mul_fp(core_busy, sample_ratio);
 	}
 
@@ -1167,10 +1164,8 @@ static int intel_pstate_set_policy(struc
 	/* Make sure min_perf_pct <= max_perf_pct */
 	limits->min_perf_pct = min(limits->max_perf_pct, limits->min_perf_pct);
 
-	limits->min_perf = div_fp(int_tofp(limits->min_perf_pct),
-				  int_tofp(100));
-	limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
-				  int_tofp(100));
+	limits->min_perf = div_fp(limits->min_perf_pct, 100);
+	limits->max_perf = div_fp(limits->max_perf_pct, 100);
 
  out:
 	intel_pstate_set_update_util_hook(policy->cpu);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] intel_pstate: Avoid pointless FRAC_BITS shifts under div_fp()
  2016-03-31 13:46 [PATCH] intel_pstate: Avoid pointless FRAC_BITS shifts under div_fp() Rafael J. Wysocki
@ 2016-04-01  7:44 ` Philippe Longepe
  2016-04-04  1:31   ` Rafael J. Wysocki
  0 siblings, 1 reply; 3+ messages in thread
From: Philippe Longepe @ 2016-04-01  7:44 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM list, Srinivas Pandruvada
  Cc: Linux Kernel Mailing List

I proposed also to simplify the intel_pstate_calc_busy function:

     core_pct = int_tofp(sample->aperf) * int_tofp(100);
     core_pct = div64_u64(core_pct, int_tofp(sample->mperf));

is equivalent to:

     core_pct = sample->aperf * int_tofp(100);
     core_pct = div64_u64(core_pct, sample->mperf);

Regards,
Philippe,

On 31/03/2016 15:46, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> There are multiple places in intel_pstate where int_tofp() is applied
> to both arguments of div_fp(), but this is pointless, because int_tofp()
> simply shifts its argument to the left by FRAC_BITS which mathematically
> is equivalent to multuplication by 2^FRAC_BITS, so if this is done
> to both arguments of a division, the extra factors will cancel each
> other during that operation anyway.
>
> Drop the pointless int_tofp() applied to div_fp() arguments throughout
> the driver.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/cpufreq/intel_pstate.c |   27 +++++++++++----------------
>   1 file changed, 11 insertions(+), 16 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -206,17 +206,17 @@ static inline void pid_reset(struct _pid
>   
>   static inline void pid_p_gain_set(struct _pid *pid, int percent)
>   {
> -	pid->p_gain = div_fp(int_tofp(percent), int_tofp(100));
> +	pid->p_gain = div_fp(percent, 100);
>   }
>   
>   static inline void pid_i_gain_set(struct _pid *pid, int percent)
>   {
> -	pid->i_gain = div_fp(int_tofp(percent), int_tofp(100));
> +	pid->i_gain = div_fp(percent, 100);
>   }
>   
>   static inline void pid_d_gain_set(struct _pid *pid, int percent)
>   {
> -	pid->d_gain = div_fp(int_tofp(percent), int_tofp(100));
> +	pid->d_gain = div_fp(percent, 100);
>   }
>   
>   static signed int pid_calc(struct _pid *pid, int32_t busy)
> @@ -394,7 +394,7 @@ static ssize_t show_turbo_pct(struct kob
>   
>   	total = cpu->pstate.turbo_pstate - cpu->pstate.min_pstate + 1;
>   	no_turbo = cpu->pstate.max_pstate - cpu->pstate.min_pstate + 1;
> -	turbo_fp = div_fp(int_tofp(no_turbo), int_tofp(total));
> +	turbo_fp = div_fp(no_turbo, total);
>   	turbo_pct = 100 - fp_toint(mul_fp(turbo_fp, int_tofp(100)));
>   	return sprintf(buf, "%u\n", turbo_pct);
>   }
> @@ -465,8 +465,7 @@ static ssize_t store_max_perf_pct(struct
>   				   limits->max_perf_pct);
>   	limits->max_perf_pct = max(limits->min_perf_pct,
>   				   limits->max_perf_pct);
> -	limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
> -				  int_tofp(100));
> +	limits->max_perf = div_fp(limits->max_perf_pct, 100);
>   
>   	if (hwp_active)
>   		intel_pstate_hwp_set_online_cpus();
> @@ -490,8 +489,7 @@ static ssize_t store_min_perf_pct(struct
>   				   limits->min_perf_pct);
>   	limits->min_perf_pct = min(limits->max_perf_pct,
>   				   limits->min_perf_pct);
> -	limits->min_perf = div_fp(int_tofp(limits->min_perf_pct),
> -				  int_tofp(100));
> +	limits->min_perf = div_fp(limits->min_perf_pct, 100);
>   
>   	if (hwp_active)
>   		intel_pstate_hwp_set_online_cpus();
> @@ -973,8 +971,8 @@ static inline int32_t get_target_pstate_
>   	 * specified pstate.
>   	 */
>   	core_busy = cpu->sample.core_pct_busy;
> -	max_pstate = int_tofp(cpu->pstate.max_pstate_physical);
> -	current_pstate = int_tofp(cpu->pstate.current_pstate);
> +	max_pstate = cpu->pstate.max_pstate_physical;
> +	current_pstate = cpu->pstate.current_pstate;
>   	core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
>   
>   	/*
> @@ -986,8 +984,7 @@ static inline int32_t get_target_pstate_
>   	duration_ns = cpu->sample.time - cpu->last_sample_time;
>   	if ((s64)duration_ns > pid_params.sample_rate_ns * 3
>   	    && cpu->last_sample_time > 0) {
> -		sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
> -				      int_tofp(duration_ns));
> +		sample_ratio = div_fp(pid_params.sample_rate_ns, duration_ns);
>   		core_busy = mul_fp(core_busy, sample_ratio);
>   	}
>   
> @@ -1167,10 +1164,8 @@ static int intel_pstate_set_policy(struc
>   	/* Make sure min_perf_pct <= max_perf_pct */
>   	limits->min_perf_pct = min(limits->max_perf_pct, limits->min_perf_pct);
>   
> -	limits->min_perf = div_fp(int_tofp(limits->min_perf_pct),
> -				  int_tofp(100));
> -	limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
> -				  int_tofp(100));
> +	limits->min_perf = div_fp(limits->min_perf_pct, 100);
> +	limits->max_perf = div_fp(limits->max_perf_pct, 100);
>   
>    out:
>   	intel_pstate_set_update_util_hook(policy->cpu);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] intel_pstate: Avoid pointless FRAC_BITS shifts under div_fp()
  2016-04-01  7:44 ` Philippe Longepe
@ 2016-04-04  1:31   ` Rafael J. Wysocki
  0 siblings, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2016-04-04  1:31 UTC (permalink / raw)
  To: Philippe Longepe, Linux PM list
  Cc: Srinivas Pandruvada, Linux Kernel Mailing List

On Friday, April 01, 2016 09:44:43 AM Philippe Longepe wrote:
> I proposed also to simplify the intel_pstate_calc_busy function:
> 
>      core_pct = int_tofp(sample->aperf) * int_tofp(100);
>      core_pct = div64_u64(core_pct, int_tofp(sample->mperf));
> 
> is equivalent to:
> 
>      core_pct = sample->aperf * int_tofp(100);
>      core_pct = div64_u64(core_pct, sample->mperf);

Right, this can be done too.

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] intel_pstate: Avoid pointless FRAC_BITS shifts under div_fp()

There are multiple places in intel_pstate where int_tofp() is applied
to both arguments of div_fp(), but this is pointless, because int_tofp()
simply shifts its argument to the left by FRAC_BITS which mathematically
is equivalent to multuplication by 2^FRAC_BITS, so if this is done
to both arguments of a division, the extra factors will cancel each
other during that operation anyway.

Drop the pointless int_tofp() applied to div_fp() arguments throughout
the driver.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/intel_pstate.c |   31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -206,17 +206,17 @@ static inline void pid_reset(struct _pid
 
 static inline void pid_p_gain_set(struct _pid *pid, int percent)
 {
-	pid->p_gain = div_fp(int_tofp(percent), int_tofp(100));
+	pid->p_gain = div_fp(percent, 100);
 }
 
 static inline void pid_i_gain_set(struct _pid *pid, int percent)
 {
-	pid->i_gain = div_fp(int_tofp(percent), int_tofp(100));
+	pid->i_gain = div_fp(percent, 100);
 }
 
 static inline void pid_d_gain_set(struct _pid *pid, int percent)
 {
-	pid->d_gain = div_fp(int_tofp(percent), int_tofp(100));
+	pid->d_gain = div_fp(percent, 100);
 }
 
 static signed int pid_calc(struct _pid *pid, int32_t busy)
@@ -394,7 +394,7 @@ static ssize_t show_turbo_pct(struct kob
 
 	total = cpu->pstate.turbo_pstate - cpu->pstate.min_pstate + 1;
 	no_turbo = cpu->pstate.max_pstate - cpu->pstate.min_pstate + 1;
-	turbo_fp = div_fp(int_tofp(no_turbo), int_tofp(total));
+	turbo_fp = div_fp(no_turbo, total);
 	turbo_pct = 100 - fp_toint(mul_fp(turbo_fp, int_tofp(100)));
 	return sprintf(buf, "%u\n", turbo_pct);
 }
@@ -465,8 +465,7 @@ static ssize_t store_max_perf_pct(struct
 				   limits->max_perf_pct);
 	limits->max_perf_pct = max(limits->min_perf_pct,
 				   limits->max_perf_pct);
-	limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
-				  int_tofp(100));
+	limits->max_perf = div_fp(limits->max_perf_pct, 100);
 
 	if (hwp_active)
 		intel_pstate_hwp_set_online_cpus();
@@ -490,8 +489,7 @@ static ssize_t store_min_perf_pct(struct
 				   limits->min_perf_pct);
 	limits->min_perf_pct = min(limits->max_perf_pct,
 				   limits->min_perf_pct);
-	limits->min_perf = div_fp(int_tofp(limits->min_perf_pct),
-				  int_tofp(100));
+	limits->min_perf = div_fp(limits->min_perf_pct, 100);
 
 	if (hwp_active)
 		intel_pstate_hwp_set_online_cpus();
@@ -876,8 +874,8 @@ static inline void intel_pstate_calc_bus
 	struct sample *sample = &cpu->sample;
 	int64_t core_pct;
 
-	core_pct = int_tofp(sample->aperf) * int_tofp(100);
-	core_pct = div64_u64(core_pct, int_tofp(sample->mperf));
+	core_pct = sample->aperf * int_tofp(100);
+	core_pct = div64_u64(core_pct, sample->mperf);
 
 	sample->core_pct_busy = (int32_t)core_pct;
 }
@@ -980,8 +978,8 @@ static inline int32_t get_target_pstate_
 	 * specified pstate.
 	 */
 	core_busy = cpu->sample.core_pct_busy;
-	max_pstate = int_tofp(cpu->pstate.max_pstate_physical);
-	current_pstate = int_tofp(cpu->pstate.current_pstate);
+	max_pstate = cpu->pstate.max_pstate_physical;
+	current_pstate = cpu->pstate.current_pstate;
 	core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
 
 	/*
@@ -992,8 +990,7 @@ static inline int32_t get_target_pstate_
 	 */
 	duration_ns = cpu->sample.time - cpu->last_sample_time;
 	if ((s64)duration_ns > pid_params.sample_rate_ns * 3) {
-		sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
-				      int_tofp(duration_ns));
+		sample_ratio = div_fp(pid_params.sample_rate_ns, duration_ns);
 		core_busy = mul_fp(core_busy, sample_ratio);
 	}
 
@@ -1176,10 +1173,8 @@ static int intel_pstate_set_policy(struc
 	/* Make sure min_perf_pct <= max_perf_pct */
 	limits->min_perf_pct = min(limits->max_perf_pct, limits->min_perf_pct);
 
-	limits->min_perf = div_fp(int_tofp(limits->min_perf_pct),
-				  int_tofp(100));
-	limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
-				  int_tofp(100));
+	limits->min_perf = div_fp(limits->min_perf_pct, 100);
+	limits->max_perf = div_fp(limits->max_perf_pct, 100);
 
  out:
 	intel_pstate_set_update_util_hook(policy->cpu);

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-04-04  1:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-31 13:46 [PATCH] intel_pstate: Avoid pointless FRAC_BITS shifts under div_fp() Rafael J. Wysocki
2016-04-01  7:44 ` Philippe Longepe
2016-04-04  1:31   ` Rafael J. Wysocki

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