[RFC PATCH 0/2] Introduce runtime modifiable Energy Model

[RFC PATCH 0/2] Introduce runtime modifiable Energy Model

[Lukasz Luba]

Hi all,

This is a follow-up patch aiming to show the users of the proposed
runtime modifiable Energy Model framework. The patches and discussion is
available here [1]. I didn't wanted to add more complexity in that
already big EM patch set and planned to add more users later.
This patch set is one of the first user. I have talked about the chip
binning in a few conferences and the need to update the EM after boot.
This Exynos is one of the platforms which adjust voltage after the EM
is registered. This is perfectly fine and alloed in the kernel, even
from a module so very late. The EM framework should just allow to
modify the power values after that.

This patch set will have to wait for merging of the dependet main EM
change. 

Regards,
Lukasz Luba

[1] https://lore.kernel.org/lkml/20231129110853.94344-1-lukasz.luba@arm.com/

Lukasz Luba (2):
  OPP: Add API to update EM after adjustment of voltage for OPPs
  soc: samsung: exynos-asv: Update Energy Model after adjusting voltage

 drivers/opp/of.c                 | 69 ++++++++++++++++++++++++++++++++
 drivers/soc/samsung/exynos-asv.c | 10 ++++-
 include/linux/pm_opp.h           |  6 +++
 3 files changed, 84 insertions(+), 1 deletion(-)

-- 
2.25.1

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

[Lukasz Luba]

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@arm.com>
---
 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;
+
+		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

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@arm.com> 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@arm.com>
> ---
>  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
>

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

[Lukasz Luba]

On 12/21/23 07:28, Xuewen Yan wrote:
> On Wed, Dec 20, 2023 at 7:02 PM Lukasz Luba <lukasz.luba@arm.com> 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@arm.com>
>> ---
>>   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?
> 

No, not in this case. It's not like registration of EM, when there
is a need to also pass the callback function. As you can see this code
operates locally and the call _get_power() just simply gets the
power in straight way. Later the whole 'runtime_table' is passed to the
EM framework to 'swap' the pointers under RCU.

Thanks Xuewen for having a look at this!

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

[Viresh Kumar]

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@arm.com>
> ---
>  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.

-- 
viresh

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

[Lukasz Luba]

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@arm.com>
>> ---
>>   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).

Regards,
Lukasz

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

[Dietmar Eggemann]

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@arm.com>
>>> ---
>>>   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?

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'.

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@arm.com>
>>>> ---
>>>>    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@arm.com/

[PATCH 2/2] soc: samsung: exynos-asv: Update Energy Model after adjusting voltage

[Lukasz Luba]

When the voltage for OPPs is adjusted there is a need to also update
Energy Model framework. The EM data contains power values which depend
on voltage values. The EM structure is used for thermal (IPA governor)
and in scheduler task placement (EAS) so it should reflect the real HW
model as best as possible to operate properly.

Based on data on Exynos5422 ASV tables the maximum power difference might
be ~29%. An Odroid-XU4 (with a random sample SoC in this chip lottery)
showed power difference for some OPPs ~20%. Therefore, it's worth to
update the EM.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/soc/samsung/exynos-asv.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c
index d60af8acc391..328f079423d6 100644
--- a/drivers/soc/samsung/exynos-asv.c
+++ b/drivers/soc/samsung/exynos-asv.c
@@ -97,9 +97,17 @@ static int exynos_asv_update_opps(struct exynos_asv *asv)
 			last_opp_table = opp_table;
 
 			ret = exynos_asv_update_cpu_opps(asv, cpu);
-			if (ret < 0)
+			if (!ret) {
+				/*
+				 * When the voltage for OPPs successfully
+				 * changed, update the EM power values to
+				 * reflect the reality and not use stale data
+				 */
+				dev_pm_opp_of_update_em(cpu);
+			} else {
 				dev_err(asv->dev, "Couldn't udate OPPs for cpu%d\n",
 					cpuid);
+			}
 		}
 
 		dev_pm_opp_put_opp_table(opp_table);
-- 
2.25.1