On 27/07/2023 16:17, Tvrtko Ursulin wrote:
Hi,
On 27/07/2023 15:10, Kamil Konieczny wrote:
Hi Tvrtko,
On 2023-07-27 at 10:20:24 +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Define the storage structure and copy over memory data as parsed by the
fdinfo helpers.
v2:
* Fix empty region map entry skip condition. (Kamil, Chris)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Rob Clark <robdclark@xxxxxxxxxxxx>
Cc: Chris Healy <cphealy@xxxxxxxxx>
Cc: Kamil Konieczny <kamil.konieczny@xxxxxxxxxxxxxxx>
---
lib/igt_drm_clients.c | 32 ++++++++++++++++++++++++++++++++
lib/igt_drm_clients.h | 11 +++++++++++
2 files changed, 43 insertions(+)
diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c
index fdea42752a81..47ad137d5f1f 100644
--- a/lib/igt_drm_clients.c
+++ b/lib/igt_drm_clients.c
@@ -103,6 +103,8 @@ igt_drm_client_update(struct igt_drm_client *c,
unsigned int pid, char *name,
c->clients->max_name_len = len;
}
+ /* Engines */
+
c->last_runtime = 0;
c->total_runtime = 0;
@@ -118,6 +120,13 @@ igt_drm_client_update(struct igt_drm_client *c,
unsigned int pid, char *name,
c->last[i] = info->busy[i];
}
+ /* Memory regions */
+ for (i = 0; i <= c->regions->max_region_id; i++) {
+ assert(i < ARRAY_SIZE(info->region_mem));
+
+ c->memory[i] = info->region_mem[i];
+ }
+
c->samples++;
c->status = IGT_DRM_CLIENT_ALIVE;
}
@@ -154,6 +163,8 @@ igt_drm_client_add(struct igt_drm_clients *clients,
c->id = info->id;
c->drm_minor = drm_minor;
c->clients = clients;
+
+ /* Engines */
c->engines = malloc(sizeof(*c->engines));
assert(c->engines);
memset(c->engines, 0, sizeof(*c->engines));
@@ -178,6 +189,27 @@ igt_drm_client_add(struct igt_drm_clients *clients,
c->last = calloc(c->engines->max_engine_id + 1, sizeof(c->last));
assert(c->val && c->last);
+ /* Memory regions */
+ c->regions = malloc(sizeof(*c->regions));
+ assert(c->regions);
+ memset(c->regions, 0, sizeof(*c->regions));
+ c->regions->names = calloc(info->last_region_index + 1,
+ sizeof(*c->regions->names));
+ assert(c->regions->names);
+
+ for (i = 0; i <= info->last_region_index; i++) {
+ /* Region map is allowed to be sparse. */
+ if (!info->region_names[i][0])
+ continue;
+
+ c->regions->names[i] = strdup(info->region_names[i]);
--------------------------------- ^
Should this be c->regions->num_regions?
No, it was supposed to carry over the same memory region index from the
region map provided to igt_parse_drm_fdinfo.
I copy pasted that concept from engine names (class names for i915) but,
given it is unused, maybe I should drop it.
Gputop does not need it, at least not yet, and I haven't thought much if
it will be useful for intel_gpu_top. Point is, it allows passing in
fixed region id to name mapping, which can simplify things and guarantee
order of memory regions in the arrays. (Otherwise the order would depend
on the order of keys in the fdinfo text.)
I think I'd like to keep this functionality for now. I do promise to rip
it out later, should I end up not needing it for intel_gpu_top after all.
Regards,
Tvrtko
