* [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core
2020-05-26 12:24 [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-05-26 12:24 ` Arkadiusz Hiler
2020-05-26 16:14 ` Lucas De Marchi
2020-05-26 12:24 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ 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>
igt_device_scan can now be used as a separate library which only depends
glib and libudev - some IGT internals are being stubbed in this case.
Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
lib/Makefile.am | 8 ++++++-
lib/drmtest.c | 1 +
lib/igt_device_scan.c | 6 ++---
lib/igt_device_scan.h | 8 ++++++-
lib/igt_tools_stub.c | 51 +++++++++++++++++++++++++++++++++++++++++++
lib/meson.build | 15 +++++++++++++
6 files changed, 83 insertions(+), 6 deletions(-)
create mode 100644 lib/igt_tools_stub.c
diff --git a/lib/Makefile.am b/lib/Makefile.am
index fba7fcab..e9753d3c 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -102,6 +102,12 @@ include Makefile.sources
libintel_tools_la_SOURCES = $(lib_source_list)
+libigt_device_scan_la_SOURCES = \
+ igt_list.c \
+ igt_tools_stub.c \
+ igt_device_scan.c \
+ igt_device_scan.h
+
libigt_perf_la_SOURCES = \
igt_perf.c \
igt_perf.h
@@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
lib_LTLIBRARIES = libi915_perf.la
-noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
+noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
noinst_HEADERS = check-ndebug.h
if !HAVE_LIBDRM_INTEL
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 70fd64c9..bced1453 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -53,6 +53,7 @@
#include "intel_io.h"
#include "igt_debugfs.h"
#include "igt_device.h"
+#include "igt_params.h"
#include "igt_gt.h"
#include "igt_kmod.h"
#include "igt_sysfs.h"
diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 7b0fc00e..be854b01 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -22,11 +22,8 @@
*
*/
-#include "igt.h"
-#include "igt_list.h"
-#include "igt_sysfs.h"
-#include "igt_device.h"
#include "igt_device_scan.h"
+#include "igt_list.h"
#include <glib.h>
#include <libudev.h>
#include <linux/limits.h>
@@ -34,6 +31,7 @@
#include <sys/types.h>
#include <dirent.h>
#include <fcntl.h>
+#include "igt_core.h"
/**
* SECTION:igt_device_scan
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index 24eafe62..ad632145 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -26,7 +26,13 @@
#define __IGT_DEVICE_SCAN_H__
#include <limits.h>
-#include <igt.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <errno.h>
enum igt_devices_print_type {
IGT_PRINT_SIMPLE,
diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
new file mode 100644
index 00000000..9a0ec621
--- /dev/null
+++ b/lib/igt_tools_stub.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright © 2020 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#include "igt_core.h"
+
+/* Stub for igt_log, this stub will simply print the msg on stderr device.
+ * Domain and log level are ignored.
+ */
+
+void igt_log(const char *domain, enum igt_log_level level, const char *format, ...)
+{
+ va_list args;
+
+ va_start(args, format);
+ vfprintf(stderr, format, args);
+ va_end(args);
+}
+
+
+/* Stub for __igt_fail_assert, this stub will simply print the msg on stderr device and
+ * exit the application, domain and log level are ignored.
+ */
+
+void __igt_fail_assert(const char *domain, const char *file,
+ const int line, const char *func, const char *assertion,
+ const char *format, ...)
+{
+ fprintf(stderr, "%s: %d %s Failed assertion: %s\n", file, line,
+ func, assertion);
+ exit(1);
+}
diff --git a/lib/meson.build b/lib/meson.build
index 99aee6ee..6cf78663 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -172,6 +172,21 @@ lib_igt_perf_build = static_library('igt_perf',
lib_igt_perf = declare_dependency(link_with : lib_igt_perf_build,
include_directories : inc)
+scan_dep = [
+ glib,
+ libudev,
+]
+
+lib_igt_device_scan_build = static_library('igt_device_scan',
+ ['igt_device_scan.c',
+ 'igt_list.c',
+ 'igt_tools_stub.c',
+ ],
+ dependencies : scan_dep,
+ include_directories : inc)
+
+lib_igt_device_scan = declare_dependency(link_with : lib_igt_device_scan_build,
+ include_directories : inc)
i915_perf_files = [
'igt_list.c',
--
2.25.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ 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-05-26 12:24 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
@ 2020-05-26 16:14 ` Lucas De Marchi
2020-05-27 7:48 ` Arkadiusz Hiler
0 siblings, 1 reply; 18+ messages in thread
From: Lucas De Marchi @ 2020-05-26 16:14 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev, Petri Latvala, Ayaz A Siddiqui
On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> igt_device_scan can now be used as a separate library which only depends
> glib and libudev - some IGT internals are being stubbed in this case.
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
> lib/Makefile.am | 8 ++++++-
> lib/drmtest.c | 1 +
> lib/igt_device_scan.c | 6 ++---
> lib/igt_device_scan.h | 8 ++++++-
> lib/igt_tools_stub.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> lib/meson.build | 15 +++++++++++++
> 6 files changed, 83 insertions(+), 6 deletions(-)
> create mode 100644 lib/igt_tools_stub.c
>
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index fba7fcab..e9753d3c 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -102,6 +102,12 @@ include Makefile.sources
>
> libintel_tools_la_SOURCES = $(lib_source_list)
>
> +libigt_device_scan_la_SOURCES = \
> + igt_list.c \
> + igt_tools_stub.c \
> + igt_device_scan.c \
> + igt_device_scan.h
> +
> libigt_perf_la_SOURCES = \
> igt_perf.c \
> igt_perf.h
> @@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
>
> lib_LTLIBRARIES = libi915_perf.la
>
> -noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
> +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
> noinst_HEADERS = check-ndebug.h
>
> if !HAVE_LIBDRM_INTEL
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 70fd64c9..bced1453 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -53,6 +53,7 @@
> #include "intel_io.h"
> #include "igt_debugfs.h"
> #include "igt_device.h"
> +#include "igt_params.h"
> #include "igt_gt.h"
> #include "igt_kmod.h"
> #include "igt_sysfs.h"
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 7b0fc00e..be854b01 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -22,11 +22,8 @@
> *
> */
>
> -#include "igt.h"
> -#include "igt_list.h"
> -#include "igt_sysfs.h"
> -#include "igt_device.h"
> #include "igt_device_scan.h"
> +#include "igt_list.h"
> #include <glib.h>
> #include <libudev.h>
> #include <linux/limits.h>
> @@ -34,6 +31,7 @@
> #include <sys/types.h>
> #include <dirent.h>
> #include <fcntl.h>
> +#include "igt_core.h"
>
> /**
> * SECTION:igt_device_scan
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index 24eafe62..ad632145 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -26,7 +26,13 @@
> #define __IGT_DEVICE_SCAN_H__
>
> #include <limits.h>
> -#include <igt.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <getopt.h>
> +#include <errno.h>
shouldn't these be sorted?
>
> enum igt_devices_print_type {
> IGT_PRINT_SIMPLE,
> diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
> new file mode 100644
> index 00000000..9a0ec621
> --- /dev/null
> +++ b/lib/igt_tools_stub.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright © 2020 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
are we still in a mix of SPDX + full text?
> +#include "igt_core.h"
> +
> +/* Stub for igt_log, this stub will simply print the msg on stderr device.
> + * Domain and log level are ignored.
> + */
> +
> +void igt_log(const char *domain, enum igt_log_level level, const char *format, ...)
> +{
> + va_list args;
> +
> + va_start(args, format);
> + vfprintf(stderr, format, args);
> + va_end(args);
> +}
> +
> +
> +/* Stub for __igt_fail_assert, this stub will simply print the msg on stderr device and
> + * exit the application, domain and log level are ignored.
> + */
> +
> +void __igt_fail_assert(const char *domain, const char *file,
> + const int line, const char *func, const char *assertion,
> + const char *format, ...)
> +{
> + fprintf(stderr, "%s: %d %s Failed assertion: %s\n", file, line,
> + func, assertion);
> + exit(1);
> +}
maybe make these weak symbols so it still "works" if we bring in igt_core?
Lucas De Marchi
> diff --git a/lib/meson.build b/lib/meson.build
> index 99aee6ee..6cf78663 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -172,6 +172,21 @@ lib_igt_perf_build = static_library('igt_perf',
> lib_igt_perf = declare_dependency(link_with : lib_igt_perf_build,
> include_directories : inc)
>
> +scan_dep = [
> + glib,
> + libudev,
> +]
> +
> +lib_igt_device_scan_build = static_library('igt_device_scan',
> + ['igt_device_scan.c',
> + 'igt_list.c',
> + 'igt_tools_stub.c',
> + ],
> + dependencies : scan_dep,
> + include_directories : inc)
> +
> +lib_igt_device_scan = declare_dependency(link_with : lib_igt_device_scan_build,
> + include_directories : inc)
>
> i915_perf_files = [
> 'igt_list.c',
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ 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-05-26 16:14 ` Lucas De Marchi
@ 2020-05-27 7:48 ` Arkadiusz Hiler
2020-05-27 8:08 ` Petri Latvala
0 siblings, 1 reply; 18+ messages in thread
From: Arkadiusz Hiler @ 2020-05-27 7:48 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: igt-dev, Petri Latvala, Ayaz A Siddiqui
On Tue, May 26, 2020 at 09:14:58AM -0700, Lucas De Marchi wrote:
> On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
> <arkadiusz.hiler@intel.com> wrote:
> >
> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> >
> > igt_device_scan can now be used as a separate library which only depends
> > glib and libudev - some IGT internals are being stubbed in this case.
> >
> > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> > lib/Makefile.am | 8 ++++++-
> > lib/drmtest.c | 1 +
> > lib/igt_device_scan.c | 6 ++---
> > lib/igt_device_scan.h | 8 ++++++-
> > lib/igt_tools_stub.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> > lib/meson.build | 15 +++++++++++++
> > 6 files changed, 83 insertions(+), 6 deletions(-)
> > create mode 100644 lib/igt_tools_stub.c
> >
> > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > index fba7fcab..e9753d3c 100644
> > --- a/lib/Makefile.am
> > +++ b/lib/Makefile.am
> > @@ -102,6 +102,12 @@ include Makefile.sources
> >
> > libintel_tools_la_SOURCES = $(lib_source_list)
> >
> > +libigt_device_scan_la_SOURCES = \
> > + igt_list.c \
> > + igt_tools_stub.c \
> > + igt_device_scan.c \
> > + igt_device_scan.h
> > +
> > libigt_perf_la_SOURCES = \
> > igt_perf.c \
> > igt_perf.h
> > @@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
> >
> > lib_LTLIBRARIES = libi915_perf.la
> >
> > -noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
> > +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
> > noinst_HEADERS = check-ndebug.h
> >
> > if !HAVE_LIBDRM_INTEL
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index 70fd64c9..bced1453 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -53,6 +53,7 @@
> > #include "intel_io.h"
> > #include "igt_debugfs.h"
> > #include "igt_device.h"
> > +#include "igt_params.h"
> > #include "igt_gt.h"
> > #include "igt_kmod.h"
> > #include "igt_sysfs.h"
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 7b0fc00e..be854b01 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -22,11 +22,8 @@
> > *
> > */
> >
> > -#include "igt.h"
> > -#include "igt_list.h"
> > -#include "igt_sysfs.h"
> > -#include "igt_device.h"
> > #include "igt_device_scan.h"
> > +#include "igt_list.h"
> > #include <glib.h>
> > #include <libudev.h>
> > #include <linux/limits.h>
> > @@ -34,6 +31,7 @@
> > #include <sys/types.h>
> > #include <dirent.h>
> > #include <fcntl.h>
> > +#include "igt_core.h"
> >
> > /**
> > * SECTION:igt_device_scan
> > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > index 24eafe62..ad632145 100644
> > --- a/lib/igt_device_scan.h
> > +++ b/lib/igt_device_scan.h
> > @@ -26,7 +26,13 @@
> > #define __IGT_DEVICE_SCAN_H__
> >
> > #include <limits.h>
> > -#include <igt.h>
> > +#include <stdbool.h>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <getopt.h>
> > +#include <errno.h>
>
> shouldn't these be sorted?
It wouldn't hurt but kernel coding style doesn't mention header sorting.
IGT doesn't enforce anything on top of it ¯\_(ツ)_/¯
> > enum igt_devices_print_type {
> > IGT_PRINT_SIMPLE,
> > diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
> > new file mode 100644
> > index 00000000..9a0ec621
> > --- /dev/null
> > +++ b/lib/igt_tools_stub.c
> > @@ -0,0 +1,51 @@
> > +/*
> > + * Copyright © 2020 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
>
> are we still in a mix of SPDX + full text?
Only stubs and drm uapi seem to have SPDX identifiers as of now. Petri
was researching the topic, I don't remember the conclusion.
> > +#include "igt_core.h"
> > +
> > +/* Stub for igt_log, this stub will simply print the msg on stderr device.
> > + * Domain and log level are ignored.
> > + */
> > +
> > +void igt_log(const char *domain, enum igt_log_level level, const char *format, ...)
> > +{
> > + va_list args;
> > +
> > + va_start(args, format);
> > + vfprintf(stderr, format, args);
> > + va_end(args);
> > +}
> > +
> > +
> > +/* Stub for __igt_fail_assert, this stub will simply print the msg on stderr device and
> > + * exit the application, domain and log level are ignored.
> > + */
> > +
> > +void __igt_fail_assert(const char *domain, const char *file,
> > + const int line, const char *func, const char *assertion,
> > + const char *format, ...)
> > +{
> > + fprintf(stderr, "%s: %d %s Failed assertion: %s\n", file, line,
> > + func, assertion);
> > + exit(1);
> > +}
>
> maybe make these weak symbols so it still "works" if we bring in igt_core?
>
> Lucas De Marchi
igt_core comes with device_scan, so you would get linking error on the
device_scan functions anyway
--
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ 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-05-27 7:48 ` Arkadiusz Hiler
@ 2020-05-27 8:08 ` Petri Latvala
0 siblings, 0 replies; 18+ messages in thread
From: Petri Latvala @ 2020-05-27 8:08 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev, Ayaz A Siddiqui
On Wed, May 27, 2020 at 10:48:42AM +0300, Arkadiusz Hiler wrote:
> On Tue, May 26, 2020 at 09:14:58AM -0700, Lucas De Marchi wrote:
> > On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
> > <arkadiusz.hiler@intel.com> wrote:
> > >
> > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > >
> > > igt_device_scan can now be used as a separate library which only depends
> > > glib and libudev - some IGT internals are being stubbed in this case.
> > >
> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > > lib/Makefile.am | 8 ++++++-
> > > lib/drmtest.c | 1 +
> > > lib/igt_device_scan.c | 6 ++---
> > > lib/igt_device_scan.h | 8 ++++++-
> > > lib/igt_tools_stub.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> > > lib/meson.build | 15 +++++++++++++
> > > 6 files changed, 83 insertions(+), 6 deletions(-)
> > > create mode 100644 lib/igt_tools_stub.c
> > >
> > > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > > index fba7fcab..e9753d3c 100644
> > > --- a/lib/Makefile.am
> > > +++ b/lib/Makefile.am
> > > @@ -102,6 +102,12 @@ include Makefile.sources
> > >
> > > libintel_tools_la_SOURCES = $(lib_source_list)
> > >
> > > +libigt_device_scan_la_SOURCES = \
> > > + igt_list.c \
> > > + igt_tools_stub.c \
> > > + igt_device_scan.c \
> > > + igt_device_scan.h
> > > +
> > > libigt_perf_la_SOURCES = \
> > > igt_perf.c \
> > > igt_perf.h
> > > @@ -121,7 +127,7 @@ pkgconfig_DATA = i915-perf.pc
> > >
> > > lib_LTLIBRARIES = libi915_perf.la
> > >
> > > -noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la
> > > +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la libigt_device_scan.la
> > > noinst_HEADERS = check-ndebug.h
> > >
> > > if !HAVE_LIBDRM_INTEL
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index 70fd64c9..bced1453 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -53,6 +53,7 @@
> > > #include "intel_io.h"
> > > #include "igt_debugfs.h"
> > > #include "igt_device.h"
> > > +#include "igt_params.h"
> > > #include "igt_gt.h"
> > > #include "igt_kmod.h"
> > > #include "igt_sysfs.h"
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index 7b0fc00e..be854b01 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -22,11 +22,8 @@
> > > *
> > > */
> > >
> > > -#include "igt.h"
> > > -#include "igt_list.h"
> > > -#include "igt_sysfs.h"
> > > -#include "igt_device.h"
> > > #include "igt_device_scan.h"
> > > +#include "igt_list.h"
> > > #include <glib.h>
> > > #include <libudev.h>
> > > #include <linux/limits.h>
> > > @@ -34,6 +31,7 @@
> > > #include <sys/types.h>
> > > #include <dirent.h>
> > > #include <fcntl.h>
> > > +#include "igt_core.h"
> > >
> > > /**
> > > * SECTION:igt_device_scan
> > > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > > index 24eafe62..ad632145 100644
> > > --- a/lib/igt_device_scan.h
> > > +++ b/lib/igt_device_scan.h
> > > @@ -26,7 +26,13 @@
> > > #define __IGT_DEVICE_SCAN_H__
> > >
> > > #include <limits.h>
> > > -#include <igt.h>
> > > +#include <stdbool.h>
> > > +#include <stdint.h>
> > > +#include <stdio.h>
> > > +#include <fcntl.h>
> > > +#include <unistd.h>
> > > +#include <getopt.h>
> > > +#include <errno.h>
> >
> > shouldn't these be sorted?
>
> It wouldn't hurt but kernel coding style doesn't mention header sorting.
> IGT doesn't enforce anything on top of it ¯\_(ツ)_/¯
If header include blocks are already sorted, keep them sorted, that's
the thumb rule I've used. That said, please use "header-from-igt.h"
and <header-from-system>.
>
> > > enum igt_devices_print_type {
> > > IGT_PRINT_SIMPLE,
> > > diff --git a/lib/igt_tools_stub.c b/lib/igt_tools_stub.c
> > > new file mode 100644
> > > index 00000000..9a0ec621
> > > --- /dev/null
> > > +++ b/lib/igt_tools_stub.c
> > > @@ -0,0 +1,51 @@
> > > +/*
> > > + * Copyright © 2020 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining a
> > > + * copy of this software and associated documentation files (the "Software"),
> > > + * to deal in the Software without restriction, including without limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the next
> > > + * paragraph) shall be included in all copies or substantial portions of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + */
> >
> > are we still in a mix of SPDX + full text?
>
> Only stubs and drm uapi seem to have SPDX identifiers as of now. Petri
> was researching the topic, I don't remember the conclusion.
Converting from a normal text block to SPDX is a no-no. Using SPDX for
new files is ok.
--
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
2020-05-26 12:24 [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-05-26 12:24 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
@ 2020-05-26 12:24 ` Arkadiusz Hiler
2020-05-26 16:26 ` Lucas De Marchi
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
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Arkadiusz Hiler @ 2020-05-26 12:24 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 be854b01..4daabbd9 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -164,6 +164,7 @@ struct igt_device {
/* For pci subsystem */
char *vendor;
char *device;
+ char *pci_slot_name;
struct igt_list_head link;
};
@@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
#define is_drm_subsystem(dev) (strequal(get_prop_subsystem(dev), "drm"))
#define is_pci_subsystem(dev) (strequal(get_prop_subsystem(dev), "pci"))
-/* Gets PCI_ID property, splits to xxxx:yyyy and stores
+/*
+ * Get PCI_SLOT_NAME property, it should be in format of
+ * xxxx:yy:zz.z
+ */
+static void set_pci_slot_name(struct igt_device *dev)
+{
+ const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
+
+ if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
+ return;
+ dev->pci_slot_name = strdup(pci_slot_name);
+}
+
+/*
+ * Gets PCI_ID property, splits to xxxx:yyyy and stores
* xxxx to dev->vendor and yyyy to dev->device for
* faster access.
*/
@@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
return NULL;
}
+static void
+__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
+{
+ if (dev->subsystem != NULL)
+ strncpy(card->subsystem, dev->subsystem,
+ sizeof(card->subsystem) - 1);
+
+ if (dev->drm_card != NULL)
+ strncpy(card->card, dev->drm_card, sizeof(card->card) - 1);
+
+ if (dev->drm_render != NULL)
+ strncpy(card->render, dev->drm_render,
+ sizeof(card->render) - 1);
+
+ if (dev->pci_slot_name != NULL)
+ strncpy(card->pci_slot_name, dev->pci_slot_name,
+ PCI_SLOT_NAME_SIZE + 1);
+}
+
+/*
+ * Iterate over all igt_devices array and find first discrete card.
+ * card->pci_slot_name will be updated only if a discrete card is present.
+ */
+void igt_device_find_first_discrete_card(struct igt_device_card *card)
+{
+ struct igt_device *dev;
+
+ igt_list_for_each_entry(dev, &igt_devs.all, link) {
+
+ if (!is_pci_subsystem(dev))
+ continue;
+
+ if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
+ __copy_dev_to_card(dev, card);
+ break;
+ }
+ }
+}
+
static struct igt_device *igt_device_from_syspath(const char *syspath)
{
struct igt_device *dev;
@@ -445,6 +499,7 @@ static void update_or_add_parent(struct udev_device *dev,
parent_idev = igt_device_new_from_udev(parent_dev);
if (is_pci_subsystem(parent_idev)) {
set_vendor_device(parent_idev);
+ set_pci_slot_name(parent_idev);
}
igt_list_add_tail(&parent_idev->link, &igt_devs.all);
}
@@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device *dev)
free(dev->drm_render);
free(dev->vendor);
free(dev->device);
+ free(dev->pci_slot_name);
g_hash_table_destroy(dev->attrs_ht);
g_hash_table_destroy(dev->props_ht);
}
+void igt_devices_free(void)
+{
+ struct igt_device *dev, *tmp;
+
+ igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
+ igt_device_free(dev);
+ free(dev);
+ }
+}
+
/**
* igt_devices_scan
* @force: enforce scanning devices
@@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter, struct igt_device_card *card)
/* We take first one if more than one card matches filter */
dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
- if (dev->subsystem != NULL)
- strncpy(card->subsystem, dev->subsystem,
- sizeof(card->subsystem) - 1);
-
- if (dev->drm_card != NULL)
- strncpy(card->card, dev->drm_card,
- sizeof(card->card) - 1);
-
- if (dev->drm_render != NULL)
- strncpy(card->render, dev->drm_render,
- sizeof(card->render) - 1);
+ __copy_dev_to_card(dev, card);
return true;
}
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index ad632145..16d75320 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -39,10 +39,13 @@ enum igt_devices_print_type {
IGT_PRINT_DETAIL,
};
+#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
+#define PCI_SLOT_NAME_SIZE 12
struct igt_device_card {
char subsystem[NAME_MAX];
char card[NAME_MAX];
char render[NAME_MAX];
+ char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
};
void igt_devices_scan(bool force);
@@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type printtype);
void igt_devices_print_vendors(void);
void igt_device_print_filter_types(void);
+void igt_devices_free(void);
+
/*
* Handle device filter collection array.
* IGT can store/retrieve filters passed by user using '--device' args.
@@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
/* Use filter to match the device and fill card structure */
bool igt_device_card_match(const char *filter, struct igt_device_card *card);
-
+void igt_device_find_first_discrete_card(struct igt_device_card *card);
int igt_open_card(struct igt_device_card *card);
int igt_open_render(struct igt_device_card *card);
--
2.25.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
2020-05-26 12:24 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
@ 2020-05-26 16:26 ` Lucas De Marchi
2020-05-27 5:32 ` Siddiqui, Ayaz A
2020-05-28 10:27 ` Tvrtko Ursulin
0 siblings, 2 replies; 18+ messages in thread
From: Lucas De Marchi @ 2020-05-26 16:26 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev, Ayaz A Siddiqui, Petri Latvala, Tvrtko Ursulin
On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> Will be used in the next patch.
>
> 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
> 2. igt_device_find_first_discrete_card(): try to find first discrete GPU
> 3. igt_devices_free(): Free device buffers created during scan
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
> lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
> lib/igt_device_scan.h | 7 +++-
> 2 files changed, 74 insertions(+), 13 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index be854b01..4daabbd9 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -164,6 +164,7 @@ struct igt_device {
> /* For pci subsystem */
> char *vendor;
> char *device;
> + char *pci_slot_name;
>
> struct igt_list_head link;
> };
> @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
> #define is_drm_subsystem(dev) (strequal(get_prop_subsystem(dev), "drm"))
> #define is_pci_subsystem(dev) (strequal(get_prop_subsystem(dev), "pci"))
>
> -/* Gets PCI_ID property, splits to xxxx:yyyy and stores
> +/*
> + * Get PCI_SLOT_NAME property, it should be in format of
> + * xxxx:yy:zz.z
> + */
> +static void set_pci_slot_name(struct igt_device *dev)
> +{
> + const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> +
> + if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
> + return;
> + dev->pci_slot_name = strdup(pci_slot_name);
> +}
> +
> +/*
> + * Gets PCI_ID property, splits to xxxx:yyyy and stores
> * xxxx to dev->vendor and yyyy to dev->device for
> * faster access.
> */
> @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
> return NULL;
> }
>
> +static void
> +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
> +{
> + if (dev->subsystem != NULL)
> + strncpy(card->subsystem, dev->subsystem,
> + sizeof(card->subsystem) - 1);
> +
> + if (dev->drm_card != NULL)
> + strncpy(card->card, dev->drm_card, sizeof(card->card) - 1);
> +
> + if (dev->drm_render != NULL)
> + strncpy(card->render, dev->drm_render,
> + sizeof(card->render) - 1);
> +
> + if (dev->pci_slot_name != NULL)
> + strncpy(card->pci_slot_name, dev->pci_slot_name,
> + PCI_SLOT_NAME_SIZE + 1);
> +}
> +
> +/*
> + * Iterate over all igt_devices array and find first discrete card.
> + * card->pci_slot_name will be updated only if a discrete card is present.
> + */
> +void igt_device_find_first_discrete_card(struct igt_device_card *card)
> +{
> + struct igt_device *dev;
> +
> + igt_list_for_each_entry(dev, &igt_devs.all, link) {
> +
> + if (!is_pci_subsystem(dev))
> + continue;
> +
> + if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
> + __copy_dev_to_card(dev, card);
why do we need to make it prefer discrete? rather than "use the first
one if no one was specified"?
Lucas De Marchi
> + break;
> + }
> + }
> +}
> +
> static struct igt_device *igt_device_from_syspath(const char *syspath)
> {
> struct igt_device *dev;
> @@ -445,6 +499,7 @@ static void update_or_add_parent(struct udev_device *dev,
> parent_idev = igt_device_new_from_udev(parent_dev);
> if (is_pci_subsystem(parent_idev)) {
> set_vendor_device(parent_idev);
> + set_pci_slot_name(parent_idev);
> }
> igt_list_add_tail(&parent_idev->link, &igt_devs.all);
> }
> @@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device *dev)
> free(dev->drm_render);
> free(dev->vendor);
> free(dev->device);
> + free(dev->pci_slot_name);
> g_hash_table_destroy(dev->attrs_ht);
> g_hash_table_destroy(dev->props_ht);
> }
>
> +void igt_devices_free(void)
> +{
> + struct igt_device *dev, *tmp;
> +
> + igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
> + igt_device_free(dev);
> + free(dev);
> + }
> +}
> +
> /**
> * igt_devices_scan
> * @force: enforce scanning devices
> @@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter, struct igt_device_card *card)
> /* We take first one if more than one card matches filter */
> dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
>
> - if (dev->subsystem != NULL)
> - strncpy(card->subsystem, dev->subsystem,
> - sizeof(card->subsystem) - 1);
> -
> - if (dev->drm_card != NULL)
> - strncpy(card->card, dev->drm_card,
> - sizeof(card->card) - 1);
> -
> - if (dev->drm_render != NULL)
> - strncpy(card->render, dev->drm_render,
> - sizeof(card->render) - 1);
> + __copy_dev_to_card(dev, card);
>
> return true;
> }
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index ad632145..16d75320 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -39,10 +39,13 @@ enum igt_devices_print_type {
> IGT_PRINT_DETAIL,
> };
>
> +#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
> +#define PCI_SLOT_NAME_SIZE 12
> struct igt_device_card {
> char subsystem[NAME_MAX];
> char card[NAME_MAX];
> char render[NAME_MAX];
> + char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
> };
>
> void igt_devices_scan(bool force);
> @@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type printtype);
> void igt_devices_print_vendors(void);
> void igt_device_print_filter_types(void);
>
> +void igt_devices_free(void);
> +
> /*
> * Handle device filter collection array.
> * IGT can store/retrieve filters passed by user using '--device' args.
> @@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
>
> /* Use filter to match the device and fill card structure */
> bool igt_device_card_match(const char *filter, struct igt_device_card *card);
> -
> +void igt_device_find_first_discrete_card(struct igt_device_card *card);
> int igt_open_card(struct igt_device_card *card);
> int igt_open_render(struct igt_device_card *card);
>
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Lucas De Marchi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
2020-05-26 16:26 ` Lucas De Marchi
@ 2020-05-27 5:32 ` Siddiqui, Ayaz A
2020-05-28 10:27 ` Tvrtko Ursulin
1 sibling, 0 replies; 18+ messages in thread
From: Siddiqui, Ayaz A @ 2020-05-27 5:32 UTC (permalink / raw)
To: Lucas De Marchi, Hiler, Arkadiusz
Cc: igt-dev, Latvala, Petri, Ursulin, Tvrtko
> -----Original Message-----
> From: Lucas De Marchi <lucas.de.marchi@gmail.com>
> Sent: Tuesday, May 26, 2020 9:57 PM
> To: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Ursulin, Tvrtko
> <tvrtko.ursulin@intel.com>; Latvala, Petri <petri.latvala@intel.com>;
> Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra
> helpers for intel_gpu_top
>
> On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> wrote:
> >
> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> >
> > Will be used in the next patch.
> >
> > 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device 2.
> > igt_device_find_first_discrete_card(): try to find first discrete GPU
> > 3. igt_devices_free(): Free device buffers created during scan
> >
> > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> > lib/igt_device_scan.c | 80
> > ++++++++++++++++++++++++++++++++++++-------
> > lib/igt_device_scan.h | 7 +++-
> > 2 files changed, 74 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c index
> > be854b01..4daabbd9 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -164,6 +164,7 @@ struct igt_device {
> > /* For pci subsystem */
> > char *vendor;
> > char *device;
> > + char *pci_slot_name;
> >
> > struct igt_list_head link;
> > };
> > @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev,
> > struct igt_device *idev) #define is_drm_subsystem(dev)
> > (strequal(get_prop_subsystem(dev), "drm")) #define
> > is_pci_subsystem(dev) (strequal(get_prop_subsystem(dev), "pci"))
> >
> > -/* Gets PCI_ID property, splits to xxxx:yyyy and stores
> > +/*
> > + * Get PCI_SLOT_NAME property, it should be in format of
> > + * xxxx:yy:zz.z
> > + */
> > +static void set_pci_slot_name(struct igt_device *dev) {
> > + const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> > +
> > + if (!pci_slot_name || (strlen(pci_slot_name) !=
> PCI_SLOT_NAME_SIZE))
> > + return;
> > + dev->pci_slot_name = strdup(pci_slot_name); }
> > +
> > +/*
> > + * Gets PCI_ID property, splits to xxxx:yyyy and stores
> > * xxxx to dev->vendor and yyyy to dev->device for
> > * faster access.
> > */
> > @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char
> *subsystem,
> > return NULL;
> > }
> >
> > +static void
> > +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card
> > +*card) {
> > + if (dev->subsystem != NULL)
> > + strncpy(card->subsystem, dev->subsystem,
> > + sizeof(card->subsystem) - 1);
> > +
> > + if (dev->drm_card != NULL)
> > + strncpy(card->card, dev->drm_card, sizeof(card->card)
> > + - 1);
> > +
> > + if (dev->drm_render != NULL)
> > + strncpy(card->render, dev->drm_render,
> > + sizeof(card->render) - 1);
> > +
> > + if (dev->pci_slot_name != NULL)
> > + strncpy(card->pci_slot_name, dev->pci_slot_name,
> > + PCI_SLOT_NAME_SIZE + 1); }
> > +
> > +/*
> > + * Iterate over all igt_devices array and find first discrete card.
> > + * card->pci_slot_name will be updated only if a discrete card is present.
> > + */
> > +void igt_device_find_first_discrete_card(struct igt_device_card
> > +*card) {
> > + struct igt_device *dev;
> > +
> > + igt_list_for_each_entry(dev, &igt_devs.all, link) {
> > +
> > + if (!is_pci_subsystem(dev))
> > + continue;
> > +
> > + if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID,
> PCI_SLOT_NAME_SIZE)) != 0) {
> > + __copy_dev_to_card(dev, card);
>
> why do we need to make it prefer discrete? rather than "use the first one if
> no one was specified"?
>
> Lucas De Marchi
>
Discreate is usually add on top of integrated GPU if both are present ,
that’s why we are giving preference to discreate.
Regards
-Ayaz
> > + break;
> > + }
> > + }
> > +}
> > +
> > static struct igt_device *igt_device_from_syspath(const char
> > *syspath) {
> > struct igt_device *dev;
> > @@ -445,6 +499,7 @@ static void update_or_add_parent(struct
> udev_device *dev,
> > parent_idev = igt_device_new_from_udev(parent_dev);
> > if (is_pci_subsystem(parent_idev)) {
> > set_vendor_device(parent_idev);
> > + set_pci_slot_name(parent_idev);
> > }
> > igt_list_add_tail(&parent_idev->link, &igt_devs.all);
> > }
> > @@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device
> *dev)
> > free(dev->drm_render);
> > free(dev->vendor);
> > free(dev->device);
> > + free(dev->pci_slot_name);
> > g_hash_table_destroy(dev->attrs_ht);
> > g_hash_table_destroy(dev->props_ht);
> > }
> >
> > +void igt_devices_free(void)
> > +{
> > + struct igt_device *dev, *tmp;
> > +
> > + igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
> > + igt_device_free(dev);
> > + free(dev);
> > + }
> > +}
> > +
> > /**
> > * igt_devices_scan
> > * @force: enforce scanning devices
> > @@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter,
> struct igt_device_card *card)
> > /* We take first one if more than one card matches filter */
> > dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
> >
> > - if (dev->subsystem != NULL)
> > - strncpy(card->subsystem, dev->subsystem,
> > - sizeof(card->subsystem) - 1);
> > -
> > - if (dev->drm_card != NULL)
> > - strncpy(card->card, dev->drm_card,
> > - sizeof(card->card) - 1);
> > -
> > - if (dev->drm_render != NULL)
> > - strncpy(card->render, dev->drm_render,
> > - sizeof(card->render) - 1);
> > + __copy_dev_to_card(dev, card);
> >
> > return true;
> > }
> > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h index
> > ad632145..16d75320 100644
> > --- a/lib/igt_device_scan.h
> > +++ b/lib/igt_device_scan.h
> > @@ -39,10 +39,13 @@ enum igt_devices_print_type {
> > IGT_PRINT_DETAIL,
> > };
> >
> > +#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
> > +#define PCI_SLOT_NAME_SIZE 12
> > struct igt_device_card {
> > char subsystem[NAME_MAX];
> > char card[NAME_MAX];
> > char render[NAME_MAX];
> > + char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
> > };
> >
> > void igt_devices_scan(bool force);
> > @@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type
> > printtype); void igt_devices_print_vendors(void); void
> > igt_device_print_filter_types(void);
> >
> > +void igt_devices_free(void);
> > +
> > /*
> > * Handle device filter collection array.
> > * IGT can store/retrieve filters passed by user using '--device' args.
> > @@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
> >
> > /* Use filter to match the device and fill card structure */ bool
> > igt_device_card_match(const char *filter, struct igt_device_card
> > *card);
> > -
> > +void igt_device_find_first_discrete_card(struct igt_device_card
> > +*card);
> > int igt_open_card(struct igt_device_card *card); int
> > igt_open_render(struct igt_device_card *card);
> >
> > --
> > 2.25.4
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
>
>
> --
> Lucas De Marchi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
2020-05-26 16:26 ` Lucas De Marchi
2020-05-27 5:32 ` Siddiqui, Ayaz A
@ 2020-05-28 10:27 ` Tvrtko Ursulin
2020-05-28 11:40 ` Arkadiusz Hiler
1 sibling, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2020-05-28 10:27 UTC (permalink / raw)
To: Lucas De Marchi, Arkadiusz Hiler
Cc: igt-dev, Tvrtko Ursulin, Ayaz A Siddiqui, Petri Latvala
On 26/05/2020 17:26, Lucas De Marchi wrote:
> On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler
> <arkadiusz.hiler@intel.com> wrote:
>>
>> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>>
>> Will be used in the next patch.
>>
>> 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
>> 2. igt_device_find_first_discrete_card(): try to find first discrete GPU
>> 3. igt_devices_free(): Free device buffers created during scan
>>
>> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> ---
>> lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
>> lib/igt_device_scan.h | 7 +++-
>> 2 files changed, 74 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>> index be854b01..4daabbd9 100644
>> --- a/lib/igt_device_scan.c
>> +++ b/lib/igt_device_scan.c
>> @@ -164,6 +164,7 @@ struct igt_device {
>> /* For pci subsystem */
>> char *vendor;
>> char *device;
>> + char *pci_slot_name;
>>
>> struct igt_list_head link;
>> };
>> @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
>> #define is_drm_subsystem(dev) (strequal(get_prop_subsystem(dev), "drm"))
>> #define is_pci_subsystem(dev) (strequal(get_prop_subsystem(dev), "pci"))
>>
>> -/* Gets PCI_ID property, splits to xxxx:yyyy and stores
>> +/*
>> + * Get PCI_SLOT_NAME property, it should be in format of
>> + * xxxx:yy:zz.z
>> + */
>> +static void set_pci_slot_name(struct igt_device *dev)
>> +{
>> + const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
>> +
>> + if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
>> + return;
>> + dev->pci_slot_name = strdup(pci_slot_name);
>> +}
>> +
>> +/*
>> + * Gets PCI_ID property, splits to xxxx:yyyy and stores
>> * xxxx to dev->vendor and yyyy to dev->device for
>> * faster access.
>> */
>> @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
>> return NULL;
>> }
>>
>> +static void
>> +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
>> +{
>> + if (dev->subsystem != NULL)
>> + strncpy(card->subsystem, dev->subsystem,
>> + sizeof(card->subsystem) - 1);
>> +
>> + if (dev->drm_card != NULL)
>> + strncpy(card->card, dev->drm_card, sizeof(card->card) - 1);
>> +
>> + if (dev->drm_render != NULL)
>> + strncpy(card->render, dev->drm_render,
>> + sizeof(card->render) - 1);
>> +
>> + if (dev->pci_slot_name != NULL)
>> + strncpy(card->pci_slot_name, dev->pci_slot_name,
>> + PCI_SLOT_NAME_SIZE + 1);
>> +}
>> +
>> +/*
>> + * Iterate over all igt_devices array and find first discrete card.
>> + * card->pci_slot_name will be updated only if a discrete card is present.
>> + */
>> +void igt_device_find_first_discrete_card(struct igt_device_card *card)
>> +{
>> + struct igt_device *dev;
>> +
>> + igt_list_for_each_entry(dev, &igt_devs.all, link) {
>> +
>> + if (!is_pci_subsystem(dev))
>> + continue;
>> +
>> + if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
>> + __copy_dev_to_card(dev, card);
>
> why do we need to make it prefer discrete? rather than "use the first
> one if no one was specified"?
I thought that makes more sense for end user, assuming igpu and
discrete, that user is more likely to want to look at the latter.
Otherwise I am also not sure if device enumeration is stable (across
boots, distros, kernel versions, udevs, systemd, whoever is involved in
providing the data device scan uses, plus the order of binding the
driver to pci devices). So from that angle as well it made more sense to
pick first discrete. Granted the approach falls apart with more than one
discrete.. Problem for another day?
Regards,
Tvrtko
>
> Lucas De Marchi
>
>> + break;
>> + }
>> + }
>> +}
>> +
>> static struct igt_device *igt_device_from_syspath(const char *syspath)
>> {
>> struct igt_device *dev;
>> @@ -445,6 +499,7 @@ static void update_or_add_parent(struct udev_device *dev,
>> parent_idev = igt_device_new_from_udev(parent_dev);
>> if (is_pci_subsystem(parent_idev)) {
>> set_vendor_device(parent_idev);
>> + set_pci_slot_name(parent_idev);
>> }
>> igt_list_add_tail(&parent_idev->link, &igt_devs.all);
>> }
>> @@ -573,10 +628,21 @@ static void igt_device_free(struct igt_device *dev)
>> free(dev->drm_render);
>> free(dev->vendor);
>> free(dev->device);
>> + free(dev->pci_slot_name);
>> g_hash_table_destroy(dev->attrs_ht);
>> g_hash_table_destroy(dev->props_ht);
>> }
>>
>> +void igt_devices_free(void)
>> +{
>> + struct igt_device *dev, *tmp;
>> +
>> + igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
>> + igt_device_free(dev);
>> + free(dev);
>> + }
>> +}
>> +
>> /**
>> * igt_devices_scan
>> * @force: enforce scanning devices
>> @@ -1196,17 +1262,7 @@ bool igt_device_card_match(const char *filter, struct igt_device_card *card)
>> /* We take first one if more than one card matches filter */
>> dev = igt_list_first_entry(&igt_devs.filtered, dev, link);
>>
>> - if (dev->subsystem != NULL)
>> - strncpy(card->subsystem, dev->subsystem,
>> - sizeof(card->subsystem) - 1);
>> -
>> - if (dev->drm_card != NULL)
>> - strncpy(card->card, dev->drm_card,
>> - sizeof(card->card) - 1);
>> -
>> - if (dev->drm_render != NULL)
>> - strncpy(card->render, dev->drm_render,
>> - sizeof(card->render) - 1);
>> + __copy_dev_to_card(dev, card);
>>
>> return true;
>> }
>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
>> index ad632145..16d75320 100644
>> --- a/lib/igt_device_scan.h
>> +++ b/lib/igt_device_scan.h
>> @@ -39,10 +39,13 @@ enum igt_devices_print_type {
>> IGT_PRINT_DETAIL,
>> };
>>
>> +#define INTEGRATED_GPU_PCI_ID "0000:00:02.0"
>> +#define PCI_SLOT_NAME_SIZE 12
>> struct igt_device_card {
>> char subsystem[NAME_MAX];
>> char card[NAME_MAX];
>> char render[NAME_MAX];
>> + char pci_slot_name[PCI_SLOT_NAME_SIZE+1];
>> };
>>
>> void igt_devices_scan(bool force);
>> @@ -51,6 +54,8 @@ void igt_devices_print(enum igt_devices_print_type printtype);
>> void igt_devices_print_vendors(void);
>> void igt_device_print_filter_types(void);
>>
>> +void igt_devices_free(void);
>> +
>> /*
>> * Handle device filter collection array.
>> * IGT can store/retrieve filters passed by user using '--device' args.
>> @@ -63,7 +68,7 @@ const char *igt_device_filter_get(int num);
>>
>> /* Use filter to match the device and fill card structure */
>> bool igt_device_card_match(const char *filter, struct igt_device_card *card);
>> -
>> +void igt_device_find_first_discrete_card(struct igt_device_card *card);
>> int igt_open_card(struct igt_device_card *card);
>> int igt_open_render(struct igt_device_card *card);
>>
>> --
>> 2.25.4
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top
2020-05-28 10:27 ` Tvrtko Ursulin
@ 2020-05-28 11:40 ` Arkadiusz Hiler
0 siblings, 0 replies; 18+ messages in thread
From: Arkadiusz Hiler @ 2020-05-28 11:40 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Tvrtko Ursulin, Petri Latvala, Ayaz A Siddiqui
On Thu, May 28, 2020 at 11:27:13AM +0100, Tvrtko Ursulin wrote:
>
> On 26/05/2020 17:26, Lucas De Marchi wrote:
> > On Tue, May 26, 2020 at 5:25 AM Arkadiusz Hiler
> > <arkadiusz.hiler@intel.com> wrote:
> > >
> > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > >
> > > Will be used in the next patch.
> > >
> > > 1. set_pci_slot_name(): stores PCI_SLOT_NAME from prop to device
> > > 2. igt_device_find_first_discrete_card(): try to find first discrete GPU
> > > 3. igt_devices_free(): Free device buffers created during scan
> > >
> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > > lib/igt_device_scan.c | 80 ++++++++++++++++++++++++++++++++++++-------
> > > lib/igt_device_scan.h | 7 +++-
> > > 2 files changed, 74 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index be854b01..4daabbd9 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -164,6 +164,7 @@ struct igt_device {
> > > /* For pci subsystem */
> > > char *vendor;
> > > char *device;
> > > + char *pci_slot_name;
> > >
> > > struct igt_list_head link;
> > > };
> > > @@ -337,7 +338,21 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
> > > #define is_drm_subsystem(dev) (strequal(get_prop_subsystem(dev), "drm"))
> > > #define is_pci_subsystem(dev) (strequal(get_prop_subsystem(dev), "pci"))
> > >
> > > -/* Gets PCI_ID property, splits to xxxx:yyyy and stores
> > > +/*
> > > + * Get PCI_SLOT_NAME property, it should be in format of
> > > + * xxxx:yy:zz.z
> > > + */
> > > +static void set_pci_slot_name(struct igt_device *dev)
> > > +{
> > > + const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> > > +
> > > + if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
> > > + return;
> > > + dev->pci_slot_name = strdup(pci_slot_name);
> > > +}
> > > +
> > > +/*
> > > + * Gets PCI_ID property, splits to xxxx:yyyy and stores
> > > * xxxx to dev->vendor and yyyy to dev->device for
> > > * faster access.
> > > */
> > > @@ -410,6 +425,45 @@ static struct igt_device *igt_device_find(const char *subsystem,
> > > return NULL;
> > > }
> > >
> > > +static void
> > > +__copy_dev_to_card(struct igt_device *dev, struct igt_device_card *card)
> > > +{
> > > + if (dev->subsystem != NULL)
> > > + strncpy(card->subsystem, dev->subsystem,
> > > + sizeof(card->subsystem) - 1);
> > > +
> > > + if (dev->drm_card != NULL)
> > > + strncpy(card->card, dev->drm_card, sizeof(card->card) - 1);
> > > +
> > > + if (dev->drm_render != NULL)
> > > + strncpy(card->render, dev->drm_render,
> > > + sizeof(card->render) - 1);
> > > +
> > > + if (dev->pci_slot_name != NULL)
> > > + strncpy(card->pci_slot_name, dev->pci_slot_name,
> > > + PCI_SLOT_NAME_SIZE + 1);
> > > +}
> > > +
> > > +/*
> > > + * Iterate over all igt_devices array and find first discrete card.
> > > + * card->pci_slot_name will be updated only if a discrete card is present.
> > > + */
> > > +void igt_device_find_first_discrete_card(struct igt_device_card *card)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + igt_list_for_each_entry(dev, &igt_devs.all, link) {
> > > +
> > > + if (!is_pci_subsystem(dev))
> > > + continue;
> > > +
> > > + if ((strncmp(dev->pci_slot_name, INTEGRATED_GPU_PCI_ID, PCI_SLOT_NAME_SIZE)) != 0) {
> > > + __copy_dev_to_card(dev, card);
> >
> > why do we need to make it prefer discrete? rather than "use the first
> > one if no one was specified"?
>
> I thought that makes more sense for end user, assuming igpu and discrete,
> that user is more likely to want to look at the latter.
>
> Otherwise I am also not sure if device enumeration is stable (across boots,
> distros, kernel versions, udevs, systemd, whoever is involved in providing
> the data device scan uses, plus the order of binding the driver to pci
> devices). So from that angle as well it made more sense to pick first
> discrete. Granted the approach falls apart with more than one discrete..
> Problem for another day?
Internally we sort the PCI devices by their syspath, so that should
help with this quarrel.
--
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ 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] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
2020-05-26 12:24 ` [igt-dev] [PATCH i-g-t 2/4] lib/igt_device_scan: Make igt_device_scan independent from igt_core Arkadiusz Hiler
2020-05-26 12:24 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_device_scan: Add extra helpers for intel_gpu_top Arkadiusz Hiler
@ 2020-05-26 12:24 ` Arkadiusz Hiler
2020-05-28 14:28 ` Tvrtko Ursulin
2020-05-26 13:17 ` [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
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ 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] 18+ 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; 18+ 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] 18+ 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-05-26 12:24 [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-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-26 13:17 ` Patchwork
2020-05-26 16:06 ` [igt-dev] [PATCH i-g-t 1/4] " Lucas De Marchi
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-05-26 13:17 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/77660/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8539 -> IGTPW_4612
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
Known issues
------------
Here are the changes found in IGTPW_4612 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [PASS][1] -> [FAIL][2] ([i915#227])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[i915#227]: https://gitlab.freedesktop.org/drm/intel/issues/227
Participating hosts (50 -> 44)
------------------------------
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5678 -> IGTPW_4612
CI-20190529: 20190529
CI_DRM_8539: 6590da76d8c6a2952b3cde70859bad41c59ed444 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4612: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
IGT_5678: c1f30ee09ac2e7eb3e8e90245239731a169a6050 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
2020-05-26 12:24 [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-05-26 13:17 ` [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-05-26 16:06 ` Lucas De Marchi
2020-05-26 17:38 ` [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/4] " Patchwork
2020-05-28 10:47 ` [igt-dev] [PATCH i-g-t 1/4] " Mika Kuoppala
6 siblings, 0 replies; 18+ messages in thread
From: Lucas De Marchi @ 2020-05-26 16:06 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev, Petri Latvala, Ayaz A Siddiqui
On Tue, May 26, 2020 at 5:24 AM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> Return value of udev_enumerate_add_match_property() was not being checked
> in scan_drm_devices().
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Lucas De Marchi
> ---
> lib/igt_device_scan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 601d6fb0..7b0fc00e 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -532,7 +532,7 @@ static void scan_drm_devices(void)
> ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
> igt_assert(!ret);
>
> - udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> + ret = udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> igt_assert(!ret);
>
> ret = udev_enumerate_scan_devices(enumerate);
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Lucas De Marchi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread* [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
2020-05-26 12:24 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
` (4 preceding siblings ...)
2020-05-26 16:06 ` [igt-dev] [PATCH i-g-t 1/4] " Lucas De Marchi
@ 2020-05-26 17:38 ` Patchwork
2020-05-28 10:47 ` [igt-dev] [PATCH i-g-t 1/4] " Mika Kuoppala
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-05-26 17:38 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/77660/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8539_full -> IGTPW_4612_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_4612_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_4612_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_4612_full:
### IGT changes ###
#### Possible regressions ####
* igt@gem_exec_flush@basic-batch-kernel-default-cmd:
- shard-hsw: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-hsw2/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw2/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html
* igt@gen9_exec_parse@cmd-crossing-page:
- shard-glk: [PASS][3] -> [TIMEOUT][4] +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk5/igt@gen9_exec_parse@cmd-crossing-page.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@gen9_exec_parse@cmd-crossing-page.html
* igt@runner@aborted:
- shard-hsw: NOTRUN -> ([FAIL][5], [FAIL][6])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw7/igt@runner@aborted.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw2/igt@runner@aborted.html
#### Warnings ####
* igt@kms_ccs@pipe-d-crc-primary-rotation-180:
- shard-glk: [SKIP][7] ([fdo#109271]) -> [TIMEOUT][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk9/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@gem_exec_reloc@basic-concurrent16}:
- shard-glk: NOTRUN -> [TIMEOUT][9]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@gem_exec_reloc@basic-concurrent16.html
Known issues
------------
Here are the changes found in IGTPW_4612_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gen9_exec_parse@allowed-all:
- shard-apl: [PASS][10] -> [DMESG-WARN][11] ([i915#1436] / [i915#716])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl1/igt@gen9_exec_parse@allowed-all.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@gen9_exec_parse@allowed-all.html
- shard-glk: [PASS][12] -> [DMESG-WARN][13] ([i915#1436] / [i915#716])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk1/igt@gen9_exec_parse@allowed-all.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk7/igt@gen9_exec_parse@allowed-all.html
* igt@gen9_exec_parse@cmd-crossing-page:
- shard-apl: [PASS][14] -> [TIMEOUT][15] ([i915#1635]) +1 similar issue
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@gen9_exec_parse@cmd-crossing-page.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl6/igt@gen9_exec_parse@cmd-crossing-page.html
* igt@i915_pm_rpm@pm-tiling:
- shard-apl: [PASS][16] -> [SKIP][17] ([fdo#109271])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl7/igt@i915_pm_rpm@pm-tiling.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl6/igt@i915_pm_rpm@pm-tiling.html
- shard-glk: [PASS][18] -> [SKIP][19] ([fdo#109271])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk6/igt@i915_pm_rpm@pm-tiling.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@i915_pm_rpm@pm-tiling.html
* igt@i915_suspend@fence-restore-tiled2untiled:
- shard-apl: [PASS][20] -> [DMESG-WARN][21] ([i915#180]) +2 similar issues
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
* igt@i915_suspend@forcewake:
- shard-kbl: [PASS][22] -> [INCOMPLETE][23] ([i915#155])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl3/igt@i915_suspend@forcewake.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl4/igt@i915_suspend@forcewake.html
* igt@kms_big_fb@linear-64bpp-rotate-0:
- shard-glk: [PASS][24] -> [FAIL][25] ([i915#1119] / [i915#118] / [i915#95])
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk7/igt@kms_big_fb@linear-64bpp-rotate-0.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-0.html
* igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen:
- shard-kbl: [PASS][26] -> [FAIL][27] ([i915#54] / [i915#93] / [i915#95])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
* igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen:
- shard-kbl: [PASS][28] -> [FAIL][29] ([i915#54])
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-256x85-onscreen.html
* igt@kms_cursor_crc@pipe-c-cursor-suspend:
- shard-kbl: [PASS][30] -> [DMESG-WARN][31] ([i915#180]) +3 similar issues
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
* igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
- shard-hsw: [PASS][32] -> [FAIL][33] ([i915#57])
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-hsw8/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
* igt@kms_flip_tiling@flip-changes-tiling:
- shard-apl: [PASS][34] -> [FAIL][35] ([i915#95])
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl6/igt@kms_flip_tiling@flip-changes-tiling.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl7/igt@kms_flip_tiling@flip-changes-tiling.html
- shard-kbl: [PASS][36] -> [FAIL][37] ([i915#699] / [i915#93] / [i915#95])
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl6/igt@kms_flip_tiling@flip-changes-tiling.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl2/igt@kms_flip_tiling@flip-changes-tiling.html
* igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
- shard-apl: [PASS][38] -> [FAIL][39] ([i915#53] / [i915#95]) +1 similar issue
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- shard-kbl: [PASS][40] -> [FAIL][41] ([i915#53] / [i915#93] / [i915#95]) +1 similar issue
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl1/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
* igt@kms_psr@psr2_no_drrs:
- shard-iclb: [PASS][42] -> [SKIP][43] ([fdo#109441]) +3 similar issues
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb6/igt@kms_psr@psr2_no_drrs.html
#### Possible fixes ####
* {igt@gem_exec_endless@dispatch@vcs0}:
- shard-apl: [SKIP][44] ([fdo#109271]) -> [PASS][45] +2 similar issues
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl4/igt@gem_exec_endless@dispatch@vcs0.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl2/igt@gem_exec_endless@dispatch@vcs0.html
* {igt@gem_exec_endless@dispatch@vcs1}:
- shard-tglb: [SKIP][46] ([i915#1929]) -> [PASS][47] +4 similar issues
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb8/igt@gem_exec_endless@dispatch@vcs1.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb2/igt@gem_exec_endless@dispatch@vcs1.html
- shard-kbl: [SKIP][48] ([fdo#109271]) -> [PASS][49] +3 similar issues
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl6/igt@gem_exec_endless@dispatch@vcs1.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl6/igt@gem_exec_endless@dispatch@vcs1.html
- shard-iclb: [SKIP][50] ([fdo#109276] / [i915#1929]) -> [PASS][51]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@gem_exec_endless@dispatch@vcs1.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb4/igt@gem_exec_endless@dispatch@vcs1.html
* {igt@gem_exec_endless@dispatch@vecs0}:
- shard-iclb: [SKIP][52] ([i915#1929]) -> [PASS][53] +3 similar issues
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@gem_exec_endless@dispatch@vecs0.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb4/igt@gem_exec_endless@dispatch@vecs0.html
* igt@i915_pm_dc@dc3co-vpb-simulation:
- shard-tglb: [SKIP][54] ([i915#1904]) -> [PASS][55]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb8/igt@i915_pm_dc@dc3co-vpb-simulation.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb7/igt@i915_pm_dc@dc3co-vpb-simulation.html
* igt@kms_big_fb@linear-32bpp-rotate-180:
- shard-apl: [FAIL][56] ([i915#1119] / [i915#95]) -> [PASS][57]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl4/igt@kms_big_fb@linear-32bpp-rotate-180.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl4/igt@kms_big_fb@linear-32bpp-rotate-180.html
- shard-kbl: [FAIL][58] ([i915#1119] / [i915#93] / [i915#95]) -> [PASS][59]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_big_fb@linear-32bpp-rotate-180.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl1/igt@kms_big_fb@linear-32bpp-rotate-180.html
* igt@kms_big_fb@x-tiled-64bpp-rotate-0:
- shard-glk: [FAIL][60] ([i915#1119] / [i915#118] / [i915#95]) -> [PASS][61]
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk8/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk5/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
* igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen:
- shard-kbl: [FAIL][62] ([i915#54] / [i915#93] / [i915#95]) -> [PASS][63]
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-hsw: [INCOMPLETE][64] ([i915#61]) -> [PASS][65]
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-hsw2/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
* {igt@kms_flip@flip-vs-suspend-interruptible@a-dp1}:
- shard-kbl: [DMESG-WARN][66] ([i915#180]) -> [PASS][67] +7 similar issues
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
* {igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1}:
- shard-apl: [FAIL][68] ([i915#407]) -> [PASS][69]
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl7/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html
* igt@kms_hdr@bpc-switch-suspend:
- shard-kbl: [DMESG-WARN][70] ([i915#180] / [i915#93] / [i915#95]) -> [PASS][71]
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_hdr@bpc-switch-suspend.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl4/igt@kms_hdr@bpc-switch-suspend.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- shard-apl: [DMESG-WARN][72] ([i915#180]) -> [PASS][73] +3 similar issues
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
* igt@kms_psr@psr2_sprite_mmap_gtt:
- shard-iclb: [SKIP][74] ([fdo#109441]) -> [PASS][75] +2 similar issues
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_gtt.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
* igt@kms_setmode@basic:
- shard-kbl: [FAIL][76] ([i915#31]) -> [PASS][77]
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl4/igt@kms_setmode@basic.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl1/igt@kms_setmode@basic.html
#### Warnings ####
* igt@i915_pm_dc@dc3co-vpb-simulation:
- shard-iclb: [SKIP][78] ([i915#588]) -> [SKIP][79] ([i915#658])
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-iclb3/igt@i915_pm_dc@dc3co-vpb-simulation.html
* igt@i915_pm_dc@dc6-dpms:
- shard-tglb: [SKIP][80] ([i915#468]) -> [FAIL][81] ([i915#454])
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb8/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_dc@dc6-psr:
- shard-tglb: [FAIL][82] ([i915#454]) -> [SKIP][83] ([i915#468])
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-tglb1/igt@i915_pm_dc@dc6-psr.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-tglb2/igt@i915_pm_dc@dc6-psr.html
* igt@kms_busy@basic-modeset-pipe-d:
- shard-apl: [TIMEOUT][84] ([i915#1635]) -> [SKIP][85] ([fdo#109271])
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl7/igt@kms_busy@basic-modeset-pipe-d.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl1/igt@kms_busy@basic-modeset-pipe-d.html
* igt@kms_ccs@pipe-d-crc-primary-rotation-180:
- shard-apl: [SKIP][86] ([fdo#109271]) -> [TIMEOUT][87] ([i915#1635]) +2 similar issues
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl6/igt@kms_ccs@pipe-d-crc-primary-rotation-180.html
* igt@kms_content_protection@legacy:
- shard-apl: [FAIL][88] ([fdo#110321] / [fdo#110336]) -> [TIMEOUT][89] ([i915#1319] / [i915#1635])
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-apl8/igt@kms_content_protection@legacy.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-apl3/igt@kms_content_protection@legacy.html
* igt@kms_content_protection@lic:
- shard-kbl: [TIMEOUT][90] ([i915#1319]) -> [FAIL][91] ([fdo#110321] / [i915#93] / [i915#95])
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-kbl7/igt@kms_content_protection@lic.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-kbl2/igt@kms_content_protection@lic.html
* igt@kms_cursor_legacy@cursorb-vs-flipa-toggle:
- shard-glk: [DMESG-FAIL][92] ([i915#1925]) -> [DMESG-WARN][93] ([i915#1926])
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8539/shard-glk1/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/shard-glk2/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
[fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
[i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
[i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
[i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
[i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
[i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1904]: https://gitlab.freedesktop.org/drm/intel/issues/1904
[i915#1925]: https://gitlab.freedesktop.org/drm/intel/issues/1925
[i915#1926]: https://gitlab.freedesktop.org/drm/intel/issues/1926
[i915#1929]: https://gitlab.freedesktop.org/drm/intel/issues/1929
[i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
[i915#407]: https://gitlab.freedesktop.org/drm/intel/issues/407
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
[i915#53]: https://gitlab.freedesktop.org/drm/intel/issues/53
[i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
[i915#57]: https://gitlab.freedesktop.org/drm/intel/issues/57
[i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
[i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#699]: https://gitlab.freedesktop.org/drm/intel/issues/699
[i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
[i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (11 -> 8)
------------------------------
Missing (3): pig-skl-6260u pig-glk-j5005 pig-icl-1065g7
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5678 -> IGTPW_4612
* Piglit: piglit_4509 -> None
CI-20190529: 20190529
CI_DRM_8539: 6590da76d8c6a2952b3cde70859bad41c59ed444 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4612: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
IGT_5678: c1f30ee09ac2e7eb3e8e90245239731a169a6050 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4612/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices
2020-05-26 12:24 [igt-dev] [PATCH i-g-t 1/4] lib/igt_device_scan: Add missing Check for return value in scan_drm_devices Arkadiusz Hiler
` (5 preceding siblings ...)
2020-05-26 17:38 ` [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/4] " Patchwork
@ 2020-05-28 10:47 ` Mika Kuoppala
6 siblings, 0 replies; 18+ messages in thread
From: Mika Kuoppala @ 2020-05-28 10:47 UTC (permalink / raw)
To: Arkadiusz Hiler, igt-dev; +Cc: Petri Latvala, Ayaz A Siddiqui
Arkadiusz Hiler <arkadiusz.hiler@intel.com> writes:
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
> Return value of udev_enumerate_add_match_property() was not being checked
> in scan_drm_devices().
>
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> lib/igt_device_scan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 601d6fb0..7b0fc00e 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -532,7 +532,7 @@ static void scan_drm_devices(void)
> ret = udev_enumerate_add_match_subsystem(enumerate, "drm");
> igt_assert(!ret);
>
> - udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> + ret = udev_enumerate_add_match_property(enumerate, "DEVNAME", "/dev/dri/*");
> igt_assert(!ret);
>
> ret = udev_enumerate_scan_devices(enumerate);
> --
> 2.25.4
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ 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 Arkadiusz Hiler
@ 2020-06-02 11:13 ` Arkadiusz Hiler
2020-06-05 12:11 ` Petri Latvala
0 siblings, 1 reply; 18+ 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] 18+ 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; 18+ 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] 18+ messages in thread