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

[Thierry Reding]

On Mon, Jan 16, 2017 at 02:38:39PM +, Emil Velikov wrote:
> On 13 January 2017 at 12:11, Mark Kettenis  wrote:
> >> From: Thierry Reding 
> >> 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 
> >
> > 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 
> 
> 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


signature.asc
Description: PGP signature
___
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

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

[Emil Velikov]

On 13 January 2017 at 12:11, Mark Kettenis  wrote:
>> From: Thierry Reding 
>> 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 
>
> 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 

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
___
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

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

[Mark Kettenis]

> From: Thierry Reding 
> 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 

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(, 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(, , 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", );
> +free(value);
> +
> +if (ret <= 0)
> +return -errno;
> +
> +value = sysfs_uevent_get(path, "DEVNUM");
> +if (!value)
> +return -ENOENT;
> +
> +ret = sscanf(value, "%03u", );
> +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", , );
> +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), );
> +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 

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

[Thierry Reding]

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 
---
 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(, 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(, , 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", );
+free(value);
+
+if (ret <= 0)
+return -errno;
+
+value = sysfs_uevent_get(path, "DEVNUM");
+if (!value)
+return -ENOENT;
+
+ret = sscanf(value, "%03u", );
+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", , );
+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), );
+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(, 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(, node, node_type, maj, min,
+