[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