On 22-10-20, 13:57, Rafael J. Wysocki wrote:
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -2182,6 +2182,9 @@ int __cpufreq_driver_target(struct cpufr
> pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n",
> policy->cpu, target_freq, relation, old_target_freq);
>
> + if (cpufreq_driver->target)
> + return cpufreq_driver->target(policy, target_freq, relation);
> +
> /*
> * This might look like a redundant call as we are checking it again
> * after finding index. But it is left intentionally for cases where
> @@ -2194,9 +2197,6 @@ int __cpufreq_driver_target(struct cpufr
> /* Save last value to restore later on errors */
> policy->restore_freq = policy->cur;
>
> - if (cpufreq_driver->target)
> - return cpufreq_driver->target(policy, target_freq, relation);
> -
> if (!cpufreq_driver->target_index)
> return -EINVAL;
>From what I understood, you want your driver to get notified about
policy->min/max changes and right now they are making it work from
within the target() callback. Your commit log talks about policy->max
and powersave combination, I think the same will be true in case of
policy->min and performance ? And also with any other governor (like
schedutil) if the target_freq doesn't change for a while.
And IMHO, this change is more like a band-aid which is going to remove
the check of target != cur for all target() type drivers (which aren't
many) and it feels like a penalty on them (which is also there for
intel-cpufreq without hwp), and that we will get into the same problem
for target_index() drivers as well if they want to do something
similar in future, i.e. skip checking for same-freq.
Maybe adding a new flag for the cpufreq-driver for force-updates would
be a better solution ? Which will make this very much driver
dependent.
--
viresh
