All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
@ 2020-06-02 11:13 Arkadiusz Hiler
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Arkadiusz Hiler @ 2020-06-02 11:13 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala, Lucas De Marchi, Ayaz A Siddiqui

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>
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

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

* [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core
  2020-06-02 11:13 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
@ 2020-06-02 11:13 ` Arkadiusz Hiler
  2020-06-05 11:48   ` Petri Latvala
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Arkadiusz Hiler @ 2020-06-02 11:13 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala, Ayaz A Siddiqui

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.

v2: (mostly) sort includes (Lucas)

Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
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 | 11 +++++-----
 lib/igt_device_scan.h |  8 ++++++-
 lib/igt_tools_stub.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
 lib/meson.build       | 15 +++++++++++++
 6 files changed, 86 insertions(+), 8 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..e4e710d4 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -55,6 +55,7 @@
 #include "igt_device.h"
 #include "igt_gt.h"
 #include "igt_kmod.h"
+#include "igt_params.h"
 #include "igt_sysfs.h"
 #include "igt_device_scan.h"
 #include "version.h"
diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 7b0fc00e..d4c5cfdf 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -22,18 +22,17 @@
  *
  */
 
-#include "igt.h"
-#include "igt_list.h"
-#include "igt_sysfs.h"
-#include "igt_device.h"
+#include "igt_core.h"
 #include "igt_device_scan.h"
+#include "igt_list.h"
+
+#include <dirent.h>
+#include <fcntl.h>
 #include <glib.h>
 #include <libudev.h>
 #include <linux/limits.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <dirent.h>
-#include <fcntl.h>
 
 /**
  * SECTION:igt_device_scan
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index 24eafe62..3526f1da 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -25,8 +25,14 @@
 #ifndef __IGT_DEVICE_SCAN_H__
 #define __IGT_DEVICE_SCAN_H__
 
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
 #include <limits.h>
-#include <igt.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <unistd.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

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

* [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
  2020-06-02 11:13 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
@ 2020-06-02 11:13 ` Arkadiusz Hiler
  2020-06-05 12:11   ` Petri Latvala
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection Arkadiusz Hiler
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Arkadiusz Hiler @ 2020-06-02 11:13 UTC (permalink / raw)
  To: igt-dev; +Cc: Tvrtko Ursulin, Petri Latvala, Ayaz A Siddiqui

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 d4c5cfdf..12fbd241 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -165,6 +165,7 @@ struct igt_device {
 	/* For pci subsystem */
 	char *vendor;
 	char *device;
+	char *pci_slot_name;
 
 	struct igt_list_head link;
 };
@@ -338,7 +339,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.
  */
@@ -411,6 +426,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;
@@ -446,6 +500,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);
 	}
@@ -574,10 +629,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
@@ -1197,17 +1263,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 3526f1da..2e73ecb7 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

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

* [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection
  2020-06-02 11:13 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
@ 2020-06-02 11:13 ` Arkadiusz Hiler
  2020-06-03 13:17   ` Tvrtko Ursulin
  2020-06-02 12:06 ` [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
  2020-06-02 18:42 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 1 reply; 11+ messages in thread
From: Arkadiusz Hiler @ 2020-06-02 11:13 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala, Ayaz A Siddiqui

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.

v2:
 * explicitly set ret to EXIT_SUCCESS after all the other uses
 * fix use after free of opt_device (Tvrtko)
 * use EXIT_FAILURE instead of "1" (Tvrtko)

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
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 | 130 ++++++++++++++++++++++++++++++++++++------
 tools/meson.build     |   2 +-
 4 files changed, 143 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..2072a3a2 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -21,6 +21,8 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include "igt_device_scan.h"
+
 #include <stdio.h>
 #include <sys/types.h>
 #include <dirent.h>
@@ -94,7 +96,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 +179,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 +200,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 +516,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 +714,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 +1104,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 +1276,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 +1310,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 +1325,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,21 +1384,50 @@ 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) {
+		ret = igt_device_card_match(opt_device, &card);
+		if (!ret) {
+			fprintf(stderr, "Requested device %s not found!\n", opt_device);
+			free(opt_device);
+			ret = EXIT_FAILURE;
+			goto exit;
+		}
+		free(opt_device);
+	} 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 = EXIT_FAILURE;
+		goto err;
 	}
 
 	ret = pmu_init(engines);
 	if (ret) {
 		fprintf(stderr,
 			"Failed to initialize PMU! (%s)\n", strerror(errno));
-		return 1;
+		ret = EXIT_FAILURE;
+		goto err;
 	}
 
+	ret = EXIT_SUCCESS;
+
 	pmu_sample(engines);
 
 	while (!stop_top) {
@@ -1384,5 +1477,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

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

* [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
  2020-06-02 11:13 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
                   ` (2 preceding siblings ...)
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection Arkadiusz Hiler
@ 2020-06-02 12:06 ` Patchwork
  2020-06-02 18:42 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-06-02 12:06 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== 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

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

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
  2020-06-02 11:13 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
                   ` (3 preceding siblings ...)
  2020-06-02 12:06 ` [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
@ 2020-06-02 18:42 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-06-02 18:42 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== 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_full -> IGTPW_4640_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

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

  * igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen:
    - shard-kbl:          [PASS][3] -> [FAIL][4] ([i915#54])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#54])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk8/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk7/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
    - shard-apl:          [PASS][7] -> [FAIL][8] ([i915#54]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl4/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl8/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x21-random:
    - shard-kbl:          [PASS][9] -> [FAIL][10] ([i915#54] / [i915#93] / [i915#95]) +4 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][11] -> [DMESG-WARN][12] ([i915#180])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_legacy@cursorb-vs-flipa-toggle:
    - shard-glk:          [PASS][13] -> [DMESG-FAIL][14] ([i915#1925] / [i915#1926])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk8/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk7/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html

  * igt@kms_cursor_legacy@pipe-c-torture-move:
    - shard-hsw:          [PASS][15] -> [DMESG-WARN][16] ([i915#128])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-hsw1/igt@kms_cursor_legacy@pipe-c-torture-move.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-hsw1/igt@kms_cursor_legacy@pipe-c-torture-move.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109349])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-iclb7/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled:
    - shard-apl:          [PASS][19] -> [FAIL][20] ([i915#52] / [i915#54] / [i915#95])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl6/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl1/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html
    - shard-kbl:          [PASS][21] -> [FAIL][22] ([i915#177] / [i915#52] / [i915#54] / [i915#93] / [i915#95])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl4/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl6/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
    - shard-glk:          [PASS][23] -> [TIMEOUT][24] ([i915#1958]) +4 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk8/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#109441]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-iclb5/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_universal_plane@universal-plane-gen9-features-pipe-c:
    - shard-glk:          [PASS][27] -> [INCOMPLETE][28] ([i915#1927] / [i915#58] / [k.org#198133])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk7/igt@kms_universal_plane@universal-plane-gen9-features-pipe-c.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk8/igt@kms_universal_plane@universal-plane-gen9-features-pipe-c.html

  * igt@kms_universal_plane@universal-plane-pipe-b-functional:
    - shard-apl:          [PASS][29] -> [FAIL][30] ([i915#331])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl7/igt@kms_universal_plane@universal-plane-pipe-b-functional.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl2/igt@kms_universal_plane@universal-plane-pipe-b-functional.html
    - shard-kbl:          [PASS][31] -> [FAIL][32] ([i915#331])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl3/igt@kms_universal_plane@universal-plane-pipe-b-functional.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl3/igt@kms_universal_plane@universal-plane-pipe-b-functional.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [PASS][33] -> [DMESG-WARN][34] ([i915#180]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Possible fixes ####

  * igt@gem_exec_reloc@basic-cpu-read:
    - shard-glk:          [TIMEOUT][35] ([i915#1958]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk6/igt@gem_exec_reloc@basic-cpu-read.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk1/igt@gem_exec_reloc@basic-cpu-read.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-glk:          [DMESG-WARN][37] ([i915#1436] / [i915#716]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk9/igt@gen9_exec_parse@allowed-all.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk5/igt@gen9_exec_parse@allowed-all.html
    - shard-kbl:          [DMESG-WARN][39] ([i915#1436] / [i915#716]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl4/igt@gen9_exec_parse@allowed-all.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl2/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][41] ([i915#1899]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-iclb8/igt@i915_pm_dc@dc6-psr.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-iclb7/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-iclb:         [INCOMPLETE][43] ([i915#189]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-iclb4/igt@i915_pm_rpm@gem-execbuf-stress.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-iclb2/igt@i915_pm_rpm@gem-execbuf-stress.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen:
    - shard-kbl:          [FAIL][45] ([i915#54] / [i915#93] / [i915#95]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html

  * igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge:
    - shard-apl:          [FAIL][47] ([i915#70] / [i915#95]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl6/igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl4/igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge.html
    - shard-kbl:          [FAIL][49] ([i915#70] / [i915#93] / [i915#95]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl7/igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl4/igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge.html

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled:
    - shard-kbl:          [FAIL][51] ([i915#177] / [i915#52] / [i915#54] / [i915#93] / [i915#95]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl6/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl7/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled.html
    - shard-apl:          [FAIL][53] ([i915#52] / [i915#54] / [i915#95]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl1/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl8/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled.html

  * igt@kms_draw_crc@fill-fb:
    - shard-apl:          [FAIL][55] ([i915#95]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl8/igt@kms_draw_crc@fill-fb.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl7/igt@kms_draw_crc@fill-fb.html

  * igt@kms_fbcon_fbt@fbc:
    - shard-kbl:          [FAIL][57] ([i915#64] / [i915#93] / [i915#95]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl7/igt@kms_fbcon_fbt@fbc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl1/igt@kms_fbcon_fbt@fbc.html
    - shard-apl:          [FAIL][59] ([i915#1525] / [i915#95]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl1/igt@kms_fbcon_fbt@fbc.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl1/igt@kms_fbcon_fbt@fbc.html

  * {igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-hdmi-a1-hdmi-a2}:
    - shard-glk:          [FAIL][61] ([i915#79]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-hdmi-a1-hdmi-a2.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-hdmi-a1-hdmi-a2.html

  * {igt@kms_flip@2x-flip-vs-rmfb-interruptible@ab-vga1-hdmi-a1}:
    - shard-hsw:          [INCOMPLETE][63] ([i915#1927] / [i915#61]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-hsw8/igt@kms_flip@2x-flip-vs-rmfb-interruptible@ab-vga1-hdmi-a1.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-hsw1/igt@kms_flip@2x-flip-vs-rmfb-interruptible@ab-vga1-hdmi-a1.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@a-dp1}:
    - shard-kbl:          [DMESG-WARN][65] ([i915#180]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@c-dp1}:
    - shard-apl:          [DMESG-WARN][67] ([i915#180]) -> [PASS][68] +6 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_panel_fitting@atomic-fastset:
    - shard-tglb:         [FAIL][69] ([i915#83]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-tglb6/igt@kms_panel_fitting@atomic-fastset.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-tglb7/igt@kms_panel_fitting@atomic-fastset.html
    - shard-iclb:         [FAIL][71] ([i915#83]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-iclb6/igt@kms_panel_fitting@atomic-fastset.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-iclb3/igt@kms_panel_fitting@atomic-fastset.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [SKIP][73] ([fdo#109441]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-iclb5/igt@kms_psr@psr2_primary_page_flip.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [FAIL][75] ([i915#1899]) -> [FAIL][76] ([i915#454])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-tglb3/igt@i915_pm_dc@dc6-psr.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-tglb1/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_content_protection@legacy:
    - shard-apl:          [TIMEOUT][77] ([i915#1319] / [i915#1635]) -> [TIMEOUT][78] ([i915#1319])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl6/igt@kms_content_protection@legacy.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl6/igt@kms_content_protection@legacy.html

  * igt@kms_content_protection@srm:
    - shard-apl:          [FAIL][79] ([fdo#110321]) -> [DMESG-FAIL][80] ([fdo#110321])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl6/igt@kms_content_protection@srm.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl8/igt@kms_content_protection@srm.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen:
    - shard-kbl:          [FAIL][81] ([i915#54] / [i915#93] / [i915#95]) -> [FAIL][82] ([i915#54])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html

  * igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy:
    - shard-glk:          [SKIP][83] ([fdo#109271]) -> [TIMEOUT][84] ([i915#1958])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-glk9/igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-glk7/igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          [FAIL][85] ([fdo#108145] / [i915#265]) -> [FAIL][86] ([fdo#108145] / [i915#265] / [i915#95])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-apl4/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-apl4/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
    - shard-kbl:          [FAIL][87] ([fdo#108145] / [i915#265]) -> [FAIL][88] ([fdo#108145] / [i915#265] / [i915#93] / [i915#95])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8571/shard-kbl2/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4640/shard-kbl7/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#128]: https://gitlab.freedesktop.org/drm/intel/issues/128
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1525]: https://gitlab.freedesktop.org/drm/intel/issues/1525
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#189]: https://gitlab.freedesktop.org/drm/intel/issues/189
  [i915#1899]: https://gitlab.freedesktop.org/drm/intel/issues/1899
  [i915#1925]: https://gitlab.freedesktop.org/drm/intel/issues/1925
  [i915#1926]: https://gitlab.freedesktop.org/drm/intel/issues/1926
  [i915#1927]: https://gitlab.freedesktop.org/drm/intel/issues/1927
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#331]: https://gitlab.freedesktop.org/drm/intel/issues/331
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#64]: https://gitlab.freedesktop.org/drm/intel/issues/64
  [i915#70]: https://gitlab.freedesktop.org/drm/intel/issues/70
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#83]: https://gitlab.freedesktop.org/drm/intel/issues/83
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

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


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

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

  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
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== 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

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

* Re: [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection Arkadiusz Hiler
@ 2020-06-03 13:17   ` Tvrtko Ursulin
  0 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2020-06-03 13:17 UTC (permalink / raw)
  To: Arkadiusz Hiler, igt-dev; +Cc: Petri Latvala, Ayaz A Siddiqui


On 02/06/2020 12:13, 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.
> 
> v2:
>   * explicitly set ret to EXIT_SUCCESS after all the other uses
>   * fix use after free of opt_device (Tvrtko)
>   * use EXIT_FAILURE instead of "1" (Tvrtko)
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> 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 | 130 ++++++++++++++++++++++++++++++++++++------
>   tools/meson.build     |   2 +-
>   4 files changed, 143 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..2072a3a2 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -21,6 +21,8 @@
>    * DEALINGS IN THE SOFTWARE.
>    */
>   
> +#include "igt_device_scan.h"
> +
>   #include <stdio.h>
>   #include <sys/types.h>
>   #include <dirent.h>
> @@ -94,7 +96,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 +179,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 +200,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 +516,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 +714,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 +1104,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 +1276,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 +1310,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 +1325,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,21 +1384,50 @@ 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) {
> +		ret = igt_device_card_match(opt_device, &card);
> +		if (!ret) {
> +			fprintf(stderr, "Requested device %s not found!\n", opt_device);
> +			free(opt_device);
> +			ret = EXIT_FAILURE;
> +			goto exit;
> +		}
> +		free(opt_device);
> +	} 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 = EXIT_FAILURE;
> +		goto err;
>   	}
>   
>   	ret = pmu_init(engines);
>   	if (ret) {
>   		fprintf(stderr,
>   			"Failed to initialize PMU! (%s)\n", strerror(errno));
> -		return 1;
> +		ret = EXIT_FAILURE;
> +		goto err;
>   	}
>   
> +	ret = EXIT_SUCCESS;
> +
>   	pmu_sample(engines);
>   
>   	while (!stop_top) {
> @@ -1384,5 +1477,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],
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I leave device scan library changes to the experts there.

Regards,

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

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

* Re: [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
@ 2020-06-05 11:48   ` Petri Latvala
  0 siblings, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2020-06-05 11:48 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev, Ayaz A Siddiqui

On Tue, Jun 02, 2020 at 02:13:28PM +0300, Arkadiusz Hiler 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.
> 
> v2: (mostly) sort includes (Lucas)
> 
> Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
> 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 | 11 +++++-----
>  lib/igt_device_scan.h |  8 ++++++-
>  lib/igt_tools_stub.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
>  lib/meson.build       | 15 +++++++++++++
>  6 files changed, 86 insertions(+), 8 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..e4e710d4 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -55,6 +55,7 @@
>  #include "igt_device.h"
>  #include "igt_gt.h"
>  #include "igt_kmod.h"
> +#include "igt_params.h"
>  #include "igt_sysfs.h"
>  #include "igt_device_scan.h"
>  #include "version.h"
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 7b0fc00e..d4c5cfdf 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -22,18 +22,17 @@
>   *
>   */
>  
> -#include "igt.h"
> -#include "igt_list.h"
> -#include "igt_sysfs.h"
> -#include "igt_device.h"
> +#include "igt_core.h"
>  #include "igt_device_scan.h"
> +#include "igt_list.h"
> +
> +#include <dirent.h>
> +#include <fcntl.h>
>  #include <glib.h>
>  #include <libudev.h>
>  #include <linux/limits.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
> -#include <dirent.h>
> -#include <fcntl.h>
>  
>  /**
>   * SECTION:igt_device_scan
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index 24eafe62..3526f1da 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -25,8 +25,14 @@
>  #ifndef __IGT_DEVICE_SCAN_H__
>  #define __IGT_DEVICE_SCAN_H__
>  
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <getopt.h>
>  #include <limits.h>
> -#include <igt.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <unistd.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)
>  



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

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

* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
  2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
@ 2020-06-05 12:11   ` Petri Latvala
  0 siblings, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2020-06-05 12:11 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev, Ayaz A Siddiqui, Tvrtko Ursulin

On Tue, Jun 02, 2020 at 02:13:29PM +0300, Arkadiusz Hiler 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 d4c5cfdf..12fbd241 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -165,6 +165,7 @@ struct igt_device {
>  	/* For pci subsystem */
>  	char *vendor;
>  	char *device;
> +	char *pci_slot_name;
>  
>  	struct igt_list_head link;
>  };
> @@ -338,7 +339,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.
>   */
> @@ -411,6 +426,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;
> +		}
> +	}
> +}

Having read the intel_gpu_top patch already, I know this function is
so it can prefer discrete over integrated i915 device if one exists
but this is more global code. Having intel or i915 somewhere in the
name of this function and the pci id macro would make it more clear
what is assumed and where the assumption holds.

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

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

* Re: [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection
  2020-05-26 12:24 ` [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection Arkadiusz Hiler
@ 2020-05-28 14:28   ` Tvrtko Ursulin
  0 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2020-05-28 14:28 UTC (permalink / raw)
  To: Arkadiusz Hiler, igt-dev; +Cc: Petri Latvala, Ayaz A Siddiqui


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

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

* [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection
  2020-05-26 12:24 [igt-dev] [PATCH i-g-t 1/4] " Arkadiusz Hiler
@ 2020-05-26 12:24 ` Arkadiusz Hiler
  2020-05-28 14:28   ` Tvrtko Ursulin
  0 siblings, 1 reply; 11+ messages in thread
From: Arkadiusz Hiler @ 2020-05-26 12:24 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala, Ayaz A Siddiqui

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

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

end of thread, other threads:[~2020-06-05 12:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-02 11:13 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
2020-06-05 11:48   ` Petri Latvala
2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
2020-06-05 12:11   ` Petri Latvala
2020-06-02 11:13 ` [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection Arkadiusz Hiler
2020-06-03 13:17   ` Tvrtko Ursulin
2020-06-02 12:06 ` [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
2020-06-02 18:42 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2020-05-26 12:24 [igt-dev] [PATCH i-g-t 1/4] " Arkadiusz Hiler
2020-05-26 12:24 ` [igt-dev] [PATCH i-g-t 4/4] tools/intel_gpu_top: Add support of discrete GPU and card selection Arkadiusz Hiler
2020-05-28 14:28   ` Tvrtko Ursulin

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