Re: [PATCH v2] cpufreq: conservative: Decrease frequency faster when the update deferred

From: Viresh Kumar
Date: Mon Nov 14 2016 - 02:04:11 EST


On 12-11-16, 23:04, Stratos Karafotis wrote:
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index fa5ece3..d787772 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -73,7 +73,19 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
> */
> if (cs_tuners->freq_step == 0)
> goto out;
> -
> + /*
> + * Decrease requested_freq for each idle period that we didn't
> + * update the frequency

Add a full stop (.) here.

> + */

I am wondering if we should be adding this code after the below 'if' block where
we check if the policy->min/max have changed ?

> + if (policy_dbs->idle_periods < UINT_MAX) {
> + unsigned int freq_target = policy_dbs->idle_periods *
> + get_freq_target(cs_tuners, policy);

I get confused every time I look at this routine (get_freq_target()). I have
sent an update to this file just now to get that fixed. If Rafael applies that
one, please rebase over it.

> + if (requested_freq > freq_target)
> + requested_freq -= freq_target;
> + else
> + requested_freq = policy->min;
> + policy_dbs->idle_periods = UINT_MAX;
> + }

Need a blank line here.

--
viresh