diff mbox

[libdrm,v2,2/4] xf86drm: Add USB support

Message ID 20170112220429.28139-3-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding Jan. 12, 2017, 10:04 p.m. UTC 
Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
infrastructure.

v2:
- make sysfs_uevent_get() more flexible using a format string

Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
 xf86drm.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xf86drm.h |  13 +++++
 2 files changed, 176 insertions(+)

Comments

Mark Kettenis Jan. 13, 2017, 12:11 p.m. UTC | #1 
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Thu, 12 Jan 2017 23:04:27 +0100
> 
> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
> infrastructure.
> 
> v2:
> - make sysfs_uevent_get() more flexible using a format string
> 
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

All this sysfs parsing stuff is highly Linux-specific and should
probably be #ifdef __linux__.  Returning -EINVAL on non-Linux
platforms for usb and host1x should be fine.

Cheers,

Mark

> ---
>  xf86drm.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  xf86drm.h |  13 +++++
>  2 files changed, 176 insertions(+)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index c123650a1e23..27cd6eb5193e 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2906,6 +2906,9 @@ static int drmParseSubsystemType(int maj, int min)
>      if (strncmp(name, "/pci", 4) == 0)
>          return DRM_BUS_PCI;
>  
> +    if (strncmp(name, "/usb", 4) == 0)
> +        return DRM_BUS_USB;
> +
>      return -EINVAL;
>  #elif defined(__OpenBSD__)
>      return DRM_BUS_PCI;
> @@ -2992,6 +2995,10 @@ static int drmCompareBusInfo(drmDevicePtr a, drmDevicePtr b)
>      switch (a->bustype) {
>      case DRM_BUS_PCI:
>          return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo));
> +
> +    case DRM_BUS_USB:
> +        return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
> +
>      default:
>          break;
>      }
> @@ -3235,6 +3242,145 @@ free_device:
>      return ret;
>  }
>  
> +static char * DRM_PRINTFLIKE(2, 3)
> +sysfs_uevent_get(const char *path, const char *fmt, ...)
> +{
> +    char filename[PATH_MAX + 1], *key, *line = NULL, *value = NULL;
> +    size_t size = 0, len;
> +    ssize_t num;
> +    va_list ap;
> +    FILE *fp;
> +
> +    va_start(ap, fmt);
> +    num = vasprintf(&key, fmt, ap);
> +    va_end(ap);
> +    len = num;
> +
> +    snprintf(filename, sizeof(filename), "%s/uevent", path);
> +
> +    fp = fopen(filename, "r");
> +    if (!fp) {
> +        free(key);
> +        return NULL;
> +    }
> +
> +    while ((num = getline(&line, &size, fp)) >= 0) {
> +        if ((strncmp(line, key, len) == 0) && (line[len] == '=')) {
> +            char *start = line + len + 1, *end = line + num - 1;
> +
> +            if (*end != '\n')
> +                end++;
> +
> +            value = strndup(start, end - start);
> +            break;
> +        }
> +    }
> +
> +    free(line);
> +    fclose(fp);
> +
> +    free(key);
> +
> +    return value;
> +}
> +
> +static int drmParseUsbBusInfo(int maj, int min, drmUsbBusInfoPtr info)
> +{
> +    char path[PATH_MAX + 1], *value;
> +    unsigned int bus, dev;
> +    int ret;
> +
> +    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> +
> +    value = sysfs_uevent_get(path, "BUSNUM");
> +    if (!value)
> +        return -ENOENT;
> +
> +    ret = sscanf(value, "%03u", &bus);
> +    free(value);
> +
> +    if (ret <= 0)
> +        return -errno;
> +
> +    value = sysfs_uevent_get(path, "DEVNUM");
> +    if (!value)
> +        return -ENOENT;
> +
> +    ret = sscanf(value, "%03u", &dev);
> +    free(value);
> +
> +    if (ret <= 0)
> +        return -errno;
> +
> +    info->bus = bus;
> +    info->dev = dev;
> +
> +    return 0;
> +}
> +
> +static int drmParseUsbDeviceInfo(int maj, int min, drmUsbDeviceInfoPtr info)
> +{
> +    char path[PATH_MAX + 1], *value;
> +    unsigned int vendor, product;
> +    int ret;
> +
> +    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> +
> +    value = sysfs_uevent_get(path, "PRODUCT");
> +    if (!value)
> +        return -ENOENT;
> +
> +    ret = sscanf(value, "%x/%x", &vendor, &product);
> +    free(value);
> +
> +    if (ret <= 0)
> +        return -errno;
> +
> +    info->vendor = vendor;
> +    info->product = product;
> +
> +    return 0;
> +}
> +
> +static int drmProcessUsbDevice(drmDevicePtr *device, const char *node,
> +                               int node_type, int maj, int min,
> +                               bool fetch_deviceinfo, uint32_t flags)
> +{
> +    drmDevicePtr dev;
> +    char *ptr;
> +    int ret;
> +
> +    dev = drmDeviceAlloc(node_type, node, sizeof(drmUsbBusInfo),
> +                         sizeof(drmUsbDeviceInfo), &ptr);
> +    if (!dev)
> +        return -ENOMEM;
> +
> +    dev->bustype = DRM_BUS_USB;
> +
> +    dev->businfo.usb = (drmUsbBusInfoPtr)ptr;
> +
> +    ret = drmParseUsbBusInfo(maj, min, dev->businfo.usb);
> +    if (ret < 0)
> +        goto free_device;
> +
> +    if (fetch_deviceinfo) {
> +        ptr += sizeof(drmUsbBusInfo);
> +        dev->deviceinfo.usb = (drmUsbDeviceInfoPtr)ptr;
> +
> +        ret = drmParseUsbDeviceInfo(maj, min, dev->deviceinfo.usb);
> +        if (ret < 0)
> +            goto free_device;
> +    }
> +
> +    *device = dev;
> +
> +    return 0;
> +
> +free_device:
> +    free(dev);
> +    return ret;
> +}
> +
>  /* Consider devices located on the same bus as duplicate and fold the respective
>   * entries into a single one.
>   *
> @@ -3410,6 +3556,14 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>                  continue;
>  
>              break;
> +
> +        case DRM_BUS_USB:
> +            ret = drmProcessUsbDevice(&d, node, node_type, maj, min, true, flags);
> +            if (ret)
> +                goto free_devices;
> +
> +            break;
> +
>          default:
>              continue;
>          }
> @@ -3541,6 +3695,15 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
>                  continue;
>  
>              break;
> +
> +        case DRM_BUS_USB:
> +            ret = drmProcessUsbDevice(&device, node, node_type, maj, min,
> +                                      devices != NULL, flags);
> +            if (ret)
> +                goto free_devices;
> +
> +            break;
> +
>          default:
>              continue;
>          }
> diff --git a/xf86drm.h b/xf86drm.h
> index b340fc46cd44..65d5321950fc 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -767,6 +767,7 @@ extern char *drmGetPrimaryDeviceNameFromFd(int fd);
>  extern char *drmGetRenderDeviceNameFromFd(int fd);
>  
>  #define DRM_BUS_PCI   0
> +#define DRM_BUS_USB   1
>  
>  typedef struct _drmPciBusInfo {
>      uint16_t domain;
> @@ -783,15 +784,27 @@ typedef struct _drmPciDeviceInfo {
>      uint8_t revision_id;
>  } drmPciDeviceInfo, *drmPciDeviceInfoPtr;
>  
> +typedef struct _drmUsbBusInfo {
> +    uint8_t bus;
> +    uint8_t dev;
> +} drmUsbBusInfo, *drmUsbBusInfoPtr;
> +
> +typedef struct _drmUsbDeviceInfo {
> +    uint16_t vendor;
> +    uint16_t product;
> +} drmUsbDeviceInfo, *drmUsbDeviceInfoPtr;
> +
>  typedef struct _drmDevice {
>      char **nodes; /* DRM_NODE_MAX sized array */
>      int available_nodes; /* DRM_NODE_* bitmask */
>      int bustype;
>      union {
>          drmPciBusInfoPtr pci;
> +        drmUsbBusInfoPtr usb;
>      } businfo;
>      union {
>          drmPciDeviceInfoPtr pci;
> +        drmUsbDeviceInfoPtr usb;
>      } deviceinfo;
>  } drmDevice, *drmDevicePtr;
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
> 
>
Emil Velikov Jan. 16, 2017, 2:38 p.m. UTC | #2 
On 13 January 2017 at 12:11, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Thierry Reding <thierry.reding@gmail.com>
>> Date: Thu, 12 Jan 2017 23:04:27 +0100
>>
>> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
>> infrastructure.
>>
>> v2:
>> - make sysfs_uevent_get() more flexible using a format string
>>
>> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
>
> All this sysfs parsing stuff is highly Linux-specific and should
> probably be #ifdef __linux__.  Returning -EINVAL on non-Linux
> platforms for usb and host1x should be fine.
>
Nicely spotted. Thierry with the above the series is
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

Can you land these in the next few days - I would love to have a
libdrm release and use drmGetDevice[s]2 in mesa.

Thanks
Emil
Thierry Reding Jan. 18, 2017, 8:56 a.m. UTC | #3 
On Mon, Jan 16, 2017 at 02:38:39PM +0000, Emil Velikov wrote:
> On 13 January 2017 at 12:11, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> From: Thierry Reding <thierry.reding@gmail.com>
> >> Date: Thu, 12 Jan 2017 23:04:27 +0100
> >>
> >> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
> >> infrastructure.
> >>
> >> v2:
> >> - make sysfs_uevent_get() more flexible using a format string
> >>
> >> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> >
> > All this sysfs parsing stuff is highly Linux-specific and should
> > probably be #ifdef __linux__.  Returning -EINVAL on non-Linux
> > platforms for usb and host1x should be fine.
> >
> Nicely spotted. Thierry with the above the series is
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> 
> Can you land these in the next few days - I would love to have a
> libdrm release and use drmGetDevice[s]2 in mesa.

Can do. I've thrown in another cleanup patch to reuse the new
sysfs_uevent_get() function (now also #ifdef __linux__) for the Linux-
specific PCI bus/device info. tests/drmdevice results are identical and
valgrind doesn't flag any leaks. I'll send out the series once more just
to be sure, but I think it should now be all fine.

Thanks,
Thierry
diff mbox

Patch

diff --git a/xf86drm.c b/xf86drm.c
index c123650a1e23..27cd6eb5193e 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2906,6 +2906,9 @@  static int drmParseSubsystemType(int maj, int min)
     if (strncmp(name, "/pci", 4) == 0)
         return DRM_BUS_PCI;
 
+    if (strncmp(name, "/usb", 4) == 0)
+        return DRM_BUS_USB;
+
     return -EINVAL;
 #elif defined(__OpenBSD__)
     return DRM_BUS_PCI;
@@ -2992,6 +2995,10 @@  static int drmCompareBusInfo(drmDevicePtr a, drmDevicePtr b)
     switch (a->bustype) {
     case DRM_BUS_PCI:
         return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo));
+
+    case DRM_BUS_USB:
+        return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
+
     default:
         break;
     }
@@ -3235,6 +3242,145 @@  free_device:
     return ret;
 }
 
+static char * DRM_PRINTFLIKE(2, 3)
+sysfs_uevent_get(const char *path, const char *fmt, ...)
+{
+    char filename[PATH_MAX + 1], *key, *line = NULL, *value = NULL;
+    size_t size = 0, len;
+    ssize_t num;
+    va_list ap;
+    FILE *fp;
+
+    va_start(ap, fmt);
+    num = vasprintf(&key, fmt, ap);
+    va_end(ap);
+    len = num;
+
+    snprintf(filename, sizeof(filename), "%s/uevent", path);
+
+    fp = fopen(filename, "r");
+    if (!fp) {
+        free(key);
+        return NULL;
+    }
+
+    while ((num = getline(&line, &size, fp)) >= 0) {
+        if ((strncmp(line, key, len) == 0) && (line[len] == '=')) {
+            char *start = line + len + 1, *end = line + num - 1;
+
+            if (*end != '\n')
+                end++;
+
+            value = strndup(start, end - start);
+            break;
+        }
+    }
+
+    free(line);
+    fclose(fp);
+
+    free(key);
+
+    return value;
+}
+
+static int drmParseUsbBusInfo(int maj, int min, drmUsbBusInfoPtr info)
+{
+    char path[PATH_MAX + 1], *value;
+    unsigned int bus, dev;
+    int ret;
+
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+
+    value = sysfs_uevent_get(path, "BUSNUM");
+    if (!value)
+        return -ENOENT;
+
+    ret = sscanf(value, "%03u", &bus);
+    free(value);
+
+    if (ret <= 0)
+        return -errno;
+
+    value = sysfs_uevent_get(path, "DEVNUM");
+    if (!value)
+        return -ENOENT;
+
+    ret = sscanf(value, "%03u", &dev);
+    free(value);
+
+    if (ret <= 0)
+        return -errno;
+
+    info->bus = bus;
+    info->dev = dev;
+
+    return 0;
+}
+
+static int drmParseUsbDeviceInfo(int maj, int min, drmUsbDeviceInfoPtr info)
+{
+    char path[PATH_MAX + 1], *value;
+    unsigned int vendor, product;
+    int ret;
+
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+
+    value = sysfs_uevent_get(path, "PRODUCT");
+    if (!value)
+        return -ENOENT;
+
+    ret = sscanf(value, "%x/%x", &vendor, &product);
+    free(value);
+
+    if (ret <= 0)
+        return -errno;
+
+    info->vendor = vendor;
+    info->product = product;
+
+    return 0;
+}
+
+static int drmProcessUsbDevice(drmDevicePtr *device, const char *node,
+                               int node_type, int maj, int min,
+                               bool fetch_deviceinfo, uint32_t flags)
+{
+    drmDevicePtr dev;
+    char *ptr;
+    int ret;
+
+    dev = drmDeviceAlloc(node_type, node, sizeof(drmUsbBusInfo),
+                         sizeof(drmUsbDeviceInfo), &ptr);
+    if (!dev)
+        return -ENOMEM;
+
+    dev->bustype = DRM_BUS_USB;
+
+    dev->businfo.usb = (drmUsbBusInfoPtr)ptr;
+
+    ret = drmParseUsbBusInfo(maj, min, dev->businfo.usb);
+    if (ret < 0)
+        goto free_device;
+
+    if (fetch_deviceinfo) {
+        ptr += sizeof(drmUsbBusInfo);
+        dev->deviceinfo.usb = (drmUsbDeviceInfoPtr)ptr;
+
+        ret = drmParseUsbDeviceInfo(maj, min, dev->deviceinfo.usb);
+        if (ret < 0)
+            goto free_device;
+    }
+
+    *device = dev;
+
+    return 0;
+
+free_device:
+    free(dev);
+    return ret;
+}
+
 /* Consider devices located on the same bus as duplicate and fold the respective
  * entries into a single one.
  *
@@ -3410,6 +3556,14 @@  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
                 continue;
 
             break;
+
+        case DRM_BUS_USB:
+            ret = drmProcessUsbDevice(&d, node, node_type, maj, min, true, flags);
+            if (ret)
+                goto free_devices;
+
+            break;
+
         default:
             continue;
         }
@@ -3541,6 +3695,15 @@  int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
                 continue;
 
             break;
+
+        case DRM_BUS_USB:
+            ret = drmProcessUsbDevice(&device, node, node_type, maj, min,
+                                      devices != NULL, flags);
+            if (ret)
+                goto free_devices;
+
+            break;
+
         default:
             continue;
         }
diff --git a/xf86drm.h b/xf86drm.h
index b340fc46cd44..65d5321950fc 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -767,6 +767,7 @@  extern char *drmGetPrimaryDeviceNameFromFd(int fd);
 extern char *drmGetRenderDeviceNameFromFd(int fd);
 
 #define DRM_BUS_PCI   0
+#define DRM_BUS_USB   1
 
 typedef struct _drmPciBusInfo {
     uint16_t domain;
@@ -783,15 +784,27 @@  typedef struct _drmPciDeviceInfo {
     uint8_t revision_id;
 } drmPciDeviceInfo, *drmPciDeviceInfoPtr;
 
+typedef struct _drmUsbBusInfo {
+    uint8_t bus;
+    uint8_t dev;
+} drmUsbBusInfo, *drmUsbBusInfoPtr;
+
+typedef struct _drmUsbDeviceInfo {
+    uint16_t vendor;
+    uint16_t product;
+} drmUsbDeviceInfo, *drmUsbDeviceInfoPtr;
+
 typedef struct _drmDevice {
     char **nodes; /* DRM_NODE_MAX sized array */
     int available_nodes; /* DRM_NODE_* bitmask */
     int bustype;
     union {
         drmPciBusInfoPtr pci;
+        drmUsbBusInfoPtr usb;
     } businfo;
     union {
         drmPciDeviceInfoPtr pci;
+        drmUsbDeviceInfoPtr usb;
     } deviceinfo;
 } drmDevice, *drmDevicePtr;