[PATCH libdrm v3 0/5] xf86drm: Add USB, platform and host1x bus support

[PATCH libdrm v3 0/5] xf86drm: Add USB, platform and host1x bus support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This series enables support for USB, platform and host1x busses in the
drmDevice infrastructure. The goal is to make use of these in Mesa for PRIME
support (via the DRI_PRIME environment variable) for devices on one of these
busses.

Changes in v3:
- add cleanup patch to reuse sysfs_uevent_get() in existing code
- guard Linux-specific sysfs parsing code with #ifdef __linux__

Changes in v2:
- add USB, platform and host1x bus support to tests/drmdevice
- make sure not to overflow path names and device fullnames
- read compatible strings into platform/host1x device info

I've tested both platform and host1x bus support on Tegra and also verified
that there aren't any leaks by running tests/drmdevice through valgrind.

Since I don't have any UDL devices myself I'm adding the X.Org development
mailing list on Cc, as suggested by Emil Velikov, in the hopes that someone
with access to that hardware will be generous enough to give these a run.

Thierry

Thierry Reding (5):
  xf86drm: Factor out drmDeviceAlloc()
  xf86drm: Add USB support
  xf86drm: Add platform and host1x bus support
  tests/drmdevice: Add USB, platform and host1x support
  xf86drm: Reuse sysfs_uevent_get()

 tests/drmdevice.c |  37 ++++
 xf86drm.c         | 619 +++++++++++++++++++++++++++++++++++++++++++++++-------
 xf86drm.h         |  41 +++-
 3 files changed, 621 insertions(+), 76 deletions(-)

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm v3 1/5] xf86drm: Factor out drmDeviceAlloc()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Subsequent patches will add support for other bus types to drmDevice and
they will duplicate a lot of the code to allocate a drmDevice. Factor
out the common code so it can be reused.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 xf86drm.c | 78 +++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 27 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 7b78dc6b6ee9..7766bfe937db 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3157,57 +3157,81 @@ void drmFreeDevices(drmDevicePtr devices[], int count)
             drmFreeDevice(&devices[i]);
 }
 
+static drmDevicePtr drmDeviceAlloc(unsigned int type, const char *node,
+                                   size_t bus_size, size_t device_size,
+                                   char **ptrp)
+{
+    size_t max_node_length, extra, size;
+    drmDevicePtr device;
+    unsigned int i;
+    char *ptr;
+
+    max_node_length = ALIGN(drmGetMaxNodeName(), sizeof(void *));
+    extra = DRM_NODE_MAX * (sizeof(void *) + max_node_length);
+
+    size = sizeof(*device) + extra + bus_size + device_size;
+
+    device = calloc(1, size);
+    if (!device)
+        return NULL;
+
+    device->available_nodes = 1 << type;
+
+    ptr = (char *)device + sizeof(*device);
+    device->nodes = (char **)ptr;
+
+    ptr += DRM_NODE_MAX * sizeof(void *);
+
+    for (i = 0; i < DRM_NODE_MAX; i++) {
+        device->nodes[i] = ptr;
+        ptr += max_node_length;
+    }
+
+    memcpy(device->nodes[type], node, max_node_length);
+
+    *ptrp = ptr;
+
+    return device;
+}
+
 static int drmProcessPciDevice(drmDevicePtr *device,
                                const char *node, int node_type,
                                int maj, int min, bool fetch_deviceinfo,
                                uint32_t flags)
 {
-    const int max_node_str = ALIGN(drmGetMaxNodeName(), sizeof(void *));
-    int ret, i;
+    drmDevicePtr dev;
     char *addr;
+    int ret;
 
-    *device = calloc(1, sizeof(drmDevice) +
-                     (DRM_NODE_MAX * (sizeof(void *) + max_node_str)) +
-                     sizeof(drmPciBusInfo) +
-                     sizeof(drmPciDeviceInfo));
-    if (!*device)
+    dev = drmDeviceAlloc(node_type, node, sizeof(drmPciBusInfo),
+                         sizeof(drmPciDeviceInfo), &addr);
+    if (!dev)
         return -ENOMEM;
 
-    addr = (char*)*device;
+    dev->bustype = DRM_BUS_PCI;
 
-    (*device)->bustype = DRM_BUS_PCI;
-    (*device)->available_nodes = 1 << node_type;
+    dev->businfo.pci = (drmPciBusInfoPtr)addr;
 
-    addr += sizeof(drmDevice);
-    (*device)->nodes = (char**)addr;
-
-    addr += DRM_NODE_MAX * sizeof(void *);
-    for (i = 0; i < DRM_NODE_MAX; i++) {
-        (*device)->nodes[i] = addr;
-        addr += max_node_str;
-    }
-    memcpy((*device)->nodes[node_type], node, max_node_str);
-
-    (*device)->businfo.pci = (drmPciBusInfoPtr)addr;
-
-    ret = drmParsePciBusInfo(maj, min, (*device)->businfo.pci);
+    ret = drmParsePciBusInfo(maj, min, dev->businfo.pci);
     if (ret)
         goto free_device;
 
     // Fetch the device info if the user has requested it
     if (fetch_deviceinfo) {
         addr += sizeof(drmPciBusInfo);
-        (*device)->deviceinfo.pci = (drmPciDeviceInfoPtr)addr;
+        dev->deviceinfo.pci = (drmPciDeviceInfoPtr)addr;
 
-        ret = drmParsePciDeviceInfo(maj, min, (*device)->deviceinfo.pci, flags);
+        ret = drmParsePciDeviceInfo(maj, min, dev->deviceinfo.pci, flags);
         if (ret)
             goto free_device;
     }
+
+    *device = dev;
+
     return 0;
 
 free_device:
-    free(*device);
-    *device = NULL;
+    free(dev);
     return ret;
 }
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm v3 2/5] xf86drm: Add USB support

[Thierry Reding]

Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
infrastructure.

v3:
- guard Linux-specific sysfs parsing code with #ifdef __linux__

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

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

diff --git a/xf86drm.c b/xf86drm.c
index 7766bfe937db..d83674e638c4 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2886,6 +2886,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
     return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
 }
 
+#ifdef __linux__
+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;
+}
+#endif
+
 static int drmParseSubsystemType(int maj, int min)
 {
 #ifdef __linux__
@@ -2906,6 +2950,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 +3039,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 +3286,113 @@ free_device:
     return ret;
 }
 
+static int drmParseUsbBusInfo(int maj, int min, drmUsbBusInfoPtr info)
+{
+#ifdef __linux__
+    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;
+#else
+#warning "Missing implementation of drmParseUsbBusInfo"
+    return -EINVAL;
+#endif
+}
+
+static int drmParseUsbDeviceInfo(int maj, int min, drmUsbDeviceInfoPtr info)
+{
+#ifdef __linux__
+    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;
+#else
+#warning "Missing implementation of drmParseUsbDeviceInfo"
+    return -EINVAL;
+#endif
+}
+
+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 +3568,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 +3707,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

Re: [PATCH libdrm v3 2/5] xf86drm: Add USB support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2471 bytes --]

Adding back dri-devel@lists.freedesktop.org

On Wed, Jan 18, 2017 at 12:21:23PM +0100, walter harms wrote:
> 
> 
> Am 18.01.2017 10:02, schrieb Thierry Reding:
> > Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
> > infrastructure.
> > 
> > v3:
> > - guard Linux-specific sysfs parsing code with #ifdef __linux__
> > 
> > v2:
> > - make sysfs_uevent_get() more flexible using a format string
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> > ---
> >  xf86drm.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  xf86drm.h |  13 +++++
> >  2 files changed, 188 insertions(+)
> > 
> > diff --git a/xf86drm.c b/xf86drm.c
> > index 7766bfe937db..d83674e638c4 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -2886,6 +2886,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
> >      return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
> >  }
> >  
> > +#ifdef __linux__
> > +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;
>        char *filename=NULL, *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);
> 
> 	since asprintf() is available you could use:
> 
>         asprintf(&filename,"%s/uevent", path);
> 
> 	same could be done for path below.

I had thought about that, but a stack-allocated string seemed
advantageous for three reasons:

	- asprintf() is a GNU extension. That's not much of an issue
	  because this is already protected by #ifdef __linux__ and
	  pretty much all C libraries I know support asprintf() and
	  friends.

	- PATH_MAX is the maximum length of a filename, so there's no
	  need to dynamically allocate, since it should nicely fit into
	  the stack pretty much everywhere (I think the largest value I
	  have ever seen for PATH_MAX is 4096 on recent Linux systems).

	- Most of the other code in xf86drm.c already uses PATH_MAX, so
	  the code remains consistent.

Given the added complexity of asprintf() and the need to free the memory
the advantages of the stack-allocated string seemed to outweigh.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm v3 2/5] xf86drm: Add USB support

[walter harms]

Am 19.01.2017 11:20, schrieb Thierry Reding:
> Adding back dri-devel@lists.freedesktop.org
> 
> On Wed, Jan 18, 2017 at 12:21:23PM +0100, walter harms wrote:
>>
>>
>> Am 18.01.2017 10:02, schrieb Thierry Reding:
>>> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
>>> infrastructure.
>>>
>>> v3:
>>> - guard Linux-specific sysfs parsing code with #ifdef __linux__
>>>
>>> v2:
>>> - make sysfs_uevent_get() more flexible using a format string
>>>
>>> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
>>> ---
>>>  xf86drm.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  xf86drm.h |  13 +++++
>>>  2 files changed, 188 insertions(+)
>>>
>>> diff --git a/xf86drm.c b/xf86drm.c
>>> index 7766bfe937db..d83674e638c4 100644
>>> --- a/xf86drm.c
>>> +++ b/xf86drm.c
>>> @@ -2886,6 +2886,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
>>>      return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
>>>  }
>>>  
>>> +#ifdef __linux__
>>> +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;
>>        char *filename=NULL, *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);
>>
>> 	since asprintf() is available you could use:
>>
>>         asprintf(&filename,"%s/uevent", path);
>>
>> 	same could be done for path below.
> 
> I had thought about that, but a stack-allocated string seemed
> advantageous for three reasons:
> 
> 	- asprintf() is a GNU extension. That's not much of an issue
> 	  because this is already protected by #ifdef __linux__ and
> 	  pretty much all C libraries I know support asprintf() and
> 	  friends.
> 
> 	- PATH_MAX is the maximum length of a filename, so there's no
> 	  need to dynamically allocate, since it should nicely fit into
> 	  the stack pretty much everywhere (I think the largest value I
> 	  have ever seen for PATH_MAX is 4096 on recent Linux systems).
> 
> 	- Most of the other code in xf86drm.c already uses PATH_MAX, so
> 	  the code remains consistent.
> 
> Given the added complexity of asprintf() and the need to free the memory
> the advantages of the stack-allocated string seemed to outweigh.
> 
> Thierry

Granted,

I like to use asprintf every time %s is used to stop malicious users in the tracks
(do not aks me how this could be done with our function). Freeing the allocated buf
is no practical problem. NTL i have no problem if you are happy with the patch, making
an informed decision is ok with me.

re,
 wh
_______________________________________________
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

[PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

ARM SoCs usually have their DRM/KMS devices on the platform bus, so add
support for that to enable these devices to be used with the drmDevice
infrastructure.

NVIDIA Tegra SoCs have an additional level in the hierarchy and DRM/KMS
devices can also be on the host1x bus. This is mostly equivalent to the
platform bus.

v3:
- guard Linux-specific sysfs parsing code with #ifdef __linux__

v2:
- be careful not to overflow the full name
- read compatible strings into device info

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 xf86drm.c | 300 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xf86drm.h |  30 ++++++-
 2 files changed, 328 insertions(+), 2 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index d83674e638c4..3c6ec1cc9e00 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2953,6 +2953,12 @@ static int drmParseSubsystemType(int maj, int min)
     if (strncmp(name, "/usb", 4) == 0)
         return DRM_BUS_USB;
 
+    if (strncmp(name, "/platform", 9) == 0)
+        return DRM_BUS_PLATFORM;
+
+    if (strncmp(name, "/host1x", 7) == 0)
+        return DRM_BUS_HOST1X;
+
     return -EINVAL;
 #elif defined(__OpenBSD__)
     return DRM_BUS_PCI;
@@ -3043,6 +3049,12 @@ static int drmCompareBusInfo(drmDevicePtr a, drmDevicePtr b)
     case DRM_BUS_USB:
         return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
 
+    case DRM_BUS_PLATFORM:
+        return memcmp(a->businfo.platform, b->businfo.platform, sizeof(drmPlatformBusInfo));
+
+    case DRM_BUS_HOST1X:
+        return memcmp(a->businfo.host1x, b->businfo.host1x, sizeof(drmHost1xBusInfo));
+
     default:
         break;
     }
@@ -3187,11 +3199,55 @@ static int drmParsePciDeviceInfo(int maj, int min,
 #endif
 }
 
+static void drmFreePlatformDevice(drmDevicePtr device)
+{
+    if (device->deviceinfo.platform) {
+        if (device->deviceinfo.platform->compatible) {
+            char **compatible = device->deviceinfo.platform->compatible;
+
+            while (*compatible) {
+                free(*compatible);
+                compatible++;
+            }
+
+            free(device->deviceinfo.platform->compatible);
+        }
+    }
+}
+
+static void drmFreeHost1xDevice(drmDevicePtr device)
+{
+    if (device->deviceinfo.host1x) {
+        if (device->deviceinfo.host1x->compatible) {
+            char **compatible = device->deviceinfo.host1x->compatible;
+
+            while (*compatible) {
+                free(*compatible);
+                compatible++;
+            }
+
+            free(device->deviceinfo.host1x->compatible);
+        }
+    }
+}
+
 void drmFreeDevice(drmDevicePtr *device)
 {
     if (device == NULL)
         return;
 
+    if (*device) {
+        switch ((*device)->bustype) {
+        case DRM_BUS_PLATFORM:
+            drmFreePlatformDevice(*device);
+            break;
+
+        case DRM_BUS_HOST1X:
+            drmFreeHost1xDevice(*device);
+            break;
+        }
+    }
+
     free(*device);
     *device = NULL;
 }
@@ -3393,6 +3449,220 @@ free_device:
     return ret;
 }
 
+static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info)
+{
+#ifdef __linux__
+    char path[PATH_MAX + 1], *name;
+
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+
+    name = sysfs_uevent_get(path, "OF_FULLNAME");
+    if (!name)
+        return -ENOENT;
+
+    strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
+    info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
+    free(name);
+
+    return 0;
+#else
+#warning "Missing implementation of drmParsePlatformBusInfo"
+    return -EINVAL;
+#endif
+}
+
+static int drmParsePlatformDeviceInfo(int maj, int min,
+                                      drmPlatformDeviceInfoPtr info)
+{
+#ifdef __linux__
+    char path[PATH_MAX + 1], *value;
+    unsigned int count, i;
+    int err;
+
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+
+    value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
+    if (!value)
+        return -ENOENT;
+
+    sscanf(value, "%u", &count);
+    free(value);
+
+    info->compatible = calloc(count + 1, sizeof(*info->compatible));
+    if (!info->compatible)
+        return -ENOMEM;
+
+    for (i = 0; i < count; i++) {
+        value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
+        if (!value) {
+            err = -ENOENT;
+            goto free;
+        }
+
+        info->compatible[i] = value;
+    }
+
+    return 0;
+
+free:
+    while (i--)
+        free(info->compatible[i]);
+
+    free(info->compatible);
+    return err;
+#else
+#warning "Missing implementation of drmParsePlatformDeviceInfo"
+    return -EINVAL;
+#endif
+}
+
+static int drmProcessPlatformDevice(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(drmPlatformBusInfo),
+                         sizeof(drmPlatformDeviceInfo), &ptr);
+    if (!dev)
+        return -ENOMEM;
+
+    dev->bustype = DRM_BUS_PLATFORM;
+
+    dev->businfo.platform = (drmPlatformBusInfoPtr)ptr;
+
+    ret = drmParsePlatformBusInfo(maj, min, dev->businfo.platform);
+    if (ret < 0)
+        goto free_device;
+
+    if (fetch_deviceinfo) {
+        ptr += sizeof(drmPlatformBusInfo);
+        dev->deviceinfo.platform = (drmPlatformDeviceInfoPtr)ptr;
+
+        ret = drmParsePlatformDeviceInfo(maj, min, dev->deviceinfo.platform);
+        if (ret < 0)
+            goto free_device;
+    }
+
+    *device = dev;
+
+    return 0;
+
+free_device:
+    free(dev);
+    return ret;
+}
+
+static int drmParseHost1xBusInfo(int maj, int min, drmHost1xBusInfoPtr info)
+{
+#ifdef __linux__
+    char path[PATH_MAX + 1], *name;
+
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+
+    name = sysfs_uevent_get(path, "OF_FULLNAME");
+    if (!name)
+        return -ENOENT;
+
+    strncpy(info->fullname, name, DRM_HOST1X_DEVICE_NAME_LEN);
+    info->fullname[DRM_HOST1X_DEVICE_NAME_LEN - 1] = '\0';
+    free(name);
+
+    return 0;
+#else
+#warning "Missing implementation of drmParseHost1xBusInfo"
+    return -EINVAL;
+#endif
+}
+
+static int drmParseHost1xDeviceInfo(int maj, int min,
+                                    drmHost1xDeviceInfoPtr info)
+{
+#ifdef __linux__
+    char path[PATH_MAX + 1], *value;
+    unsigned int count, i;
+    int err;
+
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+
+    value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
+    if (!value)
+        return -ENOENT;
+
+    sscanf(value, "%u", &count);
+    free(value);
+
+    info->compatible = calloc(count + 1, sizeof(*info->compatible));
+    if (!info->compatible)
+        return -ENOMEM;
+
+    for (i = 0; i < count; i++) {
+        value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
+        if (!value) {
+            err = -ENOENT;
+            goto free;
+        }
+
+        info->compatible[i] = value;
+    }
+
+    return 0;
+
+free:
+    while (i--)
+        free(info->compatible[i]);
+
+    free(info->compatible);
+    return err;
+#else
+#warning "Missing implementation of drmParseHost1xDeviceInfo"
+    return -EINVAL;
+#endif
+}
+
+static int drmProcessHost1xDevice(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(drmHost1xBusInfo),
+                         sizeof(drmHost1xDeviceInfo), &ptr);
+    if (!dev)
+        return -ENOMEM;
+
+    dev->bustype = DRM_BUS_HOST1X;
+
+    dev->businfo.host1x = (drmHost1xBusInfoPtr)ptr;
+
+    ret = drmParseHost1xBusInfo(maj, min, dev->businfo.host1x);
+    if (ret < 0)
+        goto free_device;
+
+    if (fetch_deviceinfo) {
+        ptr += sizeof(drmHost1xBusInfo);
+        dev->deviceinfo.host1x = (drmHost1xDeviceInfoPtr)ptr;
+
+        ret = drmParseHost1xDeviceInfo(maj, min, dev->deviceinfo.host1x);
+        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.
  *
@@ -3576,6 +3846,20 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
 
             break;
 
+        case DRM_BUS_PLATFORM:
+            ret = drmProcessPlatformDevice(&d, node, node_type, maj, min, true, flags);
+            if (ret)
+                goto free_devices;
+
+            break;
+
+        case DRM_BUS_HOST1X:
+            ret = drmProcessHost1xDevice(&d, node, node_type, maj, min, true, flags);
+            if (ret)
+                goto free_devices;
+
+            break;
+
         default:
             continue;
         }
@@ -3716,6 +4000,22 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
 
             break;
 
+        case DRM_BUS_PLATFORM:
+            ret = drmProcessPlatformDevice(&device, node, node_type, maj, min,
+                                           devices != NULL, flags);
+            if (ret)
+                goto free_devices;
+
+            break;
+
+        case DRM_BUS_HOST1X:
+            ret = drmProcessHost1xDevice(&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 65d5321950fc..0d927018f1f7 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -766,8 +766,10 @@ extern int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle);
 extern char *drmGetPrimaryDeviceNameFromFd(int fd);
 extern char *drmGetRenderDeviceNameFromFd(int fd);
 
-#define DRM_BUS_PCI   0
-#define DRM_BUS_USB   1
+#define DRM_BUS_PCI       0
+#define DRM_BUS_USB       1
+#define DRM_BUS_PLATFORM  2
+#define DRM_BUS_HOST1X    3
 
 typedef struct _drmPciBusInfo {
     uint16_t domain;
@@ -794,6 +796,26 @@ typedef struct _drmUsbDeviceInfo {
     uint16_t product;
 } drmUsbDeviceInfo, *drmUsbDeviceInfoPtr;
 
+#define DRM_PLATFORM_DEVICE_NAME_LEN 512
+
+typedef struct _drmPlatformBusInfo {
+    char fullname[DRM_PLATFORM_DEVICE_NAME_LEN];
+} drmPlatformBusInfo, *drmPlatformBusInfoPtr;
+
+typedef struct _drmPlatformDeviceInfo {
+    char **compatible; /* NULL terminated list of compatible strings */
+} drmPlatformDeviceInfo, *drmPlatformDeviceInfoPtr;
+
+#define DRM_HOST1X_DEVICE_NAME_LEN 512
+
+typedef struct _drmHost1xBusInfo {
+    char fullname[DRM_HOST1X_DEVICE_NAME_LEN];
+} drmHost1xBusInfo, *drmHost1xBusInfoPtr;
+
+typedef struct _drmHost1xDeviceInfo {
+    char **compatible; /* NULL terminated list of compatible strings */
+} drmHost1xDeviceInfo, *drmHost1xDeviceInfoPtr;
+
 typedef struct _drmDevice {
     char **nodes; /* DRM_NODE_MAX sized array */
     int available_nodes; /* DRM_NODE_* bitmask */
@@ -801,10 +823,14 @@ typedef struct _drmDevice {
     union {
         drmPciBusInfoPtr pci;
         drmUsbBusInfoPtr usb;
+        drmPlatformBusInfoPtr platform;
+        drmHost1xBusInfoPtr host1x;
     } businfo;
     union {
         drmPciDeviceInfoPtr pci;
         drmUsbDeviceInfoPtr usb;
+        drmPlatformDeviceInfoPtr platform;
+        drmHost1xDeviceInfoPtr host1x;
     } 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

Re: [PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2398 bytes --]

Adding back dri-devel@lists.freedesktop.org

On Wed, Jan 18, 2017 at 04:00:20PM +0100, walter harms wrote:
> Am 18.01.2017 10:02, schrieb Thierry Reding:
[...]
> > diff --git a/xf86drm.c b/xf86drm.c
[...]
> > @@ -3187,11 +3199,55 @@ static int drmParsePciDeviceInfo(int maj, int min,
> >  #endif
> >  }
> >  
> > +static void drmFreePlatformDevice(drmDevicePtr device)
> > +{
> > +    if (device->deviceinfo.platform) {
> 
> to save 2 indent level and improve readability :
> 
> if (! device->deviceinfo.platform ||
>     ! device->deviceinfo.platform->compatible)
> 	return;
> 
> > +        if (device->deviceinfo.platform->compatible) {
> > +            char **compatible = device->deviceinfo.platform->compatible;
> > +
> > +            while (*compatible) {
> > +                free(*compatible);
> > +                compatible++;
> > +            }
> > +
> > +            free(device->deviceinfo.platform->compatible);
> > +        }
> > +    }

I chose the above style because I thought readability wasn't impacted
and it has the advantage of making the code more easily extensible
should we ever need to add code to it.

> > +static int drmProcessHost1xDevice(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(drmHost1xBusInfo),
> > +                         sizeof(drmHost1xDeviceInfo), &ptr);
> > +    if (!dev)
> > +        return -ENOMEM;
> > +
> > +    dev->bustype = DRM_BUS_HOST1X;
> > +
> > +    dev->businfo.host1x = (drmHost1xBusInfoPtr)ptr;
> > +
> > +    ret = drmParseHost1xBusInfo(maj, min, dev->businfo.host1x);
> > +    if (ret < 0)
> > +        goto free_device;
> > +
> > +    if (fetch_deviceinfo) {
> > +        ptr += sizeof(drmHost1xBusInfo);
> > +        dev->deviceinfo.host1x = (drmHost1xDeviceInfoPtr)ptr;
> > +
> > +        ret = drmParseHost1xDeviceInfo(maj, min, dev->deviceinfo.host1x);
> > +        if (ret < 0)
> > +            goto free_device;
> > +    }
> > +
> > +    *device = dev;
> > +
> 
> do you assume that fetch_deviceinfo may change dev ?

No, why?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support

[Emil Velikov]

On 18 January 2017 at 09:02, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> ARM SoCs usually have their DRM/KMS devices on the platform bus, so add
> support for that to enable these devices to be used with the drmDevice
> infrastructure.
>
> NVIDIA Tegra SoCs have an additional level in the hierarchy and DRM/KMS
> devices can also be on the host1x bus. This is mostly equivalent to the
> platform bus.
>
Re the indentation suggestion: either one is fine imho - coding style
in libdrm is messy ;-)


> @@ -3576,6 +3846,20 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>
>              break;
>
> +        case DRM_BUS_PLATFORM:
> +            ret = drmProcessPlatformDevice(&d, node, node_type, maj, min, true, flags);
> +            if (ret)
> +                goto free_devices;
> +
> +            break;
> +
> +        case DRM_BUS_HOST1X:
> +            ret = drmProcessHost1xDevice(&d, node, node_type, maj, min, true, flags);
> +            if (ret)
> +                goto free_devices;
> +
> +            break;
> +
>          default:
>              continue;
>          }
> @@ -3716,6 +4000,22 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
>
>              break;
>
> +        case DRM_BUS_PLATFORM:
> +            ret = drmProcessPlatformDevice(&device, node, node_type, maj, min,
> +                                           devices != NULL, flags);
> +            if (ret)
> +                goto free_devices;
> +
> +            break;
> +
> +        case DRM_BUS_HOST1X:
> +            ret = drmProcessHost1xDevice(&device, node, node_type, maj, min,
> +                                         devices != NULL, flags);
> +            if (ret)
> +                goto free_devices;
> +
We want to continue on error - feel free to squash before pushing.
With that the original r-b (for the series) still stands.

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

[PATCH libdrm v3 4/5] tests/drmdevice: Add USB, platform and host1x support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Extend the drmdevice test with support for the newly added USB, platform
and host1x busses.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 tests/drmdevice.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tests/drmdevice.c b/tests/drmdevice.c
index 8c4f091a9972..9dd5098a211f 100644
--- a/tests/drmdevice.c
+++ b/tests/drmdevice.c
@@ -62,6 +62,43 @@ print_device_info(drmDevicePtr device, int i, bool print_revision)
         else
             printf("\t\t\trevision_id\tIGNORED\n");
 
+    } else if (device->bustype == DRM_BUS_USB) {
+        printf("\t\tusb\n");
+        printf("\t\t\tbus\t%03u\n", device->businfo.usb->bus);
+        printf("\t\t\tdev\t%03u\n", device->businfo.usb->dev);
+
+        printf("\tdeviceinfo\n");
+        printf("\t\tusb\n");
+        printf("\t\t\tvendor\t%04x\n", device->deviceinfo.usb->vendor);
+        printf("\t\t\tproduct\t%04x\n", device->deviceinfo.usb->product);
+    } else if (device->bustype == DRM_BUS_PLATFORM) {
+        char **compatible = device->deviceinfo.platform->compatible;
+
+        printf("\t\tplatform\n");
+        printf("\t\t\tfullname\t%s\n", device->businfo.platform->fullname);
+
+        printf("\tdeviceinfo\n");
+        printf("\t\tplatform\n");
+        printf("\t\t\tcompatible\n");
+
+        while (*compatible) {
+            printf("\t\t\t\t%s\n", *compatible);
+            compatible++;
+        }
+    } else if (device->bustype == DRM_BUS_HOST1X) {
+        char **compatible = device->deviceinfo.platform->compatible;
+
+        printf("\t\thost1x\n");
+        printf("\t\t\tfullname\t%s\n", device->businfo.host1x->fullname);
+
+        printf("\tdeviceinfo\n");
+        printf("\t\tplatform\n");
+        printf("\t\t\tcompatible\n");
+
+        while (*compatible) {
+            printf("\t\t\t\t%s\n", *compatible);
+            compatible++;
+        }
     } else {
         printf("Unknown/unhandled bustype\n");
     }
-- 
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

[PATCH libdrm v3 5/5] xf86drm: Reuse sysfs_uevent_get()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Recent patches for USB, platform and host1x bus support introduced the
sysfs_uevent_get() function that provides a generic way of parsing the
sysfs uevent file that is associated with each device in Linux.

Open-coded variants of this still exist in other places, so make those
reuse the new function to remove some code duplication.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 xf86drm.c | 66 +++++++++++++++++----------------------------------------------
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 3c6ec1cc9e00..e4a9e2eb1da4 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2971,32 +2971,21 @@ static int drmParseSubsystemType(int maj, int min)
 static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
 {
 #ifdef __linux__
-    char path[PATH_MAX + 1];
-    char data[512 + 1];
-    char *str;
-    int domain, bus, dev, func;
-    int fd, ret;
+    unsigned int domain, bus, dev, func;
+    char path[PATH_MAX + 1], *value;
+    int num;
 
-    snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/uevent", maj, min);
-    fd = open(path, O_RDONLY);
-    if (fd < 0)
-        return -errno;
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
-    ret = read(fd, data, sizeof(data)-1);
-    close(fd);
-    if (ret < 0)
-        return -errno;
-    data[ret] = '\0';
+    value = sysfs_uevent_get(path, "PCI_SLOT_NAME");
+    if (!value)
+        return -ENOENT;
 
-#define TAG "PCI_SLOT_NAME="
-    str = strstr(data, TAG);
-    if (str == NULL)
-        return -EINVAL;
+    num = sscanf(value, "%04x:%02x:%02x.%1u", &domain, &bus, &dev, &func);
+    free(value);
 
-    if (sscanf(str, TAG "%04x:%02x:%02x.%1u",
-               &domain, &bus, &dev, &func) != 4)
+    if (num != 4)
         return -EINVAL;
-#undef TAG
 
     info->domain = domain;
     info->bus = bus;
@@ -4084,13 +4073,8 @@ char *drmGetDeviceNameFromFd2(int fd)
 {
 #ifdef __linux__
     struct stat sbuf;
-    char *device_name = NULL;
+    char path[PATH_MAX + 1], *value;
     unsigned int maj, min;
-    FILE *f;
-    char buf[512];
-    static const char match[9] = "\nDEVNAME=";
-    size_t expected = 1;
-
 
     if (fstat(fd, &sbuf))
         return NULL;
@@ -4101,30 +4085,16 @@ char *drmGetDeviceNameFromFd2(int fd)
     if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
         return NULL;
 
-    snprintf(buf, sizeof(buf), "/sys/dev/char/%d:%d/uevent", maj, min);
-    if (!(f = fopen(buf, "r")))
-        return NULL;
-
-    while (expected < sizeof(match)) {
-        int c = getc(f);
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d", maj, min);
 
-        if (c == EOF) {
-            fclose(f);
-            return NULL;
-        } else if (c == match[expected] )
-            expected++;
-        else
-            expected = 0;
-    }
+    value = sysfs_uevent_get(path, "DEVNAME");
+    if (!value)
+        return NULL;
 
-    strcpy(buf, "/dev/");
-    if (fgets(buf + 5, sizeof(buf) - 5, f)) {
-        buf[strcspn(buf, "\n")] = '\0';
-        device_name = strdup(buf);
-    }
+    snprintf(path, sizeof(path), "/dev/%s", value);
+    free(value);
 
-    fclose(f);
-    return device_name;
+    return strdup(path);
 #else
     struct stat      sbuf;
     char             node[PATH_MAX + 1];
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel