[V2,0/3] OPP: Simplify required-opp handling

Message

Viresh Kumar Oct. 30, 2023, 10:24 a.m. UTC

Hello,

I wasn't able to test this locally (despite trying to hack it around) and need
help from someone who is `virt_devs` field of `struct dev_pm_opp_config`.

Pushed here:

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/required-opps

-------------------------8<-------------------------

Configuring the required OPP was never properly implemented, we just
took an exception for genpds and configured them directly, while leaving
out all other required OPP types.

Now that a standard call to dev_pm_opp_set_opp() takes care of
configuring the opp->level too, the special handling for genpds can be
avoided by simply calling dev_pm_opp_set_opp() for the required OPPs,
which shall eventually configure the corresponding level for genpds.

This also makes it possible for us to configure other type of required
OPPs (no concrete users yet though), via the same path. This is how
other frameworks take care of parent nodes, like clock, regulators, etc,
where we recursively call the same helper.

V1->V2:
- Support opp-level 0, drop vote i.e..
- Fix OPP pointer while calling dev_pm_opp_set_opp() recursively.
- Minor checks and fixes.
- Add Reviewed-by from Ulf.

--
Viresh

Viresh Kumar (3):
  OPP: Level zero is valid
  OPP: Use _set_opp_level() for single genpd case
  OPP: Call dev_pm_opp_set_opp() for required OPPs

 drivers/opp/core.c     | 176 ++++++++++++++++++++++-------------------
 drivers/opp/of.c       |  48 ++++++++---
 drivers/opp/opp.h      |   8 +-
 include/linux/pm_opp.h |  12 ++-
 4 files changed, 145 insertions(+), 99 deletions(-)

Comments

Viresh Kumar Nov. 3, 2023, 5:28 a.m. UTC | #1

On 30-10-23, 15:54, Viresh Kumar wrote:
> Hello,
> 
> I wasn't able to test this locally (despite trying to hack it around) and need
> help from someone who is `virt_devs` field of `struct dev_pm_opp_config`.
> 
> Pushed here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/required-opps
> 
> V1->V2:
> - Support opp-level 0, drop vote i.e..
> - Fix OPP pointer while calling dev_pm_opp_set_opp() recursively.
> - Minor checks and fixes.
> - Add Reviewed-by from Ulf.

Stephan, Ulf,

Any feedback on this before I merge it ?

Ulf Hansson Nov. 3, 2023, 9:20 a.m. UTC | #2

On Fri, 3 Nov 2023 at 06:28, Viresh Kumar <[e-mail]> wrote:
>
> On 30-10-23, 15:54, Viresh Kumar wrote:
> > Hello,
> >
> > I wasn't able to test this locally (despite trying to hack it around) and need
> > help from someone who is `virt_devs` field of `struct dev_pm_opp_config`.
> >
> > Pushed here:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/required-opps
> >
> > V1->V2:
> > - Support opp-level 0, drop vote i.e..
> > - Fix OPP pointer while calling dev_pm_opp_set_opp() recursively.
> > - Minor checks and fixes.
> > - Add Reviewed-by from Ulf.
>
> Stephan, Ulf,
>
> Any feedback on this before I merge it ?

I intend to review it within the next couple of days - or at least
before the merge window gets closed.

Kind regards
Uffe

Stephan Gerhold Nov. 3, 2023, 9:24 a.m. UTC | #3

On Fri, Nov 03, 2023 at 10:58:54AM +0530, Viresh Kumar wrote:
> On 30-10-23, 15:54, Viresh Kumar wrote:
> > I wasn't able to test this locally (despite trying to hack it around) and need
> > help from someone who is `virt_devs` field of `struct dev_pm_opp_config`.
> > 
> > Pushed here:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/required-opps
> > 
> > V1->V2:
> > - Support opp-level 0, drop vote i.e..
> > - Fix OPP pointer while calling dev_pm_opp_set_opp() recursively.
> > - Minor checks and fixes.
> > - Add Reviewed-by from Ulf.
> 
> Stephan, Ulf,
> 
> Any feedback on this before I merge it ?
> 

Sorry for the delay. I tested this successfully on the MSM8909 board on
Wednesday (with the single genpd, and without opp-level 0 there), but
until now didn't find time to test it on the MSM8916 board with the
multiple genpds and the opp-level 0.

The opp-level 0 works fine now, thanks for fixing that!

The warning in _link_required_opps() when using the parent genpd setup
[1] is still present though. Given that this setup is an existing
feature in the genpd core I would appreciate if we try to find a
solution before merging this patch set. It's kind of a regression
otherwise since the warning isn't present without this patch set.
Maybe someone else is already actively using such a setup.

Thanks!
Stephan

[1]: https://lore.kernel.org/linux-pm/[e-mail]/

Viresh Kumar Nov. 3, 2023, 9:27 a.m. UTC | #4

On 03-11-23, 10:24, Stephan Gerhold wrote:
> Sorry for the delay. I tested this successfully on the MSM8909 board on
> Wednesday (with the single genpd, and without opp-level 0 there), but
> until now didn't find time to test it on the MSM8916 board with the
> multiple genpds and the opp-level 0.

Thanks.

> The opp-level 0 works fine now, thanks for fixing that!
> 
> The warning in _link_required_opps() when using the parent genpd setup
> [1] is still present though. Given that this setup is an existing
> feature in the genpd core I would appreciate if we try to find a
> solution before merging this patch set. It's kind of a regression
> otherwise since the warning isn't present without this patch set.
> Maybe someone else is already actively using such a setup.

I did mention the solution that I seem fit for this case [1]. That's what I have
in mind.

Viresh Kumar Nov. 16, 2023, 10:43 a.m. UTC | #5

On 03-11-23, 10:24, Stephan Gerhold wrote:
> The warning in _link_required_opps() when using the parent genpd setup
> [1] is still present though. Given that this setup is an existing
> feature in the genpd core I would appreciate if we try to find a
> solution before merging this patch set. It's kind of a regression
> otherwise since the warning isn't present without this patch set.
> Maybe someone else is already actively using such a setup.

Can you please try V3. The warning should be gone now.