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

[Xuewen Yan]

On Wed, Dec 20, 2023 at 7:02 PM Lukasz Luba <lukasz.luba@xxxxxxx> 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)
> +{
> +       struct em_perf_table __rcu *runtime_table;
> +       struct em_perf_state *table, *new_table;
> +       struct em_perf_domain *pd;
> +       int ret, table_size, i;
> +
> +       if (IS_ERR_OR_NULL(dev))
> +               return -EINVAL;
> +
> +       pd = em_pd_get(dev);
> +       if (!pd) {
> +               dev_warn(dev, "Couldn't find Energy Model %d\n", ret);
> +               return -EINVAL;
> +       }
> +
> +       runtime_table = em_allocate_table(pd);
> +       if (!runtime_table) {
> +               dev_warn(dev, "new EM allocation failed\n");
> +               return -ENOMEM;
> +       }
> +
> +       new_table = runtime_table->state;
> +
> +       table = em_get_table(pd);
> +       /* Initialize data based on older EM table */
> +       table_size = sizeof(struct em_perf_state) * pd->nr_perf_states;
> +       memcpy(new_table, table, table_size);
> +
> +       em_put_table();
> +
> +       /* Update power values which might change due to new voltage in OPPs */
> +       for (i = 0; i < pd->nr_perf_states; i++) {
> +               unsigned long freq = new_table[i].frequency;
> +               unsigned long power;
> +
> +               ret = _get_power(dev, &power, &freq);
> +               if (ret)
> +                       goto failed;

Need we use the EM_SET_ACTIVE_POWER_CB(em_cb, _get_power) and call
em_cb->active_power?

> +
> +               new_table[i].power = power;
> +       }
> +
> +       ret = em_dev_compute_costs(dev, new_table, pd->nr_perf_states);
> +       if (ret)
> +               goto failed;
> +
> +       ret = em_dev_update_perf_domain(dev, runtime_table);
> +       if (ret)
> +               goto failed;
> +
> +       return 0;
> +
> +failed:
> +       dev_warn(dev, "EM update failed %d\n", ret);
> +       em_free_table(runtime_table);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_of_update_em);
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index ccd97bcef269..b3ab117890fc 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -464,6 +464,7 @@ struct device_node *dev_pm_opp_get_of_node(struct dev_pm_opp *opp);
>  int of_get_required_opp_performance_state(struct device_node *np, int index);
>  int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_table *opp_table);
>  int dev_pm_opp_of_register_em(struct device *dev, struct cpumask *cpus);
> +int dev_pm_opp_of_update_em(struct device *dev);
>  static inline void dev_pm_opp_of_unregister_em(struct device *dev)
>  {
>         em_dev_unregister_perf_domain(dev);
> @@ -527,6 +528,11 @@ static inline void dev_pm_opp_of_unregister_em(struct device *dev)
>  {
>  }
>
> +static inline int dev_pm_opp_of_update_em(struct device *dev)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
>  static inline int of_get_required_opp_performance_state(struct device_node *np, int index)
>  {
>         return -EOPNOTSUPP;
> --
> 2.25.1
>