On Wed, Mar 30, 2016 at 7:05 PM, Steve Muckle <steve.muckle@xxxxxxxxxx> wrote: > On 03/30/2016 04:31 AM, Rafael J. Wysocki wrote: >>>> >> +static int sugov_limits(struct cpufreq_policy *policy) >>>> >> +{ >>>> >> + struct sugov_policy *sg_policy = policy->governor_data; >>>> >> + >>>> >> + if (!policy->fast_switch_enabled) { >>>> >> + mutex_lock(&sg_policy->work_lock); >>>> >> + >>>> >> + if (policy->max < policy->cur) >>>> >> + __cpufreq_driver_target(policy, policy->max, >>>> >> + CPUFREQ_RELATION_H); >>>> >> + else if (policy->min > policy->cur) >>>> >> + __cpufreq_driver_target(policy, policy->min, >>>> >> + CPUFREQ_RELATION_L); >>>> >> + >>>> >> + mutex_unlock(&sg_policy->work_lock); >>>> >> + } >>>> >> + >>>> >> + sg_policy->need_freq_update = true; >>> > >>> > I am wondering why we need to do this for !fast_switch_enabled case? >> >> That will cause the rate limit to be ignored in the utilization update >> handler which may be necessary if it is set to a relatively large >> value (like 1 s). > > But why is that necessary for !fast_switch_enabled? In that case the > frequency has been adjusted to satisfy the new limits here, so ignoring > the rate limit shouldn't be necessary. In other words why not > > } else { > sg_policy->need_freq_update = true; > } My thinking here was that the governor might decide to use something different from the limit enforced here, so it would be good to make it do so as soon as possible. In particular in the non-frequency-invariant utilization case in which new frequency depends on the current one. That said i'm not particularly opposed to making that change if that's preferred. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html