[Mesa-dev] [PATCH 1/2] loader: refactor duplicated code into loader util lib

Eric Anholt eric at anholt.net
Thu Jan 9 19:36:30 PST 2014


Rob Clark <robdclark at gmail.com> writes:

> From: Rob Clark <robclark at freedesktop.org>
>
> All the various window system integration layers duplicate roughly the
> same code for figuring out device and driver name, pci-id's, etc.  Which
> is sad.  So extract it out into a loader util lib.

Thanks for tackling this.  It had been (low) on my list for a while.

>  static int
>  droid_open_device(void)
>  {
> @@ -773,7 +672,7 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
>        goto cleanup_display;
>     }

forgot loader_set_logger here.  Do we want to just move that to
egl_dri2.c instead of each platform_*?

> -   dri2_dpy->driver_name = (char *) droid_get_driver_name(dri2_dpy->fd);
> +   dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd, 0);
>     if (dri2_dpy->driver_name == NULL) {
>        err = "DRI2: failed to get driver name";
>        goto cleanup_device;


> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> index 444bdf1..e915c63 100644
> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> @@ -44,6 +44,7 @@ struct pipe_screen;
>  enum pipe_loader_device_type {
>     PIPE_LOADER_DEVICE_SOFTWARE,
>     PIPE_LOADER_DEVICE_PCI,
> +   PIPE_LOADER_DEVICE_PLATFORM,
>     NUM_PIPE_LOADER_DEVICE_TYPES
>  };
>  

> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
> index 927fb24..fda0ab1 100644
> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
> @@ -190,17 +117,22 @@ boolean
>  pipe_loader_drm_probe_fd(struct pipe_loader_device **dev, int fd)
>  {
>     struct pipe_loader_drm_device *ddev = CALLOC_STRUCT(pipe_loader_drm_device);
> -
> -   ddev->base.type = PIPE_LOADER_DEVICE_PCI;
> +   int vendor_id, chip_id;
> +
> +   if (loader_get_pci_id_for_fd(fd, &vendor_id, &chip_id)) {
> +      ddev->base.type = PIPE_LOADER_DEVICE_PCI;
> +      ddev->base.u.pci.vendor_id = vendor_id;
> +      ddev->base.u.pci.chip_id = chip_id;
> +   } else {
> +      ddev->base.type = PIPE_LOADER_DEVICE_PLATFORM;
> +   }
>     ddev->base.ops = &pipe_loader_drm_ops;
>     ddev->fd = fd;
>  
>     pipe_loader_drm_x_auth(fd);
>  
> -   if (!find_drm_pci_id(ddev))
> -      goto fail;
> -
> -   if (!find_drm_driver_name(ddev))
> +   ddev->base.driver_name = loader_get_driver_for_fd(fd, _LOADER_GALLIUM);
> +   if (!ddev->base.driver_name)
>        goto fail;
>  
>     *dev = &ddev->base;
> diff --git a/src/gallium/state_trackers/clover/core/device.cpp b/src/gallium/state_trackers/clover/core/device.cpp
> index e5e429a..76a49d0 100644
> --- a/src/gallium/state_trackers/clover/core/device.cpp
> +++ b/src/gallium/state_trackers/clover/core/device.cpp
> @@ -63,6 +63,7 @@ device::type() const {
>     case PIPE_LOADER_DEVICE_SOFTWARE:
>        return CL_DEVICE_TYPE_CPU;
>     case PIPE_LOADER_DEVICE_PCI:
> +   case PIPE_LOADER_DEVICE_PLATFORM:
>        return CL_DEVICE_TYPE_GPU;
>     default:
>        assert(0);
> @@ -74,6 +75,7 @@ cl_uint
>  device::vendor_id() const {
>     switch (ldev->type) {
>     case PIPE_LOADER_DEVICE_SOFTWARE:
> +   case PIPE_LOADER_DEVICE_PLATFORM:
>        return 0;
>     case PIPE_LOADER_DEVICE_PCI:
>        return ldev->u.pci.vendor_id;

These hunks look unrelated to the refactor and should be in a separate
commit enabling non-pci devices.

> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index b4b97ac..c13930c 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -44,6 +44,7 @@
>  #include "gbm_driint.h"
>  
>  #include "gbmint.h"
> +#include "loader.h"
>  
>  /* For importing wl_buffer */
>  #if HAVE_WAYLAND_PLATFORM
> @@ -270,7 +271,7 @@ dri_screen_create(struct gbm_dri_device *dri)
>     const __DRIextension **extensions;
>     int ret = 0;
>  
> -   dri->base.driver_name = dri_fd_get_driver_name(dri->base.base.fd);
> +   dri->base.driver_name = loader_get_driver_for_fd(dri->base.base.fd, 0);
>     if (dri->base.driver_name == NULL)
>        return -1;

Another missing set_logger.

> diff --git a/src/loader/loader.c b/src/loader/loader.c
> new file mode 100644
> index 0000000..3e69a59
> --- /dev/null
> +++ b/src/loader/loader.c
> @@ -0,0 +1,264 @@
> +/*
> + * Copyright (C) 2013 Rob Clark <robclark at freedesktop.org>
> + *
> + * 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.
> + *
> + * Authors:
> + *    Rob Clark <robclark at freedesktop.org>
> + */
> +
> +#include <stdarg.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include "loader.h"
> +
> +#include <xf86drm.h>
> +
> +#define __IS_LOADER
> +#include "pci_ids/pci_id_driver_map.h"
> +
> +static void default_logger(int level, const char *fmt, ...)
> +{
> +   if (level >= _LOADER_WARNING) {
> +      va_list args;
> +      va_start(args, fmt);
> +      vfprintf(stderr, fmt, args);
> +      va_end(args);
> +      fprintf(stderr, "\n");
> +   }
> +}
> +
> +static void (*log)(int level, const char *fmt, ...) = default_logger;
> +
> +#ifdef HAVE_LIBUDEV
> +#include <libudev.h>
> +
> +static inline struct udev_device *
> +udev_device_new_from_fd(struct udev *udev, int fd)
> +{
> +   struct udev_device *device;
> +   struct stat buf;
> +
> +   if (fstat(fd, &buf) < 0) {
> +      log(_LOADER_WARNING, "EGL-DRI2: failed to stat fd %d", fd);
> +      return NULL;
> +   }
> +
> +   device = udev_device_new_from_devnum(udev, 'c', buf.st_rdev);
> +   if (device == NULL) {
> +      log(_LOADER_WARNING,
> +              "EGL-DRI2: could not create udev device for fd %d", fd);
> +      return NULL;
> +   }
> +
> +   return device;
> +}
> +
> +int
> +loader_get_pci_id_for_fd(int fd, int *vendor_id, int *chip_id)
> +{
> +   struct udev *udev = NULL;
> +   struct udev_device *device = NULL, *parent;
> +   struct stat buf;
> +   const char *pci_id;
> +
> +   *chip_id = -1;
> +
> +   udev = udev_new();
> +   device = udev_device_new_from_fd(udev, fd);
> +   if (!device)
> +	   goto out;
> +
> +   parent = udev_device_get_parent(device);
> +   if (parent == NULL) {
> +      log(_LOADER_WARNING, "could not get parent device");
> +      goto out;
> +   }
> +
> +   pci_id = udev_device_get_property_value(parent, "PCI_ID");
> +   if (pci_id == NULL ||
> +       sscanf(pci_id, "%x:%x", vendor_id, chip_id) != 2) {
> +      log(_LOADER_WARNING, "malformed or no PCI ID");
> +      *chip_id = -1;
> +      goto out;
> +   }
> +
> +out:
> +   if (device)
> +      udev_device_unref(device);
> +   if (udev)
> +      udev_unref(udev);
> +
> +   return (*chip_id >= 0);
> +}
> +
> +#elif defined(PIPE_OS_ANDROID) && !defined(_EGL_NO_DRM)

I don't see gallium includes being used, so I don't think this code
would ever be built?

But it should probably be dumped in favor of a rebase of keithp's "don't
use udev" patch anyway, so I'm fine with things as is.

> +/* for i915 */
> +#include <i915_drm.h>
> +/* for radeon */
> +#include <radeon_drm.h>
> +
> +int
> +loader_get_pci_id_for_fd(int fd, int *vendor_id, int *chip_id)
> +{
> +   drmVersionPtr version;
> +
> +   *chip_id = -1;
> +
> +   version = drmGetVersion(fd);
> +   if (!version) {
> +      log(_LOADER_WARNING, "invalid drm fd");
> +      return FALSE;
> +   }
> +   if (!version->name) {
> +      log(_LOADER_WARNING, "unable to determine the driver name");
> +      drmFreeVersion(version);
> +      return FALSE;
> +   }
> +
> +   if (strcmp(version->name, "i915") == 0) {
> +      struct drm_i915_getparam gp;
> +      int ret;
> +
> +      *vendor_id = 0x8086;
> +
> +      memset(&gp, 0, sizeof(gp));
> +      gp.param = I915_PARAM_CHIPSET_ID;
> +      gp.value = chip_id;
> +      ret = drmCommandWriteRead(fd, DRM_I915_GETPARAM, &gp, sizeof(gp));
> +      if (ret) {
> +         log(_LOADER_WARNING, "failed to get param for i915");
> +	 *chip_id = -1;
> +      }
> +   }
> +   else if (strcmp(version->name, "radeon") == 0) {
> +      struct drm_radeon_info info;
> +      int ret;
> +
> +      *vendor_id = 0x1002;
> +
> +      memset(&info, 0, sizeof(info));
> +      info.request = RADEON_INFO_DEVICE_ID;
> +      info.value = (unsigned long) chip_id;
> +      ret = drmCommandWriteRead(fd, DRM_RADEON_INFO, &info, sizeof(info));
> +      if (ret) {
> +         log(_LOADER_WARNING, "failed to get info for radeon");
> +	 *chip_id = -1;
> +      }
> +   }
> +   else if (strcmp(version->name, "nouveau") == 0) {
> +      *vendor_id = 0x10de;
> +      /* not used */
> +      *chip_id = 0;
> +   }
> +   else if (strcmp(version->name, "vmwgfx") == 0) {
> +      *vendor_id = 0x15ad;
> +      /* assume SVGA II */
> +      *chip_id = 0x0405;
> +   }
> +
> +   drmFreeVersion(version);
> +
> +   return (*chip_id >= 0);
> +}
> +
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140110/31516b0f/attachment.pgp>


More information about the mesa-dev mailing list