On 03-06-21, 13:17, Ulf Hansson wrote:

On Thu, 3 Jun 2021 at 12:31, Ulf Hansson [off-list ref] wrote:

quoted

quoted

quoted

+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