From: "Andrew-sh.Cheng" <redacted>
MT8183 supports CPU DVFS and CCI DVFS, and LITTLE cpus and CCI are in the same voltage domain.
So, this series is to add drivers to handle the voltage coupling between CPU and CCI DVFS.
For SVS support, need OPP_EVENT_ADJUST_VOLTAGE and corresponding reaction.
Change since v7:
- Separate part of "[v7,6/8] cpufreq: mediatek: add opp notification for SVS support" into another patch.
- "cpufreq: mediatek: Add record of previous desired vproc value"
- This is for case of there are multiple users on Vproc.
- cpufreq will record desired voltage.
- For "[v7,8/8] arm64: dts: mediatek: add cpufreq and cci devfreq nodes for mt8183"
- Add "required-opps" for cci to make cpufreq based passive governor work.
- Depend on patches have already in K-5.12
Andrew-sh.Cheng (7):
cpufreq: mediatek: Enable clock and regulator
dt-bindings: devfreq: add compatible for mt8183 cci devfreq
devfreq: add mediatek cci devfreq
cpufreq: mediatek: Add record of previous desired vproc value
cpufreq: mediatek: add opp notification for SVS support
devfreq: mediatek: cci devfreq register opp notification for SVS
support
arm64: dts: mediatek: add cpufreq and cci devfreq nodes for mt8183
Saravana Kannan (1):
PM / devfreq: Add cpu based scaling support to passive_governor
.../devicetree/bindings/devfreq/mt8183-cci.yaml | 51 ++++
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 36 +++
arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 4 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 277 +++++++++++++++++
drivers/cpufreq/mediatek-cpufreq.c | 122 +++++++-
drivers/devfreq/Kconfig | 12 +
drivers/devfreq/Makefile | 1 +
drivers/devfreq/governor_passive.c | 329 ++++++++++++++++++++-
drivers/devfreq/mt8183-cci-devfreq.c | 225 ++++++++++++++
include/linux/devfreq.h | 29 +-
10 files changed, 1060 insertions(+), 26 deletions(-)
create mode 100644 Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: "Andrew-sh.Cheng" <redacted>
Need to enable regulator,
so that the max/min requested value will be recorded
even it is not applied right away.
Intermediate clock is not always enabled by ccf in different projects,
so cpufreq should enable it by itself.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/cpufreq/mediatek-cpufreq.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
@@ -350,6 +350,11 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)ret=PTR_ERR(proc_reg);gotoout_free_resources;}+ret=regulator_enable(proc_reg);+if(ret){+pr_warn("enable vproc for cpu%d fail\n",cpu);+gotoout_free_resources;+}/* Both presence and absence of sram regulator are valid cases. */sram_reg=regulator_get_exclusive(cpu_dev,"sram");
@@ -368,13 +373,21 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)gotoout_free_resources;}+ret=clk_prepare_enable(cpu_clk);+if(ret)+gotoout_free_opp_table;++ret=clk_prepare_enable(inter_clk);+if(ret)+gotoout_disable_mux_clock;+/* Search a safe voltage for intermediate frequency. */rate=clk_get_rate(inter_clk);opp=dev_pm_opp_find_freq_ceil(cpu_dev,&rate);if(IS_ERR(opp)){pr_err("failed to get intermediate opp for cpu%d\n",cpu);ret=PTR_ERR(opp);-gotoout_free_opp_table;+gotoout_disable_inter_clock;}info->intermediate_voltage=dev_pm_opp_get_voltage(opp);dev_pm_opp_put(opp);
@@ -393,6 +406,12 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)return0;+out_disable_inter_clock:+clk_disable_unprepare(inter_clk);++out_disable_mux_clock:+clk_disable_unprepare(cpu_clk);+out_free_opp_table:dev_pm_opp_of_cpumask_remove_table(&info->cpus);
@@ -411,14 +430,20 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)staticvoidmtk_cpu_dvfs_info_release(structmtk_cpu_dvfs_info*info){-if(!IS_ERR(info->proc_reg))+if(!IS_ERR(info->proc_reg)){+regulator_disable(info->proc_reg);regulator_put(info->proc_reg);+}if(!IS_ERR(info->sram_reg))regulator_put(info->sram_reg);-if(!IS_ERR(info->cpu_clk))+if(!IS_ERR(info->cpu_clk)){+clk_disable_unprepare(info->cpu_clk);clk_put(info->cpu_clk);-if(!IS_ERR(info->inter_clk))+}+if(!IS_ERR(info->inter_clk)){+clk_disable_unprepare(info->inter_clk);clk_put(info->inter_clk);+}dev_pm_opp_of_cpumask_remove_table(&info->cpus);}
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: "Andrew-sh.Cheng" <redacted>
Need to enable regulator,
so that the max/min requested value will be recorded
even it is not applied right away.
Intermediate clock is not always enabled by ccf in different projects,
so cpufreq should enable it by itself.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/cpufreq/mediatek-cpufreq.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
@@ -350,6 +350,11 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)ret=PTR_ERR(proc_reg);gotoout_free_resources;}+ret=regulator_enable(proc_reg);+if(ret){+pr_warn("enable vproc for cpu%d fail\n",cpu);+gotoout_free_resources;+}
Regulators are enabled by OPP core as well now, you sure this is
required ?
--
viresh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, 2021-03-30 at 10:06 +0530, Viresh Kumar wrote:
On 23-03-21, 19:33, Andrew-sh.Cheng wrote:
quoted
From: "Andrew-sh.Cheng" <redacted>
Need to enable regulator,
so that the max/min requested value will be recorded
even it is not applied right away.
Intermediate clock is not always enabled by ccf in different projects,
so cpufreq should enable it by itself.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/cpufreq/mediatek-cpufreq.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
@@ -350,6 +350,11 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)ret=PTR_ERR(proc_reg);gotoout_free_resources;}+ret=regulator_enable(proc_reg);+if(ret){+pr_warn("enable vproc for cpu%d fail\n",cpu);+gotoout_free_resources;+}
Regulators are enabled by OPP core as well now, you sure this is
required ?
Hi Viresh,
Yes.
As you mentioned, it will be enable by OPP core.
Per discuss with hotplug owner and regulator owner,
they suggest that "users should not suppose other module, will enable
regulators for them".
They suggest to add enable_regulator here.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Viresh,
Yes.
As you mentioned, it will be enable by OPP core.
Per discuss with hotplug owner and regulator owner,
they suggest that "users should not suppose other module, will enable
regulators for them".
They suggest to add enable_regulator here.
Which is fine if the modules in question aren't closely related to each other,
but OPP core and cpufreq are too closely bound to each other. So much that the
cpufreq driver can depend on the OPP core for doing it.
Though I won't Nack a patch just for that, but it was just a suggestion.
--
viresh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -0,0 +1,51 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/devfreq/mt8183-cci.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:CCI_DEVFREQ driver for MT8183.++maintainers:+-Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>++description:|+This module is used to create CCI DEVFREQ.+The performance will depend on both CCI frequency and CPU frequency.+For MT8183, CCI co-buck with Little core.+Contain CCI opp table for voltage and frequency scaling.++properties:+compatible:+const:"mediatek,mt8183-cci"++clocks:+maxItems:1++clock-names:+const:"cci"++operating-points-v2:true+opp-table:true++proc-supply:+description:+Phandle of the regulator that provides the supply voltage.++required:+-compatible+-clocks+-clock-names+-proc-supply++examples:+-|+#include <dt-bindings/clock/mt8183-clk.h>+cci:cci {+compatible = "mediatek,mt8183-cci";+clocks = <&apmixedsys CLK_APMIXED_CCIPLL>;+clock-names = "cci";+operating-points-v2 = <&cci_opp>;+proc-supply = <&mt6358_vproc12_reg>;+};+
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
@@ -8,11 +8,103 @@*/#include<linux/module.h>+#include<linux/cpu.h>+#include<linux/cpufreq.h>+#include<linux/cpumask.h>#include<linux/device.h>#include<linux/devfreq.h>+#include<linux/slab.h>#include"governor.h"-staticintdevfreq_passive_get_target_freq(structdevfreq*devfreq,+structdevfreq_cpu_state{+unsignedintcurr_freq;+unsignedintmin_freq;+unsignedintmax_freq;+unsignedintfirst_cpu;+structdevice*cpu_dev;+structopp_table*opp_table;+};++staticunsignedlongxlate_cpufreq_to_devfreq(structdevfreq_passive_data*data,+unsignedintcpu)+{+unsignedintcpu_min_freq,cpu_max_freq,cpu_curr_freq_khz,cpu_percent;+unsignedlongdev_min_freq,dev_max_freq,dev_max_state;++structdevfreq_cpu_state*cpu_state=data->cpu_state[cpu];+structdevfreq*devfreq=(structdevfreq*)data->this;+unsignedlong*dev_freq_table=devfreq->profile->freq_table;+structdev_pm_opp*opp=NULL,*p_opp=NULL;+unsignedlongcpu_curr_freq,freq;++if(!cpu_state||cpu_state->first_cpu!=cpu||+!cpu_state->opp_table||!devfreq->opp_table)+return0;++cpu_curr_freq=cpu_state->curr_freq*1000;+p_opp=devfreq_recommended_opp(cpu_state->cpu_dev,&cpu_curr_freq,0);+if(IS_ERR(p_opp))+return0;++opp=dev_pm_opp_xlate_required_opp(cpu_state->opp_table,+devfreq->opp_table,p_opp);+dev_pm_opp_put(p_opp);++if(!IS_ERR(opp)){+freq=dev_pm_opp_get_freq(opp);+dev_pm_opp_put(opp);+gotoout;+}++/* Use Interpolation if required opps is not available */+cpu_min_freq=cpu_state->min_freq;+cpu_max_freq=cpu_state->max_freq;+cpu_curr_freq_khz=cpu_state->curr_freq;++if(dev_freq_table){+/* Get minimum frequency according to sorting order */+dev_max_state=dev_freq_table[devfreq->profile->max_state-1];+if(dev_freq_table[0]<dev_max_state){+dev_min_freq=dev_freq_table[0];+dev_max_freq=dev_max_state;+}else{+dev_min_freq=dev_max_state;+dev_max_freq=dev_freq_table[0];+}+}else{+dev_min_freq=dev_pm_qos_read_value(devfreq->dev.parent,+DEV_PM_QOS_MIN_FREQUENCY);+dev_max_freq=dev_pm_qos_read_value(devfreq->dev.parent,+DEV_PM_QOS_MAX_FREQUENCY);++if(dev_max_freq<=dev_min_freq)+return0;+}+cpu_percent=((cpu_curr_freq_khz-cpu_min_freq)*100)/cpu_max_freq-cpu_min_freq;+freq=dev_min_freq+mult_frac(dev_max_freq-dev_min_freq,cpu_percent,100);++out:+returnfreq;+}++staticintget_target_freq_with_cpufreq(structdevfreq*devfreq,+unsignedlong*freq)+{+structdevfreq_passive_data*p_data=+(structdevfreq_passive_data*)devfreq->data;+unsignedintcpu;+unsignedlongtarget_freq=0;++for_each_online_cpu(cpu)+target_freq=max(target_freq,+xlate_cpufreq_to_devfreq(p_data,cpu));++*freq=target_freq;++return0;+}++staticintget_target_freq_with_devfreq(structdevfreq*devfreq,unsignedlong*freq){structdevfreq_passive_data*p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,inti,count;/*-*Ifthedevfreqdevicewithpassivegovernorhasthespecificmethod-*todeterminethenextfrequency,shouldusetheget_target_freq()-*ofstructdevfreq_passive_data.-*/-if(p_data->get_target_freq)-returnp_data->get_target_freq(devfreq,freq);--/**IftheparentandpassivedevfreqdeviceusestheOPPtable,*getthenextfrequencybyusingtheOPPtable.*/
@@ -130,16 +245,200 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb,returnNOTIFY_DONE;}+staticintcpufreq_passive_notifier_call(structnotifier_block*nb,+unsignedlongevent,void*ptr)+{+structdevfreq_passive_data*data=+container_of(nb,structdevfreq_passive_data,nb);+structdevfreq*devfreq=(structdevfreq*)data->this;+structdevfreq_cpu_state*cpu_state;+structcpufreq_freqs*cpu_freq=ptr;+unsignedintcurr_freq;+intret;++if(event!=CPUFREQ_POSTCHANGE||!cpu_freq||+!data->cpu_state[cpu_freq->policy->cpu])+return0;++cpu_state=data->cpu_state[cpu_freq->policy->cpu];+if(cpu_state->curr_freq==cpu_freq->new)+return0;++/* Backup current freq and pre-update cpu state freq*/+curr_freq=cpu_state->curr_freq;+cpu_state->curr_freq=cpu_freq->new;++mutex_lock(&devfreq->lock);+ret=update_devfreq(devfreq);+mutex_unlock(&devfreq->lock);+if(ret){+cpu_state->curr_freq=curr_freq;+dev_err(&devfreq->dev,"Couldn't update the frequency.\n");+returnret;+}++return0;+}++staticintcpufreq_passive_register(structdevfreq_passive_data**p_data)+{+structdevfreq_passive_data*data=*p_data;+structdevfreq*devfreq=(structdevfreq*)data->this;+structdevice*dev=devfreq->dev.parent;+structopp_table*opp_table=NULL;+structdevfreq_cpu_state*cpu_state;+structcpufreq_policy*policy;+structdevice*cpu_dev;+unsignedintcpu;+intret;++get_online_cpus();++data->nb.notifier_call=cpufreq_passive_notifier_call;+ret=cpufreq_register_notifier(&data->nb,+CPUFREQ_TRANSITION_NOTIFIER);+if(ret){+dev_err(dev,"Couldn't register cpufreq notifier.\n");+data->nb.notifier_call=NULL;+gotoout;+}++/* Populate devfreq_cpu_state */+for_each_online_cpu(cpu){+if(data->cpu_state[cpu])+continue;++policy=cpufreq_cpu_get(cpu);+if(!policy){+ret=-EINVAL;+gotoout;+}elseif(PTR_ERR(policy)==-EPROBE_DEFER){+ret=-EPROBE_DEFER;+gotoout;+}elseif(IS_ERR(policy)){+ret=PTR_ERR(policy);+dev_err(dev,"Couldn't get the cpufreq_poliy.\n");+gotoout;+}++cpu_state=kzalloc(sizeof(*cpu_state),GFP_KERNEL);+if(!cpu_state){+ret=-ENOMEM;+gotoout;+}++cpu_dev=get_cpu_device(cpu);+if(!cpu_dev){+dev_err(dev,"Couldn't get cpu device.\n");+ret=-ENODEV;+gotoout;+}++opp_table=dev_pm_opp_get_opp_table(cpu_dev);+if(IS_ERR(devfreq->opp_table)){+ret=PTR_ERR(opp_table);+gotoout;+}++cpu_state->cpu_dev=cpu_dev;+cpu_state->opp_table=opp_table;+cpu_state->first_cpu=cpumask_first(policy->related_cpus);+cpu_state->curr_freq=policy->cur;+cpu_state->min_freq=policy->cpuinfo.min_freq;+cpu_state->max_freq=policy->cpuinfo.max_freq;+data->cpu_state[cpu]=cpu_state;++cpufreq_cpu_put(policy);+}++out:+put_online_cpus();+if(ret)+returnret;++/* Update devfreq */+mutex_lock(&devfreq->lock);+ret=update_devfreq(devfreq);+mutex_unlock(&devfreq->lock);+if(ret)+dev_err(dev,"Couldn't update the frequency.\n");++returnret;+}++staticintcpufreq_passive_unregister(structdevfreq_passive_data**p_data)+{+structdevfreq_passive_data*data=*p_data;+structdevfreq_cpu_state*cpu_state;+intcpu;++if(data->nb.notifier_call)+cpufreq_unregister_notifier(&data->nb,+CPUFREQ_TRANSITION_NOTIFIER);++for_each_possible_cpu(cpu){+cpu_state=data->cpu_state[cpu];+if(cpu_state){+if(cpu_state->opp_table)+dev_pm_opp_put_opp_table(cpu_state->opp_table);+kfree(cpu_state);+cpu_state=NULL;+}+}++return0;+}++intregister_parent_dev_notifier(structdevfreq_passive_data**p_data)+{+structnotifier_block*nb=&(*p_data)->nb;+intret=0;++switch((*p_data)->parent_type){+caseDEVFREQ_PARENT_DEV:+nb->notifier_call=devfreq_passive_notifier_call;+ret=devfreq_register_notifier((structdevfreq*)(*p_data)->parent,nb,+DEVFREQ_TRANSITION_NOTIFIER);+break;+caseCPUFREQ_PARENT_DEV:+ret=cpufreq_passive_register(p_data);+break;+default:+ret=-EINVAL;+break;+}+returnret;+}++intunregister_parent_dev_notifier(structdevfreq_passive_data**p_data)+{+intret=0;++switch((*p_data)->parent_type){+caseDEVFREQ_PARENT_DEV:+WARN_ON(devfreq_unregister_notifier((structdevfreq*)(*p_data)->parent,+&(*p_data)->nb,+DEVFREQ_TRANSITION_NOTIFIER));+break;+caseCPUFREQ_PARENT_DEV:+cpufreq_passive_unregister(p_data);+break;+default:+ret=-EINVAL;+break;+}+returnret;+}+staticintdevfreq_passive_event_handler(structdevfreq*devfreq,unsignedintevent,void*data){structdevfreq_passive_data*p_data=(structdevfreq_passive_data*)devfreq->data;structdevfreq*parent=(structdevfreq*)p_data->parent;-structnotifier_block*nb=&p_data->nb;intret=0;-if(!parent)+if(p_data->parent_type==DEVFREQ_PARENT_DEV&&!parent)return-EPROBE_DEFER;switch(event){
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,if(!p_data->this)p_data->this=devfreq;-nb->notifier_call=devfreq_passive_notifier_call;-ret=devfreq_register_notifier(parent,nb,-DEVFREQ_TRANSITION_NOTIFIER);+ret=register_parent_dev_notifier(&p_data);break;+caseDEVFREQ_GOV_STOP:-WARN_ON(devfreq_unregister_notifier(parent,nb,-DEVFREQ_TRANSITION_NOTIFIER));+ret=unregister_parent_dev_notifier(&p_data);break;default:break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted hunk
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted hunk
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
Hi,
You are missing to add these patches to linux-pm mailing list.
Need to send them to linu-pm ML.
Also, before received this series, I tried to clean-up these patches
on testing branch[1]. So that I add my comment with my clean-up case.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
And 'Saravana Kannan [off-list ref]' is wrong email address.
Please update the email or drop this email.
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
quoted hunk
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted hunk
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted hunk
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgA6mp3Yqo$
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgA6mp3Yqo$
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
Sorry for the confusion. I make the devfreq-testing-passive-gov[1]
branch based on latest devfreq-next branch.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
First of all, if possible, I want to test them[1] with your patches in this series.
And then if there are no any problem, please let me know. After confirmed from you,
I'll send the patches of devfreq-testing-passive-gov[1] branch.
How about that?
quoted
quoted
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
quoted
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgA6mp3Yqo$
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
quoted
quoted
quoted
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
quoted
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgA6mp3Yqo$
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
quoted
quoted
quoted
quoted
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
quoted
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgA6mp3Yqo$
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
I'm sorry for delay because when I tested the patches
for devfreq parent type on Odroid-xu3, there are some problem
related to lazy linking of OPP. So I'm trying to analyze them.
Unfortunately, we need to postpone these patches to next linux
version.
[snip]
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
I'm sorry for delay because when I tested the patches
for devfreq parent type on Odroid-xu3, there are some problem
related to lazy linking of OPP. So I'm trying to analyze them.
Unfortunately, we need to postpone these patches to next linux
version.
Hi Chanwoo,
Sorry to bother you.
Do you work on this patch now?
Is there any thing that we can do?
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
I'm sorry for delay because when I tested the patches
for devfreq parent type on Odroid-xu3, there are some problem
related to lazy linking of OPP. So I'm trying to analyze them.
Unfortunately, we need to postpone these patches to next linux
version.
Hi Chanwoo Choi~
It is said that you are busy on another task recently.
May I know your plan on this patch?
Thank you.
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
I'm sorry for delay because when I tested the patches
for devfreq parent type on Odroid-xu3, there are some problem
related to lazy linking of OPP. So I'm trying to analyze them.
Unfortunately, we need to postpone these patches to next linux
version.
Hi Chanwoo Choi~
It is said that you are busy on another task recently.
May I know your plan on this patch?
Thank you.
Sorry for late work. I have a question.
When I tested exynos-bus.c with adding the 'required-opp' property
on odroid-xu3 board. I got some fail about
When calling _set_required_opps(), always _set_required_opp() returns
-EBUSY error because of following lazy linking case[1].
[1] https://elixir.bootlin.com/linux/v5.13-rc3/source/drivers/opp/core.c#L896
/* required-opps not fully initialized yet */
if (lazy_linking_pending(opp_table))
return -EBUSY;
For calling dev_pm_opp_of_add_table(), lazy_link_required_opp_table() function
will be called. But, there is constraint[2]. If is_genpd of opp_table is false,
driver/opp/of.c cannot resolve the lazy linking issue.
[2] https://elixir.bootlin.com/linux/v5.13-rc3/source/drivers/opp/of.c#L386
/* Link required OPPs for all OPPs of the newly added OPP table */
static void lazy_link_required_opp_table(struct opp_table *new_table)
{
struct opp_table *opp_table, *temp, **required_opp_tables;
struct device_node *required_np, *opp_np, *required_table_np;
struct dev_pm_opp *opp;
int i, ret;
/*
* We only support genpd's OPPs in the "required-opps" for now,
* as we don't know much about other cases.
*/
if (!new_table->is_genpd)
return;
Even if this case, there are no problem on your test case?
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
I'm sorry for delay because when I tested the patches
for devfreq parent type on Odroid-xu3, there are some problem
related to lazy linking of OPP. So I'm trying to analyze them.
Unfortunately, we need to postpone these patches to next linux
version.
Hi Chanwoo Choi~
It is said that you are busy on another task recently.
May I know your plan on this patch?
Thank you.
Hi Chanwoo~
Sorry for late reply.
Yes, we meet similar issue.
Google member Hsin-Yi had helped deal with this issue on Chrome project.
Patch segment:
@ /drivers/opp/of.c
/* Link required OPPs for all OPPs of the newly added OPP table */
static void lazy_link_required_opp_table(struct opp_table *new_table)
{
struct opp_table *opp_table, *temp, **required_opp_tables;
struct device_node *required_np, *opp_np, *required_table_np;
struct dev_pm_opp *opp;
int i, ret;
+ /*
+ * We only support genpd's OPPs in the "required-opps" for now,
+ * as we don't know much about other cases.
+ */
+ if (!new_table->is_genpd)
+ return;
Hsin-Yi replied this issue in the discussion list in the original lazy
link thread:
https://patchwork.kernel.org/project/linux-pm/patch/20190717222340.137578-4-saravanak@google.com/#23932203
Loop Hsin-YI here.
You can discuss with her if needing more detail.
Thank you both.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Chanwoo~
We will use this on Google Chrome project.
Google Hsin-Yi has test your patch + my patch set v8 [2~8]
make sure cci devfreqs runs with cpufreq.
suspend resume
speedometer2 benchmark
It is okay.
Please send the patches of devfreq-testing-passive-gov[1] branch.
I will send patch v9 base on yours latter.
Thanks for your test. I'll send the patches today.
I'm sorry for delay because when I tested the patches
for devfreq parent type on Odroid-xu3, there are some problem
related to lazy linking of OPP. So I'm trying to analyze them.
Unfortunately, we need to postpone these patches to next linux
version.
Hi Chanwoo Choi~
It is said that you are busy on another task recently.
May I know your plan on this patch?
Thank you.
Hi Chanwoo~
Sorry for late reply.
Yes, we meet similar issue.
Google member Hsin-Yi had helped deal with this issue on Chrome project.
Patch segment:
@ /drivers/opp/of.c
/* Link required OPPs for all OPPs of the newly added OPP table */
static void lazy_link_required_opp_table(struct opp_table *new_table)
{
struct opp_table *opp_table, *temp, **required_opp_tables;
struct device_node *required_np, *opp_np, *required_table_np;
struct dev_pm_opp *opp;
int i, ret;
+ /*
+ * We only support genpd's OPPs in the "required-opps" for now,
+ * as we don't know much about other cases.
+ */
+ if (!new_table->is_genpd)
+ return;
Hsin-Yi replied this issue in the discussion list in the original lazy
link thread:
https://patchwork.kernel.org/project/linux-pm/patch/20190717222340.137578-4-saravanak@google.com/#23932203
Loop Hsin-YI here.
You can discuss with her if needing more detail.
Thank you both.
Thanks. First of all, we need to resolve and discuss this issue.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Hsin-Yi Wang <hidden> Date: 2021-03-31 10:47:32
On Thu, Mar 25, 2021 at 3:58 PM Chanwoo Choi [off-list ref] wrote:
Hi,
You are missing to add these patches to linux-pm mailing list.
Need to send them to linu-pm ML.
Also, before received this series, I tried to clean-up these patches
on testing branch[1]. So that I add my comment with my clean-up case.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
And 'Saravana Kannan [off-list ref]' is wrong email address.
Please update the email or drop this email.
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
quoted
From: Saravana Kannan <redacted>
Many CPU architectures have caches that can scale independent of the
CPUs. Frequency scaling of the caches is necessary to make sure that 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 support for cpu based scaling to the
passive governor. This is accomplished by taking the current frequency
of each CPU frequency domain and then adjust 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:
* Derives the optimal devfreq device opp from required-opps property of
the parent cpu opp_table.
* 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. It is interpolated for frequencies
in between.
Andrew-sh.Cheng change
dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
after kernel-5.7
Don't return -EINVAL in devfreq_passive_event_handler()
since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
Signed-off-by: Saravana Kannan <redacted>
[Sibi: Integrated cpu-freqmap governor into passive_governor]
Signed-off-by: Sibi Sankar <redacted>
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 2 +
drivers/devfreq/governor_passive.c | 329 +++++++++++++++++++++++++++++++++++--
include/linux/devfreq.h | 29 +++-
3 files changed, 342 insertions(+), 18 deletions(-)
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev: reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
struct devfreq_cpu_state {
struct device *dev;
unsigned int first_cpu;
struct opp_table *opp_table;
unsigned int cur_freq;
unsigned int min_freq;
unsigned int max_freq;
};
quoted
+
+static unsigned long xlate_cpufreq_to_devfreq(struct devfreq_passive_data *data,
+ unsigned int cpu)
+{
+ unsigned int cpu_min_freq, cpu_max_freq, cpu_curr_freq_khz, cpu_percent;
+ unsigned long dev_min_freq, dev_max_freq, dev_max_state;
+
+ struct devfreq_cpu_state *cpu_state = data->cpu_state[cpu];
+ struct devfreq *devfreq = (struct devfreq *)data->this;
+ unsigned long *dev_freq_table = devfreq->profile->freq_table;
+ struct dev_pm_opp *opp = NULL, *p_opp = NULL;
+ unsigned long cpu_curr_freq, freq;
+
+ if (!cpu_state || cpu_state->first_cpu != cpu ||
+ !cpu_state->opp_table || !devfreq->opp_table)
+ return 0;
+
+ cpu_curr_freq = cpu_state->curr_freq * 1000;
+ p_opp = devfreq_recommended_opp(cpu_state->cpu_dev, &cpu_curr_freq, 0);
+ if (IS_ERR(p_opp))
+ return 0;
+
+ opp = dev_pm_opp_xlate_required_opp(cpu_state->opp_table,
+ devfreq->opp_table, p_opp);
+ dev_pm_opp_put(p_opp);
+
+ if (!IS_ERR(opp)) {
+ freq = dev_pm_opp_get_freq(opp);
+ dev_pm_opp_put(opp);
+ goto out;
+ }
+
+ /* Use Interpolation if required opps is not available */
+ cpu_min_freq = cpu_state->min_freq;
+ cpu_max_freq = cpu_state->max_freq;
+ cpu_curr_freq_khz = cpu_state->curr_freq;
+
+ if (dev_freq_table) {
+ /* Get minimum frequency according to sorting order */
+ dev_max_state = dev_freq_table[devfreq->profile->max_state - 1];
+ if (dev_freq_table[0] < dev_max_state) {
+ dev_min_freq = dev_freq_table[0];
+ dev_max_freq = dev_max_state;
+ } else {
+ dev_min_freq = dev_max_state;
+ dev_max_freq = dev_freq_table[0];
+ }
+ } else {
+ dev_min_freq = dev_pm_qos_read_value(devfreq->dev.parent,
+ DEV_PM_QOS_MIN_FREQUENCY);
+ dev_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
+ DEV_PM_QOS_MAX_FREQUENCY);
+
+ if (dev_max_freq <= dev_min_freq)
+ return 0;
+ }
+ cpu_percent = ((cpu_curr_freq_khz - cpu_min_freq) * 100) / cpu_max_freq - cpu_min_freq;
() is missing for denominator?
cpu_percent = ((cpu_curr_freq_khz - cpu_min_freq) * 100) /
(cpu_max_freq - cpu_min_freq);
As you knew, governor_passive.c was already used
both 'dev_pm_opp_xlate_required_opp' and 'devfreq_recommended_opp'
to get the target from OPP. So, I wan to make the common function
like 'get_taget_freq_by_required_opp' as following:
If define 'get_taget_freq_by_required_opp' as following,
it will be used for get_target_freq_with_devfreq().
After finisied the review of this patch, I'll send the patch[2].
[2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=101c5a087586ab2b5cf3370166a7e39227ca83cf
For example but this code is not tested,
static unsigned long get_taget_freq_by_required_opp(struct device *p_dev,
struct opp_table *p_opp_table,
struct opp_table *opp_table,
unsigned long freq)
{
struct dev_pm_opp *opp = NULL, *p_opp = NULL;
if (!p_dev || !p_opp_table || !opp_table || !freq)
return 0;
p_opp = devfreq_recommended_opp(p_dev, &freq, 0);
if (IS_ERR(p_opp))
return 0;
opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp);
dev_pm_opp_put(p_opp);
if (IS_ERR(opp))
return 0;
freq = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
return freq;
}
static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
unsigned long *target_freq)
{
struct devfreq_passive_data *p_data =
(struct devfreq_passive_data *)devfreq->data;
struct devfreq_cpu_data *cpu_data;
unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent;
unsigned long dev_min, dev_max;
unsigned long freq = 0;
for_each_online_cpu(cpu) {
cpu_data = p_data->cpu_data[cpu];
if (!cpu_data || cpu_data->first_cpu != cpu)
continue;
/* Get target freq via required opps */
cpu_cur = cpu_data->cur_freq * HZ_PER_KHZ;
freq = get_taget_freq_by_required_opp(cpu_data->dev,
cpu_data->opp_table,
devfreq->opp_table, cpu_cur);
if (freq) {
*target_freq = max(freq, *target_freq);
continue;
}
/* Use Interpolation if required opps is not available */
devfreq_get_freq_range(devfreq, &dev_min, &dev_max);
cpu_min = cpu_data->min_freq;
cpu_max = cpu_data->max_freq;
cpu_cur = cpu_data->cur_freq;
cpu_percent = ((cpu_cur - cpu_min) * 100) / cpu_max - cpu_min;
freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
*target_freq = max(freq, *target_freq);
}
return 0;
}
quoted
+
+static int get_target_freq_with_devfreq(struct devfreq *devfreq,
unsigned long *freq)
{
struct devfreq_passive_data *p_data
@@ -23,14 +115,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, int i, count; /*- * If the devfreq device with passive governor has the specific method- * to determine the next frequency, should use the get_target_freq()- * of struct devfreq_passive_data.- */- if (p_data->get_target_freq)- return p_data->get_target_freq(devfreq, freq);-- /* * If the parent and passive devfreq device uses the OPP table, * get the next frequency by using the OPP table. */
@@ -98,6 +182,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, return 0; }+static int devfreq_passive_get_target_freq(struct devfreq *devfreq,+ unsigned long *freq)+{+ struct devfreq_passive_data *p_data =+ (struct devfreq_passive_data *)devfreq->data;+ int ret;++ /*+ * If the devfreq device with passive governor has the specific method+ * to determine the next frequency, should use the get_target_freq()+ * of struct devfreq_passive_data.+ */+ if (p_data->get_target_freq)+ return p_data->get_target_freq(devfreq, freq);++ switch (p_data->parent_type) {+ case DEVFREQ_PARENT_DEV:+ ret = get_target_freq_with_devfreq(devfreq, freq);+ break;+ case CPUFREQ_PARENT_DEV:+ ret = get_target_freq_with_cpufreq(devfreq, freq);+ break;+ default:+ ret = -EINVAL;+ dev_err(&devfreq->dev, "Invalid parent type\n");+ break;+ }++ return ret;+}+ static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) {
In order to keep the consistent style of function name,
please change the name as following because devfreq defines
the function name as 'devfreq_regiter_notifier'
- cpufreq_passive_register -> cpufreq_passive_register_notifier
I think that you don't need to define register_parent_dev_notifier
and unregister_parent_dev_notifier as the separate functions.
Instead of the separate functions, just add the code
into devfreq_passive_event_handler.
quoted
+
static int devfreq_passive_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent = (struct devfreq *)p_data->parent;
- struct notifier_block *nb = &p_data->nb;
int ret = 0;
- if (!parent)
+ if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
return -EPROBE_DEFER;
switch (event) {
@@ -147,13 +446,11 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, if (!p_data->this) p_data->this = devfreq;- nb->notifier_call = devfreq_passive_notifier_call;- ret = devfreq_register_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER);+ ret = register_parent_dev_notifier(&p_data); break;+ case DEVFREQ_GOV_STOP:- WARN_ON(devfreq_unregister_notifier(parent, nb,- DEVFREQ_TRANSITION_NOTIFIER));+ ret = unregister_parent_dev_notifier(&p_data); break; default: break;
@@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {*usinggovernorsexceptforpassivegovernor.*Ifthedevfreqdevicehasthespecificmethodtodecide*thenextfrequency,shouldusethiscallback.+*@parent_type:parenttypeofthedevice*@this:thedevfreqinstanceofowndevice.*@nb:thenotifierblockforDEVFREQ_TRANSITION_NOTIFIERlist+*@cpu_state:thestatemin/max/currentfrequencyofallonlinecpu's**Thedevfreq_passive_datahavetosetthedevfreqinstanceofparent*devicewithgovernorsexceptforthepassivegovernor.But,don'tneedto-*initializethe'this'and'nb'fieldbecausethedevfreqcorewillhandle-*them.+*initializethe'this','nb'and'cpu_state'fieldbecausethedevfreqcore+*willhandlethem.*/structdevfreq_passive_data{/* Should set the devfreq instance of parent device */
@@ -305,9 +326,13 @@ struct devfreq_passive_data {/* Optional callback to decide the next frequency of passvice device */int(*get_target_freq)(structdevfreq*this,unsignedlong*freq);+/* Should set the type of parent device */+enumdevfreq_parent_dev_typeparent_type;+/* For passive governor's internal use. Don't need to set them */structdevfreq*this;structnotifier_blocknb;+structdevfreq_cpu_state*cpu_state[NR_CPUS];};#endif
From: "Andrew-sh.Cheng" <redacted>
This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
of the Mediatek MT8183.
On the MT8183 the CCI is supplied by the same regulator as the LITTLE
cores. The driver is notified when the regulator voltage changes
(driven by cpufreq) and adjusts the CCI frequency to the maximum
possible value.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 10 ++
drivers/devfreq/Makefile | 1 +
drivers/devfreq/mt8183-cci-devfreq.c | 198 +++++++++++++++++++++++++++++++++++
3 files changed, 209 insertions(+)
create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
@@ -0,0 +1,198 @@+// SPDX-License-Identifier: GPL-2.0+/*+*Copyright(c)2021MediaTekInc.++*Author:Andrew-sh.Cheng<andrew-sh.cheng@mediatek.com>+*/++#include<linux/clk.h>+#include<linux/devfreq.h>+#include<linux/module.h>+#include<linux/of.h>+#include<linux/platform_device.h>+#include<linux/regulator/consumer.h>+#include<linux/time.h>++#define MAX_VOLT_LIMIT (1150000)++structcci_devfreq{+structdevfreq*devfreq;+structregulator*cpu_reg;+structclk*cci_clk;+intold_vproc;+unsignedlongold_freq;+};++staticintmtk_cci_set_voltage(structcci_devfreq*cci_df,intvproc)+{+intret;++ret=regulator_set_voltage(cci_df->cpu_reg,vproc,+MAX_VOLT_LIMIT);+if(!ret)+cci_df->old_vproc=vproc;+returnret;+}++staticintmtk_cci_devfreq_target(structdevice*dev,unsignedlong*freq,+u32flags)+{+intret;+structcci_devfreq*cci_df=dev_get_drvdata(dev);+structdev_pm_opp*opp;+unsignedlongopp_rate,opp_voltage,old_voltage;++if(!cci_df)+return-EINVAL;++if(cci_df->old_freq==*freq)+return0;++opp_rate=*freq;+opp=devfreq_recommended_opp(dev,&opp_rate,1);+opp_voltage=dev_pm_opp_get_voltage(opp);+dev_pm_opp_put(opp);++old_voltage=cci_df->old_vproc;+if(old_voltage==0)+old_voltage=regulator_get_voltage(cci_df->cpu_reg);++// scale up: set voltage first then freq+if(opp_voltage>old_voltage){+ret=mtk_cci_set_voltage(cci_df,opp_voltage);+if(ret){+pr_err("cci: failed to scale up voltage\n");+returnret;+}+}++ret=clk_set_rate(cci_df->cci_clk,*freq);+if(ret){+pr_err("%s: failed cci to set rate: %d\n",__func__,+ret);+mtk_cci_set_voltage(cci_df,old_voltage);+returnret;+}++// scale down: set freq first then voltage+if(opp_voltage<old_voltage){+ret=mtk_cci_set_voltage(cci_df,opp_voltage);+if(ret){+pr_err("cci: failed to scale down voltage\n");+clk_set_rate(cci_df->cci_clk,cci_df->old_freq);+returnret;+}+}++cci_df->old_freq=*freq;++return0;+}++staticstructdevfreq_dev_profilecci_devfreq_profile={+.target=mtk_cci_devfreq_target,+};++staticintmtk_cci_devfreq_probe(structplatform_device*pdev)+{+structdevice*cci_dev=&pdev->dev;+structcci_devfreq*cci_df;+structdevfreq_passive_data*passive_data;+intret;++cci_df=devm_kzalloc(cci_dev,sizeof(*cci_df),GFP_KERNEL);+if(!cci_df)+return-ENOMEM;++cci_df->cci_clk=devm_clk_get(cci_dev,"cci_clock");+ret=PTR_ERR_OR_ZERO(cci_df->cci_clk);+if(ret){+if(ret!=-EPROBE_DEFER)+dev_err(cci_dev,"failed to get clock for CCI: %d\n",+ret);+returnret;+}+cci_df->cpu_reg=devm_regulator_get_optional(cci_dev,"proc");+ret=PTR_ERR_OR_ZERO(cci_df->cpu_reg);+if(ret){+if(ret!=-EPROBE_DEFER)+dev_err(cci_dev,"failed to get regulator for CCI: %d\n",+ret);+returnret;+}+ret=regulator_enable(cci_df->cpu_reg);+if(ret){+dev_err(cci_dev,"enable buck for cci fail\n");+returnret;+}++ret=dev_pm_opp_of_add_table(cci_dev);+if(ret){+dev_err(cci_dev,"Fail to get OPP table for CCI: %d\n",ret);+returnret;+}++platform_set_drvdata(pdev,cci_df);++passive_data=devm_kzalloc(cci_dev,sizeof(*passive_data),GFP_KERNEL);+if(!passive_data){+ret=-ENOMEM;+gotoerr_opp;+}++passive_data->parent_type=CPUFREQ_PARENT_DEV;++cci_df->devfreq=devm_devfreq_add_device(cci_dev,+&cci_devfreq_profile,+DEVFREQ_GOV_PASSIVE,+passive_data);+if(IS_ERR(cci_df->devfreq)){+ret=PTR_ERR(cci_df->devfreq);+dev_err(cci_dev,"cannot create cci devfreq device:%d\n",ret);+gotoerr_opp;+}++return0;++err_opp:+dev_pm_opp_of_remove_table(cci_dev);+returnret;+}++staticintmtk_cci_devfreq_remove(structplatform_device*pdev)+{+structdevice*cci_dev=&pdev->dev;+structcci_devfreq*cci_df;+structnotifier_block*opp_nb;++cci_df=platform_get_drvdata(pdev);+opp_nb=&cci_df->opp_nb;++dev_pm_opp_unregister_notifier(cci_dev,opp_nb);+dev_pm_opp_of_remove_table(cci_dev);+regulator_disable(cci_df->cpu_reg);++return0;+}++staticconst__maybe_unusedstructof_device_id+mediatek_cci_of_match[]={+{.compatible="mediatek,mt8183-cci"},+{},+};+MODULE_DEVICE_TABLE(of,mediatek_cci_of_match);++staticstructplatform_drivercci_devfreq_driver={+.probe=mtk_cci_devfreq_probe,+.remove=mtk_cci_devfreq_remove,+.driver={+.name="mediatek-cci-devfreq",+.of_match_table=of_match_ptr(mediatek_cci_of_match),+},+};++module_platform_driver(cci_devfreq_driver);++MODULE_DESCRIPTION("Mediatek CCI devfreq driver");+MODULE_AUTHOR("Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>");+MODULE_LICENSE("GPL v2");
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: "Andrew-sh.Cheng" <redacted>
This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
of the Mediatek MT8183.
On the MT8183 the CCI is supplied by the same regulator as the LITTLE
cores. The driver is notified when the regulator voltage changes
(driven by cpufreq) and adjusts the CCI frequency to the maximum
possible value.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 10 ++
drivers/devfreq/Makefile | 1 +
drivers/devfreq/mt8183-cci-devfreq.c | 198 +++++++++++++++++++++++++++++++++++
3 files changed, 209 insertions(+)
create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
nitpick. how about using 'old_voltage'?
because 'vproc' is not easy for understanding.
+ unsigned long old_freq;
+};
+
+static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
nitpick: how about changing 'vproc -> voltage'?
+{
+ int ret;
+
+ ret = regulator_set_voltage(cci_df->cpu_reg, vproc,
+ MAX_VOLT_LIMIT);
+ if (!ret)
+ cci_df->old_vproc = vproc;
+ return ret;
+}
+
+static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
+ u32 flags)
+{
+ int ret;
+ struct cci_devfreq *cci_df = dev_get_drvdata(dev);
+ struct dev_pm_opp *opp;
+ unsigned long opp_rate, opp_voltage, old_voltage;
+
+ if (!cci_df)
+ return -EINVAL;
+
+ if (cci_df->old_freq == *freq)
+ return 0;
+
+ opp_rate = *freq;
+ opp = devfreq_recommended_opp(dev, &opp_rate, 1);
+ opp_voltage = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+
+ old_voltage = cci_df->old_vproc;
+ if (old_voltage == 0)
+ old_voltage = regulator_get_voltage(cci_df->cpu_reg);
+
+ // scale up: set voltage first then freq
+ if (opp_voltage > old_voltage) {
+ ret = mtk_cci_set_voltage(cci_df, opp_voltage);
+ if (ret) {
+ pr_err("cci: failed to scale up voltage\n");
+ return ret;
+ }
+ }
+
+ ret = clk_set_rate(cci_df->cci_clk, *freq);
+ if (ret) {
+ pr_err("%s: failed cci to set rate: %d\n", __func__,
+ ret);
+ mtk_cci_set_voltage(cci_df, old_voltage);
+ return ret;
+ }
+
+ // scale down: set freq first then voltage
+ if (opp_voltage < old_voltage) {
+ ret = mtk_cci_set_voltage(cci_df, opp_voltage);
+ if (ret) {
+ pr_err("cci: failed to scale down voltage\n");
+ clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
+ return ret;
+ }
+ }
+
+ cci_df->old_freq = *freq;
+
+ return 0;
+}
+
+static struct devfreq_dev_profile cci_devfreq_profile = {
+ .target = mtk_cci_devfreq_target,
+};
+
+static int mtk_cci_devfreq_probe(struct platform_device *pdev)
+{
+ struct device *cci_dev = &pdev->dev;
+ struct cci_devfreq *cci_df;
+ struct devfreq_passive_data *passive_data;
+ int ret;
+
+ cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
+ if (!cci_df)
+ return -ENOMEM;
+
+ cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
+ ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(cci_dev, "failed to get clock for CCI: %d\n",
+ ret);
Use dev_err_probe() to handle EPROBE_DEFER case. It makes code more simple.
+ return ret;
+ }
+ cci_df->cpu_reg = devm_regulator_get_optional(cci_dev, "proc");
+ ret = PTR_ERR_OR_ZERO(cci_df->cpu_reg);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
+ ret);
ditto. Use dev_err_probe()
+ return ret;
+ }
+ ret = regulator_enable(cci_df->cpu_reg);
+ if (ret) {
+ dev_err(cci_dev, "enable buck for cci fail\n");
+ return ret;
+ }
+
+ ret = dev_pm_opp_of_add_table(cci_dev);
+ if (ret) {
+ dev_err(cci_dev, "Fail to get OPP table for CCI: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, cci_df);
+
+ passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
+ if (!passive_data) {
+ ret = -ENOMEM;
+ goto err_opp;
+ }
+
+ passive_data->parent_type = CPUFREQ_PARENT_DEV;
+
+ cci_df->devfreq = devm_devfreq_add_device(cci_dev,
+ &cci_devfreq_profile,
+ DEVFREQ_GOV_PASSIVE,
+ passive_data);
+ if (IS_ERR(cci_df->devfreq)) {
+ ret = PTR_ERR(cci_df->devfreq);
+ dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
+ goto err_opp;
+ }
+
+ return 0;
+
+err_opp:
+ dev_pm_opp_of_remove_table(cci_dev);
+ return ret;
+}
+
+static int mtk_cci_devfreq_remove(struct platform_device *pdev)
+{
+ struct device *cci_dev = &pdev->dev;
+ struct cci_devfreq *cci_df;
+ struct notifier_block *opp_nb;
+
+ cci_df = platform_get_drvdata(pdev);
+ opp_nb = &cci_df->opp_nb;
+
+ dev_pm_opp_unregister_notifier(cci_dev, opp_nb);
Why do you call this function without registration?
If you want to catch the OPP changes of devfreq,
you can use devfreq_register_opp_notifier/devfreq_unregister_opp_notifier
functions.
On Thu, 2021-03-25 at 16:04 +0800, Chanwoo Choi wrote:
Hi,
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
quoted
From: "Andrew-sh.Cheng" <redacted>
This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
of the Mediatek MT8183.
On the MT8183 the CCI is supplied by the same regulator as the LITTLE
cores. The driver is notified when the regulator voltage changes
(driven by cpufreq) and adjusts the CCI frequency to the maximum
possible value.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/Kconfig | 10 ++
drivers/devfreq/Makefile | 1 +
drivers/devfreq/mt8183-cci-devfreq.c | 198 +++++++++++++++++++++++++++++++++++
3 files changed, 209 insertions(+)
create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
nitpick. how about using 'old_voltage'?
because 'vproc' is not easy for understanding.
I will modify it on next patch version.
quoted
+ unsigned long old_freq;
+};
+
+static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
nitpick: how about changing 'vproc -> voltage'?
I will modify it on next patch version.
quoted
+{
+ int ret;
+
+ ret = regulator_set_voltage(cci_df->cpu_reg, vproc,
+ MAX_VOLT_LIMIT);
+ if (!ret)
+ cci_df->old_vproc = vproc;
+ return ret;
+}
+
+static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
+ u32 flags)
+{
+ int ret;
+ struct cci_devfreq *cci_df = dev_get_drvdata(dev);
+ struct dev_pm_opp *opp;
+ unsigned long opp_rate, opp_voltage, old_voltage;
+
+ if (!cci_df)
+ return -EINVAL;
+
+ if (cci_df->old_freq == *freq)
+ return 0;
+
+ opp_rate = *freq;
+ opp = devfreq_recommended_opp(dev, &opp_rate, 1);
+ opp_voltage = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+
+ old_voltage = cci_df->old_vproc;
+ if (old_voltage == 0)
+ old_voltage = regulator_get_voltage(cci_df->cpu_reg);
+
+ // scale up: set voltage first then freq
+ if (opp_voltage > old_voltage) {
+ ret = mtk_cci_set_voltage(cci_df, opp_voltage);
+ if (ret) {
+ pr_err("cci: failed to scale up voltage\n");
+ return ret;
+ }
+ }
+
+ ret = clk_set_rate(cci_df->cci_clk, *freq);
+ if (ret) {
+ pr_err("%s: failed cci to set rate: %d\n", __func__,
+ ret);
+ mtk_cci_set_voltage(cci_df, old_voltage);
+ return ret;
+ }
+
+ // scale down: set freq first then voltage
+ if (opp_voltage < old_voltage) {
+ ret = mtk_cci_set_voltage(cci_df, opp_voltage);
+ if (ret) {
+ pr_err("cci: failed to scale down voltage\n");
+ clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
+ return ret;
+ }
+ }
+
+ cci_df->old_freq = *freq;
+
+ return 0;
+}
+
+static struct devfreq_dev_profile cci_devfreq_profile = {
+ .target = mtk_cci_devfreq_target,
+};
+
+static int mtk_cci_devfreq_probe(struct platform_device *pdev)
+{
+ struct device *cci_dev = &pdev->dev;
+ struct cci_devfreq *cci_df;
+ struct devfreq_passive_data *passive_data;
+ int ret;
+
+ cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
+ if (!cci_df)
+ return -ENOMEM;
+
+ cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
+ ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(cci_dev, "failed to get clock for CCI: %d\n",
+ ret);
Use dev_err_probe() to handle EPROBE_DEFER case. It makes code more simple.
I will modify it on next patch version.
quoted
+ return ret;
+ }
+ cci_df->cpu_reg = devm_regulator_get_optional(cci_dev, "proc");
+ ret = PTR_ERR_OR_ZERO(cci_df->cpu_reg);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
+ ret);
ditto. Use dev_err_probe()
I will modify it on next patch version.
quoted
+ return ret;
+ }
+ ret = regulator_enable(cci_df->cpu_reg);
+ if (ret) {
+ dev_err(cci_dev, "enable buck for cci fail\n");
+ return ret;
+ }
+
+ ret = dev_pm_opp_of_add_table(cci_dev);
+ if (ret) {
+ dev_err(cci_dev, "Fail to get OPP table for CCI: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, cci_df);
+
+ passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
+ if (!passive_data) {
+ ret = -ENOMEM;
+ goto err_opp;
+ }
+
+ passive_data->parent_type = CPUFREQ_PARENT_DEV;
+
+ cci_df->devfreq = devm_devfreq_add_device(cci_dev,
+ &cci_devfreq_profile,
+ DEVFREQ_GOV_PASSIVE,
+ passive_data);
+ if (IS_ERR(cci_df->devfreq)) {
+ ret = PTR_ERR(cci_df->devfreq);
+ dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
+ goto err_opp;
+ }
+
+ return 0;
+
+err_opp:
+ dev_pm_opp_of_remove_table(cci_dev);
+ return ret;
+}
+
+static int mtk_cci_devfreq_remove(struct platform_device *pdev)
+{
+ struct device *cci_dev = &pdev->dev;
+ struct cci_devfreq *cci_df;
+ struct notifier_block *opp_nb;
+
+ cci_df = platform_get_drvdata(pdev);
+ opp_nb = &cci_df->opp_nb;
+
+ dev_pm_opp_unregister_notifier(cci_dev, opp_nb);
Why do you call this function without registration?
If you want to catch the OPP changes of devfreq,
you can use devfreq_register_opp_notifier/devfreq_unregister_opp_notifier
functions.
Yes, I will move it to
[V8,7/8] devfreq: mediatek: cci devfreq register opp notification for
SVS support
Need to change it as following at same line:
static const __maybe_unused struct of_device_idmediatek_cci_of_match[] = {
Hi Chanwoo,
I don't quite understand when to us __maybe_unused
This is a suggestion from patch version 2.
https://patchwork.kernel.org/patch/10876449/
Please give me some advice.
Thank you.
From: "Andrew-sh.Cheng" <redacted>
For case of share buck with other module,
the result buck voltage may not exactly the same with what we set.
Need to record the previous desired value instead of read from regulator.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/cpufreq/mediatek-cpufreq.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
From: "Andrew-sh.Cheng" <redacted>
SVS will change the voltage of opp item.
CCI devfreq need to react to change frequency.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/mt8183-cci-devfreq.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
@@ -89,6 +90,26 @@ static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,return0;}+staticintccidevfreq_opp_notifier(structnotifier_block*nb,+unsignedlongevent,void*data)+{+structdev_pm_opp*opp=data;+structcci_devfreq*cci_df=container_of(nb,structcci_devfreq,+opp_nb);+unsignedlongfreq,volt;++if(event==OPP_EVENT_ADJUST_VOLTAGE){+freq=dev_pm_opp_get_freq(opp);+/* current opp item is changed */+if(freq==cci_df->old_freq){+volt=dev_pm_opp_get_voltage(opp);+mtk_cci_set_voltage(cci_df,volt);+}+}++return0;+}+staticstructdevfreq_dev_profilecci_devfreq_profile={.target=mtk_cci_devfreq_target,};
@@ -98,12 +119,15 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)structdevice*cci_dev=&pdev->dev;structcci_devfreq*cci_df;structdevfreq_passive_data*passive_data;+structnotifier_block*opp_nb;intret;cci_df=devm_kzalloc(cci_dev,sizeof(*cci_df),GFP_KERNEL);if(!cci_df)return-ENOMEM;+opp_nb=&cci_df->opp_nb;+cci_df->cci_clk=devm_clk_get(cci_dev,"cci_clock");ret=PTR_ERR_OR_ZERO(cci_df->cci_clk);if(ret){
@@ -152,6 +176,9 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)gotoerr_opp;}+opp_nb->notifier_call=ccidevfreq_opp_notifier;+dev_pm_opp_register_notifier(cci_dev,opp_nb);+return0;err_opp:
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, 2021-03-25 at 17:11 +0900, Chanwoo Choi wrote:
Hi,
I think that you can squash this patch to patch4.
On 3/23/21 8:34 PM, Andrew-sh.Cheng wrote:
quoted
From: "Andrew-sh.Cheng" <redacted>
SVS will change the voltage of opp item.
What it the full name of SVS?
Due to the content of this patch is for SVS, so I separate it from
patch4.
SVS is Smart-Voltage-Scaling.
It will check the IC quality, and then modify the voltage field of opp
table.
The required voltage will smaller than original signed-off voltage in
opp table.
This voltage will change when temperature is changed.
cci devfreq need to raise voltage when the required voltage raise.
quoted
CCI devfreq need to react to change frequency.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/devfreq/mt8183-cci-devfreq.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
From: "Andrew-sh.Cheng" <redacted>
cpufreq should listen opp notification and do proper actions
when receiving disable and voltage adjustment events,
which are triggered when SVS is enabled.
Signed-off-by: Andrew-sh.Cheng <redacted>
---
drivers/cpufreq/mediatek-cpufreq.c | 73 ++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
@@ -239,6 +243,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,vproc=dev_pm_opp_get_voltage(opp);dev_pm_opp_put(opp);+mutex_lock(&info->lock);/**Ifthenewvoltageortheintermediatevoltageishigherthanthe*currentvoltage,scaleupvoltagefirst.
@@ -250,6 +255,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,pr_err("cpu%d: failed to scale up voltage!\n",policy->cpu);mtk_cpufreq_set_voltage(info,old_vproc);+mutex_unlock(&info->lock);returnret;}}
@@ -261,6 +267,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,policy->cpu);mtk_cpufreq_set_voltage(info,old_vproc);WARN_ON(1);+mutex_unlock(&info->lock);returnret;}
@@ -271,6 +278,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,policy->cpu);clk_set_parent(cpu_clk,armpll);mtk_cpufreq_set_voltage(info,old_vproc);+mutex_unlock(&info->lock);returnret;}
@@ -281,6 +289,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,policy->cpu);mtk_cpufreq_set_voltage(info,inter_vproc);WARN_ON(1);+mutex_unlock(&info->lock);returnret;}
@@ -296,15 +305,69 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,clk_set_parent(cpu_clk,info->inter_clk);clk_set_rate(armpll,old_freq_hz);clk_set_parent(cpu_clk,armpll);+mutex_unlock(&info->lock);returnret;}}+info->opp_freq=freq_hz;+mutex_unlock(&info->lock);+return0;}#define DYNAMIC_POWER "dynamic-power-coefficient"+staticintmtk_cpufreq_opp_notifier(structnotifier_block*nb,+unsignedlongevent,void*data)+{+structdev_pm_opp*opp=data;+structdev_pm_opp*new_opp;+structmtk_cpu_dvfs_info*info;+unsignedlongfreq,volt;+structcpufreq_policy*policy;+intret=0;++info=container_of(nb,structmtk_cpu_dvfs_info,opp_nb);++if(event==OPP_EVENT_ADJUST_VOLTAGE){+freq=dev_pm_opp_get_freq(opp);++mutex_lock(&info->lock);+if(info->opp_freq==freq){+volt=dev_pm_opp_get_voltage(opp);+ret=mtk_cpufreq_set_voltage(info,volt);+if(ret)+dev_err(info->cpu_dev,"failed to scale voltage: %d\n",+ret);+}+mutex_unlock(&info->lock);+}elseif(event==OPP_EVENT_DISABLE){+freq=dev_pm_opp_get_freq(opp);+/* case of current opp item is disabled */+if(info->opp_freq==freq){+freq=1;+new_opp=dev_pm_opp_find_freq_ceil(info->cpu_dev,+&freq);+if(!IS_ERR(new_opp)){+dev_pm_opp_put(new_opp);+policy=cpufreq_cpu_get(info->opp_cpu);+if(policy){+cpufreq_driver_target(policy,+freq/1000,+CPUFREQ_RELATION_L);+cpufreq_cpu_put(policy);+}+}else{+pr_err("%s: all opp items are disabled\n",+__func__);+}+}+}++returnnotifier_from_errno(ret);+}+staticintmtk_cpu_dvfs_info_init(structmtk_cpu_dvfs_info*info,intcpu){structdevice*cpu_dev;
@@ -400,11 +463,21 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)info->intermediate_voltage=dev_pm_opp_get_voltage(opp);dev_pm_opp_put(opp);+info->opp_cpu=cpu;+info->opp_nb.notifier_call=mtk_cpufreq_opp_notifier;+ret=dev_pm_opp_register_notifier(cpu_dev,&info->opp_nb);+if(ret){+pr_warn("cannot register opp notification\n");+gotoout_disable_inter_clock;+}++mutex_init(&info->lock);info->cpu_dev=cpu_dev;info->proc_reg=proc_reg;info->sram_reg=IS_ERR(sram_reg)?NULL:sram_reg;info->cpu_clk=cpu_clk;info->inter_clk=inter_clk;+info->opp_freq=clk_get_rate(cpu_clk);/**IfSRAMregulatorispresent,software"voltage tracking"isneeded
--
2.12.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel