On 29-10-20, 11:42, Rafael J. Wysocki wrote: > On Thu, Oct 29, 2020 at 12:10 AM Viresh Kumar <[email protected]> wrote: > > > > On 27-10-20, 16:35, Rafael J. Wysocki wrote: > > > Index: linux-pm/kernel/sched/cpufreq_schedutil.c > > > =================================================================== > > > --- linux-pm.orig/kernel/sched/cpufreq_schedutil.c > > > +++ linux-pm/kernel/sched/cpufreq_schedutil.c > > > @@ -102,11 +102,12 @@ static bool sugov_should_update_freq(str > > > static bool sugov_update_next_freq(struct sugov_policy *sg_policy, u64 > > > time, > > > unsigned int next_freq) > > > { > > > - if (sg_policy->next_freq == next_freq) > > > + if (sg_policy->next_freq == next_freq && > > > !sg_policy->need_freq_update) > > > return false; > > > > > > sg_policy->next_freq = next_freq; > > > sg_policy->last_freq_update_time = time; > > > + sg_policy->need_freq_update = false; > > > > > > return true; > > > } > > > @@ -161,10 +162,12 @@ static unsigned int get_next_freq(struct > > > > > > freq = map_util_freq(util, freq, max); > > > > > > - if (freq == sg_policy->cached_raw_freq && > > > !sg_policy->need_freq_update) > > > + if (cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS)) > > > + sg_policy->need_freq_update = true; > > > + else if (freq == sg_policy->cached_raw_freq && > > > + !sg_policy->need_freq_update) > > > return sg_policy->next_freq; > > > > > > - sg_policy->need_freq_update = false; > > > sg_policy->cached_raw_freq = freq; > > > return cpufreq_driver_resolve_freq(policy, freq); > > > } > > > > What about just this instead ? > > > > static bool sugov_update_next_freq(struct sugov_policy *sg_policy, u64 > > time, > > unsigned int next_freq) > > { > > - if (sg_policy->next_freq == next_freq) > > + if (sg_policy->next_freq == next_freq && > > + !cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS)) > > return false; > > > > sg_policy->next_freq = next_freq; > > sg_policy->last_freq_update_time = time; > > > > return true; > > } > > > > Without any changes in get_next_freq() this is not sufficient, because > get_next_freq() may skip the update too. > > If the intention is to always let the driver callback run when > CPUFREQ_NEED_UPDATE_LIMITS is set, then both get_next_freq() and > sugov_update_next_freq() need to be modified.
Right, my mistake. I was just suggesting that we may not need to touch need_freq_update at all but just check the flag. -- viresh
