Re: [PATCH v2 0/4] PM: domains: Avoid boilerplate code for DVFS in subsystem/drivers

From: Stephan Gerhold <stephan@gerhold.net>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	linux-pm@vger.kernel.org, Dmitry Osipenko <digetx@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	Roja Rani Yarubandi <rojay@codeaurora.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/4] PM: domains: Avoid boilerplate code for DVFS in subsystem/drivers
Date: Thu, 3 Jun 2021 13:12:55 +0200	[thread overview]
Message-ID: <YLi5N06Qs+gYHgYg@gerhold.net> (raw)
In-Reply-To: <20210603093438.138705-1-ulf.hansson@linaro.org>

On Thu, Jun 03, 2021 at 11:34:34AM +0200, Ulf Hansson wrote:
> Various discussions on LKML have pointed out that many subsystem/drivers for
> devices that may be attached to a genpd and which manages DVFS/OPP though the
> genpd performance states, would need very similar updates.
> 
> More precisely, they would likely have to call dev_pm_opp_set_rate|opp() to
> drop and restore OPPs (which propagates upwards into performance states votes
> in genpd), every time their devices should enter/exit a low power state, via
> their device PM callbacks.
> 
> Rather than having to add the boilerplate code for these things into the
> subsystems/drivers, this series implements the logic internally into genpd.
> 
> Concerns have been raised about this approach, mostly by myself, around that it
> limits flexibility. On the other hand, it starts to look like more and more
> people are requesting this to be manged internally in genpd, for good reasons.
> So, I think it's worth to give this a try.
> 
> In the long run, if it turns out that the flexibility was indeed needed, we can
> always deal with that as special cases on top.
> 

Do I understand your patch set correctly that you basically make the
performance state votes conditional to the "power-on" vote of the device
(which is automatically toggled during runtime/system PM)?

If yes, I think that's a good thing. It was always really confusing to me
that a device can make performance state votes if it doesn't actually
want the power domain to be powered on.

What happens if a driver calls dev_pm_genpd_set_performance_state(...)
while the device is suspended? Will that mess up the performance state
when the device resumes?

I think this might also go into the direction of my problem with the OPP
core for CPU DVFS [1] since the OPP core currently does not "power-on"
the power domains, it just sets a performance state. I got kind of stuck
with all the complexity of power domains in Linux so I think we never
solved that.

Stephan

[1]: https://lore.kernel.org/linux-pm/20200826093328.88268-1-stephan@gerhold.net/