Messages in this thread | | | From | Ulf Hansson <> | Date | Wed, 9 Jun 2021 14:25:21 +0200 | Subject | Re: [PATCH v2 3/4] PM: domains: Drop/restore performance state votes for devices at runtime PM |
| |
On Mon, 7 Jun 2021 at 06:47, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 04-06-21, 09:45, Ulf Hansson wrote: > > Starting calls from the subsystem/driver: > > > > ------ > > dev_pm_genpd_set_performance_state(dev, 100); > > "run a use case with device runtime resumed" > > ... > > "use case ends" > > dev_pm_genpd_set_performance_state(dev, 0); > > pm_runtime_put() > > ->genpd_runtime_suspend() > > gpd_data->performance_state == 0, -> gpd_data->rpm_pstate = 0; > > ... > > "new use case start" > > dev_pm_genpd_set_performance_state(dev, 100); > > pm_runtime_get_sync() > > ->genpd_runtime_resume() > > gpd_data->performance_state == 100, -> gpd_data->rpm_pstate = 0; > > (This is where we need to check for "zero" to not override the value) > > ..... > > ------ > > > > I wouldn't say that the above is the way how I see the calls to > > dev_pm_genpd_set_performance_state (or actually > > dev_pm_opp_set_rate|opp()) being deployed. The calls should rather be > > done from the subsystem/driver's ->runtime_suspend|resume() callback, > > then the path above would work in the way you suggest. > > > > Although, as we currently treat performance states and power states in > > genpd orthogonally, I wanted to make sure we could cope with both > > situations. > > I think letting the drivers to call > dev_pm_genpd_set_performance_state(dev, 0) from suspend/resume makes > it really ugly/racy as both depend on the gpd_data->performance_state > for this. It doesn't look nice. And we shouldn't try to protect such > drivers. > > Anyway, your call :)
Well, I am not sure we have an option at this point. As long as we allow performance states to be managed orthogonally to on/off states in genpd, the check in genpd_restore_performance_state() is needed.
I have started to prepare a new version of the series - and will add a comment about this in the code to try to clarify this.
[...]
Kind regards Uffe
| |