| Message ID | 20201028140847.1018-3-lukasz.luba@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | Add sustainable OPP concept | expand |
Hi Lukasz, On Wednesday 28 Oct 2020 at 14:08:45 (+0000), Lukasz Luba wrote: > +unsigned long dev_pm_opp_get_sustainable_opp_freq(struct device *dev) > +{ > + struct opp_table *opp_table; > + unsigned long freq = 0; > + > + opp_table = _find_opp_table(dev); > + if (IS_ERR(opp_table)) > + return 0; > + > + if (opp_table->sustainable_opp && opp_table->sustainable_opp->available) > + freq = dev_pm_opp_get_freq(opp_table->sustainable_opp); > + > + dev_pm_opp_put_opp_table(opp_table); > + > + return freq; > +} > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_sustainable_opp_freq); I'm guessing this is what IPA will use to find out what the sustainable frequency is right? Is PM_OPP the right place for that? It feels odd IPA will get the EM from one place, which includes the performance state, and the sustained OPP from another. Should we move that to PM_EM instead? Thanks, Quentin
Hi Quentin, On 10/30/20 11:47 AM, Quentin Perret wrote: > Hi Lukasz, > > On Wednesday 28 Oct 2020 at 14:08:45 (+0000), Lukasz Luba wrote: >> +unsigned long dev_pm_opp_get_sustainable_opp_freq(struct device *dev) >> +{ >> + struct opp_table *opp_table; >> + unsigned long freq = 0; >> + >> + opp_table = _find_opp_table(dev); >> + if (IS_ERR(opp_table)) >> + return 0; >> + >> + if (opp_table->sustainable_opp && opp_table->sustainable_opp->available) >> + freq = dev_pm_opp_get_freq(opp_table->sustainable_opp); >> + >> + dev_pm_opp_put_opp_table(opp_table); >> + >> + return freq; >> +} >> +EXPORT_SYMBOL_GPL(dev_pm_opp_get_sustainable_opp_freq); > > I'm guessing this is what IPA will use to find out what the sustainable > frequency is right? Yes, you are right. > > Is PM_OPP the right place for that? It feels odd IPA will get the EM > from one place, which includes the performance state, and the sustained > OPP from another. Should we move that to PM_EM instead? True, it might looks strange, but the OPP framework is available when we are adding the OPPs in scmi perf layer. The EM is available after we register the device, so at the end of scmi-cpufreq init. It would require a new scmi perf api function e.g. get_sustained_freq(), and a set/get function for EM, which is doable. I've discussed this approach to Viresh and he likes it better. I am happy that you are also suggesting the EM approach. I will send different patches for EM and SCMI to make that happen. Should I re-based them on top of the patch adding this milliwatts filed in EM [1]? Or do the opposite, changing the dependency order? Regards, Lukasz [1] https://lkml.org/lkml/2020/10/19/392 > > Thanks, > Quentin >
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 9915e8487f0b..bb1e68b96d0e 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -299,6 +299,32 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) } EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); +/** + * dev_pm_opp_get_sustainable_opp_freq() - Get frequency of sustainable opp + * in Hz + * @dev: device for which we do this operation + * + * Return: This function returns the frequency of the OPP marked as + * sustainable_opp if one is available, else returns 0; + */ +unsigned long dev_pm_opp_get_sustainable_opp_freq(struct device *dev) +{ + struct opp_table *opp_table; + unsigned long freq = 0; + + opp_table = _find_opp_table(dev); + if (IS_ERR(opp_table)) + return 0; + + if (opp_table->sustainable_opp && opp_table->sustainable_opp->available) + freq = dev_pm_opp_get_freq(opp_table->sustainable_opp); + + dev_pm_opp_put_opp_table(opp_table); + + return freq; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_sustainable_opp_freq); + int _get_opp_count(struct opp_table *opp_table) { struct dev_pm_opp *opp; diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 9faeb83e4b32..1f6b19bb1a95 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -815,6 +815,20 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, } } + if (of_property_read_bool(np, "opp-sustainable")) { + if (opp_table->sustainable_opp) { + /* Pick the OPP with higher rate as sustainable OPP */ + if (new_opp->rate > opp_table->sustainable_opp->rate) { + opp_table->sustainable_opp->sustainable = false; + new_opp->sustainable = true; + opp_table->sustainable_opp = new_opp; + } + } else { + new_opp->sustainable = true; + opp_table->sustainable_opp = new_opp; + } + } + if (new_opp->clock_latency_ns > opp_table->clock_latency_ns_max) opp_table->clock_latency_ns_max = new_opp->clock_latency_ns; diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index ebd930e0b3ca..45288ccbb295 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -56,6 +56,7 @@ extern struct list_head opp_tables; * @dynamic: not-created from static DT entries. * @turbo: true if turbo (boost) OPP * @suspend: true if suspend OPP + * @sustainable: true if sustainable OPP * @pstate: Device's power domain's performance state. * @rate: Frequency in hertz * @level: Performance level @@ -78,6 +79,7 @@ struct dev_pm_opp { bool dynamic; bool turbo; bool suspend; + bool sustainable; unsigned int pstate; unsigned long rate; unsigned int level; @@ -183,6 +185,7 @@ struct opp_table { unsigned int parsed_static_opps; enum opp_table_access shared_opp; struct dev_pm_opp *suspend_opp; + struct dev_pm_opp *sustainable_opp; struct mutex genpd_virt_dev_lock; struct device **genpd_virt_devs; diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index dbb484524f82..363d5a2c1ef3 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -106,6 +106,7 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev); unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev); unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev); unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev); +unsigned long dev_pm_opp_get_sustainable_opp_freq(struct device *dev); struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, unsigned long freq, @@ -215,6 +216,11 @@ static inline unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) return 0; } +static inline unsigned long dev_pm_opp_get_sustainable_opp_freq(struct device *dev) +{ + return 0; +} + static inline struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, unsigned long freq, bool available) {
Now that the OPP bindings are updated to include an optional 'opp-sustainable' property, add support to parse it from device tree and store it as part of dev_pm_opp structure. Also add and export an helper 'dev_pm_opp_get_sustainable()' that can be used to get the sustainable OPP when present. Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> --- drivers/opp/core.c | 26 ++++++++++++++++++++++++++ drivers/opp/of.c | 14 ++++++++++++++ drivers/opp/opp.h | 3 +++ include/linux/pm_opp.h | 6 ++++++ 4 files changed, 49 insertions(+)