Re: [PATCH 1/2] OPP: Add API to update EM after adjustment of voltage for OPPs

[Lukasz Luba]

On 1/4/24 17:11, Dietmar Eggemann wrote:
> On 04/01/2024 11:38, Lukasz Luba wrote:
> > Hi Viresh,
> >
> > On 12/26/23 05:12, Viresh Kumar wrote:
> > > On 20-12-23, 11:03, Lukasz Luba wrote:
> > > > There are device drivers which can modify voltage values for OPPs. It
> > > > could be due to the chip binning and those drivers have specific chip
> > > > knowledge about this. This adjustment can happen after Energy Model is
> > > > registered, thus EM can have stale data about power.
> > > >
> > > > Introduce new API function which can be used by device driver which
> > > > adjusted the voltage for OPPs. The implementation takes care about
> > > > calculating needed internal details in the new EM table ('cost' field).
> > > > It plugs in the new EM table to the framework so other subsystems would
> > > > use the correct data.
> > > >
> > > > Signed-off-by: Lukasz Luba <lukasz.luba@xxxxxxx>
> > > > ---
> > > >    drivers/opp/of.c       | 69 ++++++++++++++++++++++++++++++++++++++++++
> > > >    include/linux/pm_opp.h |  6 ++++
> > > >    2 files changed, 75 insertions(+)
> > > >
> > > > diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> > > > index 81fa27599d58..992434c0b711 100644
> > > > --- a/drivers/opp/of.c
> > > > +++ b/drivers/opp/of.c
> > > > @@ -1596,3 +1596,72 @@ int dev_pm_opp_of_register_em(struct device
> > > > *dev, struct cpumask *cpus)
> > > >        return ret;
> > > >    }
> > > >    EXPORT_SYMBOL_GPL(dev_pm_opp_of_register_em);
> > > > +
> > > > +/**
> > > > + * dev_pm_opp_of_update_em() - Update Energy Model with new power
> > > > values
> > > > + * @dev        : Device for which an Energy Model has to be registered
> > > > + *
> > > > + * This uses the "dynamic-power-coefficient" devicetree property to
> > > > calculate
> > > > + * power values for EM. It uses the new adjusted voltage values
> > > > known for OPPs
> > > > + * which have changed after boot.
> > > > + */
> > > > +int dev_pm_opp_of_update_em(struct device *dev)
> > >
> > > I don't see anything OPP or OF related in this function, I don't think
> > > it needs
> > > to be part of the OPP core. You just want to reuse _get_power() I
> > > guess, which
> > > can be exported then.
> > >
> > > This should really be part of the EM core instead.
> >
> > Thank you for having a look at this. OK, that makes sense.
> > When I finish the EM runtime modification core features and get them
> > merged, I'll continue to work on this patch set. I'll try to follow
> > your comment here and export that function (with a different name
> > probably).
>
> Just to make sure: If this is the case then you could also add
> em_dev_compute_costs() with this new patch instead providing it with the
> 'Introduce runtime modifiable Energy Model' patch-set?

You're referring to the patch 22/23 [1]. Yes, it could be skipped,
but both will go in the same merge window, so not big difference.
I tend to agree that patch 22/23 could belong to this $subject.

As soon as Rafael will merge the core runtime patches, I will
push this small one from this $subject. So it will be in a few
days delay (assuming I would get an Ack from Marek or Krzysztof
for the Exynos patch).

> This would keep dev_pm_opp_of_update_em() and em_dev_compute_costs()
> together. IIRC, all the other new EM interfaces you already use with
> your 'modifiable EM' use case: '[PATCH v5 14/23] PM: EM: Support late
> CPUs booting and capacity adjustment'.

Yes, correct, the rest of API is used (mainly from thermal/dtmp).

[1] 
https://lore.kernel.org/lkml/20240104171553.2080674-23-lukasz.luba@xxxxxxx/