On 2018-08-02 02:56, MyungJoo Ham wrote:
Many CPU architectures have caches that can scale independent of the
CPUs.
Frequency scaling of the caches is necessary to make sure the cache is
not
a performance bottleneck that leads to poor performance and power. The
same
idea applies for RAM/DDR.
To achieve this, this patch adds a generic devfreq governor that takes
the
current frequency of each CPU frequency domain and then adjusts the
frequency of the cache (or any devfreq device) based on the frequency
of
the CPUs. It listens to CPU frequency transition notifiers to keep
itself
up to date on the current CPU frequency.
To decide the frequency of the device, the governor does one of the
following:
This exactly has the same purpose with "passive" governor except for
the
single part: passive governor depends on another devfreq driver, not
cpufreq driver.
If both governors have many features in common, can you merge them into
one?
Passive governor also has "get_target_freq", which allows driver
authors
to define the mapping.
Thanks for the review and pointing me to the passive governor. I agree
that at a high level they are both doing the same. I can certainly stuff
this CPU freq to dev freq mapping into passive governor, but I think
it'll just make one huge set of code that's harder to understand and
maintain because it trying to do different things under the hood.
There are also a bunch of complexities and optimizations that come with
the cpufreq-map governor that don't fit with the passive governor.
1. It's not one CPU who's frequency we have to listen to. There are
multiple CPUs/policies we have to aggregate across.
2. We have to deal with big vs little having different needs/mappings.
3. Since it's always just CPUfreq, I can optimize the handling in the
transition notifiers. If I have 4 different devices that are scaled
based on CPU freq, I still use only 1 transition notifier. It becomes a
bit harder to do with the passive governor.
4. It requires that the device explicitly support the passive governor
and pick a mapping function. With cpufreq-map governor, the device
drivers don't need to make any changes. Whoever is making a device/board
can choose what devices to scale up base on CPU freq based on their
board and their needs. Even an end user can say, scale the GPU based on
my CPU based on interpolation if they choose to.
5. If a device has some other use for the private data, it can't work
with passive governor, but can work with cpufreq-map governor.
6. I also want to improve cpufreq-map governor to handle hotplug
correctly in later patches (needs more discussion) and that'll add more
complexity.
I think for these reasons we shouldn't combine these two governors. Let
me know what you think.
Probably, you will need to add one more notifier call to get events
from
cpufreq instead of devfreq.
* Uses a CPU frequency to device frequency mapping table
- Either one mapping table used for all CPU freq policies (typically
used
for system with homogeneous cores/clusters that have the same
OPPs).
- One mapping table per CPU freq policy (typically used for ASMP
systems
with heterogeneous CPUs with different OPPs)
OR
* Scales the device frequency in proportion to the CPU frequency. So,
if
the CPUs are running at their max frequency, the device runs at its
max
frequency. If the CPUs are running at their min frequency, the
device
runs at its min frequency. And interpolated for frequencies in
between.
If you want to satisfy these two cases to the "modified" passive
governors,
you may need to supply two "preloaded" get_target_freq functions for
struct devfreq_passive_data. If that's impossible, requiring a bit more
modifications, you may need to write alternative routines in
update_devfreq_passive().
Thanks for the pointers. I understood what you mean here.
diff --git a/drivers/devfreq/governor_cpufreq_map.c
b/drivers/devfreq/governor_cpufreq_map.c
new file mode 100644
index 0000000..084a3ff
--- /dev/null
+++ b/drivers/devfreq/governor_cpufreq_map.c
@@ -0,0 +1,583 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2015, 2018, The Linux Foundation. All rights
reserved.
+ */
Is this boilerplate fine? ( using // )
I was just following what I thought was the new standard. checkpatch
even complains about not having this.
Thanks,
Saravana
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
