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

[Viresh Kumar]

On 15-11-16, 23:25, Stratos Karafotis wrote:
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 0681fcf..808cc4d 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -66,6 +66,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  	struct dbs_data *dbs_data = policy_dbs->dbs_data;
>  	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>  	unsigned int load = dbs_update(policy);
> +	unsigned int freq_step;
> 
>  	/*
>  	 * break out if we 'cannot' reduce the speed as the user might
> @@ -82,6 +83,22 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  	if (requested_freq > policy->max || requested_freq < policy->min)
>  		requested_freq = policy->cur;
> 
> +	freq_step = get_freq_step(cs_tuners, policy);
> +
> +	/*
> +	 * Decrease requested_freq one freq_step for each idle period that
> +	 * we didn't update the frequency.
> +	 */
> +	if (policy_dbs->idle_periods < UINT_MAX) {
> +		unsigned int freq_steps = policy_dbs->idle_periods * freq_step;
> +
> +		if (requested_freq > freq_steps)
> +			requested_freq -= freq_steps;
> +		else
> +			requested_freq = policy->min;

Need a blank line here.

> +		policy_dbs->idle_periods = UINT_MAX;
> +	}
> +
>  	/* Check for frequency increase */
>  	if (load > dbs_data->up_threshold) {
>  		dbs_info->down_skip = 0;
> @@ -90,7 +107,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  		if (requested_freq == policy->max)
>  			goto out;
> 
> -		requested_freq += get_freq_step(cs_tuners, policy);
> +		requested_freq += freq_step;
>  		if (requested_freq > policy->max)
>  			requested_freq = policy->max;
> 
> @@ -106,14 +123,12 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
> 
>  	/* Check for frequency decrease */
>  	if (load < cs_tuners->down_threshold) {
> -		unsigned int freq_step;
>  		/*
>  		 * if we cannot reduce the frequency anymore, break out early
>  		 */
>  		if (requested_freq == policy->min)
>  			goto out;
> 
> -		freq_step = get_freq_step(cs_tuners, policy);
>  		if (requested_freq > freq_step)
>  			requested_freq -= freq_step;
>  		else
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index 3729474..9780f50 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -117,7 +117,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>  	struct policy_dbs_info *policy_dbs = policy->governor_data;
>  	struct dbs_data *dbs_data = policy_dbs->dbs_data;
>  	unsigned int ignore_nice = dbs_data->ignore_nice_load;
> -	unsigned int max_load = 0;
> +	unsigned int max_load = 0, idle_periods = UINT_MAX;
>  	unsigned int sampling_rate, io_busy, j;
> 
>  	/*
> @@ -214,10 +214,17 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>  			}
>  			j_cdbs->prev_load = load;
>  		}

Here as well..

> +		if (time_elapsed > 2 * sampling_rate) {
> +			unsigned int periods = time_elapsed / sampling_rate;
> 
> +			if (periods < idle_periods)
> +				idle_periods = periods;
> +		}

Here too

>  		if (load > max_load)
>  			max_load = load;
>  	}

And here..

> +	policy_dbs->idle_periods = idle_periods;
> +
>  	return max_load;
>  }
>  EXPORT_SYMBOL_GPL(dbs_update);
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index 9660cc6..10a3e0a 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -97,6 +97,7 @@ struct policy_dbs_info {
>  	struct list_head list;
>  	/* Multiplier for increasing sample delay temporarily. */
>  	unsigned int rate_mult;
> +	unsigned int idle_periods;
>  	/* Status indicators */
>  	bool is_shared;		/* This object is used by multiple CPUs */
>  	bool work_in_progress;	/* Work is being queued up or in progress */

And after fixing this trivial things, you can add

Acked-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>

-- 
viresh