Re: [RFC i-g-t] intel_gpu_top: User friendly device listing

Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> · Thu, 15 Oct 2020 09:09:02 +0100

On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Adding a new device selection print type suitable for user-facing
use cases like intel_gpu_top -L and potentially lsgpu.

Instead of:

sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
     subsystem       : drm
     drm card        : /dev/dri/card0
     parent          : sys:/sys/devices/pci0000:00/0000:00:02.0

sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
     subsystem       : drm
     drm render      : /dev/dri/renderD128
     parent          : sys:/sys/devices/pci0000:00/0000:00:02.0

sys:/sys/devices/pci0000:00/0000:00:02.0
     subsystem       : pci
     drm card        : /dev/dri/card0
     drm render      : /dev/dri/renderD128
     vendor          : 8086
     device          : 193B

New format looks like:

card0                   8086:193B    drm:/dev/dri/card0
└─renderD128                         drm:/dev/dri/renderD128

Advantages are more compact, more readable, one entry per GPU, shorter
string to copy and paste to intel_gpu_top -d, or respective usage.

Looks nice and more intuitive.

I wasn't sure about duplication of card/render name and again in filter 
string, but I have no better ideas at the moment.

Maybe one day some could replace the left column with some card names 
from some database but for now above is good enough.

1. Add -s switch to handle simple print (current default in lsgpu)

I wanted to suggest -v (verbose) or -d (details) but both are taken in 
lsgpu. -s sounds like silent or in any case not obvious what "simple" 
means. Could use just long form --verbose or --details?

2. Change to user format as default.

Yep.

Regards,

Tvrtko


--
Zbigniew


Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@xxxxxxxxx>
---
  lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
  lib/igt_device_scan.h |   1 +
  tools/intel_gpu_top.c |   3 +-
  3 files changed, 100 insertions(+), 13 deletions(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index f4d43c733314..ce0ea61dda50 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
  	printf("    %-16s: %s:%s\n", k, v1, v2);
  }
  
-static void igt_devs_print_simple(struct igt_list_head *view)
+static bool __check_empty(struct igt_list_head *view)
  {
-	struct igt_device *dev;
-
  	if (!view)
-		return;
+		return true;
  
  	if (igt_list_empty(view)) {
  		printf("No GPU devices found\n");
-		return;
+		return true;
  	}
  
+	return false;
+}
+
+static void igt_devs_print_simple(struct igt_list_head *view)
+{
+	struct igt_device *dev;
+
+	if (__check_empty(view))
+		return;
+
  	igt_list_for_each_entry(dev, view, link) {
  		printf("sys:%s\n", dev->syspath);
  		if (dev->subsystem)
@@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
  	}
  }
  
+static struct igt_device *
+__find_pci(struct igt_list_head *view, const char *drm)
+{
+	struct igt_device *dev;
+
+	igt_list_for_each_entry(dev, view, link) {
+		if (!is_pci_subsystem(dev) || !dev->drm_card)
+			continue;
+
+		if (!strcmp(dev->drm_card, drm))
+			return dev;
+	}
+
+	return NULL;
+}
+
+static void igt_devs_print_user(struct igt_list_head *view)
+{
+	struct igt_device *dev;
+
+	if (__check_empty(view))
+		return;
+
+	igt_list_for_each_entry(dev, view, link) {
+		unsigned int i, num_children;
+		struct igt_device *pci_dev;
+		struct igt_device *dev2;
+		char filter[64];
+		char *drm_name;
+		int ret;
+
+		if (!is_drm_subsystem(dev))
+			continue;
+		if (!dev->drm_card || dev->drm_render)
+			continue;
+
+		drm_name = rindex(dev->drm_card, '/');
+		if (!drm_name || !*++drm_name)
+			continue;
+
+		ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
+		igt_assert(ret < sizeof(filter));
+
+		pci_dev = __find_pci(view, dev->drm_card);
+		if (pci_dev)
+			printf("%-24s%4s:%4s    %s\n",
+			       drm_name, pci_dev->vendor, pci_dev->device,
+			       filter);
+		else
+			printf("%-24s             %s\n", drm_name, filter);
+
+		num_children = 0;
+		igt_list_for_each_entry(dev2, view, link) {
+			if (!is_drm_subsystem(dev2) || !dev2->drm_render)
+				continue;
+			if (strcmp(dev2->parent->syspath, dev->parent->syspath))
+				continue;
+
+			num_children++;
+		}
+
+		i = 0;
+		igt_list_for_each_entry(dev2, view, link) {
+			if (!is_drm_subsystem(dev2) || !dev2->drm_render)
+				continue;
+			if (strcmp(dev2->parent->syspath, dev->parent->syspath))
+				continue;
+
+			drm_name = rindex(dev2->drm_render, '/');
+			if (!drm_name || !*++drm_name)
+				continue;
+
+			ret = snprintf(filter, sizeof(filter), "drm:%s",
+				       dev2->drm_render);
+			igt_assert(ret < sizeof(filter));
+
+			printf("%s%-22s             %s\n",
+			       (++i == num_children) ? "└─" : "├─",
+			       drm_name, filter);
+		}
+	}
+}
+
  static inline void _print_key_value(const char* k, const char *v)
  {
  	printf("%-32s: %s\n", k, v);
@@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
  {
  	struct igt_device *dev;
  
-	if (!view)
+	if (__check_empty(view))
  		return;
  
-	if (igt_list_empty(view)) {
-		printf("No GPU devices found\n");
-		return;
-	}
-
  	igt_list_for_each_entry(dev, view, link) {
  		printf("========== %s:%s ==========\n",
  		       dev->subsystem, dev->syspath);
@@ -781,6 +867,7 @@ static struct print_func {
  } print_functions[] = {
  	[IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
  	[IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
+	[IGT_PRINT_USER] = { .prn = igt_devs_print_user },
  };
  
  /**
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index bd937d22752c..9e13ed9db406 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -37,6 +37,7 @@
  enum igt_devices_print_type {
  	IGT_PRINT_SIMPLE,
  	IGT_PRINT_DETAIL,
+	IGT_PRINT_USER, /* End user friendly. */
  };
  
  #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 79a936ffbe1a..b984edc656c7 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
  	unsigned int i;
  	int ret = 0, ch;
  	bool list_device = false;
-	enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
  	char *pmu_device, *opt_device = NULL;
  	struct igt_device_card card;
  
@@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
  	igt_devices_scan(false);
  
  	if (list_device) {
-		igt_devices_print(printtype);
+		igt_devices_print(IGT_PRINT_USER);
  		goto exit;
  	}
  
--
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx







