[0/4] PM / Domains: Allow performance state propagation

Hi,

This series adds performance state propagation support in genpd core.
The propagation happens from the sub-domains to their masters. More
details can be found in the individual commit logs.

This is tested on hikey960 by faking power domains in such a way that
the CPU devices have two power domains and both of them have the same
master domain. The CPU device, as well as its power domains have
"required-opps" property set and the performance requirement from the
CPU eventually configures all the domains (2 sub-domains and 1 master).

@Rajendra: Will it be possible for you to run some tests and tell me if
some stuff is still missing as per Qcom requirements ?

Based on opp/linux-next branch (which is 4.20-rc1 +
multiple-power-domain-support-in-opp-core).

--
viresh

Viresh Kumar (4):
  OPP: Add dev_pm_opp_xlate_performance_state() helper
  PM / Domains: Save OPP table pointer in genpd
  PM / Domains: Factorize dev_pm_genpd_set_performance_state()
  PM / Domains: Propagate performance state updates

 drivers/base/power/domain.c | 204 +++++++++++++++++++++++++++---------
 drivers/opp/core.c          |  49 +++++++++
 include/linux/pm_domain.h   |   6 ++
 include/linux/pm_opp.h      |   7 ++
 4 files changed, 217 insertions(+), 49 deletions(-)

-- 
2.19.1.568.g152ad8e3369a

On 21-11-18, 10:33, Rajendra Nayak wrote:
> Hi Viresh,

> 

> On 11/5/2018 12:06 PM, Viresh Kumar wrote:

> > This commit updates genpd core to start propagating performance state

> > updates to master domains that have their set_performance_state()

> > callback set.

> > 

> > A genpd handles two type of performance states now. The first one is the

> > performance state requirement put on the genpd by the devices and

> > sub-domains under it, which is already represented by

> > genpd->performance_state. The second type, introduced in this commit, is

> > the performance state requirement(s) put by the genpd on its master

> > domain(s). There is a separate value required for each master that the

> > genpd has and so a new field is added to the struct gpd_link

> > (link->performance_state), which represents the link between a genpd and

> > its master. The struct gpd_link also got another field

> > prev_performance_state, which is used by genpd core as a temporary

> > variable during transitions.

> > 

> > We need to propagate setting performance state while powering-on a genpd

> > as well, as we ignore performance state requirements from sub-domains

> > which are powered-off. For this reason _genpd_power_on() also received

> > the additional parameter, depth, which is used for hierarchical locking

> > within genpd.

> > 

> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> > ---

> >   drivers/base/power/domain.c | 107 +++++++++++++++++++++++++++++-------

> >   include/linux/pm_domain.h   |   4 ++

> >   2 files changed, 92 insertions(+), 19 deletions(-)

> > 

> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c

> > index 6d2e9b3406f1..81e02c5f753f 100644

> > --- a/drivers/base/power/domain.c

> > +++ b/drivers/base/power/domain.c

> > @@ -239,28 +239,86 @@ static void genpd_update_accounting(struct generic_pm_domain *genpd)

> >   static inline void genpd_update_accounting(struct generic_pm_domain *genpd) {}

> >   #endif

> > +static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,

> > +					   unsigned int state, int depth);

> > +

> >   static int _genpd_set_performance_state(struct generic_pm_domain *genpd,

> > -					unsigned int state)

> > +					unsigned int state, int depth)

> >   {

> > +	struct generic_pm_domain *master;

> > +	struct gpd_link *link;

> > +	unsigned int mstate;

> >   	int ret;

> >   	if (!genpd_status_on(genpd))

> >   		goto out;

> 

> This check here would mean we only propogate performance state

> to masters if the genpd is ON?


Yeah, isn't that what should we do anyway? It is similar to clk-enable
etc. Why increase the reference count if the device isn't enabled and
isn't using the clock ?

Also you can see that I have updated the genpd power-on code to start
propagation to make sure we don't miss anything.

-- 
viresh

On 21-11-18, 11:12, Rajendra Nayak wrote:
> And the reason for that seems to be that we update the genpd status to GPD_STATE_ACTIVE

> *after* we try to set the performance state, so we always hit this check which bails out

> thinking the genpd is not ON.


Thanks for looking at it. Here is the (untested) fix, please try it
out.

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 84c13695af65..92be4a224b45 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -250,9 +250,6 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
        unsigned int mstate;
        int ret;
 
-       if (!genpd_status_on(genpd))
-               goto out;
-
        /* Propagate to masters of genpd */
        list_for_each_entry(link, &genpd->slave_links, slave_node) {
                master = link->master;
@@ -286,7 +283,6 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
        if (ret)
                goto err;
 
-out:
        genpd->performance_state = state;
        return 0;
 
@@ -361,6 +357,11 @@ static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
                return 0;
 
 update_state:
+       if (!genpd_status_on(genpd)) {
+               genpd->performance_state = state;
+               return 0;
+       }
+
        return _genpd_set_performance_state(genpd, state, depth);
 }

-- 
viresh