Hi Dietmar,
On 1/20/20 2:53 PM, Dietmar Eggemann wrote:
On 16/01/2020 16:20, lukasz.luba@xxxxxxx wrote:
From: Lukasz Luba <lukasz.luba@xxxxxxx>
Add support of other devices into the Energy Model framework not only the
CPUs. Change the interface to be more unified which can handle other
devices as well.
[...]
-The source of the information about the power consumed by CPUs can vary greatly
+The source of the information about the power consumed by devices can vary greatly
from one platform to another. These power costs can be estimated using
devicetree data in some cases. In others, the firmware will know better.
Alternatively, userspace might be best positioned. And so on. In order to avoid
@@ -26,7 +28,7 @@ framework, and interested clients reading the data from it::
| Thermal (IPA) | | Scheduler (EAS) | | Other |
+---------------+ +-----------------+ +---------------+
| | em_pd_energy() |
- | | em_cpu_get() |
+ | em_dev_get() | em_cpu_get() |
Looked really hard but can't find a em_dev_get() in the code? You mean
em_get_pd() ? And why em_get_pd() and not em_pd_get()?
It was it the old implementation, I will remove 'em_dev_get()' from
the doc. The em_pd_get() is OK for me, I can change it.
+---------+ | +---------+
| | |
v v v
@@ -47,12 +49,12 @@ framework, and interested clients reading the data from it::
| Device Tree | | Firmware | | ? |
+--------------+ +---------------+ +--------------+
[...]
+There is two API functions which provide the access to the energy model:
+em_cpu_get() which takes CPU id as an argument and em_dev_get() with device
+pointer as an argument. It depends on the subsystem which interface it is
+going to use.
Would be really nice if this wouldn't be required. We should really aim
for 1 framework == 1 set of interfaces.
What happens if someone calls em_get_pd() on a CPU EM?
E.g:
static struct perf_domain *pd_init(int cpu)
{
- struct em_perf_domain *obj = em_cpu_get(cpu);
+ struct device *dev = get_cpu_device(cpu);
+ struct em_perf_domain *obj = em_pd_get(dev);
struct perf_domain *pd;
Two versions of one functionality will confuse API user from the
beginning ...
Right, I could modify the pd_init code to use one 'em_get_pd' API
and remove the 'em_cpu_get'.
[...]
+enum em_type {
+ EM_SIMPLE,
+ EM_CPU,
+};
s/EM_SIMPLE/EM_DEV ?
Right now I only see energy models and _one_ specific type (the CPU EM).
So a tag 'is a CPU EM' would suffice. No need for EM_SIMPE ...
The EM_SIMPLE is set in the em_register_perf_domain() to distinguish
CPU device which has populated 'priv' pointer and set EM_CPU.
We can just rely on 'priv == NULL' to check if we are dealing with a
CPU EM. Do you prefer this approach and get rid of em_type?
Then the code would look like:
if (em_pd->priv)
seq_puts(s, "EM_CPU\n");
else
seq_puts(s, "EM_SIMPLE\n");
Regards,
Lukasz
[...]
