All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu
@ 2019-10-24 11:05 Arkadiusz Hiler
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list Arkadiusz Hiler
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-24 11:05 UTC (permalink / raw)
  To: igt-dev; +Cc: Leo Liu, Rodrigo Siqueira, Petri Latvala

Hey,

This series aims to make running IGT on hosts with multiple GPUs manageable
without the need to unload modules / unbind devices.

Suggested reviewing order:
 * lsgpu
 * igt_core && drm_test changes
 * implementation internals in igt_device_scan.c

Changes since Zbigniew's last revision:
 * rewritten most of the parts that were using glib
 * removed multiple filter support - this will be added back when the need
   arises
 * we don't second guess the "chipset" of the device and just let the underlying
   open to fail if it has to
 * extra looging around opening device when filter is set
 * sysfs filter now has it's own prefix
 * no "platform" filter - sysfs should suffice for now, it can be added by
   someone more knowledgeable if the need arises

TODO:
 * API for opening multiple devices in a single test (e.g. for prime) - I don't
   want to design this upfront

Example usage:
  $ build/tools/lsgpu
  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/platform/vgem/drm/card1
      subsystem       : drm
      drm card        : /dev/dri/card1
      parent          : sys:/sys/devices/platform/vgem

  sys:/sys/devices/platform/vgem/drm/renderD129
      subsystem       : drm
      drm render      : /dev/dri/renderD129
      parent          : sys:/sys/devices/platform/vgem

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

  sys:/sys/devices/platform/vgem
      subsystem       : platform
      drm card        : /dev/dri/card1
      drm render      : /dev/dri/renderD129

  $ build/tools/lsgpu -d "sys:/sys/devices/pci0000:00/0000:00:02.0"
  Notice: Using --device filters
  === Device filter ===
  sys:/sys/devices/pci0000:00/0000:00:02.0

  === Testing device open ===
  Device detail:
  subsystem   : pci
  drm card    : /dev/dri/card0
  drm render  : /dev/dri/renderD128
  Device /dev/dri/card0 successfully opened
  Device /dev/dri/renderD128 successfully opened
  -------------------------------------------

  # build/tests/core_auth --run-subtest getclient-simple --device "pci:vendor=intel"
  IGT-Version: 1.24-g64068440 (x86_64) (Linux: 5.3.7-250.vanilla.knurd.1.fc30.x86_64 x86_64)
  Starting subtest: getclient-simple
  Looking for devices to open using filter: pci:vendor=intel
  Filter matched /dev/dri/card0 | /dev/dri/renderD128
  Looking for devices to open using filter: pci:vendor=intel
  Filter matched /dev/dri/card0 | /dev/dri/renderD128
  Subtest getclient-simple: SUCCESS (0.007s)

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Leo Liu <Leo.Liu@amd.com>

Arkadiusz Hiler (1):
  lib/igt_list: Update, clean-up and document igt_list

Zbigniew Kempczyński (3):
  Introduce device selection API
  Introduce device selection lsgpu tool
  Add device selection in IGT

 benchmarks/gem_wsim.c                         |    6 +-
 .../igt-gpu-tools/igt-gpu-tools-docs.xml      |    2 +
 .../igt-gpu-tools/igt_test_programs.xml       |    7 +
 lib/Makefile.sources                          |    4 +
 lib/drmtest.c                                 |   94 +-
 lib/drmtest.h                                 |    2 +
 lib/igt_chamelium.c                           |    2 +-
 lib/igt_core.c                                |   51 +-
 lib/igt_device_scan.c                         | 1199 +++++++++++++++++
 lib/igt_device_scan.h                         |   67 +
 lib/igt_dummyload.c                           |    4 +-
 lib/igt_kmod.c                                |    2 +-
 lib/igt_list.c                                |   66 +
 lib/igt_list.h                                |  165 ++-
 lib/meson.build                               |    2 +
 tests/i915/gem_exec_balancer.c                |    2 +-
 tests/vc4_purgeable_bo.c                      |    6 +-
 tools/Makefile.sources                        |    1 +
 tools/lsgpu.c                                 |  295 ++++
 tools/meson.build                             |    1 +
 20 files changed, 1873 insertions(+), 105 deletions(-)
 create mode 100644 lib/igt_device_scan.c
 create mode 100644 lib/igt_device_scan.h
 create mode 100644 lib/igt_list.c
 create mode 100644 tools/lsgpu.c

-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
@ 2019-10-24 11:05 ` Arkadiusz Hiler
  2019-10-24 11:16   ` Chris Wilson
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API Arkadiusz Hiler
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-24 11:05 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

We have diverged from how Wayland implements list both in parameter
ordering and function naming.

Let's make our side consistent to ease the cognitive dissonance for
developers working on both projects.

This patch:
 * mimics the current Wayland implementation
 * separates IGT helpers in the source files
 * adds brief explanation and code example for igt-doc
 * introduces igt_list.c as static inlines are not visible in the docs

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 benchmarks/gem_wsim.c                         |   6 +-
 .../igt-gpu-tools/igt-gpu-tools-docs.xml      |   1 +
 lib/Makefile.sources                          |   2 +
 lib/igt_chamelium.c                           |   2 +-
 lib/igt_core.c                                |   4 +-
 lib/igt_dummyload.c                           |   4 +-
 lib/igt_kmod.c                                |   2 +-
 lib/igt_list.c                                |  66 +++++++
 lib/igt_list.h                                | 165 +++++++++---------
 lib/meson.build                               |   1 +
 tests/i915/gem_exec_balancer.c                |   2 +-
 tests/vc4_purgeable_bo.c                      |   6 +-
 12 files changed, 164 insertions(+), 97 deletions(-)
 create mode 100644 lib/igt_list.c

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index 87f873b0..44745d06 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -2814,11 +2814,11 @@ static void *run_workload(void *data)
 			do_eb(wrk, w, engine, wrk->flags);
 
 			if (w->request != -1) {
-				igt_list_del(&w->rq_link);
+				igt_list_remove(&w->rq_link);
 				wrk->nrequest[w->request]--;
 			}
 			w->request = engine;
-			igt_list_add_tail(&w->rq_link, &wrk->requests[engine]);
+			igt_list_insert_tail(&wrk->requests[engine], &w->rq_link);
 			wrk->nrequest[engine]++;
 
 			if (!wrk->run)
@@ -2840,7 +2840,7 @@ static void *run_workload(void *data)
 					last_sync = true;
 
 					s->request = -1;
-					igt_list_del(&s->rq_link);
+					igt_list_remove(&s->rq_link);
 					wrk->nrequest[engine]--;
 				}
 			}
diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
index ac83272f..3f14d7be 100644
--- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
+++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
@@ -31,6 +31,7 @@
     <xi:include href="xml/igt_gvt.xml"/>
     <xi:include href="xml/igt_kmod.xml"/>
     <xi:include href="xml/igt_kms.xml"/>
+    <xi:include href="xml/igt_list.xml"/>
     <xi:include href="xml/igt_pm.xml"/>
     <xi:include href="xml/igt_primes.xml"/>
     <xi:include href="xml/igt_rand.xml"/>
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 34e0c012..1b58b6d0 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -41,6 +41,8 @@ lib_source_list =	 	\
 	igt_halffloat.h		\
 	igt_infoframe.c		\
 	igt_infoframe.h		\
+	igt_list.c		\
+	igt_list.h		\
 	igt_matrix.c		\
 	igt_matrix.h		\
 	igt_primes.c		\
diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 1b03a103..7de8ab07 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -597,7 +597,7 @@ struct chamelium_edid *chamelium_new_edid(struct chamelium *chamelium,
 	chamelium_edid->chamelium = chamelium;
 	chamelium_edid->base = malloc(edid_size);
 	memcpy(chamelium_edid->base, edid, edid_size);
-	igt_list_add(&chamelium_edid->link, &chamelium->edids);
+	igt_list_insert(&chamelium->edids, &chamelium_edid->link);
 
 	return chamelium_edid;
 }
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7bd5afa5..67b76025 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1180,7 +1180,7 @@ void __igt_subtest_group_save(int *save, int *desc)
 	if (__current_description[0] != '\0') {
 		struct description_node *new = calloc(1, sizeof(*new));
 		memcpy(new->desc, __current_description, sizeof(__current_description));
-		igt_list_add_tail(&new->link, &subgroup_descriptions);
+		igt_list_insert_tail(&subgroup_descriptions, &new->link);
 		_clear_current_description();
 		*desc = true;
 	}
@@ -1193,7 +1193,7 @@ void __igt_subtest_group_restore(int save, int desc)
 	if (desc) {
 		struct description_node *last =
 			igt_list_last_entry(&subgroup_descriptions, last, link);
-		igt_list_del(&last->link);
+		igt_list_remove(&last->link);
 		free(last);
 	}
 
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 6060878d..5d4cfb4e 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -289,7 +289,7 @@ spin_create(int fd, const struct igt_spin_factory *opts)
 	spin->out_fence = emit_recursive_batch(spin, fd, opts);
 
 	pthread_mutex_lock(&list_lock);
-	igt_list_add(&spin->link, &spin_list);
+	igt_list_insert(&spin_list, &spin->link);
 	pthread_mutex_unlock(&list_lock);
 
 	return spin;
@@ -435,7 +435,7 @@ void igt_spin_free(int fd, igt_spin_t *spin)
 		return;
 
 	pthread_mutex_lock(&list_lock);
-	igt_list_del(&spin->link);
+	igt_list_remove(&spin->link);
 	pthread_mutex_unlock(&list_lock);
 
 	if (spin->timer)
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index c3da4667..0fc5985d 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -420,7 +420,7 @@ static void tests_add(struct igt_kselftest_list *tl, struct igt_list *list)
 		if (pos->number > tl->number)
 			break;
 
-	igt_list_add_tail(&tl->link, &pos->link);
+	igt_list_insert_tail(&pos->link, &tl->link);
 }
 
 void igt_kselftest_get_tests(struct kmod_module *kmod,
diff --git a/lib/igt_list.c b/lib/igt_list.c
new file mode 100644
index 00000000..af615124
--- /dev/null
+++ b/lib/igt_list.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt_list.h"
+
+void igt_list_init(struct igt_list *list)
+{
+	list->prev = list;
+	list->next = list;
+}
+
+void igt_list_insert(struct igt_list *list, struct igt_list *elm)
+{
+	elm->prev = list;
+	elm->next = list->next;
+	list->next = elm;
+	elm->next->prev = elm;
+}
+
+
+void igt_list_remove(struct igt_list *elm)
+{
+	elm->prev->next = elm->next;
+	elm->next->prev = elm->prev;
+	elm->next = NULL;
+	elm->prev = NULL;
+}
+
+int igt_list_length(const struct igt_list *list)
+{
+	struct  igt_list *e = list->next;
+	int count = 0;
+
+	while (e != list) {
+		e = e->next;
+		count++;
+	}
+
+	return count;
+}
+
+bool igt_list_empty(const struct igt_list *list)
+{
+	return list->next == list;
+}
diff --git a/lib/igt_list.h b/lib/igt_list.h
index cadf9dd3..c73a98f1 100644
--- a/lib/igt_list.h
+++ b/lib/igt_list.h
@@ -28,9 +28,42 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-/*
- * This list data structure is a verbatim copy from wayland-util.h from the
- * Wayland project; except that wl_ prefix has been removed.
+/**
+ * SECTION:igt_list
+ * @short_description: a list implementation borrowed from the Wayland project
+ * @title: IGT List
+ * @include: igt_list.h
+ *
+ * This list data structure mimics the one from wayland-util.h from the Wayland
+ * project. A few bonus helpers are provided.
+ *
+ * igt_list is a doubly-linked list where an instance of igt_list is a head
+ * sentinel and has to be initalized. See Wayland documentation for more
+ * details.
+ *
+ * Example usage:
+ *
+ * |[<!-- language="C" -->
+ * struct igt_list foo_list;
+ *
+ * struct element {
+ *         int foo;
+ *         struct igt_list link;
+ * };
+ *
+ * struct element e1, e2, e3;
+ *
+ * igt_list_init(&foo_list);
+ * igt_list_insert(&foo_list, &e1.link);   // e1 is the first element
+ * igt_list_insert(&foo_list, &e2.link);   // e2 is now the first element
+ * igt_list_insert(&e2.link, &e3.link);    // insert e3 after e2
+ *
+ * struct igt_spin *iter;
+ * igt_list_for_each(iter, &foo_list, link) {
+ * 	printf("%d\n", iter->foo);
+ * }
+ * ]|
+ *
  */
 
 struct igt_list {
@@ -38,92 +71,56 @@ struct igt_list {
 	struct igt_list *next;
 };
 
-#define __IGT_INIT_LIST(name) { &(name), &(name) }
-#define IGT_LIST(name) struct igt_list name = __IGT_INIT_LIST(name)
-
-static inline void igt_list_init(struct igt_list *list)
-{
-	list->prev = list;
-	list->next = list;
-}
-
-static inline void __igt_list_add(struct igt_list *list,
-				  struct igt_list *prev,
-				  struct igt_list *next)
-{
-	next->prev = list;
-	list->next = next;
-	list->prev = prev;
-	prev->next = list;
-}
-
-static inline void igt_list_add(struct igt_list *elm, struct igt_list *list)
-{
-	__igt_list_add(elm, list, list->next);
-}
-
-static inline void igt_list_add_tail(struct igt_list *elm,
-				     struct igt_list *list)
-{
-	__igt_list_add(elm, list->prev, list);
-}
-
-static inline void __igt_list_del(struct igt_list *prev, struct igt_list *next)
-{
-	next->prev = prev;
-	prev->next = next;
-}
-
-static inline void igt_list_del(struct igt_list *elm)
-{
-	__igt_list_del(elm->prev, elm->next);
-}
-
-static inline void igt_list_move(struct igt_list *elm, struct igt_list *list)
-{
-	igt_list_del(elm);
-	igt_list_add(elm, list);
-}
-
-static inline void igt_list_move_tail(struct igt_list *elm,
-				      struct igt_list *list)
-{
-	igt_list_del(elm);
-	igt_list_add_tail(elm, list);
-}
-
-static inline bool igt_list_empty(const struct igt_list *list)
-{
-	return list->next == list;
-}
-
-#define container_of(ptr, sample, member)				\
-	(typeof(sample))((char *)(ptr) - offsetof(typeof(*sample), member))
-
-#define igt_list_first_entry(head, pos, member) \
-	container_of((head)->next, (pos), member)
-#define igt_list_last_entry(head, pos, member) \
-	container_of((head)->prev, (pos), member)
-
-#define igt_list_next_entry(pos, member) \
-	container_of((pos)->member.next, (pos), member)
-#define igt_list_prev_entry(pos, member) \
-	container_of((pos)->member.prev, (pos), member)
+void igt_list_init(struct igt_list *list);
+void igt_list_insert(struct igt_list *list, struct igt_list *elm);
+void igt_list_remove(struct igt_list *elm);
+int igt_list_length(const struct igt_list *list);
+bool igt_list_empty(const struct igt_list *list);
+
+#define igt_container_of(ptr, sample, member)				\
+	(__typeof__(sample))((char *)(ptr) -				\
+				offsetof(__typeof__(*sample), member))
 
 #define igt_list_for_each(pos, head, member)				\
-	for (pos = igt_list_first_entry(head, pos, member);		\
+	for (pos = igt_container_of((head)->next, pos, member);		\
 	     &pos->member != (head);					\
-	     pos = igt_list_next_entry(pos, member))
-
-#define igt_list_for_each_reverse(pos, head, member)			\
-	for (pos = igt_list_last_entry(head, pos, member);		\
-	     &pos->member != (head);					\
-	     pos = igt_list_prev_entry(pos, member))
+	     pos = igt_container_of((pos)->member.next, pos, member))
 
+/**
+ * igt_list_for_each_safe
+ *
+ * Safe against removel of the *current* list element. To achive this it
+ * requires an extra helper variable `tmp` with the same type as `pos`.
+ */
 #define igt_list_for_each_safe(pos, tmp, head, member)			\
-	for (pos = igt_list_first_entry(head, pos, member),		\
-	     tmp = igt_list_next_entry(pos, member);			\
+	for (pos = igt_container_of((head)->next, pos, member),		\
+	     tmp = igt_container_of((pos)->member.next, tmp, member); 	\
 	     &pos->member != (head);					\
-	     pos = tmp, tmp = igt_list_next_entry(pos, member))
+	     pos = tmp,							\
+	     tmp = igt_container_of((pos)->member.next, tmp, member))
+
+#define igt_list_for_each_reverse(pos, head, member)			\
+	for (pos = igt_container_of((head)->prev, pos, member);		\
+	     &pos->member != (head);					\
+	     pos = igt_container_of((pos)->member.prev, pos, member))
+
+
+/* IGT custom helpers */
+
+/**
+ * IGT_LIST
+ *
+ * Instead of having to use `igt_list_init()` list can be defined using
+ * this helper, e.g. `static IGT_LIST(list);`
+ */
+#define IGT_LIST(name) struct igt_list name = { &(name), &(name) }
+
+#define igt_list_insert_tail(list, elem) igt_list_insert((list)->prev, elem)
+
+#define igt_list_first_entry(head, pos, member) \
+	igt_container_of((head)->next, (pos), member)
+
+#define igt_list_last_entry(head, pos, member) \
+	igt_container_of((head)->prev, (pos), member)
 
 #endif /* IGT_LIST_H */
diff --git a/lib/meson.build b/lib/meson.build
index fbc0c8d1..19cb6255 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -50,6 +50,7 @@ lib_sources = [
 	'igt_fb.c',
 	'igt_core.c',
 	'igt_draw.c',
+	'igt_list.c',
 	'igt_pm.c',
 	'igt_dummyload.c',
 	'uwildmat/uwildmat.c',
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index e52f5df9..e839e0bf 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -807,7 +807,7 @@ static void bonded_slice(int i915)
 
 		*stop = 0;
 		igt_fork(child, count + 1) { /* C: arbitrary background load */
-			igt_list_del(&spin->link);
+			igt_list_remove(&spin->link);
 
 			ctx = gem_context_clone(i915, ctx,
 						I915_CONTEXT_CLONE_ENGINES, 0);
diff --git a/tests/vc4_purgeable_bo.c b/tests/vc4_purgeable_bo.c
index cfe14e70..030b1d1b 100644
--- a/tests/vc4_purgeable_bo.c
+++ b/tests/vc4_purgeable_bo.c
@@ -67,7 +67,7 @@ static void igt_vc4_alloc_mmap_max_bo(int fd, struct igt_list *list,
 		bo->size = create.size;
 		bo->map = igt_vc4_mmap_bo(fd, bo->handle, bo->size,
 					  PROT_READ | PROT_WRITE);
-		igt_list_add_tail(&bo->node, list);
+		igt_list_insert_tail(list, &bo->node);
 	}
 }
 
@@ -78,7 +78,7 @@ static void igt_vc4_unmap_free_bo_pool(int fd, struct igt_list *list)
 	while (!igt_list_empty(list)) {
 		bo = igt_list_first_entry(list, bo, node);
 		igt_assert(bo);
-		igt_list_del(&bo->node);
+		igt_list_remove(&bo->node);
 		munmap(bo->map, bo->size);
 		gem_close(fd, bo->handle);
 		free(bo);
@@ -253,7 +253,7 @@ igt_main
 		/* Trigger a purge. */
 		igt_vc4_trigger_purge(fd);
 
-		igt_list_del(&bo->node);
+		igt_list_remove(&bo->node);
 		munmap(bo->map, bo->size);
 		gem_close(fd, bo->handle);
 		free(bo);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list Arkadiusz Hiler
@ 2019-10-24 11:05 ` Arkadiusz Hiler
       [not found]   ` <20191024164030.GA7823@zkempczy-mobl2>
                     ` (2 more replies)
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool Arkadiusz Hiler
                   ` (4 subsequent siblings)
  6 siblings, 3 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-24 11:05 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Change adds device selection API based on scanning drm subsystem
using udev library.

v2 (Arek):
 * drop most of the glib code in favor of igt_list and plain C
 * make sysfs paths handling non-special - introduce sys: filter
 * drop multiple filter_* structs in favor of just two:
   - filter_class for defining filters types (e.g. sys:)
   - filter for "filter instance" - the data provided by the user
 * promote many macros to real functions for type safety
 * rename devs->devs to devs->all
 * rename devs->view to devs->filtered
 * don't expose "chip" (e.g. DRIVER_ANY) as it's unreadable as int
 * update docs to reflect those changes
 * move open functions that open igt_device_card to this patch
 * remove platform filter class for now
 * use only a single filter

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 .../igt-gpu-tools/igt-gpu-tools-docs.xml      |    1 +
 lib/Makefile.sources                          |    2 +
 lib/igt_device_scan.c                         | 1199 +++++++++++++++++
 lib/igt_device_scan.h                         |   67 +
 lib/meson.build                               |    1 +
 5 files changed, 1270 insertions(+)
 create mode 100644 lib/igt_device_scan.c
 create mode 100644 lib/igt_device_scan.h

diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
index 3f14d7be..c41cbb78 100644
--- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
+++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
@@ -23,6 +23,7 @@
     <xi:include href="xml/igt_core.xml"/>
     <xi:include href="xml/igt_debugfs.xml"/>
     <xi:include href="xml/igt_device.xml"/>
+    <xi:include href="xml/igt_device_scan.xml"/>
     <xi:include href="xml/igt_draw.xml"/>
     <xi:include href="xml/igt_dummyload.xml"/>
     <xi:include href="xml/igt_fb.xml"/>
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1b58b6d0..7e8594c5 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -25,6 +25,8 @@ lib_source_list =	 	\
 	igt_debugfs.h		\
 	igt_device.c		\
 	igt_device.h		\
+	igt_device_scan.c	\
+	igt_device_scan.h	\
 	igt_aux.c		\
 	igt_aux.h		\
 	igt_color_encoding.c	\
diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
new file mode 100644
index 00000000..71d30587
--- /dev/null
+++ b/lib/igt_device_scan.c
@@ -0,0 +1,1199 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt.h"
+#include "igt_list.h"
+#include "igt_sysfs.h"
+#include "igt_device.h"
+#include "igt_device_scan.h"
+#include <glib.h>
+#include <libudev.h>
+#include <linux/limits.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <fcntl.h>
+
+/**
+ * SECTION:igt_device_scan
+ * @short_description: Device scanning and selection
+ * @title: Device selection
+ * @include: igt.h
+ *
+ * # Device scanning
+ *
+ * Device scanning iterates over DRM subsystem using udev library
+ * to acquire DRM devices.
+ * For each DRM device we also get and store its parent to allow device
+ * selection happen in a more contextual way.
+ *
+ * Parent devices are bus devices (like PCI, platform, etc.) and contain a lot
+ * of extra data on top of the DRM device itself.
+ *
+ * # Filters
+ *
+ * Device selection can be done using filters that are using the data collected
+ * udev + some syntactic sugar.
+ *
+ * Direct device selection filter uses sysfs path to find the device:
+ *
+ * |[<!-- language="plain" -->
+ * sys:/sys/path/to/device/or/parent
+ * ]|
+ *
+ * Examples:
+ * |[<!-- language="plain" -->
+ * - sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
+ * - sys:/sys/devices/pci0000:00/0000:00:02.0
+ * - sys:/sys/devices/platform/vgem
+ * ]|
+ *
+ * The alternative is to use other implemented filters:
+ *
+ * - drm: get drm /dev/dri/... device directly
+ *
+ *   |[<!-- language="plain" -->
+ *   drm:/dev/dri/...
+ *   ]|
+ *
+ *   Loading drivers in different order can cause different ordering of
+ *   /dev/dri/card nodes which be problematic for reliable and reproducible
+ *   device selection, e.g. in automated execution setting. In such scenarios
+ *   please consider using sys, pci or platform filters instead.
+ *
+ * - pci: select device using PCI vendor and device properties
+ *   |[<!-- language="plain" -->
+ *   pci:[vendor=%04x/name][,device=%04x][,card=%d]
+ *   ]|
+ *
+ *   Filter allows device selection using vendor (hex or name), device id
+ *   (hex) and nth-card from all matches. For example if there are 4 PCI cards
+ *   installed (let two cards have 1234 and other two 1235 device id, all of
+ *   them of vendor Intel) you can select one using:
+ *
+ *   |[<!-- language="plain" -->
+ *   pci:vendor=Intel,device=1234,card=0
+ *   ]|
+ *
+ *   or
+ *
+ *   |[<!-- language="plain" -->
+ *   pci:vendor=8086,device=1234,card=0
+ *   ]|
+ *
+ *   This takes first device with 1234 id for Intel vendor (8086).
+ *
+ *   |[<!-- language="plain" -->
+ *   pci:vendor=Intel,device=1234,card=1
+ *   ]|
+ *
+ *   or
+ *
+ *   |[<!-- language="plain" -->
+ *   pci:vendor=8086,device=1234,card=1
+ *   ]|
+ *
+ *   It selects the second one.
+ *
+ *   As order the on PCI bus doesn't change (unless you'll add new device or
+ *   reorder existing one) device selection using this filter will always
+ *   return you same device regardless the order of enumeration.
+ *
+ *   Simple syntactic sugar over using the sysfs paths.
+ */
+
+#ifdef DEBUG_DEVICE_SCAN
+#define DBG(...) \
+{ \
+	struct timeval tm; \
+	gettimeofday(&tm, NULL); \
+	printf("%10ld.%03ld: ", tm.tv_sec, tm.tv_usec); \
+	printf(__VA_ARGS__); \
+	}
+
+#else
+#define DBG(...) {}
+#endif
+
+static inline bool strequal(const char *a, const char *b)
+{
+	if (a == NULL || b == NULL)
+		return false;
+
+	return strcmp(a, b) == 0;
+}
+
+struct igt_device {
+	/* Filled for drm devices */
+	struct igt_device *parent;
+
+	/* Point to vendor spec if can be found */
+
+	/* Properties / sysattrs rewriten from udev lists */
+	GHashTable *props_ht;
+	GHashTable *attrs_ht;
+
+	/* Most usable variables from udev device */
+	char *subsystem;
+	char *syspath;
+	char *devnode;
+
+	/* /dev/dri/... paths */
+	char *drm_card;
+	char *drm_render;
+
+	/* For pci subsystem */
+	char *vendor;
+	char *device;
+
+	struct igt_list link;
+};
+
+/* Scanned devices */
+static struct {
+	struct igt_list all;
+	struct igt_list filtered;
+	bool devs_scanned;
+} igt_devs;
+
+static struct {
+	const char *name;
+	const char *vendor_id;
+} pci_vendor_mapping[] = {
+	{ "intel", "8086" },
+	{ "amd", "1002" },
+	{ NULL, },
+};
+
+static const char *get_pci_vendor_id_by_name(const char *name)
+{
+	for (typeof(*pci_vendor_mapping) *vm = pci_vendor_mapping; vm->name; vm++)
+	{
+		if (!strcasecmp(name, vm->name))
+			return vm->vendor_id;
+	}
+
+	return NULL;
+}
+
+/* Reading sysattr values can take time (even seconds),
+ * we want to avoid reading such keys.
+ */
+static bool is_on_blacklist(const char *what)
+{
+	static const char *keys[] = { "config", "modalias", "modes",
+				      "resource",
+				      "resource0", "resource1", "resource2",
+				      "resource3", "resource4", "resource5",
+				      "resource0_wc", "resource1_wc", "resource2_wc",
+				      "resource3_wc", "resource4_wc", "resource5_wc",
+				      "driver",
+				      "uevent", NULL};
+	const char *key;
+	int i = 0;
+
+	if (what == NULL)
+		return false;
+
+	while ((key = keys[i++])) {
+		if (strcmp(key, what) == 0)
+			return true;
+	}
+
+	return false;
+
+}
+
+static struct igt_device *igt_device_new(void)
+{
+	struct igt_device *dev;
+
+	dev = calloc(1, sizeof(struct igt_device));
+	if (!dev)
+		return NULL;
+
+	dev->attrs_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
+					      free, free);
+	dev->props_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
+					      free, free);
+
+	if (dev->attrs_ht && dev->props_ht)
+		return dev;
+
+	return NULL;
+}
+
+static void igt_device_add_prop(struct igt_device *dev,
+				const char *key, const char *value)
+{
+	if (!key || !value)
+		return;
+
+	g_hash_table_insert(dev->props_ht, strdup(key), strdup(value));
+}
+
+static void igt_device_add_attr(struct igt_device *dev,
+				const char *key, const char *value)
+{
+	const char *v = value;
+
+	if (!key)
+		return;
+
+	/* It's possible we have symlink at key filename, but udev
+	 * library resolves only few of them
+	 */
+	if (!v) {
+		struct stat st;
+		char path[PATH_MAX];
+		char linkto[PATH_MAX];
+		int len;
+
+		snprintf(path, sizeof(path), "%s/%s", dev->syspath, key);
+		if (lstat(path, &st) != 0)
+			return;
+
+		len = readlink(path, linkto, sizeof(linkto));
+		if (len <= 0 || len == (ssize_t) sizeof(linkto))
+			return;
+		linkto[len] = '\0';
+		v = strrchr(linkto, '/');
+		if (v == NULL)
+			return;
+		v++;
+	}
+
+	g_hash_table_insert(dev->attrs_ht, strdup(key), strdup(v));
+}
+
+/* Iterate over udev properties list and rewrite it to igt_device properties
+ * hash table for instant access.
+ */
+static void get_props(struct udev_device *dev, struct igt_device *idev)
+{
+	struct udev_list_entry *entry;
+
+	entry = udev_device_get_properties_list_entry(dev);
+	while (entry) {
+		const char *name = udev_list_entry_get_name(entry);
+		const char *value = udev_list_entry_get_value(entry);
+
+		igt_device_add_prop(idev, name, value);
+		entry = udev_list_entry_get_next(entry);
+		DBG("prop: %s, val: %s\n", name, value);
+	}
+}
+
+/* Same as get_props(), but rewrites sysattrs. Resolves symbolic links
+ * not handled by udev get_sysattr_value().
+ * Function skips sysattrs from blacklist ht (acquiring some values can take
+ * seconds).
+ */
+static void get_attrs(struct udev_device *dev, struct igt_device *idev)
+{
+	struct udev_list_entry *entry;
+
+	entry = udev_device_get_sysattr_list_entry(dev);
+	while (entry) {
+		const char *key = udev_list_entry_get_name(entry);
+		const char *value;
+
+		if (is_on_blacklist(key)) {
+			entry = udev_list_entry_get_next(entry);
+			continue;
+		}
+
+		value = udev_device_get_sysattr_value(dev, key);
+		igt_device_add_attr(idev, key, value);
+		entry = udev_list_entry_get_next(entry);
+		DBG("attr: %s, val: %s\n", key, value);
+	}
+}
+
+#define get_prop(dev, prop) ((char *) g_hash_table_lookup(dev->props_ht, prop))
+#define get_attr(dev, attr) ((char *) g_hash_table_lookup(dev->attrs_ht, attr))
+#define get_prop_subsystem(dev) get_prop(dev, "SUBSYSTEM")
+#define is_drm_subsystem(dev)  (strequal(get_prop_subsystem(dev), "drm"))
+#define is_pci_subsystem(dev)  (strequal(get_prop_subsystem(dev), "pci"))
+
+/* Gets PCI_ID property, splits to xxxx:yyyy and stores
+ * xxxx to dev->vendor and yyyy to dev->device for
+ * faster access.
+ */
+static void set_vendor_device(struct igt_device *dev)
+{
+	const char *pci_id = get_prop(dev, "PCI_ID");
+
+	if (!pci_id || strlen(pci_id) != 9)
+		return;
+	dev->vendor = strndup(pci_id, 4);
+	dev->device = strndup(pci_id + 5, 4);
+}
+
+/* Allocate arrays for keeping scanned devices */
+static bool prepare_scan(void)
+{
+	if (igt_devs.all.prev == NULL || igt_devs.all.next == NULL)
+		igt_list_init(&igt_devs.all);
+
+	if (igt_devs.filtered.prev == NULL || igt_devs.filtered.next == NULL)
+		igt_list_init(&igt_devs.filtered);
+
+
+	return true;
+}
+
+static char* strdup_nullsafe(const char* str)
+{
+	if (str == NULL)
+		return NULL;
+
+	return strdup(str);
+}
+
+/* Create new igt_device from udev device.
+ * Fills structure with most usable udev device variables, properties
+ * and sysattrs.
+ */
+static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
+{
+	struct igt_device *idev = igt_device_new();
+
+	igt_assert(idev);
+	idev->syspath = strdup_nullsafe(udev_device_get_syspath(dev));
+	idev->subsystem = strdup_nullsafe(udev_device_get_subsystem(dev));
+	idev->devnode = strdup_nullsafe(udev_device_get_devnode(dev));
+
+	if (idev->devnode && strstr(idev->devnode, "/dev/dri/card"))
+		idev->drm_card = strdup(idev->devnode);
+	else if (idev->devnode && strstr(idev->devnode, "/dev/dri/render"))
+		idev->drm_render = strdup(idev->devnode);
+
+	get_props(dev, idev);
+	get_attrs(dev, idev);
+
+	return idev;
+}
+
+/* Iterate over all igt_devices array and find one matched to
+ * subsystem and syspath.
+ */
+static struct igt_device *igt_device_find(const char *subsystem,
+					  const char *syspath)
+{
+	struct igt_device *dev;
+
+	igt_list_for_each(dev, &igt_devs.all, link)
+	{
+		if (strcmp(dev->subsystem, subsystem) == 0 &&
+			strcmp(dev->syspath, syspath) == 0)
+			return dev;
+	}
+
+	return NULL;
+}
+
+static struct igt_device *igt_device_from_syspath(const char *syspath)
+{
+	struct igt_device *dev;
+
+	igt_list_for_each(dev, &igt_devs.all, link)
+	{
+		if (strcmp(dev->syspath, syspath) == 0)
+			return dev;
+	}
+
+	return NULL;
+}
+
+/* For each drm igt_device add or update its parent igt_device to the array.
+ * As card/render drm devices mostly have same parent (vkms is an exception)
+ * link to it and update corresponding drm_card / drm_render fields.
+ */
+static void update_or_add_parent(struct udev_device *dev,
+				 struct igt_device *idev)
+{
+	struct udev_device *parent_dev;
+	struct igt_device *parent_idev;
+	const char *subsystem, *syspath, *devname;
+
+	parent_dev = udev_device_get_parent(dev);
+	igt_assert(parent_dev);
+
+	subsystem = udev_device_get_subsystem(parent_dev);
+	syspath = udev_device_get_syspath(parent_dev);
+
+	parent_idev = igt_device_find(subsystem, syspath);
+	if (!parent_idev) {
+		parent_idev = igt_device_new_from_udev(parent_dev);
+		if (is_pci_subsystem(parent_idev)) {
+			set_vendor_device(parent_idev);
+		}
+		igt_list_insert_tail(&igt_devs.all, &parent_idev->link);
+	}
+	devname = udev_device_get_devnode(dev);
+	if (devname != NULL && strstr(devname, "/dev/dri/card"))
+		parent_idev->drm_card = strdup(devname);
+	else if (devname != NULL && strstr(devname, "/dev/dri/render"))
+		parent_idev->drm_render = strdup(devname);
+
+	idev->parent = parent_idev;
+}
+
+static struct igt_device *duplicate_device(struct igt_device *dev) {
+	struct igt_device *dup = malloc(sizeof(*dup));
+	memcpy(dup, dev, sizeof(*dev));
+	dup->link.prev = NULL;
+	dup->link.next = NULL;
+	return dup;
+}
+
+static gint devs_compare(const void *a, const void *b)
+{
+	struct igt_device *dev1, *dev2;
+	int ret;
+
+	dev1 = *(struct igt_device **) a;
+	dev2 = *(struct igt_device **) b;
+	ret = strcmp(dev1->subsystem, dev2->subsystem);
+	if (ret)
+		return ret;
+
+	return strcmp(dev1->syspath, dev2->syspath);
+}
+
+static void sort_all_devices(void)
+{
+	struct igt_device *dev, *tmp;
+	int len = igt_list_length(&igt_devs.all);
+	struct igt_device **devs = malloc(len * sizeof(*dev));
+
+	int i = 0;
+	igt_list_for_each_safe(dev, tmp, &igt_devs.all, link) {
+		devs[i] = dev;
+		igt_assert(i++ < len);
+		igt_list_remove(&dev->link);
+	}
+
+	qsort(devs, len, sizeof(*devs), devs_compare);
+
+	for (i = 0; i < len; ++i) {
+		igt_list_insert_tail(&igt_devs.all, &devs[i]->link);
+	}
+}
+
+/* Core scanning function.
+ *
+ * All scanned devices are kept inside igt_devs.all pointer array.
+ * Each added device is igt_device structure, which contrary to udev device
+ * has properties / sysattrs stored inside hash table instead of list.
+ *
+ * Function iterates over devices on 'drm' subsystem. For each drm device
+ * its parent is taken (bus device) and stored inside same array.
+ * Function sorts all found devices to keep same order of bus devices
+ * for providing predictable search.
+ */
+static void scan_drm_devices(void)
+{
+	struct udev *udev;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *devices, *dev_list_entry;
+	struct igt_device *dev;
+	int ret;
+
+	udev = udev_new();
+	igt_assert(udev);
+
+	enumerate = udev_enumerate_new(udev);
+	igt_assert(enumerate);
+
+	DBG("Scanning drm subsystem\n");
+	ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
+	igt_assert(!ret);
+
+	udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
+	igt_assert(!ret);
+
+	ret = udev_enumerate_scan_devices(enumerate);
+	igt_assert(!ret);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+	if (!devices)
+		return;
+
+	udev_list_entry_foreach(dev_list_entry, devices) {
+		const char *path;
+		struct udev_device *udev_dev;
+		struct igt_device *idev;
+
+		path = udev_list_entry_get_name(dev_list_entry);
+		udev_dev = udev_device_new_from_syspath(udev, path);
+		idev = igt_device_new_from_udev(udev_dev);
+		update_or_add_parent(udev_dev, idev);
+		igt_list_insert_tail(&igt_devs.all, &idev->link);
+
+		udev_device_unref(udev_dev);
+	}
+	udev_enumerate_unref(enumerate);
+	udev_unref(udev);
+
+	sort_all_devices();
+
+	igt_list_for_each(dev, &igt_devs.all, link) {
+		struct igt_device *dev_dup = duplicate_device(dev);
+		igt_list_insert_tail(&igt_devs.filtered, &dev_dup->link);
+	}
+}
+
+static void igt_device_free(struct igt_device *dev)
+{
+	free(dev->devnode);
+	free(dev->subsystem);
+	free(dev->syspath);
+	free(dev->drm_card);
+	free(dev->drm_render);
+	free(dev->vendor);
+	free(dev->device);
+	g_hash_table_destroy(dev->attrs_ht);
+	g_hash_table_destroy(dev->props_ht);
+}
+
+/**
+ * igt_devices_scan
+ * @force: enforce scanning devices
+ *
+ * Function scans udev in search of gpu devices. For first run it can be
+ * called with @force = false. If something changes during the the test
+ * or test does some module loading (new drm devices occurs during execution)
+ * function must be called again with @force = true to refresh device array.
+ */
+void igt_devices_scan(bool force)
+{
+	if (force && igt_devs.devs_scanned) {
+		struct igt_device *dev, *tmp;
+		igt_list_for_each_safe(dev, tmp, &igt_devs.all, link) {
+			igt_device_free(dev);
+			free(dev);
+		}
+
+		igt_devs.devs_scanned = false;
+	}
+
+	if (igt_devs.devs_scanned)
+		return;
+
+	prepare_scan();
+	scan_drm_devices();
+
+	igt_devs.devs_scanned = true;
+}
+
+static inline void _pr_simple(const char *k, const char *v)
+{
+	printf("    %-16s: %s\n", k, v);
+}
+
+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 *view)
+{
+	struct igt_device *dev;
+
+	if (!view)
+		return;
+
+	if (igt_list_empty(view)) {
+		printf("No GPU devices found\n");
+		return;
+	}
+
+	igt_list_for_each(dev, view, link) {
+		printf("sys:%s\n", dev->syspath);
+		if (dev->subsystem)
+			_pr_simple("subsystem", dev->subsystem);
+		if (dev->drm_card)
+			_pr_simple("drm card", dev->drm_card);
+		if (dev->drm_render)
+			_pr_simple("drm render", dev->drm_render);
+		if (is_drm_subsystem(dev)) {
+			_pr_simple2("parent", "sys",
+				   dev->parent->syspath);
+		} else {
+			if (is_pci_subsystem(dev)) {
+				_pr_simple("vendor", dev->vendor);
+				_pr_simple("device", dev->device);
+			}
+		}
+		printf("\n");
+	}
+}
+
+static inline void _print_key_value(const char* k, const char *v)
+{
+	printf("%-32s: %s\n", k, v);
+}
+
+static void print_ht(GHashTable *ht)
+{
+	GList *keys = g_hash_table_get_keys(ht);
+
+	keys = g_list_sort(keys, (GCompareFunc) strcmp);
+	while (keys) {
+		char *k = (char *) keys->data;
+		char *v = g_hash_table_lookup(ht, k);
+
+		_print_key_value(k, v);
+		keys = g_list_next(keys);
+	}
+	g_list_free(keys);
+}
+
+static void igt_devs_print_detail(struct igt_list *view)
+{
+	struct igt_device *dev;
+
+	if (!view)
+		return;
+
+	if (igt_list_empty(view)) {
+		printf("No GPU devices found\n");
+		return;
+	}
+
+	igt_list_for_each(dev, view, link) {
+		printf("========== %s:%s ==========\n",
+		       dev->subsystem, dev->syspath);
+		if (!is_drm_subsystem(dev)) {
+			_print_key_value("card device", dev->drm_card);
+			_print_key_value("render device", dev->drm_render);
+		}
+
+		printf("\n[properties]\n");
+		print_ht(dev->props_ht);
+		printf("\n[attributes]\n");
+		print_ht(dev->attrs_ht);
+		printf("\n");
+	}
+}
+
+static struct print_func {
+	void (*prn)(struct igt_list *view);
+} print_functions[] = {
+	[IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
+	[IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
+};
+
+/**
+ * igt_devices_print
+ * @printtype: IGT_PRINT_SIMPLE or IGT_PRINT_DETAIL
+ *
+ * Function can be used by external tool to print device array in simple
+ * or detailed form. This function is added here to avoid exposing
+ * internal implementation data structures.
+ */
+void igt_devices_print(enum igt_devices_print_type printtype)
+{
+	print_functions[printtype].prn(&igt_devs.filtered);
+}
+
+/**
+ * igt_devices_print_vendors
+ *
+ * Print pci id -> vendor mappings. Vendor names printed by this function
+ * can be used for filters like pci which allows passing vendor - like
+ * vendor id (8086) or as a string (Intel).
+ */
+void igt_devices_print_vendors(void)
+{
+	printf("Recognized vendors:\n");
+	printf("%-8s %-16s\n", "PCI ID", "vendor");
+	for (typeof(*pci_vendor_mapping) *vm = pci_vendor_mapping; vm->name; vm++) {
+		printf("%-8s %-16s\n", vm->vendor_id, vm->name);
+	}
+}
+
+struct filter;
+
+/* ------------------------------------------------------------------------- */
+struct filter_class {
+	struct igt_list *(*filter_function)(const struct filter_class *fcls,
+					    const struct filter *filter);
+	bool (*is_valid)(const struct filter_class *fcls,
+			 const struct filter *filter);
+	const char *name;
+	const char *help;
+	const char *detail;
+};
+
+#define FILTER_NAME_LEN 32
+#define FILTER_DATA_LEN 256
+
+struct filter {
+	struct filter_class *class;
+
+	char raw_data[FILTER_DATA_LEN];
+
+	struct {
+		char *vendor;
+		char *device;
+		char *card;
+		char *drm;
+		char *driver;
+	} data;
+};
+
+static void fill_filter_data(struct filter *filter, const char *key, const char *value)
+{
+	if (key == NULL || value == NULL) {
+		return;
+	}
+
+#define __fill_key(name) if (strcmp(key, #name) == 0) \
+	filter->data.name = strdup(value)
+	__fill_key(vendor);
+	__fill_key(device);
+	__fill_key(card);
+	__fill_key(drm);
+	__fill_key(driver);
+#undef __fill_key
+
+}
+
+static void split_filter_data(struct filter *filter)
+{
+	char *property, *key, *data, *dup;
+
+	dup = strdup(filter->raw_data);
+	data = dup;
+
+	while ((property = strsep(&data, ","))) {
+		key = strsep(&property, "=");
+		fill_filter_data(filter, key, property);
+	}
+
+	free(dup);
+}
+
+static struct filter_class *get_filter_class(const char *class_name, const struct filter *filter);
+
+static bool parse_filter(const char *fstr, struct filter *filter)
+{
+	char class_name[32];
+	if (!fstr || !filter)
+		return false;
+
+	memset(filter, 0, sizeof(*filter));
+
+
+	if (sscanf(fstr, "%31[^:]:%255s", class_name, filter->raw_data) >= 1) {
+		filter->class = get_filter_class(class_name, filter);
+		split_filter_data(filter);
+		return true;
+	}
+
+	return false;
+}
+
+static bool is_vendor_matched(struct igt_device *dev, const char *vendor)
+{
+	const char *vendor_id;
+
+	if (!dev->vendor || !vendor)
+		return false;
+
+	/* First we compare vendor id, like 8086 */
+	if (!strcasecmp(dev->vendor, vendor))
+		return true;
+
+	/* Likely we have vendor string instead of id */
+	vendor_id = get_pci_vendor_id_by_name(vendor);
+	if (!vendor_id)
+		return false;
+
+	return !strcasecmp(dev->vendor, vendor_id);
+}
+
+/* Filter which matches subsystem:/sys/... path.
+ * Used as first filter in chain.
+ */
+static struct igt_list *filter_sys(const struct filter_class *fcls,
+				   const struct filter *filter)
+{
+	struct igt_device *dev;
+	(void) fcls;
+
+	DBG("filter sys\n");
+	if (!strlen(filter->raw_data))
+		return &igt_devs.filtered;
+
+	dev = igt_device_from_syspath(filter->raw_data);
+	if (dev) {
+		struct igt_device *dup = duplicate_device(dev);
+		igt_list_insert_tail(&igt_devs.filtered, &dup->link);
+	}
+
+	return &igt_devs.filtered;
+}
+
+/* Find drm device using direct path to /dev/dri/.
+ * It extends filter_sys to allow using drm:/dev/dri/cardX and
+ * drm:/dev/dri/renderDX filter syntax.
+ */
+static struct igt_list *filter_drm(const struct filter_class *fcls,
+				   const struct filter *filter)
+{
+	struct igt_device *dev;
+	(void) fcls;
+
+	DBG("filter drm\n");
+	if (!strlen(filter->raw_data))
+		return &igt_devs.filtered;
+
+	igt_list_for_each(dev, &igt_devs.all, link) {
+		if (!is_drm_subsystem(dev))
+			continue;
+
+		if (strequal(dev->syspath, filter->raw_data) ||
+			strequal(dev->drm_card, filter->raw_data) ||
+			strequal(dev->drm_render, filter->raw_data)) {
+			struct igt_device *dup = duplicate_device(dev);
+			igt_list_insert_tail(&igt_devs.filtered, &dup->link);
+			break;
+		}
+	}
+
+
+	return &igt_devs.filtered;
+}
+
+/* Find appropriate pci device matching vendor/device/card filter arguments.
+ */
+static struct igt_list *filter_pci(const struct filter_class *fcls,
+				   const struct filter *filter)
+{
+	struct igt_device *dev;
+	int card = -1;
+	(void) fcls;
+
+	DBG("filter pci\n");
+
+	if (filter->data.card) {
+		sscanf(filter->data.card, "%d", &card);
+		if (card < 0) {
+			return &igt_devs.filtered;
+		}
+	} else {
+		card = 0;
+	}
+
+	igt_list_for_each(dev, &igt_devs.all, link) {
+		if (!is_pci_subsystem(dev))
+			continue;
+
+		/* Skip if 'vendor' doesn't match (hex or name) */
+		if (filter->data.vendor && !is_vendor_matched(dev, filter->data.vendor))
+			continue;
+
+		/* Skip if 'device' doesn't match */
+		if (filter->data.device && strcasecmp(filter->data.device, dev->device))
+			continue;
+
+		/* We get n-th card */
+		if (!card) {
+			struct igt_device *dup = duplicate_device(dev);
+			igt_list_insert_tail(&igt_devs.filtered, &dup->link);
+			break;
+		}
+		card--;
+	}
+
+	DBG("Filter pci filtered size: %d\n", igt_list_length(&igt_devs.filtered));
+
+	return &igt_devs.filtered;
+}
+
+static bool sys_path_valid(const struct filter_class *fcls,
+			   const struct filter *filter)
+{
+	struct stat st;
+
+	if (stat(filter->raw_data, &st)) {
+		igt_warn("sys_path_valid: syspath [%s], err: %s\n",
+			 filter->raw_data, strerror(errno));
+		return false;
+	}
+
+	return true;
+}
+
+
+static struct filter_class filter_definition_list[] = {
+	{
+		.name = "sys",
+		.is_valid = sys_path_valid,
+		.filter_function = filter_sys,
+		.help = "sys:/sys/devices/pci0000:00/0000:00:02.0",
+		.detail = "find device byt its sysfs path\n",
+	},
+	{
+		.name = "drm",
+		.filter_function = filter_drm,
+		.help = "drm:/dev/dri/* path",
+		.detail = "find drm device by /dev/dri/* node\n",
+	},
+	{
+		.name = "pci",
+		.filter_function = filter_pci,
+		.help = "pci:[vendor=%04x/name][,device=%04x][,card=%d]",
+		.detail = "vendor is hex number or vendor name\n",
+	},
+	{
+		.name = NULL,
+	},
+};
+
+static struct filter_class *get_filter_class(const char *class_name, const struct filter *filter)
+{
+	struct filter_class *fcls = NULL;
+	int i = 0;
+
+	while ((fcls = &filter_definition_list[i++])->name != NULL) {
+		if (strcmp(class_name, fcls->name) == 0)
+			return fcls;
+	}
+
+	return NULL;
+}
+
+/**
+ * @igt_device_print_filter_types
+ *
+ * Print all filters syntax for device selection.
+ */
+void igt_device_print_filter_types(void)
+{
+	const struct filter_class *filter = NULL;
+	int i = 0;
+
+	printf("Filter types:\n---\n");
+	printf("%-12s  %s\n---\n", "filter", "syntax");
+
+	while ((filter = &filter_definition_list[i++])->name != NULL) {
+		printf("%-12s  %s\n", filter->name, filter->help);
+		printf("%-12s  %s\n", "", filter->detail);
+	}
+}
+
+static char *device_filter;
+
+/**
+ * igt_device_is_filter_set
+ *
+ * Returns whether we have a filter set.
+ */
+bool igt_device_is_filter_set(void)
+{
+	return device_filter != NULL;
+}
+
+/* Check does filter is valid. It checks:
+ * 1. /sys/... path first
+ * 2. filter name from filter definition
+ */
+static bool is_filter_valid(const char *fstr)
+{
+	struct filter filter;
+	int ret;
+
+	ret = parse_filter(fstr, &filter);
+	if (!ret)
+		return false;
+
+	if (filter.class == NULL) {
+		igt_warn("No filter matching [%s:%s]\n", filter.class->name, filter.raw_data);
+		return false;
+	}
+
+	if (filter.class->is_valid != NULL && !filter.class->is_valid(filter.class, &filter))
+	{
+		igt_warn("Filter not valid [%s:%s]\n", filter.class->name, filter.raw_data);
+		return false;
+	}
+
+	return true;
+}
+
+/**
+ * igt_device_filter_set
+ * @filter: filter that should be set globally
+ *
+ * Returns number of filters added to filter array. Can be greater than
+ * 1 if @filters contains more than one filter separated by semicolon.
+ */
+void igt_device_filter_set(const char *filter)
+{
+	if (!is_filter_valid(filter)) {
+		igt_warn("Invalid filter: %s\n", filter);
+		return;
+	}
+
+	device_filter = strdup(filter);
+}
+
+/**
+ * igt_device_filter_free
+ *
+ * Free the filter that we store internally, effectively insetting it.
+ */
+void igt_device_filter_free(void)
+{
+	if (device_filter != NULL)
+		free(device_filter);
+}
+
+/**
+ * igt_device_filter_get
+ *
+ * Returns filter string or NULL if not set
+ */
+const char *igt_device_filter_get(void)
+{
+	return device_filter;
+}
+
+static bool igt_device_filter_apply(const char *fstr)
+{
+	struct igt_device *dev, *tmp;
+	struct filter filter;
+	bool ret;
+
+	if (!fstr)
+		return false;
+
+	ret = parse_filter(fstr, &filter);
+	if (!ret) {
+		igt_warn("Can't split filter [%s]\n", fstr);
+		return false;
+	}
+
+	/* Clean the filtered list */
+	igt_list_for_each_safe(dev, tmp, &igt_devs.filtered, link) {
+		igt_list_remove(&dev->link);
+		free(dev);
+	}
+
+	/* If filter.data contains "/sys" use direct path instead
+	 * contextual filter.
+	 */
+
+	if (!filter.class) {
+		igt_warn("No filter matching [%s:%s]\n", filter.class->name, filter.raw_data);
+		return false;
+	}
+	filter.class->filter_function(filter.class, &filter);
+
+	return true;
+}
+
+/**
+ * igt_device_card_match
+ * @filter: filter string
+ * @card: pointer to igt_device_card struct
+ *
+ * Function applies filter to match device from device array.
+ *
+ * Returns:
+ * false - no card pointer was passed or card wasn't matched,
+ * true - card matched and returned.
+ */
+bool igt_device_card_match(const char *filter, struct igt_device_card *card)
+{
+	struct igt_device *dev = NULL;
+
+	if (!card)
+		return false;
+	memset(card, 0, sizeof(*card));
+
+	igt_devices_scan(false);
+
+	if (igt_device_filter_apply(filter) == false)
+		return false;
+
+	if (igt_list_empty(&igt_devs.filtered))
+		return false;
+
+	/* We take first one if more than one card matches filter */
+	dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
+
+	if (dev->subsystem != NULL)
+		strncpy(card->subsystem, dev->subsystem,
+			     sizeof(card->subsystem) - 1);
+
+	if (dev->drm_card != NULL)
+		strncpy(card->card, dev->drm_card,
+			     sizeof(card->card) - 1);
+
+	if (dev->drm_render != NULL)
+		strncpy(card->render, dev->drm_render,
+			     sizeof(card->render) - 1);
+
+	return true;
+}
+
+/**
+ * igt_open_card:
+ * @card: pointer to igt_device_card structure
+ *
+ * Open /dev/dri/cardX device represented by igt_device_card structure.
+ * Requires filled @card argument (see igt_device_card_match() function).
+ *
+ * An open DRM fd or -1 on error
+ */
+int igt_open_card(struct igt_device_card *card)
+{
+	if (!card || !strlen(card->card))
+		return -1;
+
+	return open(card->card, O_RDWR);
+}
+
+/**
+ * igt_open_render:
+ * @card: pointer to igt_device_card structure
+ *
+ * Open /dev/dri/renderDX device represented by igt_device_card structure.
+ * Requires filled @card argument (see igt_device_card_match() function).
+ *
+ * An open DRM fd or -1 on error
+ */
+int igt_open_render(struct igt_device_card *card)
+{
+	if (!card || !strlen(card->render))
+		return -1;
+
+	return open(card->render, O_RDWR);
+}
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
new file mode 100644
index 00000000..dbde8816
--- /dev/null
+++ b/lib/igt_device_scan.h
@@ -0,0 +1,67 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_DEVICE_SCAN_H__
+#define __IGT_DEVICE_SCAN_H__
+
+#include <limits.h>
+#include <igt.h>
+
+enum igt_devices_print_type {
+	IGT_PRINT_SIMPLE,
+	IGT_PRINT_DETAIL,
+};
+
+struct igt_device_card {
+	char subsystem[NAME_MAX];
+	char card[NAME_MAX];
+	char render[NAME_MAX];
+};
+
+/* Scan udev subsystems. Do it once unless force is true, what rescans
+ * devices again.
+ */
+void igt_devices_scan(bool force);
+
+void igt_devices_print(enum igt_devices_print_type printtype);
+void igt_devices_print_vendors(void);
+void igt_device_print_filter_types(void);
+
+/*
+ * Handle device filter collection array.
+ * IGT can store/retrieve filters passed by user using '--device' args.
+ */
+
+bool igt_device_is_filter_set(void);
+void igt_device_filter_set(const char *filter);
+void igt_device_filter_free(void);
+const char *igt_device_filter_get(void);
+
+/* Use filter to match the device and fill card structure */
+bool igt_device_card_match(const char *filter, struct igt_device_card *card);
+
+int igt_open_card(struct igt_device_card *card);
+int igt_open_render(struct igt_device_card *card);
+
+#endif /* __IGT_DEVICE_SCAN_H__ */
diff --git a/lib/meson.build b/lib/meson.build
index 19cb6255..486f51d7 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -10,6 +10,7 @@ lib_sources = [
 	'igt_color_encoding.c',
 	'igt_debugfs.c',
 	'igt_device.c',
+	'igt_device_scan.c',
 	'igt_aux.c',
 	'igt_gt.c',
 	'igt_gvt.c',
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list Arkadiusz Hiler
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API Arkadiusz Hiler
@ 2019-10-24 11:05 ` Arkadiusz Hiler
  2019-10-24 13:06   ` Chris Wilson
  2019-11-18 11:58   ` Petri Latvala
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT Arkadiusz Hiler
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-24 11:05 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Tool uses device selection API to scan and display GPU devices.
It can be used to check filter correctness as well as order
of applying the filters (.igtrc, IGT_DEVICE and --device argument).

v2 (Arek):
 * don't print chip as it's no longer there
 * make it a second patch, before any alterations to igt_core or drmtest
 * use only a single filter

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 tools/Makefile.sources |   1 +
 tools/lsgpu.c          | 295 +++++++++++++++++++++++++++++++++++++++++
 tools/meson.build      |   1 +
 3 files changed, 297 insertions(+)
 create mode 100644 tools/lsgpu.c

diff --git a/tools/Makefile.sources b/tools/Makefile.sources
index d764895d..b7a43d47 100644
--- a/tools/Makefile.sources
+++ b/tools/Makefile.sources
@@ -33,6 +33,7 @@ tools_prog_lists =		\
 	intel_watermark		\
 	intel_gem_info		\
 	intel_gvtg_test     \
+	lsgpu			\
 	$(NULL)
 
 dist_bin_SCRIPTS = intel_gpu_abrt
diff --git a/tools/lsgpu.c b/tools/lsgpu.c
new file mode 100644
index 00000000..e385aaa6
--- /dev/null
+++ b/tools/lsgpu.c
@@ -0,0 +1,295 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt_device_scan.h"
+#include "igt.h"
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <glib.h>
+
+/**
+ * SECTION:lsgpu
+ * @short_description: lsgpu
+ * @title: lsgpu
+ * @include: lsgpu.c
+ *
+ * # lsgpu
+ *
+ * The devices can be scanned and displayed using 'lsgpu' tool. Tool also
+ * displays properties and sysattrs (-p switch, means print detail) which
+ * can be used during filter implementation.
+ *
+ * Tool can also be used to try out filters.
+ * To select device use '-d' or '--device' argument like:
+ *
+ * |[<!-- language="plain" -->
+ * ./lsgpu -d 'pci:vendor=Intel'
+ * === Device filter list ===
+ * [ 0]: pci:vendor=Intel
+
+ * === Testing device open ===
+ * subsystem   : pci
+ * drm card    : /dev/dri/card0
+ * drm render  : /dev/dri/renderD128
+ * Device /dev/dri/card0 successfully opened
+ * Device /dev/dri/renderD128 successfully opened
+ * ]|
+ *
+ * Additionally lsgpu tries to open the card and render nodes to verify
+ * permissions. It also uses IGT variable search order:
+ * - use --device first (it overrides IGT_DEVICE and .igtrc Common::Device
+ *   settings)
+ * - use IGT_DEVICE enviroment variable if no --device are passed
+ * - use .igtrc Common::Device if no --device nor IGT_DEVICE are passed
+ */
+
+enum {
+	OPT_PRINT_DETAIL   = 'p',
+	OPT_LIST_VENDORS   = 'v',
+	OPT_LIST_FILTERS   = 'l',
+	OPT_DEVICE         = 'd',
+	OPT_HELP           = 'h'
+};
+
+static bool g_show_vendors;
+static bool g_list_filters;
+static bool g_device;
+static bool g_help;
+static char *igt_rc_device;
+
+static const char *usage_str =
+	"usage: lsgpu [options]\n\n"
+	"Options:\n"
+	"  -p, --print-details         Print devices with details\n"
+	"  -v, --list-vendors          List recognized vendors\n"
+	"  -l, --list-filter-types     List registered device filters types\n"
+	"  -d, --device filter         Device filter, can be given multiple times\n"
+	"  -h, --help                  Show this help message and exit\n";
+
+static void test_device_open(struct igt_device_card *card)
+{
+	int fd;
+
+	if (!card)
+		return;
+
+	fd = igt_open_card(card);
+	if (fd >= 0) {
+		printf("Device %s successfully opened\n", card->card);
+		close(fd);
+	} else {
+		if (strlen(card->card))
+			printf("Cannot open card %s device\n", card->card);
+		else
+			printf("Cannot open card device, empty name\n");
+	}
+
+	fd = igt_open_render(card);
+	if (fd >= 0) {
+		printf("Device %s successfully opened\n", card->render);
+		close(fd);
+	} else {
+		if (strlen(card->render))
+			printf("Cannot open render %s device\n", card->render);
+		else
+			printf("Cannot open render device, empty name\n");
+	}
+}
+
+static void print_card(struct igt_device_card *card)
+{
+	if (!card)
+		return;
+
+	printf("subsystem   : %s\n", card->subsystem);
+	printf("drm card    : %s\n", card->card);
+	printf("drm render  : %s\n", card->render);
+}
+
+/* Partially copied from igt_core to simulate device selection order:
+ * 1. --device        (IGT_DEVICE and .igtrc Common::Device are ignored)
+ * 2. IGT_DEVICE env  (.igtrc Common::Device is ignored)
+ * 3. .igtrc          (overrides
+ */
+static void common_init_config(void)
+{
+	char *key_file_env = NULL;
+	char *key_file_loc = NULL;
+	GError *error = NULL;
+	GKeyFile *igt_key_file;
+	int ret;
+
+	/* Filter count > 0, just skip .igtrc */
+	if (igt_device_is_filter_set())
+		return;
+
+	/* Determine igt config path */
+	key_file_env = getenv("IGT_CONFIG_PATH");
+	if (key_file_env) {
+		key_file_loc = key_file_env;
+	} else {
+		key_file_loc = malloc(100);
+		snprintf(key_file_loc, 100, "%s/.igtrc", g_get_home_dir());
+	}
+
+	/* Load igt config file */
+	igt_key_file = g_key_file_new();
+	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
+					G_KEY_FILE_NONE, &error);
+	if (!ret) {
+		g_error_free(error);
+		g_key_file_free(igt_key_file);
+		igt_key_file = NULL;
+
+		return;
+	}
+
+	g_clear_error(&error);
+
+	if (!igt_rc_device)
+		igt_rc_device =	g_key_file_get_string(igt_key_file, "Common",
+						      "Device", &error);
+
+	if (igt_rc_device) {
+		igt_device_filter_set(igt_rc_device);
+		g_device = true;
+	}
+
+	g_clear_error(&error);
+}
+
+int main(int argc, char *argv[])
+{
+	static struct option long_options[] = {
+		{"print-detail",      no_argument,       NULL, OPT_PRINT_DETAIL},
+		{"list-vendors",      no_argument,       NULL, OPT_LIST_VENDORS},
+		{"list-filter-types", no_argument,       NULL, OPT_LIST_FILTERS},
+		{"device",            required_argument, NULL, OPT_DEVICE},
+		{"help",              no_argument,       NULL, OPT_HELP},
+		{0, 0, 0, 0}
+	};
+	int c, index = 0;
+	const char *env;
+	enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
+
+	env = getenv("IGT_DEVICE");
+	if (env) {
+		igt_device_filter_set(env);
+		g_device = true;
+	}
+
+	while ((c = getopt_long(argc, argv, "pvld:h",
+				long_options, &index)) != -1) {
+		switch(c) {
+
+		case OPT_PRINT_DETAIL:
+			printtype = IGT_PRINT_DETAIL;
+			break;
+		case OPT_LIST_VENDORS:
+			g_show_vendors = true;
+			break;
+		case OPT_LIST_FILTERS:
+			g_list_filters = true;
+			break;
+		case OPT_DEVICE:
+			g_device = true;
+			if (getenv("IGT_DEVICE") && igt_device_is_filter_set()) {
+				unsetenv("IGT_DEVICE");
+				igt_rc_device = NULL;
+				igt_device_filter_free();
+			}
+			igt_device_filter_set(optarg);
+			break;
+		case OPT_HELP:
+			g_help = true;
+			break;
+		}
+	}
+	common_init_config();
+
+	if (g_help) {
+		printf("%s\n", usage_str);
+		exit(0);
+	}
+
+	if (g_show_vendors) {
+		igt_devices_print_vendors();
+		return 0;
+	}
+
+	if (g_list_filters) {
+		igt_device_print_filter_types();
+		return 0;
+	}
+
+	igt_devices_scan(false);
+
+	if (getenv("IGT_DEVICE")) {
+		printf("Notice: Using IGT_DEVICE device settings [%s]\n",
+		       getenv("IGT_DEVICE"));
+	} else if (igt_rc_device) {
+		printf("Notice: Using .igtrc device settings [%s]\n",
+		       igt_rc_device);
+	} else {
+		if (igt_device_is_filter_set())
+			printf("Notice: Using --device filters\n");
+	}
+
+	if (g_device) {
+		struct igt_device_card card;
+		const char *filter = igt_device_filter_get();
+
+		if (!igt_device_is_filter_set()) {
+			return -1;
+		}
+
+		printf("=== Device filter ===\n");
+		printf("%s\n\n", filter);
+
+		printf("=== Testing device open ===\n");
+
+		if (!igt_device_card_match(filter, &card)) {
+			printf("No device found for the filter\n\n");
+			return -1;
+		}
+
+		printf("Device detail:\n");
+		print_card(&card);
+		test_device_open(&card);
+		if (printtype == IGT_PRINT_DETAIL) {
+			printf("\n");
+			igt_devices_print(printtype);
+		}
+		printf("-------------------------------------------\n");
+
+		return 0;
+	}
+
+	igt_devices_print(printtype);
+
+	return 0;
+}
diff --git a/tools/meson.build b/tools/meson.build
index eecb122b..74822a33 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -36,6 +36,7 @@ tools_progs = [
 	'intel_gem_info',
 	'intel_gvtg_test',
 	'dpcd_reg',
+	'lsgpu',
 ]
 tool_deps = igt_deps
 
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
                   ` (2 preceding siblings ...)
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool Arkadiusz Hiler
@ 2019-10-24 11:05 ` Arkadiusz Hiler
  2019-11-18 12:14   ` Petri Latvala
  2019-11-20  9:31   ` Petri Latvala
  2019-10-24 13:00 ` [igt-dev] ✓ Fi.CI.BAT: success for device selection && lsgpu Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-24 11:05 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

New IGT command line argument --device, IGT_DEVICE enviroment
and .igtrc Common::Device were added to allow selecting device
using device selection API.

v2 (Arek):
 * remove functions acting on igt_device_card
 * use only a single filter

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 .../igt-gpu-tools/igt_test_programs.xml       |  7 ++
 lib/drmtest.c                                 | 94 +++++++++++++++++--
 lib/drmtest.h                                 |  2 +
 lib/igt_core.c                                | 47 ++++++++++
 4 files changed, 142 insertions(+), 8 deletions(-)

diff --git a/docs/reference/igt-gpu-tools/igt_test_programs.xml b/docs/reference/igt-gpu-tools/igt_test_programs.xml
index b64ba474..fcce1458 100644
--- a/docs/reference/igt-gpu-tools/igt_test_programs.xml
+++ b/docs/reference/igt-gpu-tools/igt_test_programs.xml
@@ -43,6 +43,13 @@
             </para></listitem>
           </varlistentry>
 
+          <varlistentry>
+            <term><option>--device filter</option></term>
+            <listitem><para>
+                select device using filter (see "Device selection" for details)
+            </para></listitem>
+          </varlistentry>
+
           <varlistentry>
             <term><option>--help-description</option></term>
             <listitem><para>
diff --git a/lib/drmtest.c b/lib/drmtest.c
index c379a7b7..43471625 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -55,6 +55,7 @@
 #include "igt_gt.h"
 #include "igt_kmod.h"
 #include "igt_sysfs.h"
+#include "igt_device_scan.h"
 #include "version.h"
 #include "config.h"
 #include "intel_reg.h"
@@ -266,14 +267,9 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
 	return -1;
 }
 
-static int __open_driver(const char *base, int offset, unsigned int chipset)
+static void __try_modprobe(unsigned int chipset)
 {
 	static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
-	int fd;
-
-	fd = __search_and_open(base, offset, chipset);
-	if (fd != -1)
-		return fd;
 
 	pthread_mutex_lock(&mutex);
 	for (const struct module *m = modules; m->module; m++) {
@@ -285,26 +281,108 @@ static int __open_driver(const char *base, int offset, unsigned int chipset)
 		}
 	}
 	pthread_mutex_unlock(&mutex);
+}
+
+static int __open_driver(const char *base, int offset, unsigned int chipset)
+{
+	int fd;
+
+	fd = __search_and_open(base, offset, chipset);
+	if (fd != -1)
+		return fd;
+
+	__try_modprobe(chipset);
 
 	return __search_and_open(base, offset, chipset);
 }
 
+static int __open_driver_exact(const char *name, unsigned int chipset)
+{
+	int fd;
+
+	fd = open_device(name, chipset);
+	if (fd != -1)
+		return fd;
+
+	__try_modprobe(chipset);
+
+	return open_device(name, chipset);
+}
+
+/*
+ * A helper to get the first marching card in case a filter is set.
+ * It does all the extra logging around the filters for us.
+ *
+ * @card: pointer to the igt_device_card structure to be filled
+ * when a card is found.
+ *
+ * Returns:
+ * True if card according to the added filter was found,
+ * false othwerwise.
+ */
+static bool __get_the_first_card(struct igt_device_card *card)
+{
+	const char *filter;
+
+	if (igt_device_is_filter_set()) {
+		filter = igt_device_filter_get();
+		igt_info("Looking for devices to open using filter: %s\n", filter);
+
+		if (igt_device_card_match(filter, card)) {
+			igt_info("Filter matched %s | %s\n", card->card, card->render);
+			return true;
+		}
+
+		igt_warn("No card matches the filter!\n");
+	}
+
+	return false;
+}
+
 /**
  * __drm_open_driver:
  * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
  *
- * Open the first DRM device we can find, searching up to 16 device nodes
+ * Function opens device in the following order:
+ * 1. when --device arguments are present device scanning will be executed,
+ * then filter argument is used to find matching one.
+ * 2. compatibility mode - open the first DRM device we can find,
+ * searching up to 16 device nodes.
  *
  * Returns:
  * An open DRM fd or -1 on error
  */
 int __drm_open_driver(int chipset)
 {
+	if (igt_device_is_filter_set()) {
+		bool found;
+		struct igt_device_card card;
+
+		found = __get_the_first_card(&card);
+
+		if (!found || !strlen(card.card))
+			return -1;
+
+		return __open_driver_exact(card.card, chipset);
+	}
+
 	return __open_driver("/dev/dri/card", 0, chipset);
 }
 
-static int __drm_open_driver_render(int chipset)
+int __drm_open_driver_render(int chipset)
 {
+	if (igt_device_is_filter_set()) {
+		bool found;
+		struct igt_device_card card;
+
+		found = __get_the_first_card(&card);
+
+		if (!found || !strlen(card.render))
+			return -1;
+
+		return __open_driver_exact(card.render, chipset);
+	}
+
 	return __open_driver("/dev/dri/renderD", 128, chipset);
 }
 
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 614f57e6..7a985b26 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -50,6 +50,7 @@
 #define DRIVER_AMDGPU	(1 << 3)
 #define DRIVER_V3D	(1 << 4)
 #define DRIVER_PANFROST	(1 << 5)
+
 /*
  * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
  * with vgem as well as a supported driver, you can end up with a
@@ -81,6 +82,7 @@ int drm_open_driver(int chipset);
 int drm_open_driver_master(int chipset);
 int drm_open_driver_render(int chipset);
 int __drm_open_driver(int chipset);
+int __drm_open_driver_render(int chipset);
 
 void gem_quiescent_gpu(int fd);
 
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 67b76025..94121f99 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -71,6 +71,7 @@
 #include "igt_sysrq.h"
 #include "igt_rc.h"
 #include "igt_list.h"
+#include "igt_device_scan.h"
 
 #define UNW_LOCAL_ONLY
 #include <libunwind.h>
@@ -245,6 +246,9 @@
  *	[Common]
  *	FrameDumpPath=/tmp # The path to dump frames that fail comparison checks
  *
+ *	&num; Device selection filter
+ *	Device=pci:vendor=8086,card=0;vgem:
+ *
  *	&num; The following section is used for configuring the Device Under Test.
  *	&num; It is not mandatory and allows overriding default values.
  *	[DUT]
@@ -304,6 +308,7 @@ enum {
 	OPT_DEBUG,
 	OPT_INTERACTIVE_DEBUG,
 	OPT_SKIP_CRC,
+	OPT_DEVICE,
 	OPT_HELP = 'h'
 };
 
@@ -320,6 +325,7 @@ static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
 GKeyFile *igt_key_file;
 
 char *igt_frame_dump_path;
+char *igt_rc_device;
 
 static bool stderr_needs_sentinel = false;
 
@@ -644,6 +650,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
 		   "  --skip-crc-compare\n"
 		   "  --help-description\n"
 		   "  --describe\n"
+		   "  --device filter\n"
 		   "  --help|-h\n");
 	if (help_str)
 		fprintf(f, "%s\n", help_str);
@@ -733,6 +740,31 @@ static void common_init_config(void)
 
 	if (ret != 0)
 		igt_set_autoresume_delay(ret);
+
+	/* Adding filters, order .igtrc, IGT_DEVICE, --device filter */
+	if (igt_device_is_filter_set())
+		igt_debug("Notice: using --device filters:\n");
+	else {
+		if (igt_rc_device) {
+			igt_debug("Notice: using IGT_DEVICE env:\n");
+		} else {
+			igt_rc_device =	g_key_file_get_string(igt_key_file,
+							      "Common",
+							      "Device", &error);
+			g_clear_error(&error);
+			if (igt_rc_device)
+				igt_debug("Notice: using .igtrc "
+					  "Common::Device:\n");
+		}
+		if (igt_rc_device) {
+			igt_device_filter_set(igt_rc_device);
+			free(igt_rc_device);
+			igt_rc_device = NULL;
+		}
+	}
+
+	if (igt_device_is_filter_set())
+		igt_debug("[%s]\n",  igt_device_filter_get());
 }
 
 static void common_init_env(void)
@@ -767,6 +799,11 @@ static void common_init_env(void)
 	if (env) {
 		__set_forced_driver(env);
 	}
+
+	env = getenv("IGT_DEVICE");
+	if (env) {
+		igt_rc_device = strdup(env);
+	}
 }
 
 static int common_init(int *argc, char **argv,
@@ -785,6 +822,7 @@ static int common_init(int *argc, char **argv,
 		{"debug",             optional_argument, NULL, OPT_DEBUG},
 		{"interactive-debug", optional_argument, NULL, OPT_INTERACTIVE_DEBUG},
 		{"skip-crc-compare",  no_argument,       NULL, OPT_SKIP_CRC},
+		{"device",            required_argument, NULL, OPT_DEVICE},
 		{"help",              no_argument,       NULL, OPT_HELP},
 		{0, 0, 0, 0}
 	};
@@ -907,6 +945,15 @@ static int common_init(int *argc, char **argv,
 		case OPT_SKIP_CRC:
 			igt_skip_crc_compare = true;
 			goto out;
+		case OPT_DEVICE:
+			assert(optarg);
+			/* if set by env IGT_DEVICE we need to free it */
+			if (igt_rc_device) {
+				free(igt_rc_device);
+				igt_rc_device = NULL;
+			}
+			igt_device_filter_set(optarg);
+			break;
 		case OPT_HELP:
 			print_usage(help_str, false);
 			ret = -1;
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list Arkadiusz Hiler
@ 2019-10-24 11:16   ` Chris Wilson
  2019-10-24 11:20     ` Arkadiusz Hiler
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-24 11:16 UTC (permalink / raw)
  To: Arkadiusz Hiler, igt-dev; +Cc: Petri Latvala

Quoting Arkadiusz Hiler (2019-10-24 12:05:12)
> We have diverged from how Wayland implements list both in parameter
> ordering and function naming.

Nak.
 
> Let's make our side consistent to ease the cognitive dissonance for
> developers working on both projects.

Primary target audience is kernel developers.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list
  2019-10-24 11:16   ` Chris Wilson
@ 2019-10-24 11:20     ` Arkadiusz Hiler
  2019-10-24 11:23       ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-24 11:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, Petri Latvala

On Thu, Oct 24, 2019 at 12:16:31PM +0100, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-10-24 12:05:12)
> > We have diverged from how Wayland implements list both in parameter
> > ordering and function naming.
> 
> Nak.
>  
> > Let's make our side consistent to ease the cognitive dissonance for
> > developers working on both projects.
> 
> Primary target audience is kernel developers.
> -Chris

We can always go the other way around and make it as kernel-like as
possible, but the implementation was advertising itself as a wayland
copy.

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list
  2019-10-24 11:20     ` Arkadiusz Hiler
@ 2019-10-24 11:23       ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-10-24 11:23 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev, Petri Latvala

Quoting Arkadiusz Hiler (2019-10-24 12:20:10)
> On Thu, Oct 24, 2019 at 12:16:31PM +0100, Chris Wilson wrote:
> > Quoting Arkadiusz Hiler (2019-10-24 12:05:12)
> > > We have diverged from how Wayland implements list both in parameter
> > > ordering and function naming.
> > 
> > Nak.
> >  
> > > Let's make our side consistent to ease the cognitive dissonance for
> > > developers working on both projects.
> > 
> > Primary target audience is kernel developers.
> > -Chris
> 
> We can always go the other way around and make it as kernel-like as
> possible, but the implementation was advertising itself as a wayland
> copy.

I wonder why I told that fib. Take a guess...
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for device selection && lsgpu
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
                   ` (3 preceding siblings ...)
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT Arkadiusz Hiler
@ 2019-10-24 13:00 ` Patchwork
  2019-10-25 17:07 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  2019-11-12 10:14 ` [igt-dev] [PATCH i-g-t 0/4] " Tvrtko Ursulin
  6 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-24 13:00 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== Series Details ==

Series: device selection && lsgpu
URL   : https://patchwork.freedesktop.org/series/68510/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7171 -> IGTPW_3604
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/index.html

Known issues
------------

  Here are the changes found in IGTPW_3604 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_linear_blits@basic:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/fi-icl-u3/igt@gem_linear_blits@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/fi-icl-u3/igt@gem_linear_blits@basic.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8109u:       [PASS][3] -> [DMESG-FAIL][4] ([fdo#112050 ])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-write-gtt:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt.html

  * {igt@i915_selftest@live_gt_heartbeat}:
    - {fi-icl-dsi}:       [DMESG-FAIL][7] -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/fi-icl-dsi/igt@i915_selftest@live_gt_heartbeat.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/fi-icl-dsi/igt@i915_selftest@live_gt_heartbeat.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
  [fdo#112050 ]: https://bugs.freedesktop.org/show_bug.cgi?id=112050 
  [fdo#112096]: https://bugs.freedesktop.org/show_bug.cgi?id=112096


Participating hosts (51 -> 45)
------------------------------

  Additional (1): fi-cml-u2 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_5237 -> IGTPW_3604

  CI-20190529: 20190529
  CI_DRM_7171: 888c45e0d89f655ab7fa0248a10fbba9d92a6d5c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3604: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/index.html
  IGT_5237: 9a46404de7c42c8cc2d492176e956597ef28d7c4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool Arkadiusz Hiler
@ 2019-10-24 13:06   ` Chris Wilson
  2019-10-28 11:21     ` Arkadiusz Hiler
  2019-11-18 11:58   ` Petri Latvala
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-24 13:06 UTC (permalink / raw)
  To: Arkadiusz Hiler, igt-dev; +Cc: Petri Latvala

Quoting Arkadiusz Hiler (2019-10-24 12:05:14)
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> Tool uses device selection API to scan and display GPU devices.
> It can be used to check filter correctness as well as order
> of applying the filters (.igtrc, IGT_DEVICE and --device argument).

A couple of examples of use and output?
 
> v2 (Arek):
>  * don't print chip as it's no longer there
>  * make it a second patch, before any alterations to igt_core or drmtest
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for device selection && lsgpu
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
                   ` (4 preceding siblings ...)
  2019-10-24 13:00 ` [igt-dev] ✓ Fi.CI.BAT: success for device selection && lsgpu Patchwork
@ 2019-10-25 17:07 ` Patchwork
  2019-11-12 10:14 ` [igt-dev] [PATCH i-g-t 0/4] " Tvrtko Ursulin
  6 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-25 17:07 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== Series Details ==

Series: device selection && lsgpu
URL   : https://patchwork.freedesktop.org/series/68510/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7171_full -> IGTPW_3604_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_3604_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3604_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_3604_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_schedule@preempt-queue-contexts-bsd1:
    - shard-kbl:          [PASS][1] -> [FAIL][2] +8 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl6/igt@gem_exec_schedule@preempt-queue-contexts-bsd1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl1/igt@gem_exec_schedule@preempt-queue-contexts-bsd1.html

  * igt@gem_exec_schedule@preemptive-hang-blt:
    - shard-iclb:         [PASS][3] -> [SKIP][4] +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb8/igt@gem_exec_schedule@preemptive-hang-blt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-blt.html

  * igt@gem_exec_schedule@reorder-wide-render:
    - shard-glk:          [PASS][5] -> [FAIL][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-glk8/igt@gem_exec_schedule@reorder-wide-render.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-glk3/igt@gem_exec_schedule@reorder-wide-render.html
    - shard-iclb:         [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb8/igt@gem_exec_schedule@reorder-wide-render.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb1/igt@gem_exec_schedule@reorder-wide-render.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_schedule@preempt-hang-render:
    - {shard-tglb}:       NOTRUN -> [SKIP][9]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb3/igt@gem_exec_schedule@preempt-hang-render.html

  * igt@gem_exec_schedule@reorder-wide-render:
    - {shard-tglb}:       NOTRUN -> [FAIL][10]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb3/igt@gem_exec_schedule@reorder-wide-render.html

  * igt@prime_mmap@test_map_unmap:
    - {shard-tglb}:       NOTRUN -> [TIMEOUT][11] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb4/igt@prime_mmap@test_map_unmap.html

  
Known issues
------------

  Here are the changes found in IGTPW_3604_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@busy-vcs1:
    - shard-iclb:         [PASS][12] -> [SKIP][13] ([fdo#112080]) +10 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb2/igt@gem_busy@busy-vcs1.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb7/igt@gem_busy@busy-vcs1.html

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [PASS][14] -> [SKIP][15] ([fdo#109276] / [fdo#112080]) +2 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb2/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb3/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][16] -> [SKIP][17] ([fdo#110854])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb4/igt@gem_exec_balancer@smoke.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb6/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_capture@capture-vebox:
    - shard-kbl:          [PASS][18] -> [SKIP][19] ([fdo#109271]) +20 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl6/igt@gem_exec_capture@capture-vebox.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl1/igt@gem_exec_capture@capture-vebox.html

  * igt@gem_exec_parallel@bcs0-fds:
    - shard-glk:          [PASS][20] -> [INCOMPLETE][21] ([fdo#103359] / [k.org#198133])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-glk9/igt@gem_exec_parallel@bcs0-fds.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-glk3/igt@gem_exec_parallel@bcs0-fds.html

  * igt@gem_exec_schedule@preempt-queue-blt:
    - shard-kbl:          [PASS][22] -> [INCOMPLETE][23] ([fdo#103665])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl3/igt@gem_exec_schedule@preempt-queue-blt.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl1/igt@gem_exec_schedule@preempt-queue-blt.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [PASS][24] -> [SKIP][25] ([fdo#109276]) +20 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb5/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][26] -> [SKIP][27] ([fdo#111325]) +9 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb6/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_exec_schedule@reorder-wide-render:
    - shard-apl:          [PASS][28] -> [INCOMPLETE][29] ([fdo#103927])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl8/igt@gem_exec_schedule@reorder-wide-render.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl2/igt@gem_exec_schedule@reorder-wide-render.html

  * igt@gem_softpin@noreloc-s3:
    - shard-snb:          [PASS][30] -> [DMESG-WARN][31] ([fdo#102365])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-snb1/igt@gem_softpin@noreloc-s3.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-snb7/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-hsw:          [PASS][32] -> [DMESG-WARN][33] ([fdo#111870])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-hsw7/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-hsw4/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [PASS][34] -> [DMESG-WARN][35] ([fdo#111870]) +1 similar issue
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-snb2/igt@gem_userptr_blits@sync-unmap-cycles.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-snb1/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@gem_workarounds@reset-fd:
    - shard-glk:          [PASS][36] -> [SKIP][37] ([fdo#109271]) +12 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-glk4/igt@gem_workarounds@reset-fd.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-glk3/igt@gem_workarounds@reset-fd.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][38] -> [DMESG-WARN][39] ([fdo#108566]) +2 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl2/igt@i915_suspend@sysfs-reader.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl4/igt@i915_suspend@sysfs-reader.html

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-iclb:         [PASS][40] -> [SKIP][41] ([fdo#109278])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb2/igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb1/igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen:
    - shard-apl:          [PASS][42] -> [FAIL][43] ([fdo#103232])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl4/igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-size-change:
    - shard-kbl:          [PASS][44] -> [FAIL][45] ([fdo#103232]) +1 similar issue
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-size-change.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-size-change.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][46] -> [FAIL][47] ([fdo#103167]) +5 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][48] -> [DMESG-WARN][49] ([fdo#108566]) +7 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-iclb:         [PASS][50] -> [INCOMPLETE][51] ([fdo#107713] / [fdo#110042])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][52] -> [FAIL][53] ([fdo#103166])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][54] -> [SKIP][55] ([fdo#109441]) +2 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb3/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-iclb:         [PASS][56] -> [DMESG-WARN][57] ([fdo#111764])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb3/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb6/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  * igt@prime_vgem@basic-sync-default:
    - shard-iclb:         [PASS][58] -> [INCOMPLETE][59] ([fdo#107713])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb3/igt@prime_vgem@basic-sync-default.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb3/igt@prime_vgem@basic-sync-default.html

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic-invalid-context-vcs1:
    - shard-iclb:         [SKIP][60] ([fdo#112080]) -> [PASS][61] +8 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb5/igt@gem_ctx_exec@basic-invalid-context-vcs1.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb2/igt@gem_ctx_exec@basic-invalid-context-vcs1.html

  * igt@gem_ctx_isolation@vcs1-clean:
    - shard-iclb:         [SKIP][62] ([fdo#109276] / [fdo#112080]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb3/igt@gem_ctx_isolation@vcs1-clean.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb1/igt@gem_ctx_isolation@vcs1-clean.html

  * igt@gem_exec_create@basic:
    - {shard-tglb}:       [INCOMPLETE][64] ([fdo#111736]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb4/igt@gem_exec_create@basic.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb8/igt@gem_exec_create@basic.html

  * igt@gem_exec_schedule@deep-render:
    - shard-apl:          [INCOMPLETE][66] ([fdo#103927]) -> [PASS][67] +4 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl1/igt@gem_exec_schedule@deep-render.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl1/igt@gem_exec_schedule@deep-render.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][68] ([fdo#111325]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb3/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@out-order-bsd2:
    - shard-iclb:         [SKIP][70] ([fdo#109276]) -> [PASS][71] +10 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb7/igt@gem_exec_schedule@out-order-bsd2.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb2/igt@gem_exec_schedule@out-order-bsd2.html

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          [DMESG-WARN][72] ([fdo#108566]) -> [PASS][73] +4 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl4/igt@gem_exec_suspend@basic-s3.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl4/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_linear_blits@interruptible:
    - shard-apl:          [INCOMPLETE][74] ([fdo#103927] / [fdo#112067]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl6/igt@gem_linear_blits@interruptible.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl3/igt@gem_linear_blits@interruptible.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-hsw:          [DMESG-WARN][76] ([fdo#111870]) -> [PASS][77] +1 similar issue
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-hsw1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-hsw4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * {igt@i915_pm_dc@dc6-dpms}:
    - shard-iclb:         [FAIL][78] ([fdo#110548]) -> [PASS][79]
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb6/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rpm@system-suspend:
    - {shard-tglb}:       [INCOMPLETE][80] ([fdo#111747] / [fdo#111850]) -> [PASS][81]
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb4/igt@i915_pm_rpm@system-suspend.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb5/igt@i915_pm_rpm@system-suspend.html

  * igt@i915_selftest@live_hangcheck:
    - {shard-tglb}:       [INCOMPLETE][82] ([fdo#111747]) -> [PASS][83]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb5/igt@i915_selftest@live_hangcheck.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb4/igt@i915_selftest@live_hangcheck.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding:
    - shard-apl:          [FAIL][84] ([fdo#103232]) -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl1/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl8/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
    - shard-kbl:          [FAIL][86] ([fdo#103232]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-kbl1/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - {shard-tglb}:       [INCOMPLETE][88] ([fdo#111747] / [fdo#112031]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb4/igt@kms_flip@basic-flip-vs-wf_vblank.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb5/igt@kms_flip@basic-flip-vs-wf_vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - {shard-tglb}:       [INCOMPLETE][90] ([fdo#111884]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [FAIL][92] ([fdo#103167]) -> [PASS][93] +3 similar issues
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu:
    - shard-glk:          [FAIL][94] ([fdo#103167]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-glk9/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-glk7/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [DMESG-WARN][96] ([fdo#108566]) -> [PASS][97] +1 similar issue
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt:
    - {shard-tglb}:       [FAIL][98] ([fdo#103167]) -> [PASS][99] +4 similar issues
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - {shard-tglb}:       [INCOMPLETE][100] ([fdo#111832] / [fdo#111850]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-tglb7/igt@kms_frontbuffer_tracking@psr-suspend.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-tglb8/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][102] ([fdo#108341]) -> [PASS][103]
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb1/igt@kms_psr@no_drrs.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/shard-iclb5/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][104] ([fdo#109441]) -> [PASS][105] +1 similar issue
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7171/shard-iclb3/igt@kms_psr@psr2_sprit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3604/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool
  2019-10-24 13:06   ` Chris Wilson
@ 2019-10-28 11:21     ` Arkadiusz Hiler
  0 siblings, 0 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-28 11:21 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, Petri Latvala

On Thu, Oct 24, 2019 at 02:06:18PM +0100, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-10-24 12:05:14)
> > From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > 
> > Tool uses device selection API to scan and display GPU devices.
> > It can be used to check filter correctness as well as order
> > of applying the filters (.igtrc, IGT_DEVICE and --device argument).
> 
> A couple of examples of use and output?

I put some of that in the cover letter, but yeah, we can add some more
here. (command output indented with 2 spaces for readability)


$ ./build/tools/lsgpu --help
  usage: lsgpu [options]

  Options:
    -p, --print-details         Print devices with details
    -v, --list-vendors          List recognized vendors
    -l, --list-filter-types     List registered device filters types
    -d, --device filter         Device filter, can be given multiple times
    -h, --help                  Show this help message and exit

$ ./build/tools/lsgpu
  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/platform/vgem/drm/card1
      subsystem       : drm
      drm card        : /dev/dri/card1
      parent          : sys:/sys/devices/platform/vgem

  sys:/sys/devices/platform/vgem/drm/renderD129
      subsystem       : drm
      drm render      : /dev/dri/renderD129
      parent          : sys:/sys/devices/platform/vgem

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

  sys:/sys/devices/platform/vgem
      subsystem       : platform
      drm card        : /dev/dri/card1
      drm render      : /dev/dri/renderD129

$ ./build/tools/lsgpu -l
  Filter types:
  ---
  filter        syntax
  ---
  sys           sys:/sys/devices/pci0000:00/0000:00:02.0
                find device by its sysfs path

  drm           drm:/dev/dri/* path
                find drm device by /dev/dri/* node

  pci           pci:[vendor=%04x/name][,device=%04x][,card=%d]
                vendor is hex number or vendor name

$ ./build/tools/lsgpu -d pci:vendor=Intel
  Notice: Using --device filters
  === Device filter ===
  pci:vendor=Intel

  === Testing device open ===
  Device detail:
  subsystem   : pci
  drm card    : /dev/dri/card0
  drm render  : /dev/dri/renderD128
  Device /dev/dri/card0 successfully opened
  Device /dev/dri/renderD128 successfully opened
  -------------------------------------------

$ ./build/tools/lsgpu -d pci:vendor=intel
  Notice: Using --device filters
  === Device filter ===
  pci:vendor=intel

  === Testing device open ===
  Device detail:
  subsystem   : pci
  drm card    : /dev/dri/card0
  drm render  : /dev/dri/renderD128
  Device /dev/dri/card0 successfully opened
  Device /dev/dri/renderD128 successfully opened
  -------------------------------------------

$ ./build/tools/lsgpu -d pci:vendor=intel -p
  Notice: Using --device filters
  === Device filter ===
  pci:vendor=intel

  === Testing device open ===
  Device detail:
  subsystem   : pci
  drm card    : /dev/dri/card0
  drm render  : /dev/dri/renderD128
  Device /dev/dri/card0 successfully opened
  Device /dev/dri/renderD128 successfully opened

  ========== pci:/sys/devices/pci0000:00/0000:00:02.0 ==========
  card device                     : /dev/dri/card0
  render device                   : /dev/dri/renderD128

  [properties]
  DEVPATH                         : /devices/pci0000:00/0000:00:02.0
  DRIVER                          : i915
  FWUPD_GUID                      : 0x8086:0x5927
  ID_MODEL_FROM_DATABASE          : Iris Plus Graphics 650
  ID_PCI_CLASS_FROM_DATABASE      : Display controller
  ID_PCI_INTERFACE_FROM_DATABASE  : VGA controller
  ID_PCI_SUBCLASS_FROM_DATABASE   : VGA compatible controller
  ID_VENDOR_FROM_DATABASE         : Intel Corporation
  MODALIAS                        : pci:v00008086d00005927sv00008086sd00002068bc03sc00i00
  PCI_CLASS                       : 30000
  PCI_ID                          : 8086:5927
  PCI_SLOT_NAME                   : 0000:00:02.0
  PCI_SUBSYS_ID                   : 8086:2068
  SUBSYSTEM                       : pci
  USEC_INITIALIZED                : 22881171

  [attributes]
  ari_enabled                     : 0
  boot_vga                        : 1
  broken_parity_status            : 0
  class                           : 0x030000
  consistent_dma_mask_bits        : 39
  current_link_speed              : Unknown speed
  current_link_width              : 0
  d3cold_allowed                  : 1
  device                          : 0x5927
  dma_mask_bits                   : 39
  driver_override                 : (null)
  enable                          : 1
  firmware_node                   : LNXVIDEO:00
  index                           : 1
  irq                             : 129
  label                           :  CPU
  local_cpulist                   : 0-3
  local_cpus                      : f
  max_link_speed                  : Unknown speed
  max_link_width                  : 255
  msi_bus                         : 1
  numa_node                       : -1
  revision                        : 0x06
  subsystem                       : pci
  subsystem_device                : 0x2068
  subsystem_vendor                : 0x8086
  vendor                          : 0x8086

  -------------------------------------------

>  
> > v2 (Arek):
> >  * don't print chip as it's no longer there
> >  * make it a second patch, before any alterations to igt_core or drmtest
> >  * use only a single filter
> > 
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API
       [not found]   ` <20191024164030.GA7823@zkempczy-mobl2>
@ 2019-10-28 13:06     ` Arkadiusz Hiler
  0 siblings, 0 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-10-28 13:06 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev, Petri Latvala

On Thu, Oct 24, 2019 at 06:40:31PM +0200, Zbigniew Kempczyński wrote:
> On Thu, Oct 24, 2019 at 02:05:13PM +0300, Arkadiusz Hiler wrote:
> 
> <cut>
> 
> I've taken a quick look and I think this require change:
> 
> > +/**
> > + * igt_device_filter_set
> > + * @filter: filter that should be set globally
> > + *
> > + * Returns number of filters added to filter array. Can be greater than
> > + * 1 if @filters contains more than one filter separated by semicolon.
> > + */
> > +void igt_device_filter_set(const char *filter)
> > +{
> > +	if (!is_filter_valid(filter)) {
> > +		igt_warn("Invalid filter: %s\n", filter);
> > +		return;
> > +	}
> > +
> > +	device_filter = strdup(filter);
> > +}
> 
> Comment informs function returns number of filters but it returns void
> now. What we should also protect is to memleak if user passes different
> filters because we don't free previous device_filter string if already
> set.
> 
> Same in the header file.
> 
> > +
> > +/**
> > + * igt_device_filter_free
> > + *
> > + * Free the filter that we store internally, effectively insetting it.
> > + */
> > +void igt_device_filter_free(void)
> > +{
> > +	if (device_filter != NULL)
> > +		free(device_filter);
> > +}
> 
> I would also add nullyfiing device_filter here.
> 
> That's for brief look.
> 
> Thanks for reviewing and rewriting patches.

The following changes will be included in the next revision. Thanks!

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 71d30587..c3c99001 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -1050,9 +1050,6 @@ static bool is_filter_valid(const char *fstr)
 /**
  * igt_device_filter_set
  * @filter: filter that should be set globally
- *
- * Returns number of filters added to filter array. Can be greater than
- * 1 if @filters contains more than one filter separated by semicolon.
  */
 void igt_device_filter_set(const char *filter)
 {
@@ -1061,18 +1058,23 @@ void igt_device_filter_set(const char *filter)
 		return;
 	}
 
+	if (device_filter != NULL)
+		free(device_filter);
+
 	device_filter = strdup(filter);
 }
 
 /**
  * igt_device_filter_free
  *
- * Free the filter that we store internally, effectively insetting it.
+ * Free the filter that we store internally, effectively unsetting it.
  */
 void igt_device_filter_free(void)
 {
 	if (device_filter != NULL)
 		free(device_filter);
+
+	device_filter = NULL;
 }
 
 /**
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu
  2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
                   ` (5 preceding siblings ...)
  2019-10-25 17:07 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-11-12 10:14 ` Tvrtko Ursulin
  2019-12-02 12:37   ` Arkadiusz Hiler
  6 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-11-12 10:14 UTC (permalink / raw)
  To: Arkadiusz Hiler, igt-dev; +Cc: Leo Liu, Petri Latvala


On 24/10/2019 12:05, Arkadiusz Hiler wrote:
> Hey,
> 
> This series aims to make running IGT on hosts with multiple GPUs manageable
> without the need to unload modules / unbind devices.
> 
> Suggested reviewing order:
>   * lsgpu
>   * igt_core && drm_test changes
>   * implementation internals in igt_device_scan.c
> 
> Changes since Zbigniew's last revision:
>   * rewritten most of the parts that were using glib
>   * removed multiple filter support - this will be added back when the need
>     arises
>   * we don't second guess the "chipset" of the device and just let the underlying
>     open to fail if it has to
>   * extra looging around opening device when filter is set
>   * sysfs filter now has it's own prefix
>   * no "platform" filter - sysfs should suffice for now, it can be added by
>     someone more knowledgeable if the need arises
> 
> TODO:
>   * API for opening multiple devices in a single test (e.g. for prime) - I don't
>     want to design this upfront
> 
> Example usage:
>    $ build/tools/lsgpu
>    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/platform/vgem/drm/card1
>        subsystem       : drm
>        drm card        : /dev/dri/card1
>        parent          : sys:/sys/devices/platform/vgem
> 
>    sys:/sys/devices/platform/vgem/drm/renderD129
>        subsystem       : drm
>        drm render      : /dev/dri/renderD129
>        parent          : sys:/sys/devices/platform/vgem
> 
>    sys:/sys/devices/pci0000:00/0000:00:02.0
>        subsystem       : pci
>        drm card        : /dev/dri/card0
>        drm render      : /dev/dri/renderD128
>        vendor          : 8086
>        device          : 5927
> 
>    sys:/sys/devices/platform/vgem
>        subsystem       : platform
>        drm card        : /dev/dri/card1
>        drm render      : /dev/dri/renderD129
> 
>    $ build/tools/lsgpu -d "sys:/sys/devices/pci0000:00/0000:00:02.0"
>    Notice: Using --device filters
>    === Device filter ===
>    sys:/sys/devices/pci0000:00/0000:00:02.0
> 
>    === Testing device open ===
>    Device detail:
>    subsystem   : pci
>    drm card    : /dev/dri/card0
>    drm render  : /dev/dri/renderD128
>    Device /dev/dri/card0 successfully opened
>    Device /dev/dri/renderD128 successfully opened
>    -------------------------------------------
> 
>    # build/tests/core_auth --run-subtest getclient-simple --device "pci:vendor=intel"
>    IGT-Version: 1.24-g64068440 (x86_64) (Linux: 5.3.7-250.vanilla.knurd.1.fc30.x86_64 x86_64)
>    Starting subtest: getclient-simple
>    Looking for devices to open using filter: pci:vendor=intel
>    Filter matched /dev/dri/card0 | /dev/dri/renderD128
>    Looking for devices to open using filter: pci:vendor=intel
>    Filter matched /dev/dri/card0 | /dev/dri/renderD128 >    Subtest getclient-simple: SUCCESS (0.007s)

Looks very usable!

Can I be cheeky and ask if you could also cover the existing tools 
(especially intel_gpu_top (VLK-5588))?

Plus I would need a helper to get the selected device PCI string to use 
with i915 PMU? There I'd need to get a string like "0000:01:00.0" since 
the i915 PMU registers the device as either:


"/sys/bus/event_source/devices/i915" for integrated.

Or:

"/sys/bus/event_source/devices/i915-0000:01:00.0" for discrete.

So I am thinking something like igt_device_get_pci_string() could work 
for lib/igt_perf.c/i915_type_id().

Regards,

Tvrtko

> 
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: Leo Liu <Leo.Liu@amd.com>
> 
> Arkadiusz Hiler (1):
>    lib/igt_list: Update, clean-up and document igt_list
> 
> Zbigniew Kempczyński (3):
>    Introduce device selection API
>    Introduce device selection lsgpu tool
>    Add device selection in IGT
> 
>   benchmarks/gem_wsim.c                         |    6 +-
>   .../igt-gpu-tools/igt-gpu-tools-docs.xml      |    2 +
>   .../igt-gpu-tools/igt_test_programs.xml       |    7 +
>   lib/Makefile.sources                          |    4 +
>   lib/drmtest.c                                 |   94 +-
>   lib/drmtest.h                                 |    2 +
>   lib/igt_chamelium.c                           |    2 +-
>   lib/igt_core.c                                |   51 +-
>   lib/igt_device_scan.c                         | 1199 +++++++++++++++++
>   lib/igt_device_scan.h                         |   67 +
>   lib/igt_dummyload.c                           |    4 +-
>   lib/igt_kmod.c                                |    2 +-
>   lib/igt_list.c                                |   66 +
>   lib/igt_list.h                                |  165 ++-
>   lib/meson.build                               |    2 +
>   tests/i915/gem_exec_balancer.c                |    2 +-
>   tests/vc4_purgeable_bo.c                      |    6 +-
>   tools/Makefile.sources                        |    1 +
>   tools/lsgpu.c                                 |  295 ++++
>   tools/meson.build                             |    1 +
>   20 files changed, 1873 insertions(+), 105 deletions(-)
>   create mode 100644 lib/igt_device_scan.c
>   create mode 100644 lib/igt_device_scan.h
>   create mode 100644 lib/igt_list.c
>   create mode 100644 tools/lsgpu.c
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API Arkadiusz Hiler
       [not found]   ` <20191024164030.GA7823@zkempczy-mobl2>
@ 2019-11-15 13:31   ` Petri Latvala
  2019-11-18 11:55   ` Petri Latvala
  2 siblings, 0 replies; 22+ messages in thread
From: Petri Latvala @ 2019-11-15 13:31 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

On Thu, Oct 24, 2019 at 02:05:13PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> Change adds device selection API based on scanning drm subsystem
> using udev library.
> 
> v2 (Arek):
>  * drop most of the glib code in favor of igt_list and plain C
>  * make sysfs paths handling non-special - introduce sys: filter
>  * drop multiple filter_* structs in favor of just two:
>    - filter_class for defining filters types (e.g. sys:)
>    - filter for "filter instance" - the data provided by the user
>  * promote many macros to real functions for type safety
>  * rename devs->devs to devs->all
>  * rename devs->view to devs->filtered
>  * don't expose "chip" (e.g. DRIVER_ANY) as it's unreadable as int
>  * update docs to reflect those changes
>  * move open functions that open igt_device_card to this patch
>  * remove platform filter class for now
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  .../igt-gpu-tools/igt-gpu-tools-docs.xml      |    1 +
>  lib/Makefile.sources                          |    2 +
>  lib/igt_device_scan.c                         | 1199 +++++++++++++++++
>  lib/igt_device_scan.h                         |   67 +
>  lib/meson.build                               |    1 +
>  5 files changed, 1270 insertions(+)
>  create mode 100644 lib/igt_device_scan.c
>  create mode 100644 lib/igt_device_scan.h
> 
> diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> index 3f14d7be..c41cbb78 100644
> --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> @@ -23,6 +23,7 @@
>      <xi:include href="xml/igt_core.xml"/>
>      <xi:include href="xml/igt_debugfs.xml"/>
>      <xi:include href="xml/igt_device.xml"/>
> +    <xi:include href="xml/igt_device_scan.xml"/>
>      <xi:include href="xml/igt_draw.xml"/>
>      <xi:include href="xml/igt_dummyload.xml"/>
>      <xi:include href="xml/igt_fb.xml"/>
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 1b58b6d0..7e8594c5 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -25,6 +25,8 @@ lib_source_list =	 	\
>  	igt_debugfs.h		\
>  	igt_device.c		\
>  	igt_device.h		\
> +	igt_device_scan.c	\
> +	igt_device_scan.h	\
>  	igt_aux.c		\
>  	igt_aux.h		\
>  	igt_color_encoding.c	\
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> new file mode 100644
> index 00000000..71d30587
> --- /dev/null
> +++ b/lib/igt_device_scan.c
> @@ -0,0 +1,1199 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_list.h"
> +#include "igt_sysfs.h"
> +#include "igt_device.h"
> +#include "igt_device_scan.h"
> +#include <glib.h>
> +#include <libudev.h>
> +#include <linux/limits.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +#include <fcntl.h>
> +
> +/**
> + * SECTION:igt_device_scan
> + * @short_description: Device scanning and selection
> + * @title: Device selection
> + * @include: igt.h
> + *
> + * # Device scanning
> + *
> + * Device scanning iterates over DRM subsystem using udev library
> + * to acquire DRM devices.
> + * For each DRM device we also get and store its parent to allow device
> + * selection happen in a more contextual way.
> + *
> + * Parent devices are bus devices (like PCI, platform, etc.) and contain a lot
> + * of extra data on top of the DRM device itself.
> + *
> + * # Filters
> + *
> + * Device selection can be done using filters that are using the data collected
> + * udev + some syntactic sugar.
> + *
> + * Direct device selection filter uses sysfs path to find the device:
> + *
> + * |[<!-- language="plain" -->
> + * sys:/sys/path/to/device/or/parent
> + * ]|
> + *
> + * Examples:
> + * |[<!-- language="plain" -->
> + * - sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> + * - sys:/sys/devices/pci0000:00/0000:00:02.0
> + * - sys:/sys/devices/platform/vgem
> + * ]|
> + *
> + * The alternative is to use other implemented filters:
> + *
> + * - drm: get drm /dev/dri/... device directly
> + *
> + *   |[<!-- language="plain" -->
> + *   drm:/dev/dri/...
> + *   ]|
> + *
> + *   Loading drivers in different order can cause different ordering of
> + *   /dev/dri/card nodes which be problematic for reliable and reproducible
> + *   device selection, e.g. in automated execution setting. In such scenarios
> + *   please consider using sys, pci or platform filters instead.
> + *
> + * - pci: select device using PCI vendor and device properties
> + *   |[<!-- language="plain" -->
> + *   pci:[vendor=%04x/name][,device=%04x][,card=%d]
> + *   ]|
> + *
> + *   Filter allows device selection using vendor (hex or name), device id
> + *   (hex) and nth-card from all matches. For example if there are 4 PCI cards
> + *   installed (let two cards have 1234 and other two 1235 device id, all of
> + *   them of vendor Intel) you can select one using:
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=Intel,device=1234,card=0
> + *   ]|
> + *
> + *   or
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=8086,device=1234,card=0
> + *   ]|
> + *
> + *   This takes first device with 1234 id for Intel vendor (8086).
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=Intel,device=1234,card=1
> + *   ]|
> + *
> + *   or
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=8086,device=1234,card=1
> + *   ]|
> + *
> + *   It selects the second one.
> + *
> + *   As order the on PCI bus doesn't change (unless you'll add new device or
> + *   reorder existing one) device selection using this filter will always
> + *   return you same device regardless the order of enumeration.
> + *
> + *   Simple syntactic sugar over using the sysfs paths.
> + */
> +
> +#ifdef DEBUG_DEVICE_SCAN
> +#define DBG(...) \
> +{ \
> +	struct timeval tm; \
> +	gettimeofday(&tm, NULL); \
> +	printf("%10ld.%03ld: ", tm.tv_sec, tm.tv_usec); \
> +	printf(__VA_ARGS__); \
> +	}


Check indentation on }


> +
> +#else
> +#define DBG(...) {}
> +#endif
> +
> +static inline bool strequal(const char *a, const char *b)
> +{
> +	if (a == NULL || b == NULL)
> +		return false;
> +
> +	return strcmp(a, b) == 0;
> +}
> +
> +struct igt_device {
> +	/* Filled for drm devices */
> +	struct igt_device *parent;
> +
> +	/* Point to vendor spec if can be found */
> +
> +	/* Properties / sysattrs rewriten from udev lists */
> +	GHashTable *props_ht;
> +	GHashTable *attrs_ht;
> +
> +	/* Most usable variables from udev device */
> +	char *subsystem;
> +	char *syspath;
> +	char *devnode;
> +
> +	/* /dev/dri/... paths */
> +	char *drm_card;
> +	char *drm_render;
> +
> +	/* For pci subsystem */
> +	char *vendor;
> +	char *device;
> +
> +	struct igt_list link;
> +};
> +
> +/* Scanned devices */
> +static struct {
> +	struct igt_list all;
> +	struct igt_list filtered;
> +	bool devs_scanned;
> +} igt_devs;
> +
> +static struct {
> +	const char *name;
> +	const char *vendor_id;
> +} pci_vendor_mapping[] = {
> +	{ "intel", "8086" },
> +	{ "amd", "1002" },
> +	{ NULL, },
> +};
> +
> +static const char *get_pci_vendor_id_by_name(const char *name)
> +{
> +	for (typeof(*pci_vendor_mapping) *vm = pci_vendor_mapping; vm->name; vm++)
> +	{
> +		if (!strcasecmp(name, vm->name))
> +			return vm->vendor_id;
> +	}
> +
> +	return NULL;
> +}
> +
> +/* Reading sysattr values can take time (even seconds),
> + * we want to avoid reading such keys.
> + */
> +static bool is_on_blacklist(const char *what)
> +{
> +	static const char *keys[] = { "config", "modalias", "modes",
> +				      "resource",
> +				      "resource0", "resource1", "resource2",
> +				      "resource3", "resource4", "resource5",
> +				      "resource0_wc", "resource1_wc", "resource2_wc",
> +				      "resource3_wc", "resource4_wc", "resource5_wc",
> +				      "driver",
> +				      "uevent", NULL};
> +	const char *key;
> +	int i = 0;
> +
> +	if (what == NULL)
> +		return false;
> +
> +	while ((key = keys[i++])) {
> +		if (strcmp(key, what) == 0)
> +			return true;
> +	}
> +
> +	return false;
> +
> +}
> +
> +static struct igt_device *igt_device_new(void)
> +{
> +	struct igt_device *dev;
> +
> +	dev = calloc(1, sizeof(struct igt_device));
> +	if (!dev)
> +		return NULL;
> +
> +	dev->attrs_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
> +					      free, free);
> +	dev->props_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
> +					      free, free);
> +
> +	if (dev->attrs_ht && dev->props_ht)
> +		return dev;


If we reach here, we leak dev.


> +
> +	return NULL;
> +}
> +
> +static void igt_device_add_prop(struct igt_device *dev,
> +				const char *key, const char *value)
> +{
> +	if (!key || !value)
> +		return;
> +
> +	g_hash_table_insert(dev->props_ht, strdup(key), strdup(value));
> +}
> +
> +static void igt_device_add_attr(struct igt_device *dev,
> +				const char *key, const char *value)
> +{
> +	const char *v = value;
> +
> +	if (!key)
> +		return;
> +
> +	/* It's possible we have symlink at key filename, but udev
> +	 * library resolves only few of them
> +	 */
> +	if (!v) {
> +		struct stat st;
> +		char path[PATH_MAX];
> +		char linkto[PATH_MAX];
> +		int len;
> +
> +		snprintf(path, sizeof(path), "%s/%s", dev->syspath, key);
> +		if (lstat(path, &st) != 0)
> +			return;
> +
> +		len = readlink(path, linkto, sizeof(linkto));
> +		if (len <= 0 || len == (ssize_t) sizeof(linkto))
> +			return;
> +		linkto[len] = '\0';
> +		v = strrchr(linkto, '/');
> +		if (v == NULL)
> +			return;
> +		v++;
> +	}
> +
> +	g_hash_table_insert(dev->attrs_ht, strdup(key), strdup(v));
> +}
> +
> +/* Iterate over udev properties list and rewrite it to igt_device properties
> + * hash table for instant access.
> + */
> +static void get_props(struct udev_device *dev, struct igt_device *idev)
> +{
> +	struct udev_list_entry *entry;
> +
> +	entry = udev_device_get_properties_list_entry(dev);
> +	while (entry) {
> +		const char *name = udev_list_entry_get_name(entry);
> +		const char *value = udev_list_entry_get_value(entry);
> +
> +		igt_device_add_prop(idev, name, value);
> +		entry = udev_list_entry_get_next(entry);
> +		DBG("prop: %s, val: %s\n", name, value);
> +	}
> +}
> +
> +/* Same as get_props(), but rewrites sysattrs. Resolves symbolic links
> + * not handled by udev get_sysattr_value().
> + * Function skips sysattrs from blacklist ht (acquiring some values can take
> + * seconds).
> + */
> +static void get_attrs(struct udev_device *dev, struct igt_device *idev)
> +{
> +	struct udev_list_entry *entry;
> +
> +	entry = udev_device_get_sysattr_list_entry(dev);
> +	while (entry) {
> +		const char *key = udev_list_entry_get_name(entry);
> +		const char *value;
> +
> +		if (is_on_blacklist(key)) {
> +			entry = udev_list_entry_get_next(entry);
> +			continue;
> +		}
> +
> +		value = udev_device_get_sysattr_value(dev, key);
> +		igt_device_add_attr(idev, key, value);
> +		entry = udev_list_entry_get_next(entry);
> +		DBG("attr: %s, val: %s\n", key, value);
> +	}
> +}
> +
> +#define get_prop(dev, prop) ((char *) g_hash_table_lookup(dev->props_ht, prop))
> +#define get_attr(dev, attr) ((char *) g_hash_table_lookup(dev->attrs_ht, attr))
> +#define get_prop_subsystem(dev) get_prop(dev, "SUBSYSTEM")
> +#define is_drm_subsystem(dev)  (strequal(get_prop_subsystem(dev), "drm"))
> +#define is_pci_subsystem(dev)  (strequal(get_prop_subsystem(dev), "pci"))
> +
> +/* Gets PCI_ID property, splits to xxxx:yyyy and stores
> + * xxxx to dev->vendor and yyyy to dev->device for
> + * faster access.
> + */
> +static void set_vendor_device(struct igt_device *dev)
> +{
> +	const char *pci_id = get_prop(dev, "PCI_ID");
> +
> +	if (!pci_id || strlen(pci_id) != 9)
> +		return;
> +	dev->vendor = strndup(pci_id, 4);
> +	dev->device = strndup(pci_id + 5, 4);
> +}
> +
> +/* Allocate arrays for keeping scanned devices */
> +static bool prepare_scan(void)
> +{
> +	if (igt_devs.all.prev == NULL || igt_devs.all.next == NULL)
> +		igt_list_init(&igt_devs.all);
> +
> +	if (igt_devs.filtered.prev == NULL || igt_devs.filtered.next == NULL)
> +		igt_list_init(&igt_devs.filtered);
> +
> +
> +	return true;

Extra newline


> +}
> +
> +static char* strdup_nullsafe(const char* str)
> +{
> +	if (str == NULL)
> +		return NULL;
> +
> +	return strdup(str);
> +}
> +
> +/* Create new igt_device from udev device.
> + * Fills structure with most usable udev device variables, properties
> + * and sysattrs.
> + */
> +static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
> +{
> +	struct igt_device *idev = igt_device_new();
> +
> +	igt_assert(idev);
> +	idev->syspath = strdup_nullsafe(udev_device_get_syspath(dev));
> +	idev->subsystem = strdup_nullsafe(udev_device_get_subsystem(dev));
> +	idev->devnode = strdup_nullsafe(udev_device_get_devnode(dev));
> +
> +	if (idev->devnode && strstr(idev->devnode, "/dev/dri/card"))
> +		idev->drm_card = strdup(idev->devnode);
> +	else if (idev->devnode && strstr(idev->devnode, "/dev/dri/render"))
> +		idev->drm_render = strdup(idev->devnode);
> +
> +	get_props(dev, idev);
> +	get_attrs(dev, idev);
> +
> +	return idev;
> +}
> +
> +/* Iterate over all igt_devices array and find one matched to
> + * subsystem and syspath.
> + */
> +static struct igt_device *igt_device_find(const char *subsystem,
> +					  const char *syspath)
> +{
> +	struct igt_device *dev;
> +
> +	igt_list_for_each(dev, &igt_devs.all, link)
> +	{
> +		if (strcmp(dev->subsystem, subsystem) == 0 &&
> +			strcmp(dev->syspath, syspath) == 0)
> +			return dev;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct igt_device *igt_device_from_syspath(const char *syspath)
> +{
> +	struct igt_device *dev;
> +
> +	igt_list_for_each(dev, &igt_devs.all, link)
> +	{
> +		if (strcmp(dev->syspath, syspath) == 0)
> +			return dev;
> +	}
> +
> +	return NULL;
> +}
> +
> +/* For each drm igt_device add or update its parent igt_device to the array.
> + * As card/render drm devices mostly have same parent (vkms is an exception)
> + * link to it and update corresponding drm_card / drm_render fields.
> + */
> +static void update_or_add_parent(struct udev_device *dev,
> +				 struct igt_device *idev)
> +{
> +	struct udev_device *parent_dev;
> +	struct igt_device *parent_idev;
> +	const char *subsystem, *syspath, *devname;
> +
> +	parent_dev = udev_device_get_parent(dev);
> +	igt_assert(parent_dev);
> +
> +	subsystem = udev_device_get_subsystem(parent_dev);
> +	syspath = udev_device_get_syspath(parent_dev);
> +
> +	parent_idev = igt_device_find(subsystem, syspath);
> +	if (!parent_idev) {
> +		parent_idev = igt_device_new_from_udev(parent_dev);
> +		if (is_pci_subsystem(parent_idev)) {
> +			set_vendor_device(parent_idev);
> +		}
> +		igt_list_insert_tail(&igt_devs.all, &parent_idev->link);
> +	}
> +	devname = udev_device_get_devnode(dev);
> +	if (devname != NULL && strstr(devname, "/dev/dri/card"))
> +		parent_idev->drm_card = strdup(devname);
> +	else if (devname != NULL && strstr(devname, "/dev/dri/render"))
> +		parent_idev->drm_render = strdup(devname);
> +
> +	idev->parent = parent_idev;
> +}
> +
> +static struct igt_device *duplicate_device(struct igt_device *dev) {
> +	struct igt_device *dup = malloc(sizeof(*dup));
> +	memcpy(dup, dev, sizeof(*dev));
> +	dup->link.prev = NULL;
> +	dup->link.next = NULL;
> +	return dup;
> +}
> +
> +static gint devs_compare(const void *a, const void *b)
> +{
> +	struct igt_device *dev1, *dev2;
> +	int ret;
> +
> +	dev1 = *(struct igt_device **) a;
> +	dev2 = *(struct igt_device **) b;
> +	ret = strcmp(dev1->subsystem, dev2->subsystem);
> +	if (ret)
> +		return ret;
> +
> +	return strcmp(dev1->syspath, dev2->syspath);
> +}
> +
> +static void sort_all_devices(void)
> +{
> +	struct igt_device *dev, *tmp;
> +	int len = igt_list_length(&igt_devs.all);
> +	struct igt_device **devs = malloc(len * sizeof(*dev));
> +
> +	int i = 0;
> +	igt_list_for_each_safe(dev, tmp, &igt_devs.all, link) {
> +		devs[i] = dev;
> +		igt_assert(i++ < len);
> +		igt_list_remove(&dev->link);
> +	}
> +
> +	qsort(devs, len, sizeof(*devs), devs_compare);
> +
> +	for (i = 0; i < len; ++i) {
> +		igt_list_insert_tail(&igt_devs.all, &devs[i]->link);
> +	}
> +}
> +
> +/* Core scanning function.
> + *
> + * All scanned devices are kept inside igt_devs.all pointer array.
> + * Each added device is igt_device structure, which contrary to udev device
> + * has properties / sysattrs stored inside hash table instead of list.
> + *
> + * Function iterates over devices on 'drm' subsystem. For each drm device
> + * its parent is taken (bus device) and stored inside same array.
> + * Function sorts all found devices to keep same order of bus devices
> + * for providing predictable search.
> + */
> +static void scan_drm_devices(void)
> +{
> +	struct udev *udev;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	struct igt_device *dev;
> +	int ret;
> +
> +	udev = udev_new();
> +	igt_assert(udev);
> +
> +	enumerate = udev_enumerate_new(udev);
> +	igt_assert(enumerate);
> +
> +	DBG("Scanning drm subsystem\n");
> +	ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
> +	igt_assert(!ret);
> +
> +	udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> +	igt_assert(!ret);
> +
> +	ret = udev_enumerate_scan_devices(enumerate);
> +	igt_assert(!ret);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	if (!devices)
> +		return;
> +
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		const char *path;
> +		struct udev_device *udev_dev;
> +		struct igt_device *idev;
> +
> +		path = udev_list_entry_get_name(dev_list_entry);
> +		udev_dev = udev_device_new_from_syspath(udev, path);
> +		idev = igt_device_new_from_udev(udev_dev);
> +		update_or_add_parent(udev_dev, idev);
> +		igt_list_insert_tail(&igt_devs.all, &idev->link);
> +
> +		udev_device_unref(udev_dev);
> +	}
> +	udev_enumerate_unref(enumerate);
> +	udev_unref(udev);
> +
> +	sort_all_devices();
> +
> +	igt_list_for_each(dev, &igt_devs.all, link) {
> +		struct igt_device *dev_dup = duplicate_device(dev);
> +		igt_list_insert_tail(&igt_devs.filtered, &dev_dup->link);
> +	}
> +}
> +
> +static void igt_device_free(struct igt_device *dev)
> +{
> +	free(dev->devnode);
> +	free(dev->subsystem);
> +	free(dev->syspath);
> +	free(dev->drm_card);
> +	free(dev->drm_render);
> +	free(dev->vendor);
> +	free(dev->device);
> +	g_hash_table_destroy(dev->attrs_ht);
> +	g_hash_table_destroy(dev->props_ht);
> +}
> +
> +/**
> + * igt_devices_scan
> + * @force: enforce scanning devices
> + *
> + * Function scans udev in search of gpu devices. For first run it can be
> + * called with @force = false. If something changes during the the test
> + * or test does some module loading (new drm devices occurs during execution)
> + * function must be called again with @force = true to refresh device array.
> + */
> +void igt_devices_scan(bool force)
> +{
> +	if (force && igt_devs.devs_scanned) {
> +		struct igt_device *dev, *tmp;
> +		igt_list_for_each_safe(dev, tmp, &igt_devs.all, link) {
> +			igt_device_free(dev);
> +			free(dev);
> +		}
> +
> +		igt_devs.devs_scanned = false;
> +	}
> +
> +	if (igt_devs.devs_scanned)
> +		return;
> +
> +	prepare_scan();
> +	scan_drm_devices();
> +
> +	igt_devs.devs_scanned = true;
> +}
> +
> +static inline void _pr_simple(const char *k, const char *v)
> +{
> +	printf("    %-16s: %s\n", k, v);
> +}
> +
> +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 *view)
> +{
> +	struct igt_device *dev;
> +
> +	if (!view)
> +		return;
> +
> +	if (igt_list_empty(view)) {
> +		printf("No GPU devices found\n");
> +		return;
> +	}
> +
> +	igt_list_for_each(dev, view, link) {
> +		printf("sys:%s\n", dev->syspath);
> +		if (dev->subsystem)
> +			_pr_simple("subsystem", dev->subsystem);
> +		if (dev->drm_card)
> +			_pr_simple("drm card", dev->drm_card);
> +		if (dev->drm_render)
> +			_pr_simple("drm render", dev->drm_render);
> +		if (is_drm_subsystem(dev)) {
> +			_pr_simple2("parent", "sys",
> +				   dev->parent->syspath);
> +		} else {
> +			if (is_pci_subsystem(dev)) {
> +				_pr_simple("vendor", dev->vendor);
> +				_pr_simple("device", dev->device);
> +			}
> +		}
> +		printf("\n");
> +	}
> +}
> +
> +static inline void _print_key_value(const char* k, const char *v)
> +{
> +	printf("%-32s: %s\n", k, v);
> +}
> +
> +static void print_ht(GHashTable *ht)
> +{
> +	GList *keys = g_hash_table_get_keys(ht);
> +
> +	keys = g_list_sort(keys, (GCompareFunc) strcmp);
> +	while (keys) {
> +		char *k = (char *) keys->data;
> +		char *v = g_hash_table_lookup(ht, k);
> +
> +		_print_key_value(k, v);
> +		keys = g_list_next(keys);
> +	}
> +	g_list_free(keys);
> +}
> +
> +static void igt_devs_print_detail(struct igt_list *view)
> +{
> +	struct igt_device *dev;
> +
> +	if (!view)
> +		return;
> +
> +	if (igt_list_empty(view)) {
> +		printf("No GPU devices found\n");
> +		return;
> +	}
> +
> +	igt_list_for_each(dev, view, link) {
> +		printf("========== %s:%s ==========\n",
> +		       dev->subsystem, dev->syspath);
> +		if (!is_drm_subsystem(dev)) {
> +			_print_key_value("card device", dev->drm_card);
> +			_print_key_value("render device", dev->drm_render);
> +		}
> +
> +		printf("\n[properties]\n");
> +		print_ht(dev->props_ht);
> +		printf("\n[attributes]\n");
> +		print_ht(dev->attrs_ht);
> +		printf("\n");
> +	}
> +}
> +
> +static struct print_func {
> +	void (*prn)(struct igt_list *view);
> +} print_functions[] = {
> +	[IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> +	[IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> +};
> +
> +/**
> + * igt_devices_print
> + * @printtype: IGT_PRINT_SIMPLE or IGT_PRINT_DETAIL
> + *
> + * Function can be used by external tool to print device array in simple
> + * or detailed form. This function is added here to avoid exposing
> + * internal implementation data structures.
> + */
> +void igt_devices_print(enum igt_devices_print_type printtype)
> +{
> +	print_functions[printtype].prn(&igt_devs.filtered);
> +}
> +
> +/**
> + * igt_devices_print_vendors
> + *
> + * Print pci id -> vendor mappings. Vendor names printed by this function
> + * can be used for filters like pci which allows passing vendor - like
> + * vendor id (8086) or as a string (Intel).
> + */
> +void igt_devices_print_vendors(void)
> +{
> +	printf("Recognized vendors:\n");
> +	printf("%-8s %-16s\n", "PCI ID", "vendor");
> +	for (typeof(*pci_vendor_mapping) *vm = pci_vendor_mapping; vm->name; vm++) {
> +		printf("%-8s %-16s\n", vm->vendor_id, vm->name);
> +	}
> +}
> +
> +struct filter;
> +
> +/* ------------------------------------------------------------------------- */
> +struct filter_class {
> +	struct igt_list *(*filter_function)(const struct filter_class *fcls,
> +					    const struct filter *filter);
> +	bool (*is_valid)(const struct filter_class *fcls,
> +			 const struct filter *filter);
> +	const char *name;
> +	const char *help;
> +	const char *detail;
> +};
> +
> +#define FILTER_NAME_LEN 32
> +#define FILTER_DATA_LEN 256
> +
> +struct filter {
> +	struct filter_class *class;
> +
> +	char raw_data[FILTER_DATA_LEN];
> +
> +	struct {
> +		char *vendor;
> +		char *device;
> +		char *card;
> +		char *drm;
> +		char *driver;
> +	} data;
> +};
> +
> +static void fill_filter_data(struct filter *filter, const char *key, const char *value)
> +{
> +	if (key == NULL || value == NULL) {
> +		return;
> +	}
> +
> +#define __fill_key(name) if (strcmp(key, #name) == 0) \
> +	filter->data.name = strdup(value)
> +	__fill_key(vendor);
> +	__fill_key(device);
> +	__fill_key(card);
> +	__fill_key(drm);
> +	__fill_key(driver);
> +#undef __fill_key
> +
> +}
> +
> +static void split_filter_data(struct filter *filter)
> +{
> +	char *property, *key, *data, *dup;
> +
> +	dup = strdup(filter->raw_data);
> +	data = dup;
> +
> +	while ((property = strsep(&data, ","))) {
> +		key = strsep(&property, "=");
> +		fill_filter_data(filter, key, property);
> +	}
> +
> +	free(dup);
> +}
> +
> +static struct filter_class *get_filter_class(const char *class_name, const struct filter *filter);
> +
> +static bool parse_filter(const char *fstr, struct filter *filter)
> +{
> +	char class_name[32];
> +	if (!fstr || !filter)
> +		return false;
> +
> +	memset(filter, 0, sizeof(*filter));
> +
> +

Extra newline


> +	if (sscanf(fstr, "%31[^:]:%255s", class_name, filter->raw_data) >= 1) {
> +		filter->class = get_filter_class(class_name, filter);
> +		split_filter_data(filter);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool is_vendor_matched(struct igt_device *dev, const char *vendor)
> +{
> +	const char *vendor_id;
> +
> +	if (!dev->vendor || !vendor)
> +		return false;
> +
> +	/* First we compare vendor id, like 8086 */
> +	if (!strcasecmp(dev->vendor, vendor))
> +		return true;
> +
> +	/* Likely we have vendor string instead of id */
> +	vendor_id = get_pci_vendor_id_by_name(vendor);
> +	if (!vendor_id)
> +		return false;
> +
> +	return !strcasecmp(dev->vendor, vendor_id);
> +}
> +
> +/* Filter which matches subsystem:/sys/... path.
> + * Used as first filter in chain.
> + */
> +static struct igt_list *filter_sys(const struct filter_class *fcls,
> +				   const struct filter *filter)
> +{
> +	struct igt_device *dev;
> +	(void) fcls;
> +
> +	DBG("filter sys\n");
> +	if (!strlen(filter->raw_data))
> +		return &igt_devs.filtered;
> +
> +	dev = igt_device_from_syspath(filter->raw_data);
> +	if (dev) {
> +		struct igt_device *dup = duplicate_device(dev);
> +		igt_list_insert_tail(&igt_devs.filtered, &dup->link);
> +	}
> +
> +	return &igt_devs.filtered;
> +}
> +
> +/* Find drm device using direct path to /dev/dri/.
> + * It extends filter_sys to allow using drm:/dev/dri/cardX and
> + * drm:/dev/dri/renderDX filter syntax.
> + */
> +static struct igt_list *filter_drm(const struct filter_class *fcls,
> +				   const struct filter *filter)
> +{
> +	struct igt_device *dev;
> +	(void) fcls;
> +
> +	DBG("filter drm\n");
> +	if (!strlen(filter->raw_data))
> +		return &igt_devs.filtered;
> +
> +	igt_list_for_each(dev, &igt_devs.all, link) {
> +		if (!is_drm_subsystem(dev))
> +			continue;
> +
> +		if (strequal(dev->syspath, filter->raw_data) ||
> +			strequal(dev->drm_card, filter->raw_data) ||
> +			strequal(dev->drm_render, filter->raw_data)) {
> +			struct igt_device *dup = duplicate_device(dev);
> +			igt_list_insert_tail(&igt_devs.filtered, &dup->link);
> +			break;
> +		}
> +	}
> +
> +
> +	return &igt_devs.filtered;
> +}
> +
> +/* Find appropriate pci device matching vendor/device/card filter arguments.
> + */
> +static struct igt_list *filter_pci(const struct filter_class *fcls,
> +				   const struct filter *filter)
> +{
> +	struct igt_device *dev;
> +	int card = -1;
> +	(void) fcls;
> +
> +	DBG("filter pci\n");
> +
> +	if (filter->data.card) {
> +		sscanf(filter->data.card, "%d", &card);
> +		if (card < 0) {
> +			return &igt_devs.filtered;
> +		}
> +	} else {
> +		card = 0;
> +	}
> +
> +	igt_list_for_each(dev, &igt_devs.all, link) {
> +		if (!is_pci_subsystem(dev))
> +			continue;
> +
> +		/* Skip if 'vendor' doesn't match (hex or name) */
> +		if (filter->data.vendor && !is_vendor_matched(dev, filter->data.vendor))
> +			continue;
> +
> +		/* Skip if 'device' doesn't match */
> +		if (filter->data.device && strcasecmp(filter->data.device, dev->device))
> +			continue;
> +
> +		/* We get n-th card */
> +		if (!card) {
> +			struct igt_device *dup = duplicate_device(dev);
> +			igt_list_insert_tail(&igt_devs.filtered, &dup->link);
> +			break;
> +		}
> +		card--;
> +	}
> +
> +	DBG("Filter pci filtered size: %d\n", igt_list_length(&igt_devs.filtered));
> +
> +	return &igt_devs.filtered;
> +}
> +
> +static bool sys_path_valid(const struct filter_class *fcls,
> +			   const struct filter *filter)
> +{
> +	struct stat st;
> +
> +	if (stat(filter->raw_data, &st)) {
> +		igt_warn("sys_path_valid: syspath [%s], err: %s\n",
> +			 filter->raw_data, strerror(errno));
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +
> +static struct filter_class filter_definition_list[] = {
> +	{
> +		.name = "sys",
> +		.is_valid = sys_path_valid,
> +		.filter_function = filter_sys,
> +		.help = "sys:/sys/devices/pci0000:00/0000:00:02.0",
> +		.detail = "find device byt its sysfs path\n",
> +	},
> +	{
> +		.name = "drm",
> +		.filter_function = filter_drm,
> +		.help = "drm:/dev/dri/* path",
> +		.detail = "find drm device by /dev/dri/* node\n",
> +	},
> +	{
> +		.name = "pci",
> +		.filter_function = filter_pci,
> +		.help = "pci:[vendor=%04x/name][,device=%04x][,card=%d]",
> +		.detail = "vendor is hex number or vendor name\n",
> +	},
> +	{
> +		.name = NULL,
> +	},
> +};
> +
> +static struct filter_class *get_filter_class(const char *class_name, const struct filter *filter)
> +{
> +	struct filter_class *fcls = NULL;
> +	int i = 0;
> +
> +	while ((fcls = &filter_definition_list[i++])->name != NULL) {
> +		if (strcmp(class_name, fcls->name) == 0)
> +			return fcls;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * @igt_device_print_filter_types
> + *
> + * Print all filters syntax for device selection.
> + */
> +void igt_device_print_filter_types(void)
> +{
> +	const struct filter_class *filter = NULL;
> +	int i = 0;
> +
> +	printf("Filter types:\n---\n");
> +	printf("%-12s  %s\n---\n", "filter", "syntax");
> +
> +	while ((filter = &filter_definition_list[i++])->name != NULL) {
> +		printf("%-12s  %s\n", filter->name, filter->help);
> +		printf("%-12s  %s\n", "", filter->detail);
> +	}
> +}
> +
> +static char *device_filter;
> +
> +/**
> + * igt_device_is_filter_set
> + *
> + * Returns whether we have a filter set.
> + */
> +bool igt_device_is_filter_set(void)
> +{
> +	return device_filter != NULL;
> +}
> +
> +/* Check does filter is valid. It checks:
> + * 1. /sys/... path first
> + * 2. filter name from filter definition
> + */
> +static bool is_filter_valid(const char *fstr)
> +{
> +	struct filter filter;
> +	int ret;
> +
> +	ret = parse_filter(fstr, &filter);
> +	if (!ret)
> +		return false;
> +
> +	if (filter.class == NULL) {
> +		igt_warn("No filter matching [%s:%s]\n", filter.class->name, filter.raw_data);
> +		return false;
> +	}
> +
> +	if (filter.class->is_valid != NULL && !filter.class->is_valid(filter.class, &filter))
> +	{
> +		igt_warn("Filter not valid [%s:%s]\n", filter.class->name, filter.raw_data);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +/**
> + * igt_device_filter_set
> + * @filter: filter that should be set globally
> + *
> + * Returns number of filters added to filter array. Can be greater than
> + * 1 if @filters contains more than one filter separated by semicolon.
> + */
> +void igt_device_filter_set(const char *filter)
> +{
> +	if (!is_filter_valid(filter)) {
> +		igt_warn("Invalid filter: %s\n", filter);
> +		return;
> +	}
> +
> +	device_filter = strdup(filter);
> +}
> +
> +/**
> + * igt_device_filter_free
> + *
> + * Free the filter that we store internally, effectively insetting it.
> + */
> +void igt_device_filter_free(void)
> +{
> +	if (device_filter != NULL)
> +		free(device_filter);
> +}

These two functions are the ones that were already changed to not
leak.


> +
> +/**
> + * igt_device_filter_get
> + *
> + * Returns filter string or NULL if not set
> + */
> +const char *igt_device_filter_get(void)
> +{
> +	return device_filter;
> +}
> +
> +static bool igt_device_filter_apply(const char *fstr)
> +{
> +	struct igt_device *dev, *tmp;
> +	struct filter filter;
> +	bool ret;
> +
> +	if (!fstr)
> +		return false;
> +
> +	ret = parse_filter(fstr, &filter);
> +	if (!ret) {
> +		igt_warn("Can't split filter [%s]\n", fstr);
> +		return false;
> +	}
> +
> +	/* Clean the filtered list */
> +	igt_list_for_each_safe(dev, tmp, &igt_devs.filtered, link) {
> +		igt_list_remove(&dev->link);
> +		free(dev);
> +	}
> +
> +	/* If filter.data contains "/sys" use direct path instead
> +	 * contextual filter.
> +	 */
> +
> +	if (!filter.class) {
> +		igt_warn("No filter matching [%s:%s]\n", filter.class->name, filter.raw_data);
> +		return false;
> +	}
> +	filter.class->filter_function(filter.class, &filter);
> +
> +	return true;
> +}
> +
> +/**
> + * igt_device_card_match
> + * @filter: filter string
> + * @card: pointer to igt_device_card struct
> + *
> + * Function applies filter to match device from device array.
> + *
> + * Returns:
> + * false - no card pointer was passed or card wasn't matched,
> + * true - card matched and returned.
> + */
> +bool igt_device_card_match(const char *filter, struct igt_device_card *card)
> +{
> +	struct igt_device *dev = NULL;
> +
> +	if (!card)
> +		return false;
> +	memset(card, 0, sizeof(*card));
> +
> +	igt_devices_scan(false);
> +
> +	if (igt_device_filter_apply(filter) == false)
> +		return false;
> +
> +	if (igt_list_empty(&igt_devs.filtered))
> +		return false;
> +
> +	/* We take first one if more than one card matches filter */
> +	dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
> +
> +	if (dev->subsystem != NULL)
> +		strncpy(card->subsystem, dev->subsystem,
> +			     sizeof(card->subsystem) - 1);
> +
> +	if (dev->drm_card != NULL)
> +		strncpy(card->card, dev->drm_card,
> +			     sizeof(card->card) - 1);
> +
> +	if (dev->drm_render != NULL)
> +		strncpy(card->render, dev->drm_render,
> +			     sizeof(card->render) - 1);
> +
> +	return true;
> +}
> +
> +/**
> + * igt_open_card:
> + * @card: pointer to igt_device_card structure
> + *
> + * Open /dev/dri/cardX device represented by igt_device_card structure.
> + * Requires filled @card argument (see igt_device_card_match() function).
> + *
> + * An open DRM fd or -1 on error
> + */
> +int igt_open_card(struct igt_device_card *card)
> +{
> +	if (!card || !strlen(card->card))
> +		return -1;
> +
> +	return open(card->card, O_RDWR);
> +}
> +
> +/**
> + * igt_open_render:
> + * @card: pointer to igt_device_card structure
> + *
> + * Open /dev/dri/renderDX device represented by igt_device_card structure.
> + * Requires filled @card argument (see igt_device_card_match() function).
> + *
> + * An open DRM fd or -1 on error
> + */
> +int igt_open_render(struct igt_device_card *card)
> +{
> +	if (!card || !strlen(card->render))
> +		return -1;
> +
> +	return open(card->render, O_RDWR);
> +}
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> new file mode 100644
> index 00000000..dbde8816
> --- /dev/null
> +++ b/lib/igt_device_scan.h
> @@ -0,0 +1,67 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __IGT_DEVICE_SCAN_H__
> +#define __IGT_DEVICE_SCAN_H__
> +
> +#include <limits.h>
> +#include <igt.h>
> +
> +enum igt_devices_print_type {
> +	IGT_PRINT_SIMPLE,
> +	IGT_PRINT_DETAIL,
> +};
> +
> +struct igt_device_card {
> +	char subsystem[NAME_MAX];
> +	char card[NAME_MAX];
> +	char render[NAME_MAX];
> +};
> +
> +/* Scan udev subsystems. Do it once unless force is true, what rescans
> + * devices again.
> + */


ENOPARSE. Remove this comment entirely, the function definition has a
good comment with gtk-doc stuffs even.



As general feedback, multiline comments

/*
 * should be
 * like this
 */

/* instead of
 * like this
 */



This also needs rebasing to the new igt_list stuff. I tried to do it
myself but that just made things crash left and right.


Otherwise LGTM. Tentatively (assuming crashes caused by me, not this
series),
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API Arkadiusz Hiler
       [not found]   ` <20191024164030.GA7823@zkempczy-mobl2>
  2019-11-15 13:31   ` Petri Latvala
@ 2019-11-18 11:55   ` Petri Latvala
  2 siblings, 0 replies; 22+ messages in thread
From: Petri Latvala @ 2019-11-18 11:55 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

On Thu, Oct 24, 2019 at 02:05:13PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> Change adds device selection API based on scanning drm subsystem
> using udev library.
> 
> v2 (Arek):
>  * drop most of the glib code in favor of igt_list and plain C
>  * make sysfs paths handling non-special - introduce sys: filter
>  * drop multiple filter_* structs in favor of just two:
>    - filter_class for defining filters types (e.g. sys:)
>    - filter for "filter instance" - the data provided by the user
>  * promote many macros to real functions for type safety
>  * rename devs->devs to devs->all
>  * rename devs->view to devs->filtered
>  * don't expose "chip" (e.g. DRIVER_ANY) as it's unreadable as int
>  * update docs to reflect those changes
>  * move open functions that open igt_device_card to this patch
>  * remove platform filter class for now
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  .../igt-gpu-tools/igt-gpu-tools-docs.xml      |    1 +
>  lib/Makefile.sources                          |    2 +
>  lib/igt_device_scan.c                         | 1199 +++++++++++++++++
>  lib/igt_device_scan.h                         |   67 +
>  lib/meson.build                               |    1 +
>  5 files changed, 1270 insertions(+)
>  create mode 100644 lib/igt_device_scan.c
>  create mode 100644 lib/igt_device_scan.h
> 
> diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> index 3f14d7be..c41cbb78 100644
> --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> @@ -23,6 +23,7 @@
>      <xi:include href="xml/igt_core.xml"/>
>      <xi:include href="xml/igt_debugfs.xml"/>
>      <xi:include href="xml/igt_device.xml"/>
> +    <xi:include href="xml/igt_device_scan.xml"/>
>      <xi:include href="xml/igt_draw.xml"/>
>      <xi:include href="xml/igt_dummyload.xml"/>
>      <xi:include href="xml/igt_fb.xml"/>
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 1b58b6d0..7e8594c5 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -25,6 +25,8 @@ lib_source_list =	 	\
>  	igt_debugfs.h		\
>  	igt_device.c		\
>  	igt_device.h		\
> +	igt_device_scan.c	\
> +	igt_device_scan.h	\
>  	igt_aux.c		\
>  	igt_aux.h		\
>  	igt_color_encoding.c	\
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> new file mode 100644
> index 00000000..71d30587
> --- /dev/null
> +++ b/lib/igt_device_scan.c
> @@ -0,0 +1,1199 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_list.h"
> +#include "igt_sysfs.h"
> +#include "igt_device.h"
> +#include "igt_device_scan.h"
> +#include <glib.h>
> +#include <libudev.h>
> +#include <linux/limits.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +#include <fcntl.h>
> +
> +/**
> + * SECTION:igt_device_scan
> + * @short_description: Device scanning and selection
> + * @title: Device selection
> + * @include: igt.h
> + *
> + * # Device scanning
> + *
> + * Device scanning iterates over DRM subsystem using udev library
> + * to acquire DRM devices.
> + * For each DRM device we also get and store its parent to allow device
> + * selection happen in a more contextual way.
> + *
> + * Parent devices are bus devices (like PCI, platform, etc.) and contain a lot
> + * of extra data on top of the DRM device itself.
> + *
> + * # Filters
> + *
> + * Device selection can be done using filters that are using the data collected
> + * udev + some syntactic sugar.
> + *
> + * Direct device selection filter uses sysfs path to find the device:
> + *
> + * |[<!-- language="plain" -->
> + * sys:/sys/path/to/device/or/parent
> + * ]|
> + *
> + * Examples:
> + * |[<!-- language="plain" -->
> + * - sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> + * - sys:/sys/devices/pci0000:00/0000:00:02.0
> + * - sys:/sys/devices/platform/vgem
> + * ]|
> + *
> + * The alternative is to use other implemented filters:
> + *
> + * - drm: get drm /dev/dri/... device directly
> + *
> + *   |[<!-- language="plain" -->
> + *   drm:/dev/dri/...
> + *   ]|
> + *
> + *   Loading drivers in different order can cause different ordering of
> + *   /dev/dri/card nodes which be problematic for reliable and reproducible
> + *   device selection, e.g. in automated execution setting. In such scenarios
> + *   please consider using sys, pci or platform filters instead.
> + *
> + * - pci: select device using PCI vendor and device properties
> + *   |[<!-- language="plain" -->
> + *   pci:[vendor=%04x/name][,device=%04x][,card=%d]
> + *   ]|
> + *
> + *   Filter allows device selection using vendor (hex or name), device id
> + *   (hex) and nth-card from all matches. For example if there are 4 PCI cards
> + *   installed (let two cards have 1234 and other two 1235 device id, all of
> + *   them of vendor Intel) you can select one using:
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=Intel,device=1234,card=0
> + *   ]|
> + *
> + *   or
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=8086,device=1234,card=0
> + *   ]|
> + *
> + *   This takes first device with 1234 id for Intel vendor (8086).
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=Intel,device=1234,card=1
> + *   ]|
> + *
> + *   or
> + *
> + *   |[<!-- language="plain" -->
> + *   pci:vendor=8086,device=1234,card=1
> + *   ]|
> + *
> + *   It selects the second one.
> + *
> + *   As order the on PCI bus doesn't change (unless you'll add new device or
> + *   reorder existing one) device selection using this filter will always
> + *   return you same device regardless the order of enumeration.
> + *
> + *   Simple syntactic sugar over using the sysfs paths.
> + */
> +
> +#ifdef DEBUG_DEVICE_SCAN
> +#define DBG(...) \
> +{ \
> +	struct timeval tm; \
> +	gettimeofday(&tm, NULL); \
> +	printf("%10ld.%03ld: ", tm.tv_sec, tm.tv_usec); \
> +	printf(__VA_ARGS__); \
> +	}
> +
> +#else
> +#define DBG(...) {}
> +#endif
> +
> +static inline bool strequal(const char *a, const char *b)
> +{
> +	if (a == NULL || b == NULL)
> +		return false;
> +
> +	return strcmp(a, b) == 0;
> +}
> +
> +struct igt_device {
> +	/* Filled for drm devices */
> +	struct igt_device *parent;
> +
> +	/* Point to vendor spec if can be found */
> +
> +	/* Properties / sysattrs rewriten from udev lists */
> +	GHashTable *props_ht;
> +	GHashTable *attrs_ht;
> +
> +	/* Most usable variables from udev device */
> +	char *subsystem;
> +	char *syspath;
> +	char *devnode;
> +
> +	/* /dev/dri/... paths */
> +	char *drm_card;
> +	char *drm_render;
> +
> +	/* For pci subsystem */
> +	char *vendor;
> +	char *device;
> +
> +	struct igt_list link;
> +};
> +
> +/* Scanned devices */
> +static struct {
> +	struct igt_list all;
> +	struct igt_list filtered;
> +	bool devs_scanned;
> +} igt_devs;
> +
> +static struct {
> +	const char *name;
> +	const char *vendor_id;
> +} pci_vendor_mapping[] = {
> +	{ "intel", "8086" },
> +	{ "amd", "1002" },
> +	{ NULL, },
> +};
> +
> +static const char *get_pci_vendor_id_by_name(const char *name)
> +{
> +	for (typeof(*pci_vendor_mapping) *vm = pci_vendor_mapping; vm->name; vm++)
> +	{
> +		if (!strcasecmp(name, vm->name))
> +			return vm->vendor_id;
> +	}
> +
> +	return NULL;
> +}
> +
> +/* Reading sysattr values can take time (even seconds),
> + * we want to avoid reading such keys.
> + */
> +static bool is_on_blacklist(const char *what)
> +{
> +	static const char *keys[] = { "config", "modalias", "modes",
> +				      "resource",
> +				      "resource0", "resource1", "resource2",
> +				      "resource3", "resource4", "resource5",
> +				      "resource0_wc", "resource1_wc", "resource2_wc",
> +				      "resource3_wc", "resource4_wc", "resource5_wc",
> +				      "driver",
> +				      "uevent", NULL};
> +	const char *key;
> +	int i = 0;
> +
> +	if (what == NULL)
> +		return false;
> +
> +	while ((key = keys[i++])) {
> +		if (strcmp(key, what) == 0)
> +			return true;
> +	}
> +
> +	return false;
> +
> +}
> +
> +static struct igt_device *igt_device_new(void)
> +{
> +	struct igt_device *dev;
> +
> +	dev = calloc(1, sizeof(struct igt_device));
> +	if (!dev)
> +		return NULL;
> +
> +	dev->attrs_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
> +					      free, free);
> +	dev->props_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
> +					      free, free);
> +
> +	if (dev->attrs_ht && dev->props_ht)
> +		return dev;
> +
> +	return NULL;
> +}
> +
> +static void igt_device_add_prop(struct igt_device *dev,
> +				const char *key, const char *value)
> +{
> +	if (!key || !value)
> +		return;
> +
> +	g_hash_table_insert(dev->props_ht, strdup(key), strdup(value));
> +}
> +
> +static void igt_device_add_attr(struct igt_device *dev,
> +				const char *key, const char *value)
> +{
> +	const char *v = value;
> +
> +	if (!key)
> +		return;
> +
> +	/* It's possible we have symlink at key filename, but udev
> +	 * library resolves only few of them
> +	 */
> +	if (!v) {
> +		struct stat st;
> +		char path[PATH_MAX];
> +		char linkto[PATH_MAX];
> +		int len;
> +
> +		snprintf(path, sizeof(path), "%s/%s", dev->syspath, key);
> +		if (lstat(path, &st) != 0)
> +			return;
> +
> +		len = readlink(path, linkto, sizeof(linkto));
> +		if (len <= 0 || len == (ssize_t) sizeof(linkto))
> +			return;
> +		linkto[len] = '\0';
> +		v = strrchr(linkto, '/');
> +		if (v == NULL)
> +			return;
> +		v++;
> +	}
> +
> +	g_hash_table_insert(dev->attrs_ht, strdup(key), strdup(v));
> +}
> +
> +/* Iterate over udev properties list and rewrite it to igt_device properties
> + * hash table for instant access.
> + */
> +static void get_props(struct udev_device *dev, struct igt_device *idev)
> +{
> +	struct udev_list_entry *entry;
> +
> +	entry = udev_device_get_properties_list_entry(dev);
> +	while (entry) {
> +		const char *name = udev_list_entry_get_name(entry);
> +		const char *value = udev_list_entry_get_value(entry);
> +
> +		igt_device_add_prop(idev, name, value);
> +		entry = udev_list_entry_get_next(entry);
> +		DBG("prop: %s, val: %s\n", name, value);
> +	}
> +}
> +
> +/* Same as get_props(), but rewrites sysattrs. Resolves symbolic links
> + * not handled by udev get_sysattr_value().
> + * Function skips sysattrs from blacklist ht (acquiring some values can take
> + * seconds).
> + */
> +static void get_attrs(struct udev_device *dev, struct igt_device *idev)
> +{
> +	struct udev_list_entry *entry;
> +
> +	entry = udev_device_get_sysattr_list_entry(dev);
> +	while (entry) {
> +		const char *key = udev_list_entry_get_name(entry);
> +		const char *value;
> +
> +		if (is_on_blacklist(key)) {
> +			entry = udev_list_entry_get_next(entry);
> +			continue;
> +		}
> +
> +		value = udev_device_get_sysattr_value(dev, key);
> +		igt_device_add_attr(idev, key, value);
> +		entry = udev_list_entry_get_next(entry);
> +		DBG("attr: %s, val: %s\n", key, value);
> +	}
> +}
> +
> +#define get_prop(dev, prop) ((char *) g_hash_table_lookup(dev->props_ht, prop))
> +#define get_attr(dev, attr) ((char *) g_hash_table_lookup(dev->attrs_ht, attr))
> +#define get_prop_subsystem(dev) get_prop(dev, "SUBSYSTEM")
> +#define is_drm_subsystem(dev)  (strequal(get_prop_subsystem(dev), "drm"))
> +#define is_pci_subsystem(dev)  (strequal(get_prop_subsystem(dev), "pci"))
> +
> +/* Gets PCI_ID property, splits to xxxx:yyyy and stores
> + * xxxx to dev->vendor and yyyy to dev->device for
> + * faster access.
> + */
> +static void set_vendor_device(struct igt_device *dev)
> +{
> +	const char *pci_id = get_prop(dev, "PCI_ID");
> +
> +	if (!pci_id || strlen(pci_id) != 9)
> +		return;
> +	dev->vendor = strndup(pci_id, 4);
> +	dev->device = strndup(pci_id + 5, 4);
> +}
> +
> +/* Allocate arrays for keeping scanned devices */
> +static bool prepare_scan(void)
> +{
> +	if (igt_devs.all.prev == NULL || igt_devs.all.next == NULL)
> +		igt_list_init(&igt_devs.all);
> +
> +	if (igt_devs.filtered.prev == NULL || igt_devs.filtered.next == NULL)
> +		igt_list_init(&igt_devs.filtered);
> +
> +
> +	return true;
> +}
> +
> +static char* strdup_nullsafe(const char* str)
> +{
> +	if (str == NULL)
> +		return NULL;
> +
> +	return strdup(str);
> +}
> +
> +/* Create new igt_device from udev device.
> + * Fills structure with most usable udev device variables, properties
> + * and sysattrs.
> + */
> +static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
> +{
> +	struct igt_device *idev = igt_device_new();
> +
> +	igt_assert(idev);
> +	idev->syspath = strdup_nullsafe(udev_device_get_syspath(dev));
> +	idev->subsystem = strdup_nullsafe(udev_device_get_subsystem(dev));
> +	idev->devnode = strdup_nullsafe(udev_device_get_devnode(dev));
> +
> +	if (idev->devnode && strstr(idev->devnode, "/dev/dri/card"))
> +		idev->drm_card = strdup(idev->devnode);
> +	else if (idev->devnode && strstr(idev->devnode, "/dev/dri/render"))
> +		idev->drm_render = strdup(idev->devnode);
> +
> +	get_props(dev, idev);
> +	get_attrs(dev, idev);
> +
> +	return idev;
> +}
> +
> +/* Iterate over all igt_devices array and find one matched to
> + * subsystem and syspath.
> + */
> +static struct igt_device *igt_device_find(const char *subsystem,
> +					  const char *syspath)
> +{
> +	struct igt_device *dev;
> +
> +	igt_list_for_each(dev, &igt_devs.all, link)
> +	{
> +		if (strcmp(dev->subsystem, subsystem) == 0 &&
> +			strcmp(dev->syspath, syspath) == 0)
> +			return dev;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct igt_device *igt_device_from_syspath(const char *syspath)
> +{
> +	struct igt_device *dev;
> +
> +	igt_list_for_each(dev, &igt_devs.all, link)
> +	{
> +		if (strcmp(dev->syspath, syspath) == 0)
> +			return dev;
> +	}
> +
> +	return NULL;
> +}
> +
> +/* For each drm igt_device add or update its parent igt_device to the array.
> + * As card/render drm devices mostly have same parent (vkms is an exception)
> + * link to it and update corresponding drm_card / drm_render fields.
> + */
> +static void update_or_add_parent(struct udev_device *dev,
> +				 struct igt_device *idev)
> +{
> +	struct udev_device *parent_dev;
> +	struct igt_device *parent_idev;
> +	const char *subsystem, *syspath, *devname;
> +
> +	parent_dev = udev_device_get_parent(dev);
> +	igt_assert(parent_dev);
> +
> +	subsystem = udev_device_get_subsystem(parent_dev);
> +	syspath = udev_device_get_syspath(parent_dev);
> +
> +	parent_idev = igt_device_find(subsystem, syspath);
> +	if (!parent_idev) {
> +		parent_idev = igt_device_new_from_udev(parent_dev);
> +		if (is_pci_subsystem(parent_idev)) {
> +			set_vendor_device(parent_idev);
> +		}
> +		igt_list_insert_tail(&igt_devs.all, &parent_idev->link);
> +	}
> +	devname = udev_device_get_devnode(dev);
> +	if (devname != NULL && strstr(devname, "/dev/dri/card"))
> +		parent_idev->drm_card = strdup(devname);
> +	else if (devname != NULL && strstr(devname, "/dev/dri/render"))
> +		parent_idev->drm_render = strdup(devname);
> +
> +	idev->parent = parent_idev;
> +}
> +
> +static struct igt_device *duplicate_device(struct igt_device *dev) {
> +	struct igt_device *dup = malloc(sizeof(*dup));
> +	memcpy(dup, dev, sizeof(*dev));
> +	dup->link.prev = NULL;
> +	dup->link.next = NULL;
> +	return dup;
> +}
> +
> +static gint devs_compare(const void *a, const void *b)
> +{
> +	struct igt_device *dev1, *dev2;
> +	int ret;
> +
> +	dev1 = *(struct igt_device **) a;
> +	dev2 = *(struct igt_device **) b;
> +	ret = strcmp(dev1->subsystem, dev2->subsystem);
> +	if (ret)
> +		return ret;
> +
> +	return strcmp(dev1->syspath, dev2->syspath);
> +}
> +
> +static void sort_all_devices(void)
> +{
> +	struct igt_device *dev, *tmp;
> +	int len = igt_list_length(&igt_devs.all);
> +	struct igt_device **devs = malloc(len * sizeof(*dev));
> +
> +	int i = 0;
> +	igt_list_for_each_safe(dev, tmp, &igt_devs.all, link) {
> +		devs[i] = dev;
> +		igt_assert(i++ < len);
> +		igt_list_remove(&dev->link);
> +	}
> +
> +	qsort(devs, len, sizeof(*devs), devs_compare);
> +
> +	for (i = 0; i < len; ++i) {
> +		igt_list_insert_tail(&igt_devs.all, &devs[i]->link);
> +	}

Last minute finding: This leaks memory.

free(devs);


-- 
Petri Latvala




> +}
> +
> +/* Core scanning function.
> + *
> + * All scanned devices are kept inside igt_devs.all pointer array.
> + * Each added device is igt_device structure, which contrary to udev device
> + * has properties / sysattrs stored inside hash table instead of list.
> + *
> + * Function iterates over devices on 'drm' subsystem. For each drm device
> + * its parent is taken (bus device) and stored inside same array.
> + * Function sorts all found devices to keep same order of bus devices
> + * for providing predictable search.
> + */
> +static void scan_drm_devices(void)
> +{
> +	struct udev *udev;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	struct igt_device *dev;
> +	int ret;
> +
> +	udev = udev_new();
> +	igt_assert(udev);
> +
> +	enumerate = udev_enumerate_new(udev);
> +	igt_assert(enumerate);
> +
> +	DBG("Scanning drm subsystem\n");
> +	ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
> +	igt_assert(!ret);
> +
> +	udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> +	igt_assert(!ret);
> +
> +	ret = udev_enumerate_scan_devices(enumerate);
> +	igt_assert(!ret);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	if (!devices)
> +		return;
> +
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		const char *path;
> +		struct udev_device *udev_dev;
> +		struct igt_device *idev;
> +
> +		path = udev_list_entry_get_name(dev_list_entry);
> +		udev_dev = udev_device_new_from_syspath(udev, path);
> +		idev = igt_device_new_from_udev(udev_dev);
> +		update_or_add_parent(udev_dev, idev);
> +		igt_list_insert_tail(&igt_devs.all, &idev->link);
> +
> +		udev_device_unref(udev_dev);
> +	}
> +	udev_enumerate_unref(enumerate);
> +	udev_unref(udev);
> +
> +	sort_all_devices();
> +
> +	igt_list_for_each(dev, &igt_devs.all, link) {
> +		struct igt_device *dev_dup = duplicate_device(dev);
> +		igt_list_insert_tail(&igt_devs.filtered, &dev_dup->link);
> +	}
> +}
> +
> +static void igt_device_free(struct igt_device *dev)
> +{
> +	free(dev->devnode);
> +	free(dev->subsystem);
> +	free(dev->syspath);
> +	free(dev->drm_card);
> +	free(dev->drm_render);
> +	free(dev->vendor);
> +	free(dev->device);
> +	g_hash_table_destroy(dev->attrs_ht);
> +	g_hash_table_destroy(dev->props_ht);
> +}
> +
> +/**
> + * igt_devices_scan
> + * @force: enforce scanning devices
> + *
> + * Function scans udev in search of gpu devices. For first run it can be
> + * called with @force = false. If something changes during the the test
> + * or test does some module loading (new drm devices occurs during execution)
> + * function must be called again with @force = true to refresh device array.
> + */
> +void igt_devices_scan(bool force)
> +{
> +	if (force && igt_devs.devs_scanned) {
> +		struct igt_device *dev, *tmp;
> +		igt_list_for_each_safe(dev, tmp, &igt_devs.all, link) {
> +			igt_device_free(dev);
> +			free(dev);
> +		}
> +
> +		igt_devs.devs_scanned = false;
> +	}
> +
> +	if (igt_devs.devs_scanned)
> +		return;
> +
> +	prepare_scan();
> +	scan_drm_devices();
> +
> +	igt_devs.devs_scanned = true;
> +}
> +
> +static inline void _pr_simple(const char *k, const char *v)
> +{
> +	printf("    %-16s: %s\n", k, v);
> +}
> +
> +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 *view)
> +{
> +	struct igt_device *dev;
> +
> +	if (!view)
> +		return;
> +
> +	if (igt_list_empty(view)) {
> +		printf("No GPU devices found\n");
> +		return;
> +	}
> +
> +	igt_list_for_each(dev, view, link) {
> +		printf("sys:%s\n", dev->syspath);
> +		if (dev->subsystem)
> +			_pr_simple("subsystem", dev->subsystem);
> +		if (dev->drm_card)
> +			_pr_simple("drm card", dev->drm_card);
> +		if (dev->drm_render)
> +			_pr_simple("drm render", dev->drm_render);
> +		if (is_drm_subsystem(dev)) {
> +			_pr_simple2("parent", "sys",
> +				   dev->parent->syspath);
> +		} else {
> +			if (is_pci_subsystem(dev)) {
> +				_pr_simple("vendor", dev->vendor);
> +				_pr_simple("device", dev->device);
> +			}
> +		}
> +		printf("\n");
> +	}
> +}
> +
> +static inline void _print_key_value(const char* k, const char *v)
> +{
> +	printf("%-32s: %s\n", k, v);
> +}
> +
> +static void print_ht(GHashTable *ht)
> +{
> +	GList *keys = g_hash_table_get_keys(ht);
> +
> +	keys = g_list_sort(keys, (GCompareFunc) strcmp);
> +	while (keys) {
> +		char *k = (char *) keys->data;
> +		char *v = g_hash_table_lookup(ht, k);
> +
> +		_print_key_value(k, v);
> +		keys = g_list_next(keys);
> +	}
> +	g_list_free(keys);
> +}
> +
> +static void igt_devs_print_detail(struct igt_list *view)
> +{
> +	struct igt_device *dev;
> +
> +	if (!view)
> +		return;
> +
> +	if (igt_list_empty(view)) {
> +		printf("No GPU devices found\n");
> +		return;
> +	}
> +
> +	igt_list_for_each(dev, view, link) {
> +		printf("========== %s:%s ==========\n",
> +		       dev->subsystem, dev->syspath);
> +		if (!is_drm_subsystem(dev)) {
> +			_print_key_value("card device", dev->drm_card);
> +			_print_key_value("render device", dev->drm_render);
> +		}
> +
> +		printf("\n[properties]\n");
> +		print_ht(dev->props_ht);
> +		printf("\n[attributes]\n");
> +		print_ht(dev->attrs_ht);
> +		printf("\n");
> +	}
> +}
> +
> +static struct print_func {
> +	void (*prn)(struct igt_list *view);
> +} print_functions[] = {
> +	[IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> +	[IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> +};
> +
> +/**
> + * igt_devices_print
> + * @printtype: IGT_PRINT_SIMPLE or IGT_PRINT_DETAIL
> + *
> + * Function can be used by external tool to print device array in simple
> + * or detailed form. This function is added here to avoid exposing
> + * internal implementation data structures.
> + */
> +void igt_devices_print(enum igt_devices_print_type printtype)
> +{
> +	print_functions[printtype].prn(&igt_devs.filtered);
> +}
> +
> +/**
> + * igt_devices_print_vendors
> + *
> + * Print pci id -> vendor mappings. Vendor names printed by this function
> + * can be used for filters like pci which allows passing vendor - like
> + * vendor id (8086) or as a string (Intel).
> + */
> +void igt_devices_print_vendors(void)
> +{
> +	printf("Recognized vendors:\n");
> +	printf("%-8s %-16s\n", "PCI ID", "vendor");
> +	for (typeof(*pci_vendor_mapping) *vm = pci_vendor_mapping; vm->name; vm++) {
> +		printf("%-8s %-16s\n", vm->vendor_id, vm->name);
> +	}
> +}
> +
> +struct filter;
> +
> +/* ------------------------------------------------------------------------- */
> +struct filter_class {
> +	struct igt_list *(*filter_function)(const struct filter_class *fcls,
> +					    const struct filter *filter);
> +	bool (*is_valid)(const struct filter_class *fcls,
> +			 const struct filter *filter);
> +	const char *name;
> +	const char *help;
> +	const char *detail;
> +};
> +
> +#define FILTER_NAME_LEN 32
> +#define FILTER_DATA_LEN 256
> +
> +struct filter {
> +	struct filter_class *class;
> +
> +	char raw_data[FILTER_DATA_LEN];
> +
> +	struct {
> +		char *vendor;
> +		char *device;
> +		char *card;
> +		char *drm;
> +		char *driver;
> +	} data;
> +};
> +
> +static void fill_filter_data(struct filter *filter, const char *key, const char *value)
> +{
> +	if (key == NULL || value == NULL) {
> +		return;
> +	}
> +
> +#define __fill_key(name) if (strcmp(key, #name) == 0) \
> +	filter->data.name = strdup(value)
> +	__fill_key(vendor);
> +	__fill_key(device);
> +	__fill_key(card);
> +	__fill_key(drm);
> +	__fill_key(driver);
> +#undef __fill_key
> +
> +}
> +
> +static void split_filter_data(struct filter *filter)
> +{
> +	char *property, *key, *data, *dup;
> +
> +	dup = strdup(filter->raw_data);
> +	data = dup;
> +
> +	while ((property = strsep(&data, ","))) {
> +		key = strsep(&property, "=");
> +		fill_filter_data(filter, key, property);
> +	}
> +
> +	free(dup);
> +}
> +
> +static struct filter_class *get_filter_class(const char *class_name, const struct filter *filter);
> +
> +static bool parse_filter(const char *fstr, struct filter *filter)
> +{
> +	char class_name[32];
> +	if (!fstr || !filter)
> +		return false;
> +
> +	memset(filter, 0, sizeof(*filter));
> +
> +
> +	if (sscanf(fstr, "%31[^:]:%255s", class_name, filter->raw_data) >= 1) {
> +		filter->class = get_filter_class(class_name, filter);
> +		split_filter_data(filter);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool is_vendor_matched(struct igt_device *dev, const char *vendor)
> +{
> +	const char *vendor_id;
> +
> +	if (!dev->vendor || !vendor)
> +		return false;
> +
> +	/* First we compare vendor id, like 8086 */
> +	if (!strcasecmp(dev->vendor, vendor))
> +		return true;
> +
> +	/* Likely we have vendor string instead of id */
> +	vendor_id = get_pci_vendor_id_by_name(vendor);
> +	if (!vendor_id)
> +		return false;
> +
> +	return !strcasecmp(dev->vendor, vendor_id);
> +}
> +
> +/* Filter which matches subsystem:/sys/... path.
> + * Used as first filter in chain.
> + */
> +static struct igt_list *filter_sys(const struct filter_class *fcls,
> +				   const struct filter *filter)
> +{
> +	struct igt_device *dev;
> +	(void) fcls;
> +
> +	DBG("filter sys\n");
> +	if (!strlen(filter->raw_data))
> +		return &igt_devs.filtered;
> +
> +	dev = igt_device_from_syspath(filter->raw_data);
> +	if (dev) {
> +		struct igt_device *dup = duplicate_device(dev);
> +		igt_list_insert_tail(&igt_devs.filtered, &dup->link);
> +	}
> +
> +	return &igt_devs.filtered;
> +}
> +
> +/* Find drm device using direct path to /dev/dri/.
> + * It extends filter_sys to allow using drm:/dev/dri/cardX and
> + * drm:/dev/dri/renderDX filter syntax.
> + */
> +static struct igt_list *filter_drm(const struct filter_class *fcls,
> +				   const struct filter *filter)
> +{
> +	struct igt_device *dev;
> +	(void) fcls;
> +
> +	DBG("filter drm\n");
> +	if (!strlen(filter->raw_data))
> +		return &igt_devs.filtered;
> +
> +	igt_list_for_each(dev, &igt_devs.all, link) {
> +		if (!is_drm_subsystem(dev))
> +			continue;
> +
> +		if (strequal(dev->syspath, filter->raw_data) ||
> +			strequal(dev->drm_card, filter->raw_data) ||
> +			strequal(dev->drm_render, filter->raw_data)) {
> +			struct igt_device *dup = duplicate_device(dev);
> +			igt_list_insert_tail(&igt_devs.filtered, &dup->link);
> +			break;
> +		}
> +	}
> +
> +
> +	return &igt_devs.filtered;
> +}
> +
> +/* Find appropriate pci device matching vendor/device/card filter arguments.
> + */
> +static struct igt_list *filter_pci(const struct filter_class *fcls,
> +				   const struct filter *filter)
> +{
> +	struct igt_device *dev;
> +	int card = -1;
> +	(void) fcls;
> +
> +	DBG("filter pci\n");
> +
> +	if (filter->data.card) {
> +		sscanf(filter->data.card, "%d", &card);
> +		if (card < 0) {
> +			return &igt_devs.filtered;
> +		}
> +	} else {
> +		card = 0;
> +	}
> +
> +	igt_list_for_each(dev, &igt_devs.all, link) {
> +		if (!is_pci_subsystem(dev))
> +			continue;
> +
> +		/* Skip if 'vendor' doesn't match (hex or name) */
> +		if (filter->data.vendor && !is_vendor_matched(dev, filter->data.vendor))
> +			continue;
> +
> +		/* Skip if 'device' doesn't match */
> +		if (filter->data.device && strcasecmp(filter->data.device, dev->device))
> +			continue;
> +
> +		/* We get n-th card */
> +		if (!card) {
> +			struct igt_device *dup = duplicate_device(dev);
> +			igt_list_insert_tail(&igt_devs.filtered, &dup->link);
> +			break;
> +		}
> +		card--;
> +	}
> +
> +	DBG("Filter pci filtered size: %d\n", igt_list_length(&igt_devs.filtered));
> +
> +	return &igt_devs.filtered;
> +}
> +
> +static bool sys_path_valid(const struct filter_class *fcls,
> +			   const struct filter *filter)
> +{
> +	struct stat st;
> +
> +	if (stat(filter->raw_data, &st)) {
> +		igt_warn("sys_path_valid: syspath [%s], err: %s\n",
> +			 filter->raw_data, strerror(errno));
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +
> +static struct filter_class filter_definition_list[] = {
> +	{
> +		.name = "sys",
> +		.is_valid = sys_path_valid,
> +		.filter_function = filter_sys,
> +		.help = "sys:/sys/devices/pci0000:00/0000:00:02.0",
> +		.detail = "find device byt its sysfs path\n",
> +	},
> +	{
> +		.name = "drm",
> +		.filter_function = filter_drm,
> +		.help = "drm:/dev/dri/* path",
> +		.detail = "find drm device by /dev/dri/* node\n",
> +	},
> +	{
> +		.name = "pci",
> +		.filter_function = filter_pci,
> +		.help = "pci:[vendor=%04x/name][,device=%04x][,card=%d]",
> +		.detail = "vendor is hex number or vendor name\n",
> +	},
> +	{
> +		.name = NULL,
> +	},
> +};
> +
> +static struct filter_class *get_filter_class(const char *class_name, const struct filter *filter)
> +{
> +	struct filter_class *fcls = NULL;
> +	int i = 0;
> +
> +	while ((fcls = &filter_definition_list[i++])->name != NULL) {
> +		if (strcmp(class_name, fcls->name) == 0)
> +			return fcls;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * @igt_device_print_filter_types
> + *
> + * Print all filters syntax for device selection.
> + */
> +void igt_device_print_filter_types(void)
> +{
> +	const struct filter_class *filter = NULL;
> +	int i = 0;
> +
> +	printf("Filter types:\n---\n");
> +	printf("%-12s  %s\n---\n", "filter", "syntax");
> +
> +	while ((filter = &filter_definition_list[i++])->name != NULL) {
> +		printf("%-12s  %s\n", filter->name, filter->help);
> +		printf("%-12s  %s\n", "", filter->detail);
> +	}
> +}
> +
> +static char *device_filter;
> +
> +/**
> + * igt_device_is_filter_set
> + *
> + * Returns whether we have a filter set.
> + */
> +bool igt_device_is_filter_set(void)
> +{
> +	return device_filter != NULL;
> +}
> +
> +/* Check does filter is valid. It checks:
> + * 1. /sys/... path first
> + * 2. filter name from filter definition
> + */
> +static bool is_filter_valid(const char *fstr)
> +{
> +	struct filter filter;
> +	int ret;
> +
> +	ret = parse_filter(fstr, &filter);
> +	if (!ret)
> +		return false;
> +
> +	if (filter.class == NULL) {
> +		igt_warn("No filter matching [%s:%s]\n", filter.class->name, filter.raw_data);
> +		return false;
> +	}
> +
> +	if (filter.class->is_valid != NULL && !filter.class->is_valid(filter.class, &filter))
> +	{
> +		igt_warn("Filter not valid [%s:%s]\n", filter.class->name, filter.raw_data);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +/**
> + * igt_device_filter_set
> + * @filter: filter that should be set globally
> + *
> + * Returns number of filters added to filter array. Can be greater than
> + * 1 if @filters contains more than one filter separated by semicolon.
> + */
> +void igt_device_filter_set(const char *filter)
> +{
> +	if (!is_filter_valid(filter)) {
> +		igt_warn("Invalid filter: %s\n", filter);
> +		return;
> +	}
> +
> +	device_filter = strdup(filter);
> +}
> +
> +/**
> + * igt_device_filter_free
> + *
> + * Free the filter that we store internally, effectively insetting it.
> + */
> +void igt_device_filter_free(void)
> +{
> +	if (device_filter != NULL)
> +		free(device_filter);
> +}
> +
> +/**
> + * igt_device_filter_get
> + *
> + * Returns filter string or NULL if not set
> + */
> +const char *igt_device_filter_get(void)
> +{
> +	return device_filter;
> +}
> +
> +static bool igt_device_filter_apply(const char *fstr)
> +{
> +	struct igt_device *dev, *tmp;
> +	struct filter filter;
> +	bool ret;
> +
> +	if (!fstr)
> +		return false;
> +
> +	ret = parse_filter(fstr, &filter);
> +	if (!ret) {
> +		igt_warn("Can't split filter [%s]\n", fstr);
> +		return false;
> +	}
> +
> +	/* Clean the filtered list */
> +	igt_list_for_each_safe(dev, tmp, &igt_devs.filtered, link) {
> +		igt_list_remove(&dev->link);
> +		free(dev);
> +	}
> +
> +	/* If filter.data contains "/sys" use direct path instead
> +	 * contextual filter.
> +	 */
> +
> +	if (!filter.class) {
> +		igt_warn("No filter matching [%s:%s]\n", filter.class->name, filter.raw_data);
> +		return false;
> +	}
> +	filter.class->filter_function(filter.class, &filter);
> +
> +	return true;
> +}
> +
> +/**
> + * igt_device_card_match
> + * @filter: filter string
> + * @card: pointer to igt_device_card struct
> + *
> + * Function applies filter to match device from device array.
> + *
> + * Returns:
> + * false - no card pointer was passed or card wasn't matched,
> + * true - card matched and returned.
> + */
> +bool igt_device_card_match(const char *filter, struct igt_device_card *card)
> +{
> +	struct igt_device *dev = NULL;
> +
> +	if (!card)
> +		return false;
> +	memset(card, 0, sizeof(*card));
> +
> +	igt_devices_scan(false);
> +
> +	if (igt_device_filter_apply(filter) == false)
> +		return false;
> +
> +	if (igt_list_empty(&igt_devs.filtered))
> +		return false;
> +
> +	/* We take first one if more than one card matches filter */
> +	dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
> +
> +	if (dev->subsystem != NULL)
> +		strncpy(card->subsystem, dev->subsystem,
> +			     sizeof(card->subsystem) - 1);
> +
> +	if (dev->drm_card != NULL)
> +		strncpy(card->card, dev->drm_card,
> +			     sizeof(card->card) - 1);
> +
> +	if (dev->drm_render != NULL)
> +		strncpy(card->render, dev->drm_render,
> +			     sizeof(card->render) - 1);
> +
> +	return true;
> +}
> +
> +/**
> + * igt_open_card:
> + * @card: pointer to igt_device_card structure
> + *
> + * Open /dev/dri/cardX device represented by igt_device_card structure.
> + * Requires filled @card argument (see igt_device_card_match() function).
> + *
> + * An open DRM fd or -1 on error
> + */
> +int igt_open_card(struct igt_device_card *card)
> +{
> +	if (!card || !strlen(card->card))
> +		return -1;
> +
> +	return open(card->card, O_RDWR);
> +}
> +
> +/**
> + * igt_open_render:
> + * @card: pointer to igt_device_card structure
> + *
> + * Open /dev/dri/renderDX device represented by igt_device_card structure.
> + * Requires filled @card argument (see igt_device_card_match() function).
> + *
> + * An open DRM fd or -1 on error
> + */
> +int igt_open_render(struct igt_device_card *card)
> +{
> +	if (!card || !strlen(card->render))
> +		return -1;
> +
> +	return open(card->render, O_RDWR);
> +}
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> new file mode 100644
> index 00000000..dbde8816
> --- /dev/null
> +++ b/lib/igt_device_scan.h
> @@ -0,0 +1,67 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __IGT_DEVICE_SCAN_H__
> +#define __IGT_DEVICE_SCAN_H__
> +
> +#include <limits.h>
> +#include <igt.h>
> +
> +enum igt_devices_print_type {
> +	IGT_PRINT_SIMPLE,
> +	IGT_PRINT_DETAIL,
> +};
> +
> +struct igt_device_card {
> +	char subsystem[NAME_MAX];
> +	char card[NAME_MAX];
> +	char render[NAME_MAX];
> +};
> +
> +/* Scan udev subsystems. Do it once unless force is true, what rescans
> + * devices again.
> + */
> +void igt_devices_scan(bool force);
> +
> +void igt_devices_print(enum igt_devices_print_type printtype);
> +void igt_devices_print_vendors(void);
> +void igt_device_print_filter_types(void);
> +
> +/*
> + * Handle device filter collection array.
> + * IGT can store/retrieve filters passed by user using '--device' args.
> + */
> +
> +bool igt_device_is_filter_set(void);
> +void igt_device_filter_set(const char *filter);
> +void igt_device_filter_free(void);
> +const char *igt_device_filter_get(void);
> +
> +/* Use filter to match the device and fill card structure */
> +bool igt_device_card_match(const char *filter, struct igt_device_card *card);
> +
> +int igt_open_card(struct igt_device_card *card);
> +int igt_open_render(struct igt_device_card *card);
> +
> +#endif /* __IGT_DEVICE_SCAN_H__ */
> diff --git a/lib/meson.build b/lib/meson.build
> index 19cb6255..486f51d7 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -10,6 +10,7 @@ lib_sources = [
>  	'igt_color_encoding.c',
>  	'igt_debugfs.c',
>  	'igt_device.c',
> +	'igt_device_scan.c',
>  	'igt_aux.c',
>  	'igt_gt.c',
>  	'igt_gvt.c',
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool Arkadiusz Hiler
  2019-10-24 13:06   ` Chris Wilson
@ 2019-11-18 11:58   ` Petri Latvala
  1 sibling, 0 replies; 22+ messages in thread
From: Petri Latvala @ 2019-11-18 11:58 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

On Thu, Oct 24, 2019 at 02:05:14PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> Tool uses device selection API to scan and display GPU devices.
> It can be used to check filter correctness as well as order
> of applying the filters (.igtrc, IGT_DEVICE and --device argument).
> 
> v2 (Arek):
>  * don't print chip as it's no longer there
>  * make it a second patch, before any alterations to igt_core or drmtest
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  tools/Makefile.sources |   1 +
>  tools/lsgpu.c          | 295 +++++++++++++++++++++++++++++++++++++++++
>  tools/meson.build      |   1 +
>  3 files changed, 297 insertions(+)
>  create mode 100644 tools/lsgpu.c
> 
> diff --git a/tools/Makefile.sources b/tools/Makefile.sources
> index d764895d..b7a43d47 100644
> --- a/tools/Makefile.sources
> +++ b/tools/Makefile.sources
> @@ -33,6 +33,7 @@ tools_prog_lists =		\
>  	intel_watermark		\
>  	intel_gem_info		\
>  	intel_gvtg_test     \
> +	lsgpu			\
>  	$(NULL)
>  
>  dist_bin_SCRIPTS = intel_gpu_abrt
> diff --git a/tools/lsgpu.c b/tools/lsgpu.c
> new file mode 100644
> index 00000000..e385aaa6
> --- /dev/null
> +++ b/tools/lsgpu.c
> @@ -0,0 +1,295 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt_device_scan.h"
> +#include "igt.h"
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <glib.h>
> +
> +/**
> + * SECTION:lsgpu
> + * @short_description: lsgpu
> + * @title: lsgpu
> + * @include: lsgpu.c
> + *
> + * # lsgpu
> + *
> + * The devices can be scanned and displayed using 'lsgpu' tool. Tool also
> + * displays properties and sysattrs (-p switch, means print detail) which
> + * can be used during filter implementation.
> + *
> + * Tool can also be used to try out filters.
> + * To select device use '-d' or '--device' argument like:
> + *
> + * |[<!-- language="plain" -->
> + * ./lsgpu -d 'pci:vendor=Intel'
> + * === Device filter list ===
> + * [ 0]: pci:vendor=Intel
> +
> + * === Testing device open ===
> + * subsystem   : pci
> + * drm card    : /dev/dri/card0
> + * drm render  : /dev/dri/renderD128
> + * Device /dev/dri/card0 successfully opened
> + * Device /dev/dri/renderD128 successfully opened
> + * ]|
> + *
> + * Additionally lsgpu tries to open the card and render nodes to verify
> + * permissions. It also uses IGT variable search order:
> + * - use --device first (it overrides IGT_DEVICE and .igtrc Common::Device
> + *   settings)
> + * - use IGT_DEVICE enviroment variable if no --device are passed
> + * - use .igtrc Common::Device if no --device nor IGT_DEVICE are passed
> + */
> +
> +enum {
> +	OPT_PRINT_DETAIL   = 'p',
> +	OPT_LIST_VENDORS   = 'v',
> +	OPT_LIST_FILTERS   = 'l',
> +	OPT_DEVICE         = 'd',
> +	OPT_HELP           = 'h'
> +};
> +
> +static bool g_show_vendors;
> +static bool g_list_filters;
> +static bool g_device;
> +static bool g_help;
> +static char *igt_rc_device;
> +
> +static const char *usage_str =
> +	"usage: lsgpu [options]\n\n"
> +	"Options:\n"
> +	"  -p, --print-details         Print devices with details\n"
> +	"  -v, --list-vendors          List recognized vendors\n"
> +	"  -l, --list-filter-types     List registered device filters types\n"
> +	"  -d, --device filter         Device filter, can be given multiple times\n"
> +	"  -h, --help                  Show this help message and exit\n";
> +
> +static void test_device_open(struct igt_device_card *card)
> +{
> +	int fd;
> +
> +	if (!card)
> +		return;
> +
> +	fd = igt_open_card(card);
> +	if (fd >= 0) {
> +		printf("Device %s successfully opened\n", card->card);
> +		close(fd);
> +	} else {
> +		if (strlen(card->card))
> +			printf("Cannot open card %s device\n", card->card);
> +		else
> +			printf("Cannot open card device, empty name\n");
> +	}
> +
> +	fd = igt_open_render(card);
> +	if (fd >= 0) {
> +		printf("Device %s successfully opened\n", card->render);
> +		close(fd);
> +	} else {
> +		if (strlen(card->render))
> +			printf("Cannot open render %s device\n", card->render);
> +		else
> +			printf("Cannot open render device, empty name\n");
> +	}
> +}
> +
> +static void print_card(struct igt_device_card *card)
> +{
> +	if (!card)
> +		return;
> +
> +	printf("subsystem   : %s\n", card->subsystem);
> +	printf("drm card    : %s\n", card->card);
> +	printf("drm render  : %s\n", card->render);
> +}
> +
> +/* Partially copied from igt_core to simulate device selection order:
> + * 1. --device        (IGT_DEVICE and .igtrc Common::Device are ignored)
> + * 2. IGT_DEVICE env  (.igtrc Common::Device is ignored)
> + * 3. .igtrc          (overrides
> + */

Is that the reason to not use igt_load_igtrc?


> +static void common_init_config(void)
> +{
> +	char *key_file_env = NULL;
> +	char *key_file_loc = NULL;
> +	GError *error = NULL;
> +	GKeyFile *igt_key_file;
> +	int ret;
> +
> +	/* Filter count > 0, just skip .igtrc */
> +	if (igt_device_is_filter_set())
> +		return;
> +
> +	/* Determine igt config path */
> +	key_file_env = getenv("IGT_CONFIG_PATH");
> +	if (key_file_env) {
> +		key_file_loc = key_file_env;
> +	} else {
> +		key_file_loc = malloc(100);

This malloc is not free()d.


Otherwise LGTM.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT Arkadiusz Hiler
@ 2019-11-18 12:14   ` Petri Latvala
  2019-11-19 14:18     ` Arkadiusz Hiler
  2019-11-20  9:31   ` Petri Latvala
  1 sibling, 1 reply; 22+ messages in thread
From: Petri Latvala @ 2019-11-18 12:14 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

On Thu, Oct 24, 2019 at 02:05:15PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> New IGT command line argument --device, IGT_DEVICE enviroment
> and .igtrc Common::Device were added to allow selecting device
> using device selection API.
> 
> v2 (Arek):
>  * remove functions acting on igt_device_card
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  .../igt-gpu-tools/igt_test_programs.xml       |  7 ++
>  lib/drmtest.c                                 | 94 +++++++++++++++++--
>  lib/drmtest.h                                 |  2 +
>  lib/igt_core.c                                | 47 ++++++++++
>  4 files changed, 142 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/reference/igt-gpu-tools/igt_test_programs.xml b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> index b64ba474..fcce1458 100644
> --- a/docs/reference/igt-gpu-tools/igt_test_programs.xml
> +++ b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> @@ -43,6 +43,13 @@
>              </para></listitem>
>            </varlistentry>
>  
> +          <varlistentry>
> +            <term><option>--device filter</option></term>
> +            <listitem><para>
> +                select device using filter (see "Device selection" for details)
> +            </para></listitem>
> +          </varlistentry>
> +
>            <varlistentry>
>              <term><option>--help-description</option></term>
>              <listitem><para>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index c379a7b7..43471625 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -55,6 +55,7 @@
>  #include "igt_gt.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
> +#include "igt_device_scan.h"
>  #include "version.h"
>  #include "config.h"
>  #include "intel_reg.h"
> @@ -266,14 +267,9 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
>  	return -1;
>  }
>  
> -static int __open_driver(const char *base, int offset, unsigned int chipset)
> +static void __try_modprobe(unsigned int chipset)
>  {
>  	static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> -	int fd;
> -
> -	fd = __search_and_open(base, offset, chipset);
> -	if (fd != -1)
> -		return fd;
>  
>  	pthread_mutex_lock(&mutex);
>  	for (const struct module *m = modules; m->module; m++) {
> @@ -285,26 +281,108 @@ static int __open_driver(const char *base, int offset, unsigned int chipset)
>  		}
>  	}
>  	pthread_mutex_unlock(&mutex);
> +}
> +
> +static int __open_driver(const char *base, int offset, unsigned int chipset)
> +{
> +	int fd;
> +
> +	fd = __search_and_open(base, offset, chipset);
> +	if (fd != -1)
> +		return fd;
> +
> +	__try_modprobe(chipset);
>  
>  	return __search_and_open(base, offset, chipset);
>  }
>  
> +static int __open_driver_exact(const char *name, unsigned int chipset)
> +{
> +	int fd;
> +
> +	fd = open_device(name, chipset);
> +	if (fd != -1)
> +		return fd;
> +
> +	__try_modprobe(chipset);
> +
> +	return open_device(name, chipset);
> +}
> +
> +/*
> + * A helper to get the first marching card in case a filter is set.

Cards can do what?

'matching' might be more appropriate.


> + * It does all the extra logging around the filters for us.
> + *
> + * @card: pointer to the igt_device_card structure to be filled
> + * when a card is found.
> + *
> + * Returns:
> + * True if card according to the added filter was found,
> + * false othwerwise.
> + */
> +static bool __get_the_first_card(struct igt_device_card *card)
> +{
> +	const char *filter;
> +
> +	if (igt_device_is_filter_set()) {
> +		filter = igt_device_filter_get();
> +		igt_info("Looking for devices to open using filter: %s\n", filter);
> +
> +		if (igt_device_card_match(filter, card)) {
> +			igt_info("Filter matched %s | %s\n", card->card, card->render);
> +			return true;
> +		}
> +
> +		igt_warn("No card matches the filter!\n");
> +	}
> +
> +	return false;
> +}
> +
>  /**
>   * __drm_open_driver:
>   * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
>   *
> - * Open the first DRM device we can find, searching up to 16 device nodes
> + * Function opens device in the following order:
> + * 1. when --device arguments are present device scanning will be executed,
> + * then filter argument is used to find matching one.
> + * 2. compatibility mode - open the first DRM device we can find,
> + * searching up to 16 device nodes.
>   *
>   * Returns:
>   * An open DRM fd or -1 on error
>   */
>  int __drm_open_driver(int chipset)
>  {
> +	if (igt_device_is_filter_set()) {
> +		bool found;
> +		struct igt_device_card card;
> +
> +		found = __get_the_first_card(&card);
> +
> +		if (!found || !strlen(card.card))
> +			return -1;
> +
> +		return __open_driver_exact(card.card, chipset);
> +	}
> +
>  	return __open_driver("/dev/dri/card", 0, chipset);
>  }

I'm a little confused how this all works with IGT_FORCE_DRIVER? And
how does this work for tests that open two different drivers?


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT
  2019-11-18 12:14   ` Petri Latvala
@ 2019-11-19 14:18     ` Arkadiusz Hiler
  0 siblings, 0 replies; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-11-19 14:18 UTC (permalink / raw)
  To: Petri Latvala; +Cc: igt-dev

On Mon, Nov 18, 2019 at 02:14:04PM +0200, Petri Latvala wrote:
> On Thu, Oct 24, 2019 at 02:05:15PM +0300, Arkadiusz Hiler wrote:
> > From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > 
> > New IGT command line argument --device, IGT_DEVICE enviroment
> > and .igtrc Common::Device were added to allow selecting device
> > using device selection API.
> > 
> > v2 (Arek):
> >  * remove functions acting on igt_device_card
> >  * use only a single filter
> > 
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> >  .../igt-gpu-tools/igt_test_programs.xml       |  7 ++
> >  lib/drmtest.c                                 | 94 +++++++++++++++++--
> >  lib/drmtest.h                                 |  2 +
> >  lib/igt_core.c                                | 47 ++++++++++
> >  4 files changed, 142 insertions(+), 8 deletions(-)
> > 
> > diff --git a/docs/reference/igt-gpu-tools/igt_test_programs.xml b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> > index b64ba474..fcce1458 100644
> > --- a/docs/reference/igt-gpu-tools/igt_test_programs.xml
> > +++ b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> > @@ -43,6 +43,13 @@
> >              </para></listitem>
> >            </varlistentry>
> >  
> > +          <varlistentry>
> > +            <term><option>--device filter</option></term>
> > +            <listitem><para>
> > +                select device using filter (see "Device selection" for details)
> > +            </para></listitem>
> > +          </varlistentry>
> > +
> >            <varlistentry>
> >              <term><option>--help-description</option></term>
> >              <listitem><para>
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index c379a7b7..43471625 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -55,6 +55,7 @@
> >  #include "igt_gt.h"
> >  #include "igt_kmod.h"
> >  #include "igt_sysfs.h"
> > +#include "igt_device_scan.h"
> >  #include "version.h"
> >  #include "config.h"
> >  #include "intel_reg.h"
> > @@ -266,14 +267,9 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
> >  	return -1;
> >  }
> >  
> > -static int __open_driver(const char *base, int offset, unsigned int chipset)
> > +static void __try_modprobe(unsigned int chipset)
> >  {
> >  	static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> > -	int fd;
> > -
> > -	fd = __search_and_open(base, offset, chipset);
> > -	if (fd != -1)
> > -		return fd;
> >  
> >  	pthread_mutex_lock(&mutex);
> >  	for (const struct module *m = modules; m->module; m++) {
> > @@ -285,26 +281,108 @@ static int __open_driver(const char *base, int offset, unsigned int chipset)
> >  		}
> >  	}
> >  	pthread_mutex_unlock(&mutex);
> > +}
> > +
> > +static int __open_driver(const char *base, int offset, unsigned int chipset)
> > +{
> > +	int fd;
> > +
> > +	fd = __search_and_open(base, offset, chipset);
> > +	if (fd != -1)
> > +		return fd;
> > +
> > +	__try_modprobe(chipset);
> >  
> >  	return __search_and_open(base, offset, chipset);
> >  }
> >  
> > +static int __open_driver_exact(const char *name, unsigned int chipset)
> > +{
> > +	int fd;
> > +
> > +	fd = open_device(name, chipset);
> > +	if (fd != -1)
> > +		return fd;
> > +
> > +	__try_modprobe(chipset);
> > +
> > +	return open_device(name, chipset);
> > +}
> > +
> > +/*
> > + * A helper to get the first marching card in case a filter is set.
> 
> Cards can do what?

Right, it should have said "fleeing"...

"A helper to ge tthe first fleeing card"

> 'matching' might be more appropriate.

Ah, right :-)

> > + * It does all the extra logging around the filters for us.
> > + *
> > + * @card: pointer to the igt_device_card structure to be filled
> > + * when a card is found.
> > + *
> > + * Returns:
> > + * True if card according to the added filter was found,
> > + * false othwerwise.
> > + */
> > +static bool __get_the_first_card(struct igt_device_card *card)
> > +{
> > +	const char *filter;
> > +
> > +	if (igt_device_is_filter_set()) {
> > +		filter = igt_device_filter_get();
> > +		igt_info("Looking for devices to open using filter: %s\n", filter);
> > +
> > +		if (igt_device_card_match(filter, card)) {
> > +			igt_info("Filter matched %s | %s\n", card->card, card->render);
> > +			return true;
> > +		}
> > +
> > +		igt_warn("No card matches the filter!\n");
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >  /**
> >   * __drm_open_driver:
> >   * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
> >   *
> > - * Open the first DRM device we can find, searching up to 16 device nodes
> > + * Function opens device in the following order:
> > + * 1. when --device arguments are present device scanning will be executed,
> > + * then filter argument is used to find matching one.
> > + * 2. compatibility mode - open the first DRM device we can find,
> > + * searching up to 16 device nodes.
> >   *
> >   * Returns:
> >   * An open DRM fd or -1 on error
> >   */
> >  int __drm_open_driver(int chipset)
> >  {
> > +	if (igt_device_is_filter_set()) {
> > +		bool found;
> > +		struct igt_device_card card;
> > +
> > +		found = __get_the_first_card(&card);
> > +
> > +		if (!found || !strlen(card.card))
> > +			return -1;
> > +
> > +		return __open_driver_exact(card.card, chipset);
> > +	}
> > +
> >  	return __open_driver("/dev/dri/card", 0, chipset);
> >  }
> 
> I'm a little confused how this all works with IGT_FORCE_DRIVER?

IGT_FORCE_DRIVER will work if no filter is used. We may consider
deprecating this later on.

> And how does this work for tests that open two different drivers?

This will break, but this is IMO ok and they are the prefect thing to
get us working on API for opening multiple devices :-)

There was some proposal in the original patches by Zbigniew, but IMO the
API requires a little bit more work and though from a consumer POV
before getting it in.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT
  2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT Arkadiusz Hiler
  2019-11-18 12:14   ` Petri Latvala
@ 2019-11-20  9:31   ` Petri Latvala
  1 sibling, 0 replies; 22+ messages in thread
From: Petri Latvala @ 2019-11-20  9:31 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

On Thu, Oct 24, 2019 at 02:05:15PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> New IGT command line argument --device, IGT_DEVICE enviroment
> and .igtrc Common::Device were added to allow selecting device
> using device selection API.
> 
> v2 (Arek):
>  * remove functions acting on igt_device_card
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  .../igt-gpu-tools/igt_test_programs.xml       |  7 ++
>  lib/drmtest.c                                 | 94 +++++++++++++++++--
>  lib/drmtest.h                                 |  2 +
>  lib/igt_core.c                                | 47 ++++++++++
>  4 files changed, 142 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/reference/igt-gpu-tools/igt_test_programs.xml b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> index b64ba474..fcce1458 100644
> --- a/docs/reference/igt-gpu-tools/igt_test_programs.xml
> +++ b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> @@ -43,6 +43,13 @@
>              </para></listitem>
>            </varlistentry>
>  
> +          <varlistentry>
> +            <term><option>--device filter</option></term>
> +            <listitem><para>
> +                select device using filter (see "Device selection" for details)
> +            </para></listitem>
> +          </varlistentry>
> +
>            <varlistentry>
>              <term><option>--help-description</option></term>
>              <listitem><para>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index c379a7b7..43471625 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -55,6 +55,7 @@
>  #include "igt_gt.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
> +#include "igt_device_scan.h"
>  #include "version.h"
>  #include "config.h"
>  #include "intel_reg.h"
> @@ -266,14 +267,9 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
>  	return -1;
>  }
>  
> -static int __open_driver(const char *base, int offset, unsigned int chipset)
> +static void __try_modprobe(unsigned int chipset)
>  {
>  	static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> -	int fd;
> -
> -	fd = __search_and_open(base, offset, chipset);
> -	if (fd != -1)
> -		return fd;
>  
>  	pthread_mutex_lock(&mutex);
>  	for (const struct module *m = modules; m->module; m++) {
> @@ -285,26 +281,108 @@ static int __open_driver(const char *base, int offset, unsigned int chipset)
>  		}
>  	}
>  	pthread_mutex_unlock(&mutex);
> +}
> +
> +static int __open_driver(const char *base, int offset, unsigned int chipset)
> +{
> +	int fd;
> +
> +	fd = __search_and_open(base, offset, chipset);
> +	if (fd != -1)
> +		return fd;
> +
> +	__try_modprobe(chipset);
>  
>  	return __search_and_open(base, offset, chipset);
>  }
>  
> +static int __open_driver_exact(const char *name, unsigned int chipset)
> +{
> +	int fd;
> +
> +	fd = open_device(name, chipset);
> +	if (fd != -1)
> +		return fd;
> +
> +	__try_modprobe(chipset);
> +
> +	return open_device(name, chipset);
> +}
> +
> +/*
> + * A helper to get the first marching card in case a filter is set.
> + * It does all the extra logging around the filters for us.
> + *
> + * @card: pointer to the igt_device_card structure to be filled
> + * when a card is found.
> + *
> + * Returns:
> + * True if card according to the added filter was found,
> + * false othwerwise.
> + */
> +static bool __get_the_first_card(struct igt_device_card *card)
> +{
> +	const char *filter;
> +
> +	if (igt_device_is_filter_set()) {
> +		filter = igt_device_filter_get();
> +		igt_info("Looking for devices to open using filter: %s\n", filter);
> +
> +		if (igt_device_card_match(filter, card)) {
> +			igt_info("Filter matched %s | %s\n", card->card, card->render);
> +			return true;
> +		}
> +
> +		igt_warn("No card matches the filter!\n");
> +	}
> +
> +	return false;
> +}
> +
>  /**
>   * __drm_open_driver:
>   * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
>   *
> - * Open the first DRM device we can find, searching up to 16 device nodes
> + * Function opens device in the following order:
> + * 1. when --device arguments are present device scanning will be executed,
> + * then filter argument is used to find matching one.
> + * 2. compatibility mode - open the first DRM device we can find,
> + * searching up to 16 device nodes.
>   *
>   * Returns:
>   * An open DRM fd or -1 on error
>   */
>  int __drm_open_driver(int chipset)
>  {
> +	if (igt_device_is_filter_set()) {
> +		bool found;
> +		struct igt_device_card card;
> +
> +		found = __get_the_first_card(&card);
> +
> +		if (!found || !strlen(card.card))
> +			return -1;
> +
> +		return __open_driver_exact(card.card, chipset);
> +	}
> +
>  	return __open_driver("/dev/dri/card", 0, chipset);
>  }
>  
> -static int __drm_open_driver_render(int chipset)
> +int __drm_open_driver_render(int chipset)
>  {
> +	if (igt_device_is_filter_set()) {
> +		bool found;
> +		struct igt_device_card card;
> +
> +		found = __get_the_first_card(&card);
> +
> +		if (!found || !strlen(card.render))
> +			return -1;
> +
> +		return __open_driver_exact(card.render, chipset);
> +	}
> +
>  	return __open_driver("/dev/dri/renderD", 128, chipset);
>  }
>  
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 614f57e6..7a985b26 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -50,6 +50,7 @@
>  #define DRIVER_AMDGPU	(1 << 3)
>  #define DRIVER_V3D	(1 << 4)
>  #define DRIVER_PANFROST	(1 << 5)
> +
>  /*
>   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
>   * with vgem as well as a supported driver, you can end up with a
> @@ -81,6 +82,7 @@ int drm_open_driver(int chipset);
>  int drm_open_driver_master(int chipset);
>  int drm_open_driver_render(int chipset);
>  int __drm_open_driver(int chipset);
> +int __drm_open_driver_render(int chipset);
>  
>  void gem_quiescent_gpu(int fd);
>  
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 67b76025..94121f99 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -71,6 +71,7 @@
>  #include "igt_sysrq.h"
>  #include "igt_rc.h"
>  #include "igt_list.h"
> +#include "igt_device_scan.h"
>  
>  #define UNW_LOCAL_ONLY
>  #include <libunwind.h>
> @@ -245,6 +246,9 @@
>   *	[Common]
>   *	FrameDumpPath=/tmp # The path to dump frames that fail comparison checks
>   *
> + *	&num; Device selection filter
> + *	Device=pci:vendor=8086,card=0;vgem:
> + *
>   *	&num; The following section is used for configuring the Device Under Test.
>   *	&num; It is not mandatory and allows overriding default values.
>   *	[DUT]
> @@ -304,6 +308,7 @@ enum {
>  	OPT_DEBUG,
>  	OPT_INTERACTIVE_DEBUG,
>  	OPT_SKIP_CRC,
> +	OPT_DEVICE,
>  	OPT_HELP = 'h'
>  };
>  
> @@ -320,6 +325,7 @@ static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
>  GKeyFile *igt_key_file;
>  
>  char *igt_frame_dump_path;
> +char *igt_rc_device;
>  
>  static bool stderr_needs_sentinel = false;
>  
> @@ -644,6 +650,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>  		   "  --skip-crc-compare\n"
>  		   "  --help-description\n"
>  		   "  --describe\n"
> +		   "  --device filter\n"
>  		   "  --help|-h\n");
>  	if (help_str)
>  		fprintf(f, "%s\n", help_str);
> @@ -733,6 +740,31 @@ static void common_init_config(void)
>  
>  	if (ret != 0)
>  		igt_set_autoresume_delay(ret);
> +
> +	/* Adding filters, order .igtrc, IGT_DEVICE, --device filter */
> +	if (igt_device_is_filter_set())
> +		igt_debug("Notice: using --device filters:\n");
> +	else {
> +		if (igt_rc_device) {
> +			igt_debug("Notice: using IGT_DEVICE env:\n");
> +		} else {
> +			igt_rc_device =	g_key_file_get_string(igt_key_file,
> +							      "Common",
> +							      "Device", &error);
> +			g_clear_error(&error);
> +			if (igt_rc_device)
> +				igt_debug("Notice: using .igtrc "
> +					  "Common::Device:\n");
> +		}
> +		if (igt_rc_device) {
> +			igt_device_filter_set(igt_rc_device);
> +			free(igt_rc_device);
> +			igt_rc_device = NULL;
> +		}
> +	}
> +
> +	if (igt_device_is_filter_set())
> +		igt_debug("[%s]\n",  igt_device_filter_get());

                                   ^  extra space


With that and the earlier comments,

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu
  2019-11-12 10:14 ` [igt-dev] [PATCH i-g-t 0/4] " Tvrtko Ursulin
@ 2019-12-02 12:37   ` Arkadiusz Hiler
  2019-12-03 13:59     ` Tvrtko Ursulin
  0 siblings, 1 reply; 22+ messages in thread
From: Arkadiusz Hiler @ 2019-12-02 12:37 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev, Leo Liu, Petri Latvala

On Tue, Nov 12, 2019 at 10:14:13AM +0000, Tvrtko Ursulin wrote:
> 
> On 24/10/2019 12:05, Arkadiusz Hiler wrote:
> > Hey,
> > 
> > This series aims to make running IGT on hosts with multiple GPUs manageable
> > without the need to unload modules / unbind devices.
> > 
> > Suggested reviewing order:
> >   * lsgpu
> >   * igt_core && drm_test changes
> >   * implementation internals in igt_device_scan.c
> > 
> > Changes since Zbigniew's last revision:
> >   * rewritten most of the parts that were using glib
> >   * removed multiple filter support - this will be added back when the need
> >     arises
> >   * we don't second guess the "chipset" of the device and just let the underlying
> >     open to fail if it has to
> >   * extra looging around opening device when filter is set
> >   * sysfs filter now has it's own prefix
> >   * no "platform" filter - sysfs should suffice for now, it can be added by
> >     someone more knowledgeable if the need arises
> > 
> > TODO:
> >   * API for opening multiple devices in a single test (e.g. for prime) - I don't
> >     want to design this upfront
> > 
> > Example usage:
> >    $ build/tools/lsgpu
> >    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/platform/vgem/drm/card1
> >        subsystem       : drm
> >        drm card        : /dev/dri/card1
> >        parent          : sys:/sys/devices/platform/vgem
> > 
> >    sys:/sys/devices/platform/vgem/drm/renderD129
> >        subsystem       : drm
> >        drm render      : /dev/dri/renderD129
> >        parent          : sys:/sys/devices/platform/vgem
> > 
> >    sys:/sys/devices/pci0000:00/0000:00:02.0
> >        subsystem       : pci
> >        drm card        : /dev/dri/card0
> >        drm render      : /dev/dri/renderD128
> >        vendor          : 8086
> >        device          : 5927
> > 
> >    sys:/sys/devices/platform/vgem
> >        subsystem       : platform
> >        drm card        : /dev/dri/card1
> >        drm render      : /dev/dri/renderD129
> > 
> >    $ build/tools/lsgpu -d "sys:/sys/devices/pci0000:00/0000:00:02.0"
> >    Notice: Using --device filters
> >    === Device filter ===
> >    sys:/sys/devices/pci0000:00/0000:00:02.0
> > 
> >    === Testing device open ===
> >    Device detail:
> >    subsystem   : pci
> >    drm card    : /dev/dri/card0
> >    drm render  : /dev/dri/renderD128
> >    Device /dev/dri/card0 successfully opened
> >    Device /dev/dri/renderD128 successfully opened
> >    -------------------------------------------
> > 
> >    # build/tests/core_auth --run-subtest getclient-simple --device "pci:vendor=intel"
> >    IGT-Version: 1.24-g64068440 (x86_64) (Linux: 5.3.7-250.vanilla.knurd.1.fc30.x86_64 x86_64)
> >    Starting subtest: getclient-simple
> >    Looking for devices to open using filter: pci:vendor=intel
> >    Filter matched /dev/dri/card0 | /dev/dri/renderD128
> >    Looking for devices to open using filter: pci:vendor=intel
> >    Filter matched /dev/dri/card0 | /dev/dri/renderD128 >    Subtest getclient-simple: SUCCESS (0.007s)
> 
> Looks very usable!
> 
> Can I be cheeky and ask if you could also cover the existing tools
> (especially intel_gpu_top (VLK-5588))?

I am happy to do this but I would prefer to do that in the folowup
patches to not bloat this series any more.

https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/51

> Plus I would need a helper to get the selected device PCI string to use with
> i915 PMU? There I'd need to get a string like "0000:01:00.0" since the i915
> PMU registers the device as either:
> 
> 
> "/sys/bus/event_source/devices/i915" for integrated.
> 
> Or:
> 
> "/sys/bus/event_source/devices/i915-0000:01:00.0" for discrete.
> 
> So I am thinking something like igt_device_get_pci_string() could work for
> lib/igt_perf.c/i915_type_id().

same as above

https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/52
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu
  2019-12-02 12:37   ` Arkadiusz Hiler
@ 2019-12-03 13:59     ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-12-03 13:59 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev, Leo Liu, Petri Latvala


On 02/12/2019 12:37, Arkadiusz Hiler wrote:
> On Tue, Nov 12, 2019 at 10:14:13AM +0000, Tvrtko Ursulin wrote:
>>
>> On 24/10/2019 12:05, Arkadiusz Hiler wrote:
>>> Hey,
>>>
>>> This series aims to make running IGT on hosts with multiple GPUs manageable
>>> without the need to unload modules / unbind devices.
>>>
>>> Suggested reviewing order:
>>>    * lsgpu
>>>    * igt_core && drm_test changes
>>>    * implementation internals in igt_device_scan.c
>>>
>>> Changes since Zbigniew's last revision:
>>>    * rewritten most of the parts that were using glib
>>>    * removed multiple filter support - this will be added back when the need
>>>      arises
>>>    * we don't second guess the "chipset" of the device and just let the underlying
>>>      open to fail if it has to
>>>    * extra looging around opening device when filter is set
>>>    * sysfs filter now has it's own prefix
>>>    * no "platform" filter - sysfs should suffice for now, it can be added by
>>>      someone more knowledgeable if the need arises
>>>
>>> TODO:
>>>    * API for opening multiple devices in a single test (e.g. for prime) - I don't
>>>      want to design this upfront
>>>
>>> Example usage:
>>>     $ build/tools/lsgpu
>>>     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/platform/vgem/drm/card1
>>>         subsystem       : drm
>>>         drm card        : /dev/dri/card1
>>>         parent          : sys:/sys/devices/platform/vgem
>>>
>>>     sys:/sys/devices/platform/vgem/drm/renderD129
>>>         subsystem       : drm
>>>         drm render      : /dev/dri/renderD129
>>>         parent          : sys:/sys/devices/platform/vgem
>>>
>>>     sys:/sys/devices/pci0000:00/0000:00:02.0
>>>         subsystem       : pci
>>>         drm card        : /dev/dri/card0
>>>         drm render      : /dev/dri/renderD128
>>>         vendor          : 8086
>>>         device          : 5927
>>>
>>>     sys:/sys/devices/platform/vgem
>>>         subsystem       : platform
>>>         drm card        : /dev/dri/card1
>>>         drm render      : /dev/dri/renderD129
>>>
>>>     $ build/tools/lsgpu -d "sys:/sys/devices/pci0000:00/0000:00:02.0"
>>>     Notice: Using --device filters
>>>     === Device filter ===
>>>     sys:/sys/devices/pci0000:00/0000:00:02.0
>>>
>>>     === Testing device open ===
>>>     Device detail:
>>>     subsystem   : pci
>>>     drm card    : /dev/dri/card0
>>>     drm render  : /dev/dri/renderD128
>>>     Device /dev/dri/card0 successfully opened
>>>     Device /dev/dri/renderD128 successfully opened
>>>     -------------------------------------------
>>>
>>>     # build/tests/core_auth --run-subtest getclient-simple --device "pci:vendor=intel"
>>>     IGT-Version: 1.24-g64068440 (x86_64) (Linux: 5.3.7-250.vanilla.knurd.1.fc30.x86_64 x86_64)
>>>     Starting subtest: getclient-simple
>>>     Looking for devices to open using filter: pci:vendor=intel
>>>     Filter matched /dev/dri/card0 | /dev/dri/renderD128
>>>     Looking for devices to open using filter: pci:vendor=intel
>>>     Filter matched /dev/dri/card0 | /dev/dri/renderD128 >    Subtest getclient-simple: SUCCESS (0.007s)
>>
>> Looks very usable!
>>
>> Can I be cheeky and ask if you could also cover the existing tools
>> (especially intel_gpu_top (VLK-5588))?
> 
> I am happy to do this but I would prefer to do that in the folowup
> patches to not bloat this series any more.
> 
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/51

Thanks a lot!

>> Plus I would need a helper to get the selected device PCI string to use with
>> i915 PMU? There I'd need to get a string like "0000:01:00.0" since the i915
>> PMU registers the device as either:
>>
>>
>> "/sys/bus/event_source/devices/i915" for integrated.
>>
>> Or:
>>
>> "/sys/bus/event_source/devices/i915-0000:01:00.0" for discrete.
>>
>> So I am thinking something like igt_device_get_pci_string() could work for
>> lib/igt_perf.c/i915_type_id().
> 
> same as above
> 
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/52

And thanks again!

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-12-03 13:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 11:05 [igt-dev] [PATCH i-g-t 0/4] device selection && lsgpu Arkadiusz Hiler
2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_list: Update, clean-up and document igt_list Arkadiusz Hiler
2019-10-24 11:16   ` Chris Wilson
2019-10-24 11:20     ` Arkadiusz Hiler
2019-10-24 11:23       ` Chris Wilson
2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 2/4] Introduce device selection API Arkadiusz Hiler
     [not found]   ` <20191024164030.GA7823@zkempczy-mobl2>
2019-10-28 13:06     ` Arkadiusz Hiler
2019-11-15 13:31   ` Petri Latvala
2019-11-18 11:55   ` Petri Latvala
2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool Arkadiusz Hiler
2019-10-24 13:06   ` Chris Wilson
2019-10-28 11:21     ` Arkadiusz Hiler
2019-11-18 11:58   ` Petri Latvala
2019-10-24 11:05 ` [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT Arkadiusz Hiler
2019-11-18 12:14   ` Petri Latvala
2019-11-19 14:18     ` Arkadiusz Hiler
2019-11-20  9:31   ` Petri Latvala
2019-10-24 13:00 ` [igt-dev] ✓ Fi.CI.BAT: success for device selection && lsgpu Patchwork
2019-10-25 17:07 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2019-11-12 10:14 ` [igt-dev] [PATCH i-g-t 0/4] " Tvrtko Ursulin
2019-12-02 12:37   ` Arkadiusz Hiler
2019-12-03 13:59     ` Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.