cpufreq: intel_pstate: Simplify code in intel_pstate_adjust_busy_pstate

Simplify the code by removing the inline functions pstate_increase and pstate_decrease and use directly the intel_pstate_set_pstate.

Signed-off-by Stratos Karafotis <[email protected]>

---
 drivers/cpufreq/intel_pstate.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

drivers/cpufreq/intel_pstate.c +1 -17

index 3a49269..26a0262 100644
@@ -588,21 +588,6 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
        pstate_funcs.set(cpu, pstate);
 }
 
-static inline void intel_pstate_pstate_increase(struct cpudata *cpu, int steps)
-{
-       int target;
-       target = cpu->pstate.current_pstate + steps;
-
-       intel_pstate_set_pstate(cpu, target);
-}
-
-static inline void intel_pstate_pstate_decrease(struct cpudata *cpu, int steps)
-{
-       int target;
-       target = cpu->pstate.current_pstate - steps;
-       intel_pstate_set_pstate(cpu, target);
-}
-
 static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 {
        cpu->pstate.min_pstate = pstate_funcs.get_min();

@@ -695,20 +680,15 @@ static inline void intel_pstate_calc_scaled_busy(struct cpudata *cpu)
 static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
 {
        struct _pid *pid;
-       signed int ctl = 0;
-       int steps;
+       signed int ctl;
 
        pid = &cpu->pid;
        intel_pstate_calc_scaled_busy(cpu);
 
        ctl = pid_calc(pid, cpu->sample.busy_scaled);

Quoted 48 lines · Stratos Karafotis

Simplify the code by removing the inline functions pstate_increase and pstate_decrease and use directly the intel_pstate_set_pstate.

Signed-off-by Stratos Karafotis <[email protected]>

---
 drivers/cpufreq/intel_pstate.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

drivers/cpufreq/intel_pstate.c +1 -17

index 3a49269..26a0262 100644
@@ -588,21 +588,6 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
        pstate_funcs.set(cpu, pstate);
 }
 
-static inline void intel_pstate_pstate_increase(struct cpudata *cpu, int steps)
-{
-       int target;
-       target = cpu->pstate.current_pstate + steps;
-
-       intel_pstate_set_pstate(cpu, target);
-}
-
-static inline void intel_pstate_pstate_decrease(struct cpudata *cpu, int steps)
-{
-       int target;
-       target = cpu->pstate.current_pstate - steps;
-       intel_pstate_set_pstate(cpu, target);
-}
-
 static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 {
        cpu->pstate.min_pstate = pstate_funcs.get_min();

@@ -695,20 +680,15 @@ static inline void intel_pstate_calc_scaled_busy(struct cpudata *cpu)
 static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
 {
        struct _pid *pid;
-       signed int ctl = 0;
-       int steps;
+       signed int ctl;
 
        pid = &cpu->pid;
        intel_pstate_calc_scaled_busy(cpu);
 
        ctl = pid_calc(pid, cpu->sample.busy_scaled);
 

drivers/cpufreq/intel_pstate.c static inline void intel_pstate_calc_scaled_busy(struct cpudata *cpu) · line 692

Quoted 9 lines

drivers/cpufreq/intel_pstate.c +2 -6

-       steps = abs(ctl);
-
-       if (ctl < 0)
-               intel_pstate_pstate_increase(cpu, steps);
-       else
-               intel_pstate_pstate_decrease(cpu, steps);
+       /* Negative values of ctl increase the pstate and vice versa */
+       intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);
 }

I am not very good at this driver but there is some obvious functional change here. Earlier we used to pass 'cpu->pstate.current_pstate {-|+} steps' and now you are doing '-ctl' only

Quoted 5 lines · Viresh Kumar

Quoted 4 lines · Stratos Karafotis

Simplify the code by removing the inline functions pstate_increase and pstate_decrease and use directly the intel_pstate_set_pstate.

Quoted 44 lines

Quoted 44 lines

Signed-off-by Stratos Karafotis <[email protected]>

---
  drivers/cpufreq/intel_pstate.c | 26 +++-----------------------
  1 file changed, 3 insertions(+), 23 deletions(-)

drivers/cpufreq/intel_pstate.c +1 -17

index 3a49269..26a0262 100644
@@ -588,21 +588,6 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
         pstate_funcs.set(cpu, pstate);
  }
 
-static inline void intel_pstate_pstate_increase(struct cpudata *cpu, int steps)
-{
-       int target;
-       target = cpu->pstate.current_pstate + steps;
-
-       intel_pstate_set_pstate(cpu, target);
-}
-
-static inline void intel_pstate_pstate_decrease(struct cpudata *cpu, int steps)
-{
-       int target;
-       target = cpu->pstate.current_pstate - steps;
-       intel_pstate_set_pstate(cpu, target);
-}
-
  static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
  {
         cpu->pstate.min_pstate = pstate_funcs.get_min();

@@ -695,20 +680,15 @@ static inline void intel_pstate_calc_scaled_busy(struct cpudata *cpu)
  static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
  {
         struct _pid *pid;
-       signed int ctl = 0;
-       int steps;
+       signed int ctl;
 
         pid = &cpu->pid;
         intel_pstate_calc_scaled_busy(cpu);
 
         ctl = pid_calc(pid, cpu->sample.busy_scaled);
 

Quoted 14 lines

drivers/cpufreq/intel_pstate.c static inline void intel_pstate_calc_scaled_busy(struct cpudata *cpu) · line 692

Quoted 9 lines

drivers/cpufreq/intel_pstate.c +2 -6

-       steps = abs(ctl);
-
-       if (ctl < 0)
-               intel_pstate_pstate_increase(cpu, steps);
-       else
-               intel_pstate_pstate_decrease(cpu, steps);
+       /* Negative values of ctl increase the pstate and vice versa */
+       intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);
  }

I am not very good at this driver but there is some obvious functional change here. Earlier we used to pass 'cpu->pstate.current_pstate {-|+} steps' and now you are doing '-ctl' only

The original code is:

	if (ctl < 0)
		intel_pstate_pstate_increase(cpu, steps);
	else
		intel_pstate_pstate_decrease(cpu, steps);

Without inlines functions intel_pstate_pstate_increase() and intel_pstate_pstate_decrease() we get:

	if (ctl < 0)
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate + steps);
	else
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - steps);

But steps = abs(ctl), so:

	if (ctl < 0)
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate + abs(ctl));
	else
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - abs(ctl));

By definition, abs(ctl) = ctl if ctl >= 0, -ctl if ctl < 0. Thus:

	if (ctl < 0)
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate + (-ctl));
	else
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);

And:

	if (ctl < 0)
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);
	else
		intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);

Finally remove the unnecessary if statement.

	intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);

So, this is equivalent with the original code.

Thanks,
Stratos
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines · Stratos Karafotis

Quoted 5 lines · Viresh Kumar

Quoted 4 lines · Stratos Karafotis

Simplify the code by removing the inline functions pstate_increase and pstate_decrease and use directly the intel_pstate_set_pstate.

Doesn't apply without your scaled_busy change spin this patch with out the scaled_busy change and explain the change more fully in the commit message to cover Viresh's question and I am good with this change.