[igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[Arkadiusz Hiler]

From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>

Return value of udev_enumerate_add_match_property() was not being checked
in scan_drm_devices().

Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 lib/igt_device_scan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 601d6fb0..7b0fc00e 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -532,7 +532,7 @@ static void scan_drm_devices(void)
 	ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
 	igt_assert(!ret);
 
-	udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
+	ret = udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
 	igt_assert(!ret);
 
 	ret = udev_enumerate_scan_devices(enumerate);
-- 
2.25.4

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

[igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core

[Arkadiusz Hiler]

From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>

igt_device_scan can now be used as a separate library which only depends
glib and libudev - some IGT internals are being stubbed in this case.

Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 lib/Makefile.am       |  8 ++++++-
 lib/drmtest.c         |  1 +
 lib/igt_device_scan.c |  6 ++---
 lib/igt_device_scan.h |  8 ++++++-
 lib/igt_tools_stub.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
 lib/meson.build       | 15 +++++++++++++
 6 files changed, 83 insertions(+), 6 deletions(-)
 create mode 100644 lib/igt_tools_stub.c

diff --git a/lib/Makefile.am b/lib/Makefile.am
index fba7fcab..e9753d3c 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -102,6 +102,12 @@ include Makefile.sources
 
 libintel_tools_la_SOURCES = $(lib_source_list)
 
+libigt_device_scan_la_SOURCES = \
+	igt_list.c		\
+	igt_tools_stub.c	\
+	igt_device_scan.c	\
+	igt_device_scan.h
+
 libigt_perf_la_SOURCES = \
 	igt_perf.c	 \
 	igt_perf.h
@@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
 
 lib_LTLIBRARIES = libi915_perf.la
 
-noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
+noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
 noinst_HEADERS = check-ndebug.h
 
 if !HAVE_LIBDRM_INTEL
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 70fd64c9..bced1453 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -53,6 +53,7 @@
 #include "intel_io.h"
 #include "igt_debugfs.h"
 #include "igt_device.h"
+#include "igt_params.h"
 #include "igt_gt.h"
 #include "igt_kmod.h"
 #include "igt_sysfs.h"
diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 7b0fc00e..be854b01 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -22,11 +22,8 @@
  *
  */
 
-#include "igt.h"
-#include "igt_list.h"
-#include "igt_sysfs.h"
-#include "igt_device.h"
 #include "igt_device_scan.h"
+#include "igt_list.h"
 #include <glib.h>
 #include <libudev.h>
 #include <linux/limits.h>
@@ -34,6 +31,7 @@
 #include <sys/types.h>
 #include <dirent.h>
 #include <fcntl.h>
+#include "igt_core.h"
 
 /**
  * SECTION:igt_device_scan
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index 24eafe62..ad632145 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -26,7 +26,13 @@
 #define __IGT_DEVICE_SCAN_H__
 
 #include <limits.h>
-#include <igt.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <errno.h>
 
 enum igt_devices_print_type {
 	IGT_PRINT_SIMPLE,
diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
new file mode 100644
index 00000000..9a0ec621
--- /dev/null
+++ b/lib/igt_tools_stub.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright © 2020 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_core.h"
+
+/* Stub for igt_log, this stub will simply print the msg on stderr device.
+ * Domain and log level are ignored.
+ */
+
+void igt_log(const char *domain, enum igt_log_level level, const char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
+}
+
+
+/* Stub for __igt_fail_assert, this stub will simply print the msg on stderr device and
+ * exit the application, domain and log level are ignored.
+ */
+
+void __igt_fail_assert(const char *domain, const char *file,
+		       const int line, const char *func, const char *assertion,
+		       const char *format, ...)
+{
+	fprintf(stderr, "%s: %d %s Failed assertion: %s\n", file, line,
+		func, assertion);
+	exit(1);
+}
diff --git a/lib/meson.build b/lib/meson.build
index 99aee6ee..6cf78663 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -172,6 +172,21 @@ lib_igt_perf_build = static_library('igt_perf',
 lib_igt_perf = declare_dependency(link_with : lib_igt_perf_build,
 				  include_directories : inc)
 
+scan_dep = [
+	glib,
+	libudev,
+]
+
+lib_igt_device_scan_build = static_library('igt_device_scan',
+	['igt_device_scan.c',
+	'igt_list.c',
+	'igt_tools_stub.c',
+	],
+	dependencies : scan_dep,
+	include_directories : inc)
+
+lib_igt_device_scan = declare_dependency(link_with : lib_igt_device_scan_build,
+				  include_directories : inc)
 
 i915_perf_files = [
   'igt_list.c',
-- 
2.25.4

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

[igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top

[Arkadiusz Hiler]

From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>

Will be used in the next patch.

1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
2. igt_device_find_first_discrete_card(): try to find first discrete GPU
3. igt_devices_free(): Free device buffers created during scan

Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
 lib/igt_device_scan.h |  7 +++-
 2 files changed, 74 insertions(+), 13 deletions(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index be854b01..4daabbd9 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -164,6 +164,7 @@ struct igt_device {
 	/* For pci subsystem */
 	char *vendor;
 	char *device;
+	char *pci_slot_name;
 
 	struct igt_list_head link;
 };
@@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
 #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
+/*
+ * Get PCI_SLOT_NAME property, it should be in format of
+ * xxxx:yy:zz.z
+ */
+static void set_pci_slot_name(struct igt_device *dev)
+{
+	const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
+
+	if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
+		return;
+	dev->pci_slot_name = strdup(pci_slot_name);
+}
+
+/*
+ * Gets PCI_ID property, splits to xxxx:yyyy and stores
  * xxxx to dev->vendor and yyyy to dev->device for
  * faster access.
  */
@@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
 	return NULL;
 }
 
+static void
+__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
+{
+	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);
+
+	if (dev->pci_slot_name != NULL)
+		strncpy(card->pci_slot_name, dev->pci_slot_name,
+			PCI_SLOT_NAME_SIZE + 1);
+}
+
+/*
+ * Iterate over all igt_devices array and find first discrete card.
+ * card->pci_slot_name will be updated only if a discrete card is present.
+ */
+void igt_device_find_first_discrete_card(struct igt_device_card *card)
+{
+	struct igt_device *dev;
+
+	igt_list_for_each_entry(dev, &igt_devs.all, link) {
+
+		if (!is_pci_subsystem(dev))
+			continue;
+
+		if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
+			__copy_dev_to_card(dev, card);
+			break;
+		}
+	}
+}
+
 static struct igt_device *igt_device_from_syspath(const char *syspath)
 {
 	struct igt_device *dev;
@@ -445,6 +499,7 @@ static void update_or_add_parent(struct udev_device *dev,
 		parent_idev = igt_device_new_from_udev(parent_dev);
 		if (is_pci_subsystem(parent_idev)) {
 			set_vendor_device(parent_idev);
+			set_pci_slot_name(parent_idev);
 		}
 		igt_list_add_tail(&parent_idev->link, &igt_devs.all);
 	}
@@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device *dev)
 	free(dev->drm_render);
 	free(dev->vendor);
 	free(dev->device);
+	free(dev->pci_slot_name);
 	g_hash_table_destroy(dev->attrs_ht);
 	g_hash_table_destroy(dev->props_ht);
 }
 
+void igt_devices_free(void)
+{
+	struct igt_device *dev, *tmp;
+
+	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
+		igt_device_free(dev);
+		free(dev);
+	}
+}
+
 /**
  * igt_devices_scan
  * @force: enforce scanning devices
@@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter, struct igt_device_card *card)
 	/* 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);
+	__copy_dev_to_card(dev, card);
 
 	return true;
 }
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index ad632145..16d75320 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -39,10 +39,13 @@ enum igt_devices_print_type {
 	IGT_PRINT_DETAIL,
 };
 
+#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
+#define PCI_SLOT_NAME_SIZE 12
 struct igt_device_card {
 	char subsystem[NAME_MAX];
 	char card[NAME_MAX];
 	char render[NAME_MAX];
+	char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
 };
 
 void igt_devices_scan(bool force);
@@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type printtype);
 void igt_devices_print_vendors(void);
 void igt_device_print_filter_types(void);
 
+void igt_devices_free(void);
+
 /*
  * Handle device filter collection array.
  * IGT can store/retrieve filters passed by user using '--device' args.
@@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
 
 /* Use filter to match the device and fill card structure */
 bool igt_device_card_match(const char *filter, struct igt_device_card *card);
-
+void igt_device_find_first_discrete_card(struct igt_device_card *card);
 int igt_open_card(struct igt_device_card *card);
 int igt_open_render(struct igt_device_card *card);
 
-- 
2.25.4

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

[igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection

[Arkadiusz Hiler]

From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>

In intel_gpu_top device path was hard coded for integrated GPU.

With this patch we:
 * use igt_device_scan library for device scanning,
 * make discrete GPU the default one,
 * provided options for card selection.

Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 man/intel_gpu_top.rst |  29 +++++++++-
 tools/Makefile.am     |   2 +-
 tools/intel_gpu_top.c | 127 ++++++++++++++++++++++++++++++++++++------
 tools/meson.build     |   2 +-
 4 files changed, 140 insertions(+), 20 deletions(-)

diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
index d487baca..5552e969 100644
--- a/man/intel_gpu_top.rst
+++ b/man/intel_gpu_top.rst
@@ -7,9 +7,9 @@ Display a top-like summary of Intel GPU usage
 ---------------------------------------------
 .. include:: defs.rst
 :Author: IGT Developers <igt-dev@lists.freedesktop.org>
-:Date: 2019-02-08
+:Date: 2020-03-18
 :Version: |PACKAGE_STRING|
-:Copyright: 2009,2011,2012,2016,2018,2019 Intel Corporation
+:Copyright: 2009,2011,2012,2016,2018,2019,2020 Intel Corporation
 :Manual section: |MANUAL_SECTION|
 :Manual group: |MANUAL_GROUP|
 
@@ -43,6 +43,31 @@ OPTIONS
 
 -s <ms>
     Refresh period in milliseconds.
+-L
+    List available GPUs on the platform.
+-d
+    Select a specific GPU using supported filter.
+
+
+DEVICE SELECTION
+================
+
+User can select specific GPU for performance monitoring on platform where multiple GPUs are available.
+A GPU can be selected by sysfs path, drm node or using various PCI sub filters.
+
+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
 
 LIMITATIONS
 ===========
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 9352b41c..f97f9e08 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -24,4 +24,4 @@ AM_CFLAGS = $(DEBUG_CFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \
 LDADD = $(top_builddir)/lib/libintel_tools.la
 AM_LDFLAGS = -Wl,--as-needed
 
-intel_gpu_top_LDADD = $(top_builddir)/lib/libigt_perf.la
+intel_gpu_top_LDADD = $(top_builddir)/lib/libigt_perf.la $(top_builddir)/lib/libigt_device_scan.la $(LIBUDEV_LIBS) $(GLIB_LIBS)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 8197482d..b3bbe505 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -22,6 +22,7 @@
  */
 
 #include <stdio.h>
+#include "igt_device_scan.h"
 #include <sys/types.h>
 #include <dirent.h>
 #include <stdint.h>
@@ -94,7 +95,16 @@ struct engines {
 	struct pmu_counter imc_reads;
 	struct pmu_counter imc_writes;
 
+	bool discrete;
+	char *device;
+
+	/* Do not edit below this line.
+	 * This structure is reallocated every time a new engine is
+	 * found and size is increased by sizeof (engine).
+	 */
+
 	struct engine engine;
+
 };
 
 static uint64_t
@@ -168,14 +178,20 @@ static int engine_cmp(const void *__a, const void *__b)
 		return a->instance - b->instance;
 }
 
-static struct engines *discover_engines(void)
+#define is_igpu_pci(x) (strcmp(x, "0000:00:02.0") == 0)
+#define is_igpu(x) (strcmp(x, "i915") == 0)
+
+static struct engines *discover_engines(char *device)
 {
-	const char *sysfs_root = "/sys/devices/i915/events";
+	char sysfs_root[PATH_MAX];
 	struct engines *engines;
 	struct dirent *dent;
 	int ret = 0;
 	DIR *d;
 
+	snprintf(sysfs_root, sizeof(sysfs_root),
+		 "/sys/devices/%s/events", device);
+
 	engines = malloc(sizeof(struct engines));
 	if (!engines)
 		return NULL;
@@ -183,6 +199,8 @@ static struct engines *discover_engines(void)
 	memset(engines, 0, sizeof(*engines));
 
 	engines->num_engines = 0;
+	engines->device = device;
+	engines->discrete = !is_igpu(device);
 
 	d = opendir(sysfs_root);
 	if (!d)
@@ -497,7 +515,7 @@ static int pmu_init(struct engines *engines)
 	}
 
 	engines->rapl_fd = -1;
-	if (rapl_type_id()) {
+	if (!engines->discrete && rapl_type_id()) {
 		engines->rapl_scale = rapl_gpu_power_scale();
 		engines->rapl_unit = rapl_gpu_power_unit();
 		if (!engines->rapl_unit)
@@ -695,8 +713,11 @@ usage(const char *appname)
 		"\t[-l]            List plain text data.\n"
 		"\t[-o <file|->]   Output to specified file or '-' for standard out.\n"
 		"\t[-s <ms>]       Refresh period in milliseconds (default %ums).\n"
+		"\t[-L]            List all cards.\n"
+		"\t[-d <device>]   Device filter, please check manual page for more details.\n"
 		"\n",
 		appname, DEFAULT_PERIOD_MS);
+	igt_device_print_filter_types();
 }
 
 static enum {
@@ -1082,13 +1103,18 @@ print_header(struct engines *engines, double t,
 	if (output_mode == INTERACTIVE) {
 		printf("\033[H\033[J");
 
-		if (lines++ < con_h)
-			printf("intel-gpu-top - %s/%s MHz;  %s%% RC6; %s %s; %s irqs/s\n",
-			       freq_items[1].buf, freq_items[0].buf,
-			       rc6_items[0].buf, power_items[0].buf,
-			       engines->rapl_unit,
-			       irq_items[0].buf);
-
+		if (lines++ < con_h) {
+			if (!engines->discrete)
+				printf("intel-gpu-top - %s/%s MHz;  %s%% RC6; %s %s; %s irqs/s\n",
+					freq_items[1].buf, freq_items[0].buf,
+					rc6_items[0].buf, power_items[0].buf,
+					engines->rapl_unit,
+					irq_items[0].buf);
+			else
+				printf("intel-gpu-top - %s/%s MHz;  %s%% RC6; %s irqs/s\n",
+					freq_items[1].buf, freq_items[0].buf,
+					rc6_items[0].buf, irq_items[0].buf);
+		}
 		if (lines++ < con_h)
 			printf("\n");
 	}
@@ -1249,6 +1275,33 @@ static void sigint_handler(int  sig)
 	stop_top = true;
 }
 
+/* tr_pmu_name()
+ *
+ * Transliterate pci_slot_id to sysfs device name entry for discrete GPU.
+ * Discrete GPU PCI ID   ("xxxx:yy:zz.z")       device = "i915_xxxx_yy_zz.z".
+ */
+static char *tr_pmu_name(struct igt_device_card *card)
+{
+	int ret;
+	const int bufsize = 18;
+	char *buf, *device = NULL;
+
+	assert(card->pci_slot_name[0]);
+
+	device = malloc(bufsize);
+	assert(device);
+
+	ret = snprintf(device, bufsize, "i915_%s", card->pci_slot_name);
+	assert(ret == (bufsize-1));
+
+	buf = device;
+	for (; *buf; buf++)
+		if (*buf == ':')
+			*buf = '_';
+
+	return device;
+}
+
 int main(int argc, char **argv)
 {
 	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
@@ -1256,10 +1309,14 @@ int main(int argc, char **argv)
 	char *output_path = NULL;
 	struct engines *engines;
 	unsigned int i;
-	int ret, ch;
+	int ret = 0, ch;
+	bool list_device = false;
+	enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
+	char *pmu_device, *opt_device = NULL;
+	struct igt_device_card card;
 
 	/* Parse options */
-	while ((ch = getopt(argc, argv, "o:s:Jlh")) != -1) {
+	while ((ch = getopt(argc, argv, "o:s:d:JLlh")) != -1) {
 		switch (ch) {
 		case 'o':
 			output_path = optarg;
@@ -1267,9 +1324,15 @@ int main(int argc, char **argv)
 		case 's':
 			period_us = atoi(optarg) * 1000;
 			break;
+		case 'd':
+			opt_device = strdup(optarg);
+			break;
 		case 'J':
 			output_mode = JSON;
 			break;
+		case 'L':
+			list_device = true;
+			break;
 		case 'l':
 			output_mode = STDOUT;
 			break;
@@ -1320,19 +1383,46 @@ int main(int argc, char **argv)
 		break;
 	};
 
-	engines = discover_engines();
+	igt_devices_scan(false);
+
+	if (list_device) {
+		igt_devices_print(printtype);
+		goto exit;
+	}
+
+	if (opt_device != NULL) {
+		/* Free opt device now as its not needed further. */
+		ret = igt_device_card_match(opt_device, &card);
+		free(opt_device);
+		if (!ret) {
+			fprintf(stderr, "Requested device %s not found!\n", opt_device);
+			ret = 1;
+			goto exit;
+		}
+	} else {
+		igt_device_find_first_discrete_card(&card);
+	}
+
+	if (card.pci_slot_name[0] && !is_igpu_pci(card.pci_slot_name))
+		pmu_device = tr_pmu_name(&card);
+	else
+		pmu_device = strdup("i915");
+
+	engines = discover_engines(pmu_device);
 	if (!engines) {
 		fprintf(stderr,
 			"Failed to detect engines! (%s)\n(Kernel 4.16 or newer is required for i915 PMU support.)\n",
 			strerror(errno));
-		return 1;
+		ret = 1;
+		goto err;
 	}
 
 	ret = pmu_init(engines);
 	if (ret) {
 		fprintf(stderr,
 			"Failed to initialize PMU! (%s)\n", strerror(errno));
-		return 1;
+		ret = 1;
+		goto err;
 	}
 
 	pmu_sample(engines);
@@ -1384,5 +1474,10 @@ int main(int argc, char **argv)
 		usleep(period_us);
 	}
 
-	return 0;
+err:
+	free(engines);
+	free(pmu_device);
+exit:
+	igt_devices_free();
+	return ret;
 }
diff --git a/tools/meson.build b/tools/meson.build
index 59b56d5d..34f95e79 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -93,7 +93,7 @@ install_subdir('registers', install_dir : datadir,
 executable('intel_gpu_top', 'intel_gpu_top.c',
 	   install : true,
 	   install_rpath : bindir_rpathdir,
-	   dependencies : lib_igt_perf)
+	   dependencies : [lib_igt_perf,lib_igt_device_scan])
 
 executable('amd_hdmi_compliance', 'amd_hdmi_compliance.c',
 	   dependencies : [tool_deps],
-- 
2.25.4

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
URL   : https://patchwork.freedesktop.org/series/77660/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8539 -> IGTPW_4612
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][1] -> [FAIL][2] ([i915#227])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [i915#227]: https://gitlab.freedesktop.org/drm/intel/issues/227


Participating hosts (50 -> 44)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5678 -> IGTPW_4612

  CI-20190529: 20190529
  CI_DRM_8539: 6590da76d8c6a2952b3cde70859bad41c59ed444 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4612: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
  IGT_5678: c1f30ee09ac2e7eb3e8e90245239731a169a6050 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[Lucas De Marchi]

On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> Return value of udev_enumerate_add_match_property() was not being checked
> in scan_drm_devices().
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

> ---
>  lib/igt_device_scan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 601d6fb0..7b0fc00e 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -532,7 +532,7 @@ static void scan_drm_devices(void)
>         ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
>         igt_assert(!ret);
>
> -       udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> +       ret = udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
>         igt_assert(!ret);
>
>         ret = udev_enumerate_scan_devices(enumerate);
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev



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

Re: [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core

[Lucas De Marchi]

On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> igt_device_scan can now be used as a separate library which only depends
> glib and libudev - some IGT internals are being stubbed in this case.
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  lib/Makefile.am       |  8 ++++++-
>  lib/drmtest.c         |  1 +
>  lib/igt_device_scan.c |  6 ++---
>  lib/igt_device_scan.h |  8 ++++++-
>  lib/igt_tools_stub.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
>  lib/meson.build       | 15 +++++++++++++
>  6 files changed, 83 insertions(+), 6 deletions(-)
>  create mode 100644 lib/igt_tools_stub.c
>
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index fba7fcab..e9753d3c 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -102,6 +102,12 @@ include Makefile.sources
>
>  libintel_tools_la_SOURCES = $(lib_source_list)
>
> +libigt_device_scan_la_SOURCES = \
> +       igt_list.c              \
> +       igt_tools_stub.c        \
> +       igt_device_scan.c       \
> +       igt_device_scan.h
> +
>  libigt_perf_la_SOURCES = \
>         igt_perf.c       \
>         igt_perf.h
> @@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
>
>  lib_LTLIBRARIES = libi915_perf.la
>
> -noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
> +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
>  noinst_HEADERS = check-ndebug.h
>
>  if !HAVE_LIBDRM_INTEL
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 70fd64c9..bced1453 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -53,6 +53,7 @@
>  #include "intel_io.h"
>  #include "igt_debugfs.h"
>  #include "igt_device.h"
> +#include "igt_params.h"
>  #include "igt_gt.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 7b0fc00e..be854b01 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -22,11 +22,8 @@
>   *
>   */
>
> -#include "igt.h"
> -#include "igt_list.h"
> -#include "igt_sysfs.h"
> -#include "igt_device.h"
>  #include "igt_device_scan.h"
> +#include "igt_list.h"
>  #include <glib.h>
>  #include <libudev.h>
>  #include <linux/limits.h>
> @@ -34,6 +31,7 @@
>  #include <sys/types.h>
>  #include <dirent.h>
>  #include <fcntl.h>
> +#include "igt_core.h"
>
>  /**
>   * SECTION:igt_device_scan
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index 24eafe62..ad632145 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -26,7 +26,13 @@
>  #define __IGT_DEVICE_SCAN_H__
>
>  #include <limits.h>
> -#include <igt.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <getopt.h>
> +#include <errno.h>

shouldn't these be sorted?

>
>  enum igt_devices_print_type {
>         IGT_PRINT_SIMPLE,
> diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
> new file mode 100644
> index 00000000..9a0ec621
> --- /dev/null
> +++ b/lib/igt_tools_stub.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright © 2020 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.
> + *
> + */

are we still in a mix of SPDX  + full text?

> +#include "igt_core.h"
> +
> +/* Stub for igt_log, this stub will simply print the msg on stderr device.
> + * Domain and log level are ignored.
> + */
> +
> +void igt_log(const char *domain, enum igt_log_level level, const char *format, ...)
> +{
> +       va_list args;
> +
> +       va_start(args, format);
> +       vfprintf(stderr, format, args);
> +       va_end(args);
> +}
> +
> +
> +/* Stub for __igt_fail_assert, this stub will simply print the msg on stderr device and
> + * exit the application, domain and log level are ignored.
> + */
> +
> +void __igt_fail_assert(const char *domain, const char *file,
> +                      const int line, const char *func, const char *assertion,
> +                      const char *format, ...)
> +{
> +       fprintf(stderr, "%s: %d %s Failed assertion: %s\n", file, line,
> +               func, assertion);
> +       exit(1);
> +}

maybe make these weak symbols so it still "works" if we bring in igt_core?

Lucas De Marchi

> diff --git a/lib/meson.build b/lib/meson.build
> index 99aee6ee..6cf78663 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -172,6 +172,21 @@ lib_igt_perf_build = static_library('igt_perf',
>  lib_igt_perf = declare_dependency(link_with : lib_igt_perf_build,
>                                   include_directories : inc)
>
> +scan_dep = [
> +       glib,
> +       libudev,
> +]
> +
> +lib_igt_device_scan_build = static_library('igt_device_scan',
> +       ['igt_device_scan.c',
> +       'igt_list.c',
> +       'igt_tools_stub.c',
> +       ],
> +       dependencies : scan_dep,
> +       include_directories : inc)
> +
> +lib_igt_device_scan = declare_dependency(link_with : lib_igt_device_scan_build,
> +                                 include_directories : inc)
>
>  i915_perf_files = [
>    'igt_list.c',
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top

[Lucas De Marchi]

On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> Will be used in the next patch.
>
> 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
> 2. igt_device_find_first_discrete_card(): try to find first discrete GPU
> 3. igt_devices_free(): Free device buffers created during scan
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
>  lib/igt_device_scan.h |  7 +++-
>  2 files changed, 74 insertions(+), 13 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index be854b01..4daabbd9 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -164,6 +164,7 @@ struct igt_device {
>         /* For pci subsystem */
>         char *vendor;
>         char *device;
> +       char *pci_slot_name;
>
>         struct igt_list_head link;
>  };
> @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
>  #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
> +/*
> + * Get PCI_SLOT_NAME property, it should be in format of
> + * xxxx:yy:zz.z
> + */
> +static void set_pci_slot_name(struct igt_device *dev)
> +{
> +       const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> +
> +       if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
> +               return;
> +       dev->pci_slot_name = strdup(pci_slot_name);
> +}
> +
> +/*
> + * Gets PCI_ID property, splits to xxxx:yyyy and stores
>   * xxxx to dev->vendor and yyyy to dev->device for
>   * faster access.
>   */
> @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
>         return NULL;
>  }
>
> +static void
> +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
> +{
> +       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);
> +
> +       if (dev->pci_slot_name != NULL)
> +               strncpy(card->pci_slot_name, dev->pci_slot_name,
> +                       PCI_SLOT_NAME_SIZE + 1);
> +}
> +
> +/*
> + * Iterate over all igt_devices array and find first discrete card.
> + * card->pci_slot_name will be updated only if a discrete card is present.
> + */
> +void igt_device_find_first_discrete_card(struct igt_device_card *card)
> +{
> +       struct igt_device *dev;
> +
> +       igt_list_for_each_entry(dev, &igt_devs.all, link) {
> +
> +               if (!is_pci_subsystem(dev))
> +                       continue;
> +
> +               if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
> +                       __copy_dev_to_card(dev, card);

why do we need to make it prefer discrete? rather than "use the first
one if no one was specified"?

Lucas De Marchi

> +                       break;
> +               }
> +       }
> +}
> +
>  static struct igt_device *igt_device_from_syspath(const char *syspath)
>  {
>         struct igt_device *dev;
> @@ -445,6 +499,7 @@ static void update_or_add_parent(struct udev_device *dev,
>                 parent_idev = igt_device_new_from_udev(parent_dev);
>                 if (is_pci_subsystem(parent_idev)) {
>                         set_vendor_device(parent_idev);
> +                       set_pci_slot_name(parent_idev);
>                 }
>                 igt_list_add_tail(&parent_idev->link, &igt_devs.all);
>         }
> @@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device *dev)
>         free(dev->drm_render);
>         free(dev->vendor);
>         free(dev->device);
> +       free(dev->pci_slot_name);
>         g_hash_table_destroy(dev->attrs_ht);
>         g_hash_table_destroy(dev->props_ht);
>  }
>
> +void igt_devices_free(void)
> +{
> +       struct igt_device *dev, *tmp;
> +
> +       igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
> +               igt_device_free(dev);
> +               free(dev);
> +       }
> +}
> +
>  /**
>   * igt_devices_scan
>   * @force: enforce scanning devices
> @@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter, struct igt_device_card *card)
>         /* 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);
> +       __copy_dev_to_card(dev, card);
>
>         return true;
>  }
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index ad632145..16d75320 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -39,10 +39,13 @@ enum igt_devices_print_type {
>         IGT_PRINT_DETAIL,
>  };
>
> +#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
> +#define PCI_SLOT_NAME_SIZE 12
>  struct igt_device_card {
>         char subsystem[NAME_MAX];
>         char card[NAME_MAX];
>         char render[NAME_MAX];
> +       char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
>  };
>
>  void igt_devices_scan(bool force);
> @@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type printtype);
>  void igt_devices_print_vendors(void);
>  void igt_device_print_filter_types(void);
>
> +void igt_devices_free(void);
> +
>  /*
>   * Handle device filter collection array.
>   * IGT can store/retrieve filters passed by user using '--device' args.
> @@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
>
>  /* Use filter to match the device and fill card structure */
>  bool igt_device_card_match(const char *filter, struct igt_device_card *card);
> -
> +void igt_device_find_first_discrete_card(struct igt_device_card *card);
>  int igt_open_card(struct igt_device_card *card);
>  int igt_open_render(struct igt_device_card *card);
>
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev



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

[igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
URL   : https://patchwork.freedesktop.org/series/77660/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8539_full -> IGTPW_4612_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4612_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4612_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_4612/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_flush@basic-batch-kernel-default-cmd:
    - shard-hsw:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-hsw2/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw2/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html

  * igt@gen9_exec_parse@cmd-crossing-page:
    - shard-glk:          [PASS][3] -> [TIMEOUT][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk5/igt@gen9_exec_parse@cmd-crossing-page.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@gen9_exec_parse@cmd-crossing-page.html

  * igt@runner@aborted:
    - shard-hsw:          NOTRUN -> ([FAIL][5], [FAIL][6])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw7/igt@runner@aborted.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw2/igt@runner@aborted.html

  
#### Warnings ####

  * igt@kms_ccs@pipe-d-crc-primary-rotation-180:
    - shard-glk:          [SKIP][7] ([fdo#109271]) -> [TIMEOUT][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk9/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html

  
#### Suppressed ####

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

  * {igt@gem_exec_reloc@basic-concurrent16}:
    - shard-glk:          NOTRUN -> [TIMEOUT][9]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@gem_exec_reloc@basic-concurrent16.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [PASS][10] -> [DMESG-WARN][11] ([i915#1436] / [i915#716])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl1/igt@gen9_exec_parse@allowed-all.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@gen9_exec_parse@allowed-all.html
    - shard-glk:          [PASS][12] -> [DMESG-WARN][13] ([i915#1436] / [i915#716])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk1/igt@gen9_exec_parse@allowed-all.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk7/igt@gen9_exec_parse@allowed-all.html

  * igt@gen9_exec_parse@cmd-crossing-page:
    - shard-apl:          [PASS][14] -> [TIMEOUT][15] ([i915#1635]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@gen9_exec_parse@cmd-crossing-page.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl6/igt@gen9_exec_parse@cmd-crossing-page.html

  * igt@i915_pm_rpm@pm-tiling:
    - shard-apl:          [PASS][16] -> [SKIP][17] ([fdo#109271])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl7/igt@i915_pm_rpm@pm-tiling.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl6/igt@i915_pm_rpm@pm-tiling.html
    - shard-glk:          [PASS][18] -> [SKIP][19] ([fdo#109271])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk6/igt@i915_pm_rpm@pm-tiling.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@i915_pm_rpm@pm-tiling.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][20] -> [DMESG-WARN][21] ([i915#180]) +2 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@i915_suspend@forcewake:
    - shard-kbl:          [PASS][22] -> [INCOMPLETE][23] ([i915#155])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl3/igt@i915_suspend@forcewake.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl4/igt@i915_suspend@forcewake.html

  * igt@kms_big_fb@linear-64bpp-rotate-0:
    - shard-glk:          [PASS][24] -> [FAIL][25] ([i915#1119] / [i915#118] / [i915#95])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk7/igt@kms_big_fb@linear-64bpp-rotate-0.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen:
    - shard-kbl:          [PASS][26] -> [FAIL][27] ([i915#54] / [i915#93] / [i915#95])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen:
    - shard-kbl:          [PASS][28] -> [FAIL][29] ([i915#54])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [PASS][30] -> [DMESG-WARN][31] ([i915#180]) +3 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-hsw:          [PASS][32] -> [FAIL][33] ([i915#57])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-hsw8/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html

  * igt@kms_flip_tiling@flip-changes-tiling:
    - shard-apl:          [PASS][34] -> [FAIL][35] ([i915#95])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl6/igt@kms_flip_tiling@flip-changes-tiling.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl7/igt@kms_flip_tiling@flip-changes-tiling.html
    - shard-kbl:          [PASS][36] -> [FAIL][37] ([i915#699] / [i915#93] / [i915#95])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl6/igt@kms_flip_tiling@flip-changes-tiling.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl2/igt@kms_flip_tiling@flip-changes-tiling.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - shard-apl:          [PASS][38] -> [FAIL][39] ([i915#53] / [i915#95]) +1 similar issue
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [PASS][40] -> [FAIL][41] ([i915#53] / [i915#93] / [i915#95]) +1 similar issue
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl1/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [PASS][42] -> [SKIP][43] ([fdo#109441]) +3 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb6/igt@kms_psr@psr2_no_drrs.html

  
#### Possible fixes ####

  * {igt@gem_exec_endless@dispatch@vcs0}:
    - shard-apl:          [SKIP][44] ([fdo#109271]) -> [PASS][45] +2 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl4/igt@gem_exec_endless@dispatch@vcs0.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl2/igt@gem_exec_endless@dispatch@vcs0.html

  * {igt@gem_exec_endless@dispatch@vcs1}:
    - shard-tglb:         [SKIP][46] ([i915#1929]) -> [PASS][47] +4 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb8/igt@gem_exec_endless@dispatch@vcs1.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb2/igt@gem_exec_endless@dispatch@vcs1.html
    - shard-kbl:          [SKIP][48] ([fdo#109271]) -> [PASS][49] +3 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl6/igt@gem_exec_endless@dispatch@vcs1.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl6/igt@gem_exec_endless@dispatch@vcs1.html
    - shard-iclb:         [SKIP][50] ([fdo#109276] / [i915#1929]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@gem_exec_endless@dispatch@vcs1.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb4/igt@gem_exec_endless@dispatch@vcs1.html

  * {igt@gem_exec_endless@dispatch@vecs0}:
    - shard-iclb:         [SKIP][52] ([i915#1929]) -> [PASS][53] +3 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@gem_exec_endless@dispatch@vecs0.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb4/igt@gem_exec_endless@dispatch@vecs0.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-tglb:         [SKIP][54] ([i915#1904]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb8/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb7/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@kms_big_fb@linear-32bpp-rotate-180:
    - shard-apl:          [FAIL][56] ([i915#1119] / [i915#95]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl4/igt@kms_big_fb@linear-32bpp-rotate-180.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl4/igt@kms_big_fb@linear-32bpp-rotate-180.html
    - shard-kbl:          [FAIL][58] ([i915#1119] / [i915#93] / [i915#95]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_big_fb@linear-32bpp-rotate-180.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl1/igt@kms_big_fb@linear-32bpp-rotate-180.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-0:
    - shard-glk:          [FAIL][60] ([i915#1119] / [i915#118] / [i915#95]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk8/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen:
    - shard-kbl:          [FAIL][62] ([i915#54] / [i915#93] / [i915#95]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          [INCOMPLETE][64] ([i915#61]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw2/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@a-dp1}:
    - shard-kbl:          [DMESG-WARN][66] ([i915#180]) -> [PASS][67] +7 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * {igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1}:
    - shard-apl:          [FAIL][68] ([i915#407]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl7/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-kbl:          [DMESG-WARN][70] ([i915#180] / [i915#93] / [i915#95]) -> [PASS][71]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_hdr@bpc-switch-suspend.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl4/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][72] ([i915#180]) -> [PASS][73] +3 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][74] ([fdo#109441]) -> [PASS][75] +2 similar issues
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][76] ([i915#31]) -> [PASS][77]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_setmode@basic.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl1/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][78] ([i915#588]) -> [SKIP][79] ([i915#658])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb3/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [SKIP][80] ([i915#468]) -> [FAIL][81] ([i915#454])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb8/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [FAIL][82] ([i915#454]) -> [SKIP][83] ([i915#468])
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb1/igt@i915_pm_dc@dc6-psr.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb2/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_busy@basic-modeset-pipe-d:
    - shard-apl:          [TIMEOUT][84] ([i915#1635]) -> [SKIP][85] ([fdo#109271])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl7/igt@kms_busy@basic-modeset-pipe-d.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl1/igt@kms_busy@basic-modeset-pipe-d.html

  * igt@kms_ccs@pipe-d-crc-primary-rotation-180:
    - shard-apl:          [SKIP][86] ([fdo#109271]) -> [TIMEOUT][87] ([i915#1635]) +2 similar issues
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl6/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html

  * igt@kms_content_protection@legacy:
    - shard-apl:          [FAIL][88] ([fdo#110321] / [fdo#110336]) -> [TIMEOUT][89] ([i915#1319] / [i915#1635])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@kms_content_protection@legacy.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@kms_content_protection@legacy.html

  * igt@kms_content_protection@lic:
    - shard-kbl:          [TIMEOUT][90] ([i915#1319]) -> [FAIL][91] ([fdo#110321] / [i915#93] / [i915#95])
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl7/igt@kms_content_protection@lic.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl2/igt@kms_content_protection@lic.html

  * igt@kms_cursor_legacy@cursorb-vs-flipa-toggle:
    - shard-glk:          [DMESG-FAIL][92] ([i915#1925]) -> [DMESG-WARN][93] ([i915#1926])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk1/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk2/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1904]: https://gitlab.freedesktop.org/drm/intel/issues/1904
  [i915#1925]: https://gitlab.freedesktop.org/drm/intel/issues/1925
  [i915#1926]: https://gitlab.freedesktop.org/drm/intel/issues/1926
  [i915#1929]: https://gitlab.freedesktop.org/drm/intel/issues/1929
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#407]: https://gitlab.freedesktop.org/drm/intel/issues/407
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#53]: https://gitlab.freedesktop.org/drm/intel/issues/53
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#57]: https://gitlab.freedesktop.org/drm/intel/issues/57
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#699]: https://gitlab.freedesktop.org/drm/intel/issues/699
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 8)
------------------------------

  Missing    (3): pig-skl-6260u pig-glk-j5005 pig-icl-1065g7 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5678 -> IGTPW_4612
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_8539: 6590da76d8c6a2952b3cde70859bad41c59ed444 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4612: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
  IGT_5678: c1f30ee09ac2e7eb3e8e90245239731a169a6050 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top

[Siddiqui, Ayaz A]

> -----Original Message-----
> From: Lucas De Marchi <lucas.de.marchi@gmail.com>
> Sent: Tuesday, May 26, 2020 9:57 PM
> To: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Ursulin, Tvrtko
> <tvrtko.ursulin@intel.com>; Latvala, Petri <petri.latvala@intel.com>;
> Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra
> helpers for intel_gpu_top
> 
> On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> wrote:
> >
> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> >
> > Will be used in the next patch.
> >
> > 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device 2.
> > igt_device_find_first_discrete_card(): try to find first discrete GPU
> > 3. igt_devices_free(): Free device buffers created during scan
> >
> > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> >  lib/igt_device_scan.c | 80
> > ++++++++++++++++++++++++++++++++++++-------
> >  lib/igt_device_scan.h |  7 +++-
> >  2 files changed, 74 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c index
> > be854b01..4daabbd9 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -164,6 +164,7 @@ struct igt_device {
> >         /* For pci subsystem */
> >         char *vendor;
> >         char *device;
> > +       char *pci_slot_name;
> >
> >         struct igt_list_head link;
> >  };
> > @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev,
> > struct igt_device *idev)  #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
> > +/*
> > + * Get PCI_SLOT_NAME property, it should be in format of
> > + * xxxx:yy:zz.z
> > + */
> > +static void set_pci_slot_name(struct igt_device *dev) {
> > +       const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> > +
> > +       if (!pci_slot_name || (strlen(pci_slot_name) !=
> PCI_SLOT_NAME_SIZE))
> > +               return;
> > +       dev->pci_slot_name = strdup(pci_slot_name); }
> > +
> > +/*
> > + * Gets PCI_ID property, splits to xxxx:yyyy and stores
> >   * xxxx to dev->vendor and yyyy to dev->device for
> >   * faster access.
> >   */
> > @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char
> *subsystem,
> >         return NULL;
> >  }
> >
> > +static void
> > +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card
> > +*card) {
> > +       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);
> > +
> > +       if (dev->pci_slot_name != NULL)
> > +               strncpy(card->pci_slot_name, dev->pci_slot_name,
> > +                       PCI_SLOT_NAME_SIZE + 1); }
> > +
> > +/*
> > + * Iterate over all igt_devices array and find first discrete card.
> > + * card->pci_slot_name will be updated only if a discrete card is present.
> > + */
> > +void igt_device_find_first_discrete_card(struct igt_device_card
> > +*card) {
> > +       struct igt_device *dev;
> > +
> > +       igt_list_for_each_entry(dev, &igt_devs.all, link) {
> > +
> > +               if (!is_pci_subsystem(dev))
> > +                       continue;
> > +
> > +               if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID,
> PCI_SLOT_NAME_SIZE)) != 0) {
> > +                       __copy_dev_to_card(dev, card);
> 
> why do we need to make it prefer discrete? rather than "use the first one if
> no one was specified"?
> 
> Lucas De Marchi
> 
Discreate is usually add on top of integrated GPU if both are present , 
that’s why we are giving preference to discreate. 

Regards
-Ayaz

> > +                       break;
> > +               }
> > +       }
> > +}
> > +
> >  static struct igt_device *igt_device_from_syspath(const char
> > *syspath)  {
> >         struct igt_device *dev;
> > @@ -445,6 +499,7 @@ static void update_or_add_parent(struct
> udev_device *dev,
> >                 parent_idev = igt_device_new_from_udev(parent_dev);
> >                 if (is_pci_subsystem(parent_idev)) {
> >                         set_vendor_device(parent_idev);
> > +                       set_pci_slot_name(parent_idev);
> >                 }
> >                 igt_list_add_tail(&parent_idev->link, &igt_devs.all);
> >         }
> > @@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device
> *dev)
> >         free(dev->drm_render);
> >         free(dev->vendor);
> >         free(dev->device);
> > +       free(dev->pci_slot_name);
> >         g_hash_table_destroy(dev->attrs_ht);
> >         g_hash_table_destroy(dev->props_ht);
> >  }
> >
> > +void igt_devices_free(void)
> > +{
> > +       struct igt_device *dev, *tmp;
> > +
> > +       igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
> > +               igt_device_free(dev);
> > +               free(dev);
> > +       }
> > +}
> > +
> >  /**
> >   * igt_devices_scan
> >   * @force: enforce scanning devices
> > @@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter,
> struct igt_device_card *card)
> >         /* 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);
> > +       __copy_dev_to_card(dev, card);
> >
> >         return true;
> >  }
> > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h index
> > ad632145..16d75320 100644
> > --- a/lib/igt_device_scan.h
> > +++ b/lib/igt_device_scan.h
> > @@ -39,10 +39,13 @@ enum igt_devices_print_type {
> >         IGT_PRINT_DETAIL,
> >  };
> >
> > +#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
> > +#define PCI_SLOT_NAME_SIZE 12
> >  struct igt_device_card {
> >         char subsystem[NAME_MAX];
> >         char card[NAME_MAX];
> >         char render[NAME_MAX];
> > +       char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
> >  };
> >
> >  void igt_devices_scan(bool force);
> > @@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type
> > printtype);  void igt_devices_print_vendors(void);  void
> > igt_device_print_filter_types(void);
> >
> > +void igt_devices_free(void);
> > +
> >  /*
> >   * Handle device filter collection array.
> >   * IGT can store/retrieve filters passed by user using '--device' args.
> > @@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
> >
> >  /* Use filter to match the device and fill card structure */  bool
> > igt_device_card_match(const char *filter, struct igt_device_card
> > *card);
> > -
> > +void igt_device_find_first_discrete_card(struct igt_device_card
> > +*card);
> >  int igt_open_card(struct igt_device_card *card);  int
> > igt_open_render(struct igt_device_card *card);
> >
> > --
> > 2.25.4
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> 
> 
> --
> Lucas De Marchi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core

[Arkadiusz Hiler]

On Tue, May 26, 2020 at 09:14:58AM -0700, Lucas De Marchi wrote:
> On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
> <arkadiusz.hiler@intel.com> wrote:
> >
> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> >
> > igt_device_scan can now be used as a separate library which only depends
> > glib and libudev - some IGT internals are being stubbed in this case.
> >
> > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> >  lib/Makefile.am       |  8 ++++++-
> >  lib/drmtest.c         |  1 +
> >  lib/igt_device_scan.c |  6 ++---
> >  lib/igt_device_scan.h |  8 ++++++-
> >  lib/igt_tools_stub.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
> >  lib/meson.build       | 15 +++++++++++++
> >  6 files changed, 83 insertions(+), 6 deletions(-)
> >  create mode 100644 lib/igt_tools_stub.c
> >
> > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > index fba7fcab..e9753d3c 100644
> > --- a/lib/Makefile.am
> > +++ b/lib/Makefile.am
> > @@ -102,6 +102,12 @@ include Makefile.sources
> >
> >  libintel_tools_la_SOURCES = $(lib_source_list)
> >
> > +libigt_device_scan_la_SOURCES = \
> > +       igt_list.c              \
> > +       igt_tools_stub.c        \
> > +       igt_device_scan.c       \
> > +       igt_device_scan.h
> > +
> >  libigt_perf_la_SOURCES = \
> >         igt_perf.c       \
> >         igt_perf.h
> > @@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
> >
> >  lib_LTLIBRARIES = libi915_perf.la
> >
> > -noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
> > +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
> >  noinst_HEADERS = check-ndebug.h
> >
> >  if !HAVE_LIBDRM_INTEL
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index 70fd64c9..bced1453 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -53,6 +53,7 @@
> >  #include "intel_io.h"
> >  #include "igt_debugfs.h"
> >  #include "igt_device.h"
> > +#include "igt_params.h"
> >  #include "igt_gt.h"
> >  #include "igt_kmod.h"
> >  #include "igt_sysfs.h"
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 7b0fc00e..be854b01 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -22,11 +22,8 @@
> >   *
> >   */
> >
> > -#include "igt.h"
> > -#include "igt_list.h"
> > -#include "igt_sysfs.h"
> > -#include "igt_device.h"
> >  #include "igt_device_scan.h"
> > +#include "igt_list.h"
> >  #include <glib.h>
> >  #include <libudev.h>
> >  #include <linux/limits.h>
> > @@ -34,6 +31,7 @@
> >  #include <sys/types.h>
> >  #include <dirent.h>
> >  #include <fcntl.h>
> > +#include "igt_core.h"
> >
> >  /**
> >   * SECTION:igt_device_scan
> > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > index 24eafe62..ad632145 100644
> > --- a/lib/igt_device_scan.h
> > +++ b/lib/igt_device_scan.h
> > @@ -26,7 +26,13 @@
> >  #define __IGT_DEVICE_SCAN_H__
> >
> >  #include <limits.h>
> > -#include <igt.h>
> > +#include <stdbool.h>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <getopt.h>
> > +#include <errno.h>
> 
> shouldn't these be sorted?

It wouldn't hurt but kernel coding style doesn't mention header sorting.
IGT doesn't enforce anything on top of it ¯\_(ツ)_/¯

> >  enum igt_devices_print_type {
> >         IGT_PRINT_SIMPLE,
> > diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
> > new file mode 100644
> > index 00000000..9a0ec621
> > --- /dev/null
> > +++ b/lib/igt_tools_stub.c
> > @@ -0,0 +1,51 @@
> > +/*
> > + * Copyright © 2020 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.
> > + *
> > + */
> 
> are we still in a mix of SPDX  + full text?

Only stubs and drm uapi seem to have SPDX identifiers as of now. Petri
was researching the topic, I don't remember the conclusion.

> > +#include "igt_core.h"
> > +
> > +/* Stub for igt_log, this stub will simply print the msg on stderr device.
> > + * Domain and log level are ignored.
> > + */
> > +
> > +void igt_log(const char *domain, enum igt_log_level level, const char *format, ...)
> > +{
> > +       va_list args;
> > +
> > +       va_start(args, format);
> > +       vfprintf(stderr, format, args);
> > +       va_end(args);
> > +}
> > +
> > +
> > +/* Stub for __igt_fail_assert, this stub will simply print the msg on stderr device and
> > + * exit the application, domain and log level are ignored.
> > + */
> > +
> > +void __igt_fail_assert(const char *domain, const char *file,
> > +                      const int line, const char *func, const char *assertion,
> > +                      const char *format, ...)
> > +{
> > +       fprintf(stderr, "%s: %d %s Failed assertion: %s\n", file, line,
> > +               func, assertion);
> > +       exit(1);
> > +}
> 
> maybe make these weak symbols so it still "works" if we bring in igt_core?
> 
> Lucas De Marchi

igt_core comes with device_scan, so you would get linking error on the
device_scan functions anyway

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

Re: [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core

[Petri Latvala]

On Wed, May 27, 2020 at 10:48:42AM +0300, Arkadiusz Hiler wrote:
> On Tue, May 26, 2020 at 09:14:58AM -0700, Lucas De Marchi wrote:
> > On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
> > <arkadiusz.hiler@intel.com> wrote:
> > >
> > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > >
> > > igt_device_scan can now be used as a separate library which only depends
> > > glib and libudev - some IGT internals are being stubbed in this case.
> > >
> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > >  lib/Makefile.am       |  8 ++++++-
> > >  lib/drmtest.c         |  1 +
> > >  lib/igt_device_scan.c |  6 ++---
> > >  lib/igt_device_scan.h |  8 ++++++-
> > >  lib/igt_tools_stub.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
> > >  lib/meson.build       | 15 +++++++++++++
> > >  6 files changed, 83 insertions(+), 6 deletions(-)
> > >  create mode 100644 lib/igt_tools_stub.c
> > >
> > > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > > index fba7fcab..e9753d3c 100644
> > > --- a/lib/Makefile.am
> > > +++ b/lib/Makefile.am
> > > @@ -102,6 +102,12 @@ include Makefile.sources
> > >
> > >  libintel_tools_la_SOURCES = $(lib_source_list)
> > >
> > > +libigt_device_scan_la_SOURCES = \
> > > +       igt_list.c              \
> > > +       igt_tools_stub.c        \
> > > +       igt_device_scan.c       \
> > > +       igt_device_scan.h
> > > +
> > >  libigt_perf_la_SOURCES = \
> > >         igt_perf.c       \
> > >         igt_perf.h
> > > @@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
> > >
> > >  lib_LTLIBRARIES = libi915_perf.la
> > >
> > > -noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
> > > +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
> > >  noinst_HEADERS = check-ndebug.h
> > >
> > >  if !HAVE_LIBDRM_INTEL
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index 70fd64c9..bced1453 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -53,6 +53,7 @@
> > >  #include "intel_io.h"
> > >  #include "igt_debugfs.h"
> > >  #include "igt_device.h"
> > > +#include "igt_params.h"
> > >  #include "igt_gt.h"
> > >  #include "igt_kmod.h"
> > >  #include "igt_sysfs.h"
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index 7b0fc00e..be854b01 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -22,11 +22,8 @@
> > >   *
> > >   */
> > >
> > > -#include "igt.h"
> > > -#include "igt_list.h"
> > > -#include "igt_sysfs.h"
> > > -#include "igt_device.h"
> > >  #include "igt_device_scan.h"
> > > +#include "igt_list.h"
> > >  #include <glib.h>
> > >  #include <libudev.h>
> > >  #include <linux/limits.h>
> > > @@ -34,6 +31,7 @@
> > >  #include <sys/types.h>
> > >  #include <dirent.h>
> > >  #include <fcntl.h>
> > > +#include "igt_core.h"
> > >
> > >  /**
> > >   * SECTION:igt_device_scan
> > > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > > index 24eafe62..ad632145 100644
> > > --- a/lib/igt_device_scan.h
> > > +++ b/lib/igt_device_scan.h
> > > @@ -26,7 +26,13 @@
> > >  #define __IGT_DEVICE_SCAN_H__
> > >
> > >  #include <limits.h>
> > > -#include <igt.h>
> > > +#include <stdbool.h>
> > > +#include <stdint.h>
> > > +#include <stdio.h>
> > > +#include <fcntl.h>
> > > +#include <unistd.h>
> > > +#include <getopt.h>
> > > +#include <errno.h>
> > 
> > shouldn't these be sorted?
> 
> It wouldn't hurt but kernel coding style doesn't mention header sorting.
> IGT doesn't enforce anything on top of it ¯\_(ツ)_/¯

If header include blocks are already sorted, keep them sorted, that's
the thumb rule I've used. That said, please use "header-from-igt.h"
and <header-from-system>.


> 
> > >  enum igt_devices_print_type {
> > >         IGT_PRINT_SIMPLE,
> > > diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
> > > new file mode 100644
> > > index 00000000..9a0ec621
> > > --- /dev/null
> > > +++ b/lib/igt_tools_stub.c
> > > @@ -0,0 +1,51 @@
> > > +/*
> > > + * Copyright © 2020 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.
> > > + *
> > > + */
> > 
> > are we still in a mix of SPDX  + full text?
> 
> Only stubs and drm uapi seem to have SPDX identifiers as of now. Petri
> was researching the topic, I don't remember the conclusion.

Converting from a normal text block to SPDX is a no-no. Using SPDX for
new files is ok.


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

Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top

[Tvrtko Ursulin]

On 26/05/2020 17:26, Lucas De Marchi wrote:
> On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler
> <arkadiusz.hiler@intel.com> wrote:
>>
>> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>>
>> Will be used in the next patch.
>>
>> 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
>> 2. igt_device_find_first_discrete_card(): try to find first discrete GPU
>> 3. igt_devices_free(): Free device buffers created during scan
>>
>> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> ---
>>   lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
>>   lib/igt_device_scan.h |  7 +++-
>>   2 files changed, 74 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>> index be854b01..4daabbd9 100644
>> --- a/lib/igt_device_scan.c
>> +++ b/lib/igt_device_scan.c
>> @@ -164,6 +164,7 @@ struct igt_device {
>>          /* For pci subsystem */
>>          char *vendor;
>>          char *device;
>> +       char *pci_slot_name;
>>
>>          struct igt_list_head link;
>>   };
>> @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
>>   #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
>> +/*
>> + * Get PCI_SLOT_NAME property, it should be in format of
>> + * xxxx:yy:zz.z
>> + */
>> +static void set_pci_slot_name(struct igt_device *dev)
>> +{
>> +       const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
>> +
>> +       if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
>> +               return;
>> +       dev->pci_slot_name = strdup(pci_slot_name);
>> +}
>> +
>> +/*
>> + * Gets PCI_ID property, splits to xxxx:yyyy and stores
>>    * xxxx to dev->vendor and yyyy to dev->device for
>>    * faster access.
>>    */
>> @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
>>          return NULL;
>>   }
>>
>> +static void
>> +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
>> +{
>> +       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);
>> +
>> +       if (dev->pci_slot_name != NULL)
>> +               strncpy(card->pci_slot_name, dev->pci_slot_name,
>> +                       PCI_SLOT_NAME_SIZE + 1);
>> +}
>> +
>> +/*
>> + * Iterate over all igt_devices array and find first discrete card.
>> + * card->pci_slot_name will be updated only if a discrete card is present.
>> + */
>> +void igt_device_find_first_discrete_card(struct igt_device_card *card)
>> +{
>> +       struct igt_device *dev;
>> +
>> +       igt_list_for_each_entry(dev, &igt_devs.all, link) {
>> +
>> +               if (!is_pci_subsystem(dev))
>> +                       continue;
>> +
>> +               if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
>> +                       __copy_dev_to_card(dev, card);
> 
> why do we need to make it prefer discrete? rather than "use the first
> one if no one was specified"?

I thought that makes more sense for end user, assuming igpu and 
discrete, that user is more likely to want to look at the latter.

Otherwise I am also not sure if device enumeration is stable (across 
boots, distros, kernel versions, udevs, systemd, whoever is involved in 
providing the data device scan uses, plus the order of binding the 
driver to pci devices). So from that angle as well it made more sense to 
pick first discrete. Granted the approach falls apart with more than one 
discrete.. Problem for another day?

Regards,

Tvrtko

> 
> Lucas De Marchi
> 
>> +                       break;
>> +               }
>> +       }
>> +}
>> +
>>   static struct igt_device *igt_device_from_syspath(const char *syspath)
>>   {
>>          struct igt_device *dev;
>> @@ -445,6 +499,7 @@ static void update_or_add_parent(struct udev_device *dev,
>>                  parent_idev = igt_device_new_from_udev(parent_dev);
>>                  if (is_pci_subsystem(parent_idev)) {
>>                          set_vendor_device(parent_idev);
>> +                       set_pci_slot_name(parent_idev);
>>                  }
>>                  igt_list_add_tail(&parent_idev->link, &igt_devs.all);
>>          }
>> @@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device *dev)
>>          free(dev->drm_render);
>>          free(dev->vendor);
>>          free(dev->device);
>> +       free(dev->pci_slot_name);
>>          g_hash_table_destroy(dev->attrs_ht);
>>          g_hash_table_destroy(dev->props_ht);
>>   }
>>
>> +void igt_devices_free(void)
>> +{
>> +       struct igt_device *dev, *tmp;
>> +
>> +       igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
>> +               igt_device_free(dev);
>> +               free(dev);
>> +       }
>> +}
>> +
>>   /**
>>    * igt_devices_scan
>>    * @force: enforce scanning devices
>> @@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter, struct igt_device_card *card)
>>          /* 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);
>> +       __copy_dev_to_card(dev, card);
>>
>>          return true;
>>   }
>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
>> index ad632145..16d75320 100644
>> --- a/lib/igt_device_scan.h
>> +++ b/lib/igt_device_scan.h
>> @@ -39,10 +39,13 @@ enum igt_devices_print_type {
>>          IGT_PRINT_DETAIL,
>>   };
>>
>> +#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
>> +#define PCI_SLOT_NAME_SIZE 12
>>   struct igt_device_card {
>>          char subsystem[NAME_MAX];
>>          char card[NAME_MAX];
>>          char render[NAME_MAX];
>> +       char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
>>   };
>>
>>   void igt_devices_scan(bool force);
>> @@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type printtype);
>>   void igt_devices_print_vendors(void);
>>   void igt_device_print_filter_types(void);
>>
>> +void igt_devices_free(void);
>> +
>>   /*
>>    * Handle device filter collection array.
>>    * IGT can store/retrieve filters passed by user using '--device' args.
>> @@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
>>
>>   /* Use filter to match the device and fill card structure */
>>   bool igt_device_card_match(const char *filter, struct igt_device_card *card);
>> -
>> +void igt_device_find_first_discrete_card(struct igt_device_card *card);
>>   int igt_open_card(struct igt_device_card *card);
>>   int igt_open_render(struct igt_device_card *card);
>>
>> --
>> 2.25.4
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> 
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[Mika Kuoppala]

Arkadiusz Hiler <arkadiusz.hiler@intel.com> writes:

> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> Return value of udev_enumerate_add_match_property() was not being checked
> in scan_drm_devices().
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  lib/igt_device_scan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 601d6fb0..7b0fc00e 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -532,7 +532,7 @@ static void scan_drm_devices(void)
>  	ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
>  	igt_assert(!ret);
>  
> -	udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> +	ret = udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
>  	igt_assert(!ret);
>  
>  	ret = udev_enumerate_scan_devices(enumerate);
> -- 
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top

[Arkadiusz Hiler]

On Thu, May 28, 2020 at 11:27:13AM +0100, Tvrtko Ursulin wrote:
> 
> On 26/05/2020 17:26, Lucas De Marchi wrote:
> > On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler
> > <arkadiusz.hiler@intel.com> wrote:
> > > 
> > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > 
> > > Will be used in the next patch.
> > > 
> > > 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
> > > 2. igt_device_find_first_discrete_card(): try to find first discrete GPU
> > > 3. igt_devices_free(): Free device buffers created during scan
> > > 
> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > >   lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
> > >   lib/igt_device_scan.h |  7 +++-
> > >   2 files changed, 74 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index be854b01..4daabbd9 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -164,6 +164,7 @@ struct igt_device {
> > >          /* For pci subsystem */
> > >          char *vendor;
> > >          char *device;
> > > +       char *pci_slot_name;
> > > 
> > >          struct igt_list_head link;
> > >   };
> > > @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
> > >   #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
> > > +/*
> > > + * Get PCI_SLOT_NAME property, it should be in format of
> > > + * xxxx:yy:zz.z
> > > + */
> > > +static void set_pci_slot_name(struct igt_device *dev)
> > > +{
> > > +       const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> > > +
> > > +       if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
> > > +               return;
> > > +       dev->pci_slot_name = strdup(pci_slot_name);
> > > +}
> > > +
> > > +/*
> > > + * Gets PCI_ID property, splits to xxxx:yyyy and stores
> > >    * xxxx to dev->vendor and yyyy to dev->device for
> > >    * faster access.
> > >    */
> > > @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
> > >          return NULL;
> > >   }
> > > 
> > > +static void
> > > +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
> > > +{
> > > +       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);
> > > +
> > > +       if (dev->pci_slot_name != NULL)
> > > +               strncpy(card->pci_slot_name, dev->pci_slot_name,
> > > +                       PCI_SLOT_NAME_SIZE + 1);
> > > +}
> > > +
> > > +/*
> > > + * Iterate over all igt_devices array and find first discrete card.
> > > + * card->pci_slot_name will be updated only if a discrete card is present.
> > > + */
> > > +void igt_device_find_first_discrete_card(struct igt_device_card *card)
> > > +{
> > > +       struct igt_device *dev;
> > > +
> > > +       igt_list_for_each_entry(dev, &igt_devs.all, link) {
> > > +
> > > +               if (!is_pci_subsystem(dev))
> > > +                       continue;
> > > +
> > > +               if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
> > > +                       __copy_dev_to_card(dev, card);
> > 
> > why do we need to make it prefer discrete? rather than "use the first
> > one if no one was specified"?
> 
> I thought that makes more sense for end user, assuming igpu and discrete,
> that user is more likely to want to look at the latter.
> 
> Otherwise I am also not sure if device enumeration is stable (across boots,
> distros, kernel versions, udevs, systemd, whoever is involved in providing
> the data device scan uses, plus the order of binding the driver to pci
> devices). So from that angle as well it made more sense to pick first
> discrete. Granted the approach falls apart with more than one discrete..
> Problem for another day?

Internally we sort the PCI devices by their syspath, so that should
help with this quarrel.

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

Re: [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection

[Tvrtko Ursulin]

On 26/05/2020 13:24, Arkadiusz Hiler wrote:
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> 
> In intel_gpu_top device path was hard coded for integrated GPU.
> 
> With this patch we:
>   * use igt_device_scan library for device scanning,
>   * make discrete GPU the default one,
>   * provided options for card selection.
> 
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>   man/intel_gpu_top.rst |  29 +++++++++-
>   tools/Makefile.am     |   2 +-
>   tools/intel_gpu_top.c | 127 ++++++++++++++++++++++++++++++++++++------
>   tools/meson.build     |   2 +-
>   4 files changed, 140 insertions(+), 20 deletions(-)
> 
> diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
> index d487baca..5552e969 100644
> --- a/man/intel_gpu_top.rst
> +++ b/man/intel_gpu_top.rst
> @@ -7,9 +7,9 @@ Display a top-like summary of Intel GPU usage
>   ---------------------------------------------
>   .. include:: defs.rst
>   :Author: IGT Developers <igt-dev@lists.freedesktop.org>
> -:Date: 2019-02-08
> +:Date: 2020-03-18
>   :Version: |PACKAGE_STRING|
> -:Copyright: 2009,2011,2012,2016,2018,2019 Intel Corporation
> +:Copyright: 2009,2011,2012,2016,2018,2019,2020 Intel Corporation
>   :Manual section: |MANUAL_SECTION|
>   :Manual group: |MANUAL_GROUP|
>   
> @@ -43,6 +43,31 @@ OPTIONS
>   
>   -s <ms>
>       Refresh period in milliseconds.
> +-L
> +    List available GPUs on the platform.
> +-d
> +    Select a specific GPU using supported filter.
> +
> +
> +DEVICE SELECTION
> +================
> +
> +User can select specific GPU for performance monitoring on platform where multiple GPUs are available.
> +A GPU can be selected by sysfs path, drm node or using various PCI sub filters.
> +
> +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
>   
>   LIMITATIONS
>   ===========
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 9352b41c..f97f9e08 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -24,4 +24,4 @@ AM_CFLAGS = $(DEBUG_CFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \
>   LDADD = $(top_builddir)/lib/libintel_tools.la
>   AM_LDFLAGS = -Wl,--as-needed
>   
> -intel_gpu_top_LDADD = $(top_builddir)/lib/libigt_perf.la
> +intel_gpu_top_LDADD = $(top_builddir)/lib/libigt_perf.la $(top_builddir)/lib/libigt_device_scan.la $(LIBUDEV_LIBS) $(GLIB_LIBS)
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 8197482d..b3bbe505 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -22,6 +22,7 @@
>    */
>   
>   #include <stdio.h>
> +#include "igt_device_scan.h"
>   #include <sys/types.h>
>   #include <dirent.h>
>   #include <stdint.h>
> @@ -94,7 +95,16 @@ struct engines {
>   	struct pmu_counter imc_reads;
>   	struct pmu_counter imc_writes;
>   
> +	bool discrete;
> +	char *device;
> +
> +	/* Do not edit below this line.
> +	 * This structure is reallocated every time a new engine is
> +	 * found and size is increased by sizeof (engine).
> +	 */
> +
>   	struct engine engine;
> +
>   };
>   
>   static uint64_t
> @@ -168,14 +178,20 @@ static int engine_cmp(const void *__a, const void *__b)
>   		return a->instance - b->instance;
>   }
>   
> -static struct engines *discover_engines(void)
> +#define is_igpu_pci(x) (strcmp(x, "0000:00:02.0") == 0)
> +#define is_igpu(x) (strcmp(x, "i915") == 0)
> +
> +static struct engines *discover_engines(char *device)
>   {
> -	const char *sysfs_root = "/sys/devices/i915/events";
> +	char sysfs_root[PATH_MAX];
>   	struct engines *engines;
>   	struct dirent *dent;
>   	int ret = 0;
>   	DIR *d;
>   
> +	snprintf(sysfs_root, sizeof(sysfs_root),
> +		 "/sys/devices/%s/events", device);
> +
>   	engines = malloc(sizeof(struct engines));
>   	if (!engines)
>   		return NULL;
> @@ -183,6 +199,8 @@ static struct engines *discover_engines(void)
>   	memset(engines, 0, sizeof(*engines));
>   
>   	engines->num_engines = 0;
> +	engines->device = device;
> +	engines->discrete = !is_igpu(device);
>   
>   	d = opendir(sysfs_root);
>   	if (!d)
> @@ -497,7 +515,7 @@ static int pmu_init(struct engines *engines)
>   	}
>   
>   	engines->rapl_fd = -1;
> -	if (rapl_type_id()) {
> +	if (!engines->discrete && rapl_type_id()) {
>   		engines->rapl_scale = rapl_gpu_power_scale();
>   		engines->rapl_unit = rapl_gpu_power_unit();
>   		if (!engines->rapl_unit)
> @@ -695,8 +713,11 @@ usage(const char *appname)
>   		"\t[-l]            List plain text data.\n"
>   		"\t[-o <file|->]   Output to specified file or '-' for standard out.\n"
>   		"\t[-s <ms>]       Refresh period in milliseconds (default %ums).\n"
> +		"\t[-L]            List all cards.\n"
> +		"\t[-d <device>]   Device filter, please check manual page for more details.\n"
>   		"\n",
>   		appname, DEFAULT_PERIOD_MS);
> +	igt_device_print_filter_types();
>   }
>   
>   static enum {
> @@ -1082,13 +1103,18 @@ print_header(struct engines *engines, double t,
>   	if (output_mode == INTERACTIVE) {
>   		printf("\033[H\033[J");
>   
> -		if (lines++ < con_h)
> -			printf("intel-gpu-top - %s/%s MHz;  %s%% RC6; %s %s; %s irqs/s\n",
> -			       freq_items[1].buf, freq_items[0].buf,
> -			       rc6_items[0].buf, power_items[0].buf,
> -			       engines->rapl_unit,
> -			       irq_items[0].buf);
> -
> +		if (lines++ < con_h) {
> +			if (!engines->discrete)
> +				printf("intel-gpu-top - %s/%s MHz;  %s%% RC6; %s %s; %s irqs/s\n",
> +					freq_items[1].buf, freq_items[0].buf,
> +					rc6_items[0].buf, power_items[0].buf,
> +					engines->rapl_unit,
> +					irq_items[0].buf);
> +			else
> +				printf("intel-gpu-top - %s/%s MHz;  %s%% RC6; %s irqs/s\n",
> +					freq_items[1].buf, freq_items[0].buf,
> +					rc6_items[0].buf, irq_items[0].buf);
> +		}
>   		if (lines++ < con_h)
>   			printf("\n");
>   	}
> @@ -1249,6 +1275,33 @@ static void sigint_handler(int  sig)
>   	stop_top = true;
>   }
>   
> +/* tr_pmu_name()
> + *
> + * Transliterate pci_slot_id to sysfs device name entry for discrete GPU.
> + * Discrete GPU PCI ID   ("xxxx:yy:zz.z")       device = "i915_xxxx_yy_zz.z".
> + */
> +static char *tr_pmu_name(struct igt_device_card *card)
> +{
> +	int ret;
> +	const int bufsize = 18;
> +	char *buf, *device = NULL;
> +
> +	assert(card->pci_slot_name[0]);
> +
> +	device = malloc(bufsize);
> +	assert(device);
> +
> +	ret = snprintf(device, bufsize, "i915_%s", card->pci_slot_name);
> +	assert(ret == (bufsize-1));
> +
> +	buf = device;
> +	for (; *buf; buf++)
> +		if (*buf == ':')
> +			*buf = '_';
> +
> +	return device;
> +}
> +
>   int main(int argc, char **argv)
>   {
>   	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
> @@ -1256,10 +1309,14 @@ int main(int argc, char **argv)
>   	char *output_path = NULL;
>   	struct engines *engines;
>   	unsigned int i;
> -	int ret, ch;
> +	int ret = 0, ch;
> +	bool list_device = false;
> +	enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> +	char *pmu_device, *opt_device = NULL;
> +	struct igt_device_card card;
>   
>   	/* Parse options */
> -	while ((ch = getopt(argc, argv, "o:s:Jlh")) != -1) {
> +	while ((ch = getopt(argc, argv, "o:s:d:JLlh")) != -1) {
>   		switch (ch) {
>   		case 'o':
>   			output_path = optarg;
> @@ -1267,9 +1324,15 @@ int main(int argc, char **argv)
>   		case 's':
>   			period_us = atoi(optarg) * 1000;
>   			break;
> +		case 'd':
> +			opt_device = strdup(optarg);
> +			break;
>   		case 'J':
>   			output_mode = JSON;
>   			break;
> +		case 'L':
> +			list_device = true;
> +			break;
>   		case 'l':
>   			output_mode = STDOUT;
>   			break;
> @@ -1320,19 +1383,46 @@ int main(int argc, char **argv)
>   		break;
>   	};
>   
> -	engines = discover_engines();
> +	igt_devices_scan(false);
> +
> +	if (list_device) {
> +		igt_devices_print(printtype);

I am not really happy with the output here, but that criticism goes to 
the lsgpu and library as well. It seems that we are listing multiple 
entries per card (not talking about render nodes!) so it is trial and 
error to copy & paste the right one for giving to "-D".

My opens:

Should lsgpu list master and render nodes separately? I would say not by 
default, or at least list them in a hierarchical manner akin to lsblk.

Should the output be condensed to one master entry per physical GPU by 
default? (And be the one which works when passed in to "-D". Not all do, 
because some don't have the PCI data, while they do point to the same 
DRM card, AFAIR.

> +		goto exit;
> +	}
> +
> +	if (opt_device != NULL) {
> +		/* Free opt device now as its not needed further. */
> +		ret = igt_device_card_match(opt_device, &card);
> +		free(opt_device);
> +		if (!ret) {
> +			fprintf(stderr, "Requested device %s not found!\n", opt_device);

opt_device is use after free here.

> +			ret = 1;

Instead these ones (here and below) we could use EXIT_FAILURE for 
readability.

> +			goto exit;
> +		}
> +	} else {
> +		igt_device_find_first_discrete_card(&card);
> +	}
> +
> +	if (card.pci_slot_name[0] && !is_igpu_pci(card.pci_slot_name))
> +		pmu_device = tr_pmu_name(&card);
> +	else
> +		pmu_device = strdup("i915");
> +
> +	engines = discover_engines(pmu_device);
>   	if (!engines) {
>   		fprintf(stderr,
>   			"Failed to detect engines! (%s)\n(Kernel 4.16 or newer is required for i915 PMU support.)\n",
>   			strerror(errno));
> -		return 1;
> +		ret = 1;
> +		goto err;
>   	}
>   
>   	ret = pmu_init(engines);
>   	if (ret) {
>   		fprintf(stderr,
>   			"Failed to initialize PMU! (%s)\n", strerror(errno));
> -		return 1;
> +		ret = 1;
> +		goto err;
>   	}
>   
>   	pmu_sample(engines);
> @@ -1384,5 +1474,10 @@ int main(int argc, char **argv)
>   		usleep(period_us);
>   	}
>   
> -	return 0;
> +err:
> +	free(engines);
> +	free(pmu_device);
> +exit:
> +	igt_devices_free();
> +	return ret;
>   }
> diff --git a/tools/meson.build b/tools/meson.build
> index 59b56d5d..34f95e79 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -93,7 +93,7 @@ install_subdir('registers', install_dir : datadir,
>   executable('intel_gpu_top', 'intel_gpu_top.c',
>   	   install : true,
>   	   install_rpath : bindir_rpathdir,
> -	   dependencies : lib_igt_perf)
> +	   dependencies : [lib_igt_perf,lib_igt_device_scan])
>   
>   executable('amd_hdmi_compliance', 'amd_hdmi_compliance.c',
>   	   dependencies : [tool_deps],
> 

Apart from two minor nits and a generic open on lspgu behaviour I tested 
this and it worked.

Regards,

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
URL   : https://patchwork.freedesktop.org/series/77914/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8571 -> IGTPW_4640
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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


Changes
-------

  No changes found


Participating hosts (50 -> 44)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5690 -> IGTPW_4640

  CI-20190529: 20190529
  CI_DRM_8571: 0536dff30eff69abcf6355bdd9b9fdf45a560099 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4640: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/index.html
  IGT_5690: bea881189520a9cccbb1c1cb454ac5b6fdaea40e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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