[Mesa-dev] [PATCH 02/16] loader: slim down loader_get_pci_id_for_fd implementation(s)

[Nicolai Hähnle]

On 11.10.2016 20:31, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
>
> Currently mesa has three code paths in the loader - libudev, manual
> sysfs and drm ioctl one.
>
> Considering the issues we had with libudev - strip those down in favour
> of the libdrm drm device API. The latter can be implemented in any way
> depending on the platform and can be reused by others.
>
> Cc: Jonathan Gray <jsg at jsg.id.au>
> Cc: Jean-Sébastien Pédron <dumbbell at FreeBSD.org>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> Jonathan, Jean-Sébastien I believe I've prodded you guys for a *BSD
> implementation of the drm*Device API a while back. With this commit
> you'll be 'forced' to prep some ;-)
>
> About the implementation itself feel free to use libudev/equivalent (we
> cannot do so on Linux, since due to fun interactions with Steam's one)
> or any other means available on your platform. Fwiw I would strongly
> suggest _against_ adding DRM specific ioctls just for this purpose but
> at the end of the day it's your code/userbase.
>
> On the FreeBSD/DragonFly side this means that you no londer require a
> handful of patches for mesa, which is always a plus.
> ---
>  src/loader/loader.c | 172 +++++-----------------------------------------------
>  1 file changed, 16 insertions(+), 156 deletions(-)
>
> diff --git a/src/loader/loader.c b/src/loader/loader.c
> index 3e60e4c..41ea016 100644
> --- a/src/loader/loader.c
> +++ b/src/loader/loader.c
[snip]
>  static int
>  drm_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, "MESA-LOADER: invalid drm fd\n");
> -      return 0;
> -   }
> -   if (!version->name) {
> -      log_(_LOADER_WARNING, "MESA-LOADER: unable to determine the driver name\n");
> -      drmFreeVersion(version);
> -      return 0;
> -   }
> -
> -   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, "MESA-LOADER: failed to get param for i915\n");
> -	 *chip_id = -1;
> +   drmDevicePtr device;
> +   int ret;
> +
> +   if (drmGetDevice(fd, &device) == 0) {
> +      if (device->bustype == DRM_BUS_PCI) {
> +         *vendor_id = device->deviceinfo.pci->vendor_id;
> +         *chip_id = device->deviceinfo.pci->device_id;
> +         ret = 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, "MESA-LOADER: failed to get info for radeon\n");
> -	 *chip_id = -1;
> +      else {
> +         log_(_LOADER_WARNING, "MESA-LOADER: device is not located on the PCI bus\n");
> +         ret = 0;
>        }
> +      drmFreeDevice(&device);
>     }
> -   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;
> +   else {
> +      log_(_LOADER_WARNING, "MESA-LOADER: device is not located on the PCI bus\n");
> +      ret = 0;
>     }

This should probably be a different error/warning message.

Nicolai

>
> -   drmFreeVersion(version);
> -
> -   return (*chip_id >= 0);
> +   return ret;
>  }
>  #endif
>
> @@ -585,14 +453,6 @@ drm_get_pci_id_for_fd(int fd, int *vendor_id, int *chip_id)
>  int
>  loader_get_pci_id_for_fd(int fd, int *vendor_id, int *chip_id)
>  {
> -#if HAVE_LIBUDEV
> -   if (libudev_get_pci_id_for_fd(fd, vendor_id, chip_id))
> -      return 1;
> -#endif
> -#if HAVE_SYSFS
> -   if (sysfs_get_pci_id_for_fd(fd, vendor_id, chip_id))
> -      return 1;
> -#endif
>  #if HAVE_LIBDRM
>     if (drm_get_pci_id_for_fd(fd, vendor_id, chip_id))
>        return 1;
>