Re: [PATCH V2 1/9] cpufreq: Auto-register with energy model if asked
From: Quentin Perret <hidden>
Date: 2021-08-11 13:03:08
Also in:
lkml
On Wednesday 11 Aug 2021 at 17:28:39 (+0530), Viresh Kumar wrote:
quoted hunk ↗ jump to hunk
Many cpufreq drivers register with the energy model for each policy and do exactly the same thing. Follow the footsteps of thermal-cooling, to get it done from the cpufreq core itself. Provide a new callback, which will be called, if present, by the cpufreq core at the right moment (more on that in the code's comment). Also provide a generic implementation that uses dev_pm_opp_of_register_em(). This also allows us to register with the EM at a later point of time, compared to ->init(), from where the EM core can access cpufreq policy directly using cpufreq_cpu_get() type of helpers and perform other work, like marking few frequencies inefficient, this will be done separately. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq.c | 12 ++++++++++++ include/linux/cpufreq.h | 14 ++++++++++++++ 2 files changed, 26 insertions(+)diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 06c526d66dd3..75974e7d6cc5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c@@ -1493,6 +1493,18 @@ static int cpufreq_online(unsigned int cpu) write_unlock_irqrestore(&cpufreq_driver_lock, flags); } + /* + * Register with the energy model before sched_cpufreq_governor_change() + * is called, which will result in rebuilding of the sched domains, + * which should only be done once the energy model is properly + * initialized for the policy first. + * + * Also, this should be called before the policy is registered with + * cooling framework. + */ + if (cpufreq_driver->register_em) + cpufreq_driver->register_em(policy);
Maybe move that to the 'if (new_policy)' block above? There is currently no need to re-register the EM on CPU hotplug.
quoted hunk ↗ jump to hunk
ret = cpufreq_init_policy(policy); if (ret) { pr_err("%s: Failed to initialize policy for cpu: %d (%d)\n",diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 9fd719475fcd..1295621f6c28 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h@@ -9,10 +9,12 @@ #define _LINUX_CPUFREQ_H #include <linux/clk.h> +#include <linux/cpu.h> #include <linux/cpumask.h> #include <linux/completion.h> #include <linux/kobject.h> #include <linux/notifier.h> +#include <linux/pm_opp.h> #include <linux/pm_qos.h> #include <linux/spinlock.h> #include <linux/sysfs.h>@@ -373,6 +375,12 @@ struct cpufreq_driver { /* platform specific boost support code */ bool boost_enabled; int (*set_boost)(struct cpufreq_policy *policy, int state); + + /* + * Set by drivers that want the core to automatically register the + * policy's devices with Energy Model. + */ + void (*register_em)(struct cpufreq_policy *policy); }; /* flags */@@ -1046,4 +1054,10 @@ unsigned int cpufreq_generic_get(unsigned int cpu); void cpufreq_generic_init(struct cpufreq_policy *policy, struct cpufreq_frequency_table *table, unsigned int transition_latency); + +static inline void cpufreq_register_em_with_opp(struct cpufreq_policy *policy) +{ + dev_pm_opp_of_register_em(get_cpu_device(policy->cpu), + policy->related_cpus); +}
I was thinking this could go in pm_opp.h instead, but it doesn't really matter. So, with the first comment above fixed: Reviewed-by: Quentin Perret <redacted> Thanks, Quentin