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.
This is build/boot tested by the bot for a couple of boards.
https://gitlab.com/vireshk/pmko/-/pipelines/351525873
Note that I haven't picked any of the Reviewed-by tags from the first version
since the idea is very much changed here.
V1->V2:
- Add a callback instead of flag.
- Register before governor is initialized.
- Update scmi driver as well.
- Don't unregister from the EM core.
--
Viresh
Viresh Kumar (9):
cpufreq: Auto-register with energy model if asked
cpufreq: dt: Use auto-registration for energy model
cpufreq: imx6q: Use auto-registration for energy model
cpufreq: mediatek: Use auto-registration for energy model
cpufreq: omap: Use auto-registration for energy model
cpufreq: qcom-cpufreq-hw: Use auto-registration for energy model
cpufreq: scpi: Use auto-registration for energy model
cpufreq: vexpress: Use auto-registration for energy model
cpufreq: scmi: Use .register_em() callback
drivers/cpufreq/cpufreq-dt.c | 3 +-
drivers/cpufreq/cpufreq.c | 12 ++++++
drivers/cpufreq/imx6q-cpufreq.c | 2 +-
drivers/cpufreq/mediatek-cpufreq.c | 3 +-
drivers/cpufreq/omap-cpufreq.c | 2 +-
drivers/cpufreq/qcom-cpufreq-hw.c | 3 +-
drivers/cpufreq/scmi-cpufreq.c | 55 +++++++++++++++-----------
drivers/cpufreq/scpi-cpufreq.c | 3 +-
drivers/cpufreq/vexpress-spc-cpufreq.c | 3 +-
include/linux/cpufreq.h | 14 +++++++
10 files changed, 65 insertions(+), 35 deletions(-)
--
2.31.1.272.g89b43f80a514
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Set the newly added .register_em() callback with
cpufreq_register_em_with_opp() to automatically register with the EM
core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/imx6q-cpufreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Set the newly added .register_em() callback with
cpufreq_register_em_with_opp() to automatically register with the EM
core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/mediatek-cpufreq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Set the newly added .register_em() callback with
cpufreq_register_em_with_opp() to automatically register with the EM
core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/vexpress-spc-cpufreq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Set the newly added .register_em() callback with
cpufreq_register_em_with_opp() to automatically register with the EM
core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scpi-cpufreq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
@@ -123,9 +125,6 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)structdevice*cpu_dev;structscmi_data*priv;structcpufreq_frequency_table*freq_table;-structem_data_callbackem_cb=EM_DATA_CB(scmi_get_cpu_power);-cpumask_var_topp_shared_cpus;-boolpower_scale_mw;cpu_dev=get_cpu_device(policy->cpu);if(!cpu_dev){
@@ -133,9 +132,15 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)return-ENODEV;}-if(!zalloc_cpumask_var(&opp_shared_cpus,GFP_KERNEL))+priv=kzalloc(sizeof(*priv),GFP_KERNEL);+if(!priv)return-ENOMEM;+if(!zalloc_cpumask_var(&priv->opp_shared_cpus,GFP_KERNEL)){+ret=-ENOMEM;+gotoout_free_priv;+}+/* Obtain CPUs that share SCMI performance controls */ret=scmi_get_sharing_cpus(cpu_dev,policy->cpus);if(ret){
@@ -148,14 +153,14 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)*TheOPP'sharingcpus'infomaycomefromDTthroughanemptyopp*tableandopp-shared.*/-ret=dev_pm_opp_of_get_sharing_cpus(cpu_dev,opp_shared_cpus);-if(ret||!cpumask_weight(opp_shared_cpus)){+ret=dev_pm_opp_of_get_sharing_cpus(cpu_dev,priv->opp_shared_cpus);+if(ret||!cpumask_weight(priv->opp_shared_cpus)){/**Eitheropp-tableisnotsetornoopp-sharedwasfound.*UsetheCPUmaskfromSCMItodesignateCPUssharinganOPP*table.*/-cpumask_copy(opp_shared_cpus,policy->cpus);+cpumask_copy(priv->opp_shared_cpus,policy->cpus);}/*
@@ -180,7 +185,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)gotoout_free_opp;}-ret=dev_pm_opp_set_sharing_cpus(cpu_dev,opp_shared_cpus);+ret=dev_pm_opp_set_sharing_cpus(cpu_dev,priv->opp_shared_cpus);if(ret){dev_err(cpu_dev,"%s: failed to mark OPPs as shared: %d\n",__func__,ret);
@@ -188,21 +193,13 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)gotoout_free_opp;}-power_scale_mw=perf_ops->power_scale_mw_get(ph);-em_dev_register_perf_domain(cpu_dev,nr_opp,&em_cb,-opp_shared_cpus,power_scale_mw);-}--priv=kzalloc(sizeof(*priv),GFP_KERNEL);-if(!priv){-ret=-ENOMEM;-gotoout_free_opp;+priv->nr_opp=nr_opp;}ret=dev_pm_opp_init_cpufreq_table(cpu_dev,&freq_table);if(ret){dev_err(cpu_dev,"failed to init cpufreq table: %d\n",ret);-gotoout_free_priv;+gotoout_free_opp;}priv->cpu_dev=cpu_dev;
@@ -223,17 +220,16 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)policy->fast_switch_possible=perf_ops->fast_switch_possible(ph,cpu_dev);-free_cpumask_var(opp_shared_cpus);return0;-out_free_priv:-kfree(priv);-out_free_opp:dev_pm_opp_remove_all_dynamic(cpu_dev);out_free_cpumask:-free_cpumask_var(opp_shared_cpus);+free_cpumask_var(priv->opp_shared_cpus);++out_free_priv:+kfree(priv);returnret;}
@@ -244,11 +240,23 @@ static int scmi_cpufreq_exit(struct cpufreq_policy *policy)dev_pm_opp_free_cpufreq_table(priv->cpu_dev,&policy->freq_table);dev_pm_opp_remove_all_dynamic(priv->cpu_dev);+free_cpumask_var(priv->opp_shared_cpus);kfree(priv);return0;}+staticvoidscmi_cpufreq_register_em(structcpufreq_policy*policy)+{+structem_data_callbackem_cb=EM_DATA_CB(scmi_get_cpu_power);+boolpower_scale_mw=perf_ops->power_scale_mw_get(ph);+structscmi_data*priv=policy->driver_data;++em_dev_register_perf_domain(get_cpu_device(policy->cpu),priv->nr_opp,+&em_cb,priv->opp_shared_cpus,+power_scale_mw);+}+staticstructcpufreq_driverscmi_cpufreq_driver={.name="scmi",.flags=CPUFREQ_HAVE_GOVERNOR_PER_POLICY|
On Wednesday 11 Aug 2021 at 17:28:47 (+0530), Viresh Kumar wrote:
quoted hunk
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
Can we use policy->related_cpus and friends directly in the callback
instead? That should simplify the patch a bit.
Also, we can probably afford calling dev_pm_opp_get_opp_count() from the
em_register callback as it is not a hot path, which would avoid wasting
some 'resident' memory here that is only used during init.
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lukasz Luba <lukasz.luba@arm.com> Date: 2021-08-11 14:09:20
On 8/11/21 2:17 PM, Quentin Perret wrote:
On Wednesday 11 Aug 2021 at 17:28:47 (+0530), Viresh Kumar wrote:
quoted
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
Can we use policy->related_cpus and friends directly in the callback
Unfortunately not. This tricky setup code was introduced because we may
have a platform with per-CPU policy, so single bit set in
policy->related_cpus, but we want EAS to be still working on set
of CPUs. That's why we construct temporary cpumask and pass it to EM.
instead? That should simplify the patch a bit.
Also, we can probably afford calling dev_pm_opp_get_opp_count() from the
em_register callback as it is not a hot path, which would avoid wasting
some 'resident' memory here that is only used during init.
Thanks,
Quentin
On Wednesday 11 Aug 2021 at 15:09:13 (+0100), Lukasz Luba wrote:
On 8/11/21 2:17 PM, Quentin Perret wrote:
quoted
On Wednesday 11 Aug 2021 at 17:28:47 (+0530), Viresh Kumar wrote:
quoted
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
Can we use policy->related_cpus and friends directly in the callback
Unfortunately not. This tricky setup code was introduced because we may
have a platform with per-CPU policy, so single bit set in
policy->related_cpus, but we want EAS to be still working on set
of CPUs. That's why we construct temporary cpumask and pass it to EM.
Aha, I see this now. Hmm, those platforms better have AMUs then,
otherwise PELT signals will be wonky ...
I was going to suggest using dev_pm_opp_get_sharing_cpus() from the
callback instead, but maybe that's overkill as we'd need to allocate a
temporary cpumask and all. So n/m this patch should be fine as is.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lukasz Luba <lukasz.luba@arm.com> Date: 2021-08-11 15:54:31
On 8/11/21 3:39 PM, Quentin Perret wrote:
On Wednesday 11 Aug 2021 at 15:09:13 (+0100), Lukasz Luba wrote:
quoted
On 8/11/21 2:17 PM, Quentin Perret wrote:
quoted
On Wednesday 11 Aug 2021 at 17:28:47 (+0530), Viresh Kumar wrote:
quoted
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
Can we use policy->related_cpus and friends directly in the callback
Unfortunately not. This tricky setup code was introduced because we may
have a platform with per-CPU policy, so single bit set in
policy->related_cpus, but we want EAS to be still working on set
of CPUs. That's why we construct temporary cpumask and pass it to EM.
Aha, I see this now. Hmm, those platforms better have AMUs then,
otherwise PELT signals will be wonky ...
That's the plan, to have the AMUs. We suggest that, but reality would
tell... ;)
I was going to suggest using dev_pm_opp_get_sharing_cpus() from the
callback instead, but maybe that's overkill as we'd need to allocate a
temporary cpumask and all. So n/m this patch should be fine as is.
From: Lukasz Luba <lukasz.luba@arm.com> Date: 2021-08-11 16:33:44
On 8/11/21 12:58 PM, Viresh Kumar wrote:
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
I would free the priv->opp_shared_cpus mask here, since we don't
need it anymore and memory can be reclaimed. Don't forget this
setup would be called N CPUs times, on this per-CPU policy platform.
If freed here, then also there wouldn't be a need to free it in
scmi_cpufreq_exit() so you can remove it from there.
I will test&review this patch on Monday when I re-flash custom FW to my
Juno (just to be sure that this per-CPU cpufreq + shared EM/EAS works).
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Also, we can probably afford calling dev_pm_opp_get_opp_count() from the
em_register callback as it is not a hot path, which would avoid wasting
some 'resident' memory here that is only used during init.
We also need to make sure that OPPs are available in init(), else we
fail. So, we can't really move that out.
--
viresh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Set the newly added .register_em() callback to register with the EM
after the cpufreq policy is properly initialized.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 23 deletions(-)
I would free the priv->opp_shared_cpus mask here, since we don't
need it anymore and memory can be reclaimed.
Yes, we don't need it anymore, but this isn't a good place to undo
what init() has done. Moreover, it is possible that register_em() may
not get called at all, if some error has occurred after init() has
successfully returned. It is always better to use exit() for such
things. It won't hurt a lot to keep this around anyway.
Don't forget this
setup would be called N CPUs times, on this per-CPU policy platform.
Yes, but EM will just ignore this call. Though I have made a change
here now to check for non-zero nr_opp to avoid the unnecessary call.
If freed here, then also there wouldn't be a need to free it in
scmi_cpufreq_exit() so you can remove it from there.