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

Arkadiusz Hiler arkadiusz.hiler at intel.com
Wed May 27 07:48:42 UTC 2020


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 at intel.com> wrote:
> >
> > From: Ayaz A Siddiqui <ayaz.siddiqui at 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 at intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at 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


More information about the igt-dev mailing list