Re: [RESEND][PATCH v2 3/4] PM: EM: Add em_dev_update_chip_binning()

From: Lukasz Luba
Date: Thu Mar 28 2024 - 03:31:09 EST




On 3/28/24 07:21, Dietmar Eggemann wrote:
On 27/03/2024 13:55, Dietmar Eggemann wrote:
On 26/03/2024 21:32, Lukasz Luba wrote:


On 3/26/24 10:09, Dietmar Eggemann wrote:
On 22/03/2024 12:08, Lukasz Luba wrote:

[...]

+    return em_recalc_and_update(dev, pd, em_table);
+}
+EXPORT_SYMBOL_GPL(em_dev_update_chip_binning);

In the previous version of 'chip-binning' you were using the new EM
interface em_dev_compute_costs() (1) which is now replaced by
em_recalc_and_update() -> em_compute_costs().

https://lkml.kernel.org/r/20231220110339.1065505-2-lukasz.luba@xxxxxxx

Which leaves (1) still unused.

That was why my concern back then that we shouldn't introduce EM
interfaces without a user:

https://lkml.kernel.org/r/8fc499cf-fca1-4465-bff7-a93dfd36f3c8@xxxxxxx

What happens now with em_dev_compute_costs()?


For now it's not used, but modules which will create new EMs
with custom power values will use it. When such a module have
e.g. 5 EMs for one PD and only switches on one of them, then
this em_dev_compute_costs() will be used at setup for those
5 EMs. Later it won't be used.
I don't wanted to combine the registration of new EM with
the compute cost, because that will create overhead in the
switching to new EM code path. Now we have only ~3us, which
was the main goal.

When our scmi-cpufreq get the support for EM update this
compute cost will be used there.

OK, I see. I checked the reloadable EM test module and
em_dev_compute_costs() is used there like you described it.

I had a second look and IMHO the layout is like this:

Internal (1) and external (2,3) 'reloadable EM' use cases:
(EM alloc and free not depicted)

1. Late CPUs booting (CPU capacity adjustment)

e3f1164fc9ee PM: EM: Support late CPUs booting and capacity adjustment

schedule_delayed_work(&em_update_work, ...)

em_update_workfn()
em_check_capacity_update()
em_adjust_new_capacity()
em_recalc_and_update() <-- (i)
em_compute_costs() <-- (ii)
em_dev_update_perf_domain() <-- external

2. Reload EM from driver

22ea02848c07 PM: EM: Add em_dev_compute_costs()
977230d5d503 PM: EM: Introduce em_dev_update_perf_domain() for EM
updates

em_dev_compute_costs() <-- external
em_compute_costs() <-- (ii)
em_dev_update_perf_domain() <-- external

3. Chip binning

this patchset PM: EM: Add em_dev_update_chip_binning()

em_dev_update_chip_binning() <-- external
em_recalc_and_update() <-- (i)
em_compute_costs() <-- (ii)
em_dev_update_perf_domain() <-- external





Yes, that's correct.