Re: [PATCH 0/2] cpufreq: Use sorted frequency tables

[Steve Muckle <steve.muckle@linaro.org>]

On Wed, Jun 01, 2016 at 12:50:41AM +0200, Rafael J. Wysocki wrote:
> On Tue, May 31, 2016 at 1:36 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > Hi Guys,
> >
> > This work in inspired by some of the concerns raised by Steve in one of
> > his patchset.
> >
> > Currently, the cpufreq drivers aren't required to provide a sorted list
> > of frequencies to the cpufreq-core and so traversing that list match a
> > target frequency is very inefficient.
> >
> > This is not bearable by, for example, the fast-switch path of schedutil
> > governor and so we have moved the traversing logic local to the
> > acpi-cpufreq driver for now. That is better handled in the core, but it
> > has to be efficient.
> >
> > OTOH, even for traditional governors without a fast-switch path, it
> > would be much better to be able to traverse this table quickly.
> >
> > The ideal solution would be to keep a single freq-table in struct
> > cpufreq_policy, that will be sorted as well. But there are few
> > dependencies due to which it can't be done today (Hint: cpufreq drivers
> > are abusing the 'index' passed to them, to refer to multiple arrays).
> >
> > And so for now, lets create a separate table local to the cpufreq-core
> > only.
> >
> > To use that, another API cpufreq_find_target_index() is created as well
> > and few users are migrated to it.
> >
> > Lightly tested on Exynos board, frequencies were getting selected as
> > expected.
> 
> I'm not particularly liking this due to the possible confusion that
> may result from it.
> 
> Perhaps we can require drivers implementing ->fast_switch to sort
> their frequency tables to start with?  Or maybe make the core check
> whether or not the table is sorted and in what order and handle it
> accordingly?
> 
> Let's just think about the design here for a while, OK?

This would be required beyond drivers implementing fast_switch -
cpufreq_driver_resolve_freq() (which started this discussion) is called
for slow path transitions as well.

Checking the table type and performing the associated lookup seems
workable to me though it adds a bit of complexity.

Also what about leaving it as is? I didn't fully catch the concern with
abuse in the series I posted, and it pushes this complexity of dealing
with the freq table efficiently down into the driver, which is best
suited for that IMO.

Another thought is that it'd be nice to eventually reduce
cpufreq_driver_{fast_switch,resolve_freq} into simple inline functions
so that we could jump to the driver directly from schedutil, eliminating
a function call.

thanks,
Steve