[Spice-devel] [PATCH] common: Add a udev helper to identify GPU Vendor (v2)

Frediano Ziglio freddy77 at gmail.com
Thu Oct 5 16:44:10 UTC 2023


Il giorno mar 3 ott 2023 alle ore 20:36 Frediano Ziglio
<freddy77 at gmail.com> ha scritto:
>
> Il giorno lun 2 ott 2023 alle ore 06:41 Vivek Kasireddy
> <vivek.kasireddy at intel.com> ha scritto:
> >
> > Given that libudev is widely available on many Linux distros, we
> > can use the relevant APIs to iterate over all the devices associated
> > with the drm subsystem to figure out if a specific vendor GPU
> > is available or not.
> >
> > This capability (identifying GPU Vendor) is useful to determine
> > whether to launch Gstreamer pipeline using h/w accelerated
> > plugins. On systems where libudev is not available (Windows,
> > MacOS, etc) we'd have to make this determination based on the
> > availability of the relevant plugins in the Gstreamer registry.
> >
> > v2: (Frediano)
> > - Added autoconf support
> > - Added license text
> > - Added pragma once and SPICE_BEGIN/END_DECLS to the header
> > - Checked the vendor id udev attribute of the pci device to
> >   determine a vendor GPU instead of checking the driver name
> >
>
> Thanks. Do you want this part of the message to be part of the final
> commit? Usually we don't put history... and not much useful IMHO.
>
> > Cc: Frediano Ziglio <freddy77 at gmail.com>
> > Cc: Gerd Hoffmann <kraxel at redhat.com>
> > Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
> > Cc: Dongwon Kim <dongwon.kim at intel.com>
> > Cc: Hazwan Arif Mazlan <hazwan.arif.mazlan at intel.com>
> > Cc: Jin Chung Teng <jin.chung.teng at intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > ---
> >  common/meson.build |  2 ++
> >  common/udev.c      | 78 ++++++++++++++++++++++++++++++++++++++++++++++
> >  common/udev.h      | 33 ++++++++++++++++++++
> >  configure.ac       |  1 +
> >  m4/spice-deps.m4   | 12 +++++++
> >  meson.build        |  7 +++++
> >  6 files changed, 133 insertions(+)
> >  create mode 100644 common/udev.c
> >  create mode 100644 common/udev.h
> >
> > diff --git a/common/meson.build b/common/meson.build
> > index 09e3ea7..63bd2db 100644
> > --- a/common/meson.build
> > +++ b/common/meson.build
> > @@ -39,6 +39,8 @@ spice_common_sources = [
> >    'snd_codec.h',
> >    'utils.c',
> >    'utils.h',
> > +  'udev.c',
> > +  'udev.h',
> >    'verify.h',
> >    'recorder.h'
> >  ]
> > diff --git a/common/udev.c b/common/udev.c
> > new file mode 100644
> > index 0000000..3985fdd
> > --- /dev/null
> > +++ b/common/udev.c
> > @@ -0,0 +1,78 @@
> > +/*
> > +   Copyright (C) 2023 Intel Corporation.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > +*/
> > +
> > +#include <config.h>
> > +
> > +#ifdef HAVE_UDEV
> > +#include <libudev.h>
> > +#include <stdbool.h>
> > +#include <stdlib.h>
> > +#include "udev.h"
> > +
> > +GpuVendor spice_udev_detect_gpu(long gpu_vendor)
>
> long type here looks a bit weird, considering that vendor numbers are
> encoded as 16-bit unsigned integer in PCI specifications; maybe just
> int?
>
> > +{
> > +    struct udev *udev;
> > +    struct udev_device *drm_dev, *pci_dev;
> > +    struct udev_enumerate *udev_enum;
> > +    struct udev_list_entry *entry, *devices;
> > +    const char *path, *vendor_id;
> > +    GpuVendor vendor = VENDOR_GPU_NOTDETECTED;
> > +
> > +    udev = udev_new();
> > +    if (!udev) {
> > +        return vendor;
>
> wondering if in this case VENDOR_GPU_UNKNOWN would be better, if we
> fail to get udev object we are not sure if there's the device or not.
>
> > +    }
> > +
> > +    udev_enum = udev_enumerate_new(udev);
> > +    if (udev_enum) {
> > +        udev_enumerate_add_match_subsystem(udev_enum, "drm");
> > +        udev_enumerate_add_match_sysname(udev_enum, "card[0-9]");
> > +        udev_enumerate_scan_devices(udev_enum);
> > +        devices = udev_enumerate_get_list_entry(udev_enum);
> > +
> > +        udev_list_entry_foreach(entry, devices) {
> > +            path = udev_list_entry_get_name(entry);
> > +            drm_dev = udev_device_new_from_syspath(udev, path);
> > +            if (!drm_dev) {
> > +                continue;
> > +            }
> > +
> > +            pci_dev = udev_device_get_parent_with_subsystem_devtype(drm_dev,
> > +                                                                    "pci", NULL);
> > +            if (pci_dev) {
> > +                vendor_id = udev_device_get_sysattr_value(pci_dev, "vendor");
> > +                if (vendor_id && strtol(vendor_id, NULL, 16) == gpu_vendor) {
> > +                    vendor = VENDOR_GPU_DETECTED;
> > +                    udev_device_unref(drm_dev);
> > +                    break;
> > +                }
> > +            }
> > +            udev_device_unref(drm_dev);
> > +        }
> > +        udev_enumerate_unref(udev_enum);
> > +    }
> > +    udev_unref(udev);
> > +
> > +    return vendor;
> > +}
> > +#else
> > +GpuVendor spice_udev_detect_gpu(long gpu_vendor)
>
> This won't compile as GpuVendor is not defined (udev.h not included)
>
> > +{
> > +    return VENDOR_GPU_UNKNOWN;
> > +}
> > +#endif
> > +
> > diff --git a/common/udev.h b/common/udev.h
> > new file mode 100644
> > index 0000000..6a3d068
> > --- /dev/null
> > +++ b/common/udev.h
> > @@ -0,0 +1,33 @@
> > +/*
> > +   Copyright (C) 2023 Intel Corporation.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > +*/
> > +
> > +#pragma once
> > +#include <spice/macros.h>
> > +
> > +#define INTEL_VENDOR_ID 0x8086
> > +
> > +typedef enum {
> > +    VENDOR_GPU_UNKNOWN,
> > +    VENDOR_GPU_DETECTED,
> > +    VENDOR_GPU_NOTDETECTED,
> > +} GpuVendor;
> > +
> > +SPICE_BEGIN_DECLS
> > +
> > +GpuVendor spice_udev_detect_gpu(long gpu_vendor);
> > +
> > +SPICE_END_DECLS
> > diff --git a/configure.ac b/configure.ac
> > index 0d4c22b..42e873d 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -61,6 +61,7 @@ SPICE_CHECK_GLIB2
> >  SPICE_CHECK_OPUS
> >  SPICE_CHECK_OPENSSL
> >  SPICE_CHECK_GDK_PIXBUF
> > +SPICE_CHECK_UDEV
> >
> >  SPICE_COMMON_CFLAGS='$(PIXMAN_CFLAGS) $(SMARTCARD_CFLAGS) $(GLIB2_CFLAGS) $(OPUS_CFLAGS) $(OPENSSL_CFLAGS)'
> >  SPICE_COMMON_CFLAGS="$SPICE_COMMON_CFLAGS -DG_LOG_DOMAIN=\\\"Spice\\\""
> > diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4
> > index e11fc4e..6a07ee6 100644
> > --- a/m4/spice-deps.m4
> > +++ b/m4/spice-deps.m4
> > @@ -302,6 +302,18 @@ AC_DEFUN([SPICE_CHECK_OPENSSL], [
> >      PKG_CHECK_MODULES(OPENSSL, openssl)
> >  ])
> >
> > +# SPICE_CHECK_UDEV
> > +# -----------------
> > +# Check for the availability of libudev. If found, it will help to determine
> > +# if a given vendor GPU is available or not.
> > +#------------------
> > +AC_DEFUN([SPICE_CHECK_UDEV], [
> > +    PKG_CHECK_MODULES([UDEV], [libudev], [have_udev=yes],[have_udev=no])
> > +    if test "x$have_udev" = "xyes"; then
> > +      AC_DEFINE([HAVE_UDEV], 1, [whether libudev is available to identify GPU])
> > +    fi
> > +])
> > +
> >  # SPICE_CHECK_INSTRUMENTATION
> >  # -----------------
> >  # Check for the availability of an instrumentation library.
> > diff --git a/meson.build b/meson.build
> > index eeccecd..cafbf03 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -147,6 +147,13 @@ if smartcard_dep.found()
> >    spice_common_config_data.set('USE_SMARTCARD', '1')
> >  endif
> >
> > +#udev
> > +udev_dep = dependency('libudev')
> > +if udev_dep.found()
> > +  spice_common_deps += udev_dep
> > +  spice_common_config_data.set('HAVE_UDEV', '1')
> > +endif
> > +
> >  #
> >  # global C defines
> >  #
>
> Wrote some small fixups (also addressing some comments) at
> https://gitlab.freedesktop.org/fziglio/spice-common/-/commits/udev/
> (ignore the test, just for linking/Windows/CI checks).
>

Hi,
   added another fixup to the list, see
https://gitlab.freedesktop.org/fziglio/spice-common/-/commit/c4053f413e3e072f6fdc074a2adbb2b42df579f7
(libudev should not be required otherwise it won't compile on systems
not providing it)

Frediano


More information about the Spice-devel mailing list