On Tuesday, May 31, 2016 06:08:56 PM Steve Muckle wrote:

On Wed, Jun 01, 2016 at 12:50:41AM +0200, Rafael J. Wysocki wrote:

quoted

On Tue, May 31, 2016 at 1:36 PM, Viresh Kumar [off-list ref] wrote:

quoted

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.

The concern is that all drivers using frequency tables would probably
implement the callbacks in question in a very similar way, leading to
quite a bit of code duplication.  That's rarely a good thing.

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.

Well, let's do one thing at a time. :-)

Thanks,
Rafael