Re: [PATCH] cpufreq: ondemand: Introduces stepped frequency increase

[Corrado Zoccolo]

Hi Michael,
On Wed, Jul 8, 2009 at 4:18 PM, Michael S. Zick<lkml@xxxxxxxxxxxx> wrote:
> On Wed July 8 2009, Corrado Zoccolo wrote:
>> The patch introduces a new sysfs tunable cpufreq/ondemand/freq_step,
>> as found in conservative governor, to chose the frequency increase step,
>> expressed as percentage (default = 100 is previous behaviour).
>>
>> This allows fine tuning powersaving on mobile CPUs, since smaller steps will allow to:
>> * absorb punctual load spikes
>> * stabilize at the needed frequency, without passing for more power consuming states, and
>>
>
> Has this been tested on VIA C7-M and similar VIA products?
> Reason I ask is because they only step in increments of the
> clock multiplier - which varies among the models.
>
Using the correct clock multipliers and voltages is driver's duty.
This change affects the governor, instead, that selects, among the
available ones,
which one will be used, according to a policy.

Corrado

> Also, the factory recommendation is to stay on the freq/voltage
> curve for each product.
> Only the programmer accessable VID (Voltage IDentifier) codes
> are (on-silicon) lookup table mapped to the VRM (Voltage Regulator Module)
> control lines - not all products provide an "on curve" mapping
> for each possible multiplier step.
>
> How about a few conditional statements in this bit of code, please.
>
> Mike
>> Signed-off-by: Corrado Zoccolo czoccolo@xxxxxxxxx
>>
>> ---
>> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
>> index e741c33..baa7b5e 100644
>> --- a/drivers/cpufreq/cpufreq_ondemand.c
>> +++ b/drivers/cpufreq/cpufreq_ondemand.c
>> @@ -83,6 +83,7 @@ struct cpu_dbs_info_s {
>> Â Â Â unsigned int freq_lo;
>> Â Â Â unsigned int freq_lo_jiffies;
>> Â Â Â unsigned int freq_hi_jiffies;
>> + Â Â int requested_delta;
>> Â Â Â int cpu;
>> Â Â Â unsigned int enable:1,
>> Â Â Â Â Â Â Â sample_type:1;
>> @@ -112,11 +113,13 @@ static struct dbs_tuners {
>> Â Â Â unsigned int down_differential;
>> Â Â Â unsigned int ignore_nice;
>> Â Â Â unsigned int powersave_bias;
>> + Â Â unsigned int freq_step;
>> Â} dbs_tuners_ins = {
>> Â Â Â .up_threshold = DEF_FREQUENCY_UP_THRESHOLD,
>> Â Â Â .down_differential = DEF_FREQUENCY_DOWN_DIFFERENTIAL,
>> Â Â Â .ignore_nice = 0,
>> Â Â Â .powersave_bias = 0,
>> + Â Â .freq_step = 100,
>> Â};
>>
>> Âstatic inline cputime64_t get_cpu_idle_time_jiffy(unsigned int cpu,
>> @@ -261,6 +264,7 @@ show_one(sampling_rate, sampling_rate);
>> Âshow_one(up_threshold, up_threshold);
>> Âshow_one(ignore_nice_load, ignore_nice);
>> Âshow_one(powersave_bias, powersave_bias);
>> +show_one(freq_step, freq_step);
>>
>> Âstatic ssize_t store_sampling_rate(struct cpufreq_policy *unused,
>> Â Â Â Â Â Â Â const char *buf, size_t count)
>> @@ -358,6 +362,28 @@ static ssize_t store_powersave_bias(struct cpufreq_policy *unused,
>> Â Â Â return count;
>> Â}
>>
>> +static ssize_t store_freq_step(struct cpufreq_policy *policy,
>> + Â Â Â Â Â Â const char *buf, size_t count)
>> +{
>> + Â Â unsigned int input;
>> + Â Â int ret;
>> + Â Â ret = sscanf(buf, "%u", &input);
>> +
>> + Â Â if (ret != 1)
>> + Â Â Â Â Â Â return -EINVAL;
>> +
>> + Â Â if (input > 100)
>> + Â Â Â Â Â Â input = 100;
>> +
>> + Â Â /* no need to test here if freq_step is zero as the user might actually
>> + Â Â Â* want this, they would be crazy though :) */
>> + Â Â mutex_lock(&dbs_mutex);
>> + Â Â dbs_tuners_ins.freq_step = input;
>> + Â Â mutex_unlock(&dbs_mutex);
>> +
>> + Â Â return count;
>> +}
>> +
>> Â#define define_one_rw(_name) \
>> Âstatic struct freq_attr _name = \
>> Â__ATTR(_name, 0644, show_##_name, store_##_name)
>> @@ -366,6 +392,7 @@ define_one_rw(sampling_rate);
>> Âdefine_one_rw(up_threshold);
>> Âdefine_one_rw(ignore_nice_load);
>> Âdefine_one_rw(powersave_bias);
>> +define_one_rw(freq_step);
>>
>> Âstatic struct attribute *dbs_attributes[] = {
>> Â Â Â &sampling_rate_max.attr,
>> @@ -374,6 +401,7 @@ static struct attribute *dbs_attributes[] = {
>> Â Â Â &up_threshold.attr,
>> Â Â Â &ignore_nice_load.attr,
>> Â Â Â &powersave_bias.attr,
>> + Â Â &freq_step.attr,
>> Â Â Â NULL
>> Â};
>>
>> @@ -464,19 +492,30 @@ static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
>>
>> Â Â Â /* Check for frequency increase */
>> Â Â Â if (max_load_freq > dbs_tuners_ins.up_threshold * policy->cur) {
>> + Â Â Â Â Â Â unsigned int freq_target = this_dbs_info->requested_delta
>> + Â Â Â Â Â Â Â Â Â Â + policy->cur;
>> + Â Â Â Â Â Â unsigned int freq_step;
>> +
>> Â Â Â Â Â Â Â /* if we are already at full speed then break out early */
>> - Â Â Â Â Â Â if (!dbs_tuners_ins.powersave_bias) {
>> - Â Â Â Â Â Â Â Â Â Â if (policy->cur == policy->max)
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â return;
>> + Â Â Â Â Â Â if (freq_target == policy->max)
>> + Â Â Â Â Â Â Â Â Â Â return;
>> +
>> + Â Â Â Â Â Â freq_step = (dbs_tuners_ins.freq_step * (policy->max-policy->min))
>> + Â Â Â Â Â Â Â Â Â Â / 100;
>>
>> - Â Â Â Â Â Â Â Â Â Â __cpufreq_driver_target(policy, policy->max,
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â CPUFREQ_RELATION_H);
>> + Â Â Â Â Â Â freq_target += max(freq_step, 5U);
>> + Â Â Â Â Â Â freq_target = max(policy->min, min(policy->max, freq_target));
>> +
>> + Â Â Â Â Â Â if (!dbs_tuners_ins.powersave_bias) {
>> + Â Â Â Â Â Â Â Â Â Â __cpufreq_driver_target(policy, freq_target,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CPUFREQ_RELATION_H);
>> Â Â Â Â Â Â Â } else {
>> - Â Â Â Â Â Â Â Â Â Â int freq = powersave_bias_target(policy, policy->max,
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CPUFREQ_RELATION_H);
>> + Â Â Â Â Â Â Â Â Â Â unsigned int freq = powersave_bias_target(policy, freq_target,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CPUFREQ_RELATION_H);
>> Â Â Â Â Â Â Â Â Â Â Â __cpufreq_driver_target(policy, freq,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CPUFREQ_RELATION_L);
>> Â Â Â Â Â Â Â }
>> + Â Â Â Â Â Â this_dbs_info->requested_delta = freq_target - policy->cur;
>> Â Â Â Â Â Â Â return;
>> Â Â Â }
>>
>> @@ -507,6 +546,7 @@ static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
>> Â Â Â Â Â Â Â Â Â Â Â __cpufreq_driver_target(policy, freq,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CPUFREQ_RELATION_L);
>> Â Â Â Â Â Â Â }
>> + Â Â Â Â Â Â this_dbs_info->requested_delta = freq_next - policy->cur;
>> Â Â Â }
>> Â}
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at Âhttp://www.tux.org/lkml/
>>
>>
>
>
>



-- 
__________________________________________________________________________

dott. Corrado Zoccolo                          mailto:czoccolo@xxxxxxxxx
PhD - Department of Computer Science - University of Pisa, Italy
--------------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/