[PATCH i-g-t] tools/gputop: Fix engine columns with amdgpu

[Lucas De Marchi]

On Wed, Apr 03, 2024 at 04:00:57PM +0100, Tvrtko Ursulin wrote:
>From: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>
>Amdgpu kernel driver skips output of fdinfo keys for unused engines. That
>is completely legal but corrupts the column formatting in the current
>code.
>
>Fix it by simply treating change in detected engines used by a client
>as trigger to re-emit a new header. This ensures columns are always

That would be kind of ugly if each client uses a different engine.

Why not outputing all the engine classes regardless of what the
clients are using?

Lucas De Marchi

>correctly aligned, albeit with a cost of potentially duplicating the
>header for the same DRM minor.
>
>This is considered good enough for a reference implementation. The
>alternative would be to add some real per DRM minor state tracking which
>sounds like an overkill, at least until gputop gains a nicer (any) UI.
>
>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>---
> tools/gputop.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
>diff --git a/tools/gputop.c b/tools/gputop.c
>index 71e28f43ee4c..76075c5dde8b 100644
>--- a/tools/gputop.c
>+++ b/tools/gputop.c
>@@ -119,11 +119,36 @@ print_client_header(struct igt_drm_client *c, int lines, int con_w, int con_h,
> 	return lines;
> }
>
>+static bool
>+engines_identical(const struct igt_drm_client *c,
>+		  const struct igt_drm_client *pc)
>+{
>+	unsigned int i;
>+
>+	if (c->engines->num_engines != pc->engines->num_engines ||
>+	    c->engines->max_engine_id != pc->engines->max_engine_id)
>+		return false;
>+
>+	for (i = 0; i <= c->engines->max_engine_id; i++)
>+		if (c->engines->capacity[i] != pc->engines->capacity[i] ||
>+		    !!c->engines->names[i] != !!pc->engines->names[i] ||
>+		    strcmp(c->engines->names[i], pc->engines->names[i]))
>+			return false;
>+
>+	return true;
>+}
>
> static bool
> newheader(const struct igt_drm_client *c, const struct igt_drm_client *pc)
> {
>-	return !pc || c->drm_minor != pc->drm_minor;
>+	return !pc || c->drm_minor != pc->drm_minor ||
>+	       /*
>+		* Below is a a hack for drivers like amdgpu which omit listing
>+		* unused engines. Simply treat them as separate minors which
>+		* will ensure the per-engine columns are correctly sized in all
>+		* cases.
>+		*/
>+	       !engines_identical(c, pc);
> }
>
> static int
>-- 
>2.44.0
>