Viresh Kumar <viresh.ku...@linaro.org> writes: > Some platforms have the capability to configure the performance state of > their Power Domains. The performance levels are represented by positive > integer values, a lower value represents lower performance state. > > This patch registers the power domain framework for PM QOS performance > notifier in order to manage performance state of power domains.
It seems to me it doesm't just register, but actually keeps track of the performance_state by always tracking the max. > This also allows the power domain drivers to implement a > ->set_performance_state() callback, which will be called by the power > domain core from the notifier routine. > > Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org> > --- > drivers/base/power/domain.c | 98 > ++++++++++++++++++++++++++++++++++++++++++++- > include/linux/pm_domain.h | 5 +++ > 2 files changed, 101 insertions(+), 2 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index a73d79670a64..1158a07f92de 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -367,6 +367,88 @@ static int genpd_dev_pm_qos_notifier(struct > notifier_block *nb, > return NOTIFY_DONE; > } > > +static void update_domain_performance_state(struct generic_pm_domain *genpd, > + int depth) > +{ > + struct generic_pm_domain_data *pd_data; > + struct generic_pm_domain *subdomain; > + struct pm_domain_data *pdd; > + unsigned int state = 0; > + struct gpd_link *link; > + > + /* Traverse all devices within the domain */ > + list_for_each_entry(pdd, &genpd->dev_list, list_node) { > + pd_data = to_gpd_data(pdd); > + > + if (pd_data->performance_state > state) > + state = pd_data->performance_state; > + } This seems to only update the state if it's bigger. Maybe I'm missing something here, but it seems like won't be able to lower the performance_state after it's been raised? > + /* Traverse all subdomains within the domain */ > + list_for_each_entry(link, &genpd->master_links, master_node) { > + subdomain = link->slave; > + > + if (subdomain->performance_state > state) > + state = subdomain->performance_state; > + } So subdomains are always assumed to influence the performance_state of the parent domains? Is that always the case? I suspect this should be probably be a reasonable default assumption, but maybe controlled with a flag. > + if (genpd->performance_state == state) > + return; > + > + genpd->performance_state = state; > + > + if (genpd->set_performance_state) { > + genpd->set_performance_state(genpd, state); > + return; > + } So is zero not a valid performance_state? That doesn't seem quite right to me, but either way, it should be documented. > + /* Propagate only if this domain has a single parent */ Why? This limitation should be explained in the cover letter and changelog. I would also expect some sort of WARN here since this could otherwise be a rather silent failures. [...] Kevin