| Message ID | cedbfc3a7fffc4c400908c4c3af518bea408d318.1541399301.git.viresh.kumar@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PM / Domains: Allow performance state propagation | expand |
On 21-11-18, 11:48, Rajendra Nayak wrote: > > > On 11/21/2018 11:36 AM, Viresh Kumar wrote: > > On 21-11-18, 10:47, Viresh Kumar wrote: > > > On 21-11-18, 10:34, Rajendra Nayak wrote: > > > > > > > > > > > > On 11/5/2018 12:06 PM, Viresh Kumar wrote: > > > > > Introduce a new helper dev_pm_opp_xlate_performance_state() which will > > > > > be used to translate from pstate of a device to another one. > > > > > > > > > > Initially this will be used by genpd to find pstate of a master domain > > > > > using its sub-domain's pstate. > > > > > > > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > > --- > > > > > drivers/opp/core.c | 49 ++++++++++++++++++++++++++++++++++++++++++ > > > > > include/linux/pm_opp.h | 7 ++++++ > > > > > 2 files changed, 56 insertions(+) > > > > > > > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > > > > index 0eaa954b3f6c..010a4268e8dd 100644 > > > > > --- a/drivers/opp/core.c > > > > > +++ b/drivers/opp/core.c > > > > > @@ -1707,6 +1707,55 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, > > > > > dev_err(virt_dev, "Failed to find required device entry\n"); > > > > > } > > > > > +/** > > > > > + * dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table. > > > > > + * @src_table: OPP table which has dst_table as one of its required OPP table. > > > > > > > > So I have a case where the src_table and dst_table are shared/same. Can you explain how would > > > > it work in such a case? > > > > > > Can you give the example, as I am finding some issues with such shared > > > tables. Though the code may work just fine btw. > > > > I may have found the problem you are facing here. Please try this diff > > and tell me if you hitting it, check this in dmesg. > > Yes, I do seem to be hitting this. So there are few complexities in the case where an OPP table points to itself in the required-opp field. I looked at solving it up in the opp core but that gets more and more messy. Now there is actually a assumption within the OPP core. Your Mx domain should get initialized before the Cx domain, as that is when the OPP tables are created as well. This is because Cx's OPP table will point to Mx's OPP table (doesn't matter if they share the same table or not) and so Mx's OPP table should come first. Can you check if that is already the case for you? If not, please try doing it and lemme know if it works. It should. I just want to avoid too much complexity in OPP core without much use. -- viresh
On 23-11-18, 14:41, Viresh Kumar wrote: > On 22-11-18, 11:38, Viresh Kumar wrote: > > So there are few complexities in the case where an OPP table points to itself in > > the required-opp field. I looked at solving it up in the opp core but that gets > > more and more messy. > > > > Now there is actually a assumption within the OPP core. Your Mx domain should > > get initialized before the Cx domain, as that is when the OPP tables are created > > as well. This is because Cx's OPP table will point to Mx's OPP table (doesn't > > matter if they share the same table or not) and so Mx's OPP table should come > > first. > > > > Can you check if that is already the case for you? If not, please try doing it > > and lemme know if it works. It should. > > > > I just want to avoid too much complexity in OPP core without much use. > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 7f338d39809a..09df3fbdb555 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1734,7 +1734,7 @@ unsigned int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, > int i; > > for (i = 0; i < src_table->required_opp_count; i++) { > - if (src_table->required_opp_tables[i] == dst_table) > + if (src_table->required_opp_tables[i]->np == dst_table->np) > break; > } > > Try this fix and it should make it work for you. > > Having said that, the way you are representing all your domains with a > single OPP table look incorrect. You are using the same OPP table for > around 10 power domains, all provided by a single provider. This is > absolutely fine. But the Mx domain doesn't have any dependency on any > other domain actually and so its OPPs should never have the > "required-opps" property, but it has those properties in your setup as > you are trying to use the same OPP table. That may all work fine with > the above patch (which is required anyway as it fixes a valid issue), > but you may see a error warning that something failed for Mx domain, > as it has required-opps property but no required device or genpd. > > Anyway, please test above first and then you can fix your tables :) Here is the fix you need for the case where cx and mx have 1:1 mapping and we don't need to duplicate OPP tables in DT. diff --git a/drivers/opp/core.c b/drivers/opp/core.c index ad944d75b40b..99b71f60b6d8 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1727,6 +1727,16 @@ unsigned int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, unsigned int dest_pstate = 0; int i; + /* + * Normally the src_table will have the "required_opps" property set to + * point to one of the OPPs in the dst_table, but in some cases the + * genpd and its master have one to one mapping of performance states + * and so none of them have the "required-opps" property set. Return the + * pstate of the src_table as it is in such cases. + */ + if (!src_table->required_opp_count) + return pstate; + for (i = 0; i < src_table->required_opp_count; i++) { if (src_table->required_opp_tables[i]->np == dst_table->np) break; -- viresh
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 0eaa954b3f6c..010a4268e8dd 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1707,6 +1707,55 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, dev_err(virt_dev, "Failed to find required device entry\n"); } +/** + * dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table. + * @src_table: OPP table which has dst_table as one of its required OPP table. + * @dst_table: Required OPP table of the src_table. + * @pstate: Current performance state of the src_table. + * + * This Returns pstate of the OPP (present in @dst_table) pointed out by the + * "required-opps" property of the OPP (present in @src_table) which has + * performance state set to @pstate. + * + * Return: Positive performance state on success, otherwise 0 on errors. + */ +unsigned int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, + struct opp_table *dst_table, + unsigned int pstate) +{ + struct dev_pm_opp *opp; + unsigned int dest_pstate = 0; + int i; + + for (i = 0; i < src_table->required_opp_count; i++) { + if (src_table->required_opp_tables[i] == dst_table) + break; + } + + if (unlikely(i == src_table->required_opp_count)) { + pr_err("%s: Couldn't find matching OPP table (%p: %p)\n", + __func__, src_table, dst_table); + return 0; + } + + mutex_lock(&src_table->lock); + + list_for_each_entry(opp, &src_table->opp_list, node) { + if (opp->pstate == pstate) { + dest_pstate = opp->required_opps[i]->pstate; + goto unlock; + } + } + + pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table, + dst_table); + +unlock: + mutex_unlock(&src_table->lock); + + return dest_pstate; +} + /** * dev_pm_opp_add() - Add an OPP table from a table definitions * @dev: device for which we do this operation diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 2b2c3fd985ab..5a64a81a1789 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -128,6 +128,7 @@ struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*s void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table); struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, struct device *virt_dev, int index); void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, struct device *virt_dev); +unsigned int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate); int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq); int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask); int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask); @@ -280,6 +281,12 @@ static inline struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev } static inline void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, struct device *virt_dev) {} + +static inline unsigned int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate) +{ + return 0; +} + static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) { return -ENOTSUPP;
Introduce a new helper dev_pm_opp_xlate_performance_state() which will be used to translate from pstate of a device to another one. Initially this will be used by genpd to find pstate of a master domain using its sub-domain's pstate. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/opp/core.c | 49 ++++++++++++++++++++++++++++++++++++++++++ include/linux/pm_opp.h | 7 ++++++ 2 files changed, 56 insertions(+) -- 2.19.1.568.g152ad8e3369a