Messages in this thread | | | Date | Fri, 4 Jun 2021 09:23:12 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH v2 3/4] PM: domains: Drop/restore performance state votes for devices at runtime PM |
| |
On 03-06-21, 13:17, Ulf Hansson wrote: > On Thu, 3 Jun 2021 at 12:31, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > +static int genpd_drop_performance_state(struct device *dev) > > > > +{ > > > > + unsigned int prev_state = dev_gpd_data(dev)->performance_state; > > > > + > > > > + if (!genpd_set_performance_state(dev, 0)) > > > > + return prev_state; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static void genpd_restore_performance_state(struct device *dev, > > > > + unsigned int state) > > > > +{ > > > > + if (state) > > > > > > I will skip this check, as we are checking it in > > > genpd_set_performance_state() anyway ? > > > > I don't want us to override OPP votes made by the subsystem/driver > > level runtime PM callbacks. For example, if the drivers manage this > > thing themselves, that should be preserved. > > > > That said, by the check above I want to avoid setting the state to > > zero internally by genpd, if the driver level ->runtime_resume() > > callback has already restored the state. > > Ehh, forget about what I said about the ->runtime_resume() callback. > > I am mostly trying to avoid restoring a state that is zero, just to be > sure nobody else on some different level outside gendp, have decided > to set a new OPP in-between our calls to > genpd_drop|restore_performance state.
What stops the core to call genpd_drop_performance_state() in the first place here, if the driver was doing its own thing ? If that gets called, then restore should be without any checks IMO. The state should already be 0 at this point of time, I don't know why this will get called again with state 0, but it will have no effect.
Can you give some sort of flow sequence where I can see the problem a bit more clearly ?
-- viresh
| |