23.08.2021 13:46, Ulf Hansson пишет:

quoted

quoted

...
dev_pm_opp_set_rate(rate)
pm_runtime_get_noresume()
pm_runtime_set_active()
pm_runtime_enable()
...
pm_runtime_put()
...

We need to call genpd_set_performance_state() independently of whether
the device is runtime suspended or not.

I don't see where is the problem in yours example.

pm_runtime_suspended() = false while RPM is disabled. When device is
resumed, the rpm_pstate=0, so it won't change the pstate on resume.

Yes, you are certainly correct, my bad! I mixed it up with
pm_runtime_status_suspended(), which only cares about the status.

So, after a second thought, your suggestion sounds very much
reasonable to me! I have also tried to consider all different
scenarios, including the system suspend/resume path, but I think it
should be fine.

It could be improved slightly to cover more cases.

I also think that a patch like the above should be considered as a
fix, because it actually fixes a problem, according to what I said in
my earlier reply, below.

Fixes : 5937c3ce2122 ("PM: domains: Drop/restore performance state
votes for devices at runtime PM").

quoted

quoted

Although, it actually seems like good idea to update
dev_gpd_data(dev)->rpm_pstate = state here, as to make sure
genpd_runtime_resume() doesn't restore an old/invalid value that was
saved while dropping the performance state vote for the device in
genpd_runtime_suspend() earlier.

Let me send a patch for this shortly, to close this window of a possible error.

It will also remove the need to resume device just to change the clock
rate, like I needed to do it in the PWM patch of this series.

Do you want to send the patch formally? Or do you prefer it if I do it?

I'll send the patch.