[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

We tried several different ways already for the enumeration interface (libpciaccess, libudev, etc). But we ran into some problems with these options for example when run Steam games which ships 32bit libraries (including libudev) in the steam runtime, so finally we decided to use sysfs directly to avoid introducing some additional dependencies into libdrm.

Regards,
Jammy

-----Original Message-----
From: Emil Velikov [mailto:emil.l.velikov at gmail.com] 
Sent: Thursday, August 13, 2015 11:27 PM
To: Daniel Vetter
Cc: Zhou, Jammy; ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 13 August 2015 at 16:07, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Thu, Aug 13, 2015 at 11:33:41AM +0800, Jammy Zhou wrote:
>> From: Emil Velikov <emil.l.velikov at gmail.com>
>>
>> For mutiple GPU support, the devices on the system should be 
>> enumerated to get necessary information about each device, and the 
>> drmGetDevices interface is added for this. Currently only PCI devices 
>> are supported for the enumeration.
>>
>> Typical usage:
>> int count;
>> drmDevicePtr *foo;
>> count = drmGetDevices(NULL, 0);
>> foo = calloc(count, sizeof(drmDevicePtr)); count = drmGetDevices(foo, 
>> count);
>> /* find proper device, open correct device node, etc */ 
>> drmFreeDevices(foo, count); free(foo);
>>
>> v2: change according to feedback from Emil
>>
>> Signed-off-by: Jammy Zhou <Jammy.Zhou at amd.com>
>> ---
>>  xf86drm.c | 351 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  xf86drm.h |  34 ++++++
>>  2 files changed, 385 insertions(+)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 5e02969..237663b 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -55,6 +55,7 @@
>>  #ifdef HAVE_SYS_MKDEV_H
>>  # include <sys/mkdev.h> /* defines major(), minor(), and makedev() 
>> on Solaris */  #endif
>> +#include <math.h>
>>
>>  /* Not all systems have MAP_FAILED defined */  #ifndef MAP_FAILED @@ 
>> -2829,3 +2830,353 @@ char *drmGetRenderDeviceNameFromFd(int fd)  {
>>       return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);  }
>> +
>> +#ifdef __linux__
>> +static int drmParseSubsystemType(const char *str) {
>> +    char link[PATH_MAX + 1] = "";
>> +    char *name;
>> +
>> +    if (readlink(str, link, PATH_MAX) < 0)
>> +        return -EINVAL;
>> +
>> +    name = strrchr(link, '/');
>> +    if (!name)
>> +        return -EINVAL;
>> +
>> +    name++;
>> +
>> +    if (strncmp(name, "pci", 3) == 0)
>> +        return DRM_BUS_PCI;
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +static int drmParsePciBusInfo(const char *str, drmPciBusInfoPtr 
>> +info) {
>> +    int domain, bus, dev, func;
>> +    char *value;
>> +
>> +    if (str == NULL)
>> +        return -EINVAL;
>> +
>> +    value = strstr(str, "PCI_SLOT_NAME=");
>> +    if (value == NULL)
>> +        return -EINVAL;
>> +
>> +    value += strlen("PCI_SLOT_NAME=");
>> +
>> +    if (sscanf(value, "%04x:%02x:%02x.%1u",
>> +               &domain, &bus, &dev, &func) != 4)
>> +        return -EINVAL;
>> +
>> +    info->domain = domain;
>> +    info->bus = bus;
>> +    info->dev = dev;
>> +    info->func = func;
>> +
>> +    return 0;
>> +}
>> +
>> +static int drmSameDevice(drmDevicePtr a, drmDevicePtr b) {
>> +    if (a->bustype != b->bustype)
>> +        return 0;
>> +
>> +    switch (a->bustype) {
>> +    case DRM_BUS_PCI:
>> +        if (memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo)) == 0)
>> +            return 1;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int drmGetNodeType(const char *name) {
>> +    if (strncmp(name, DRM_PRIMARY_MINOR_NAME,
>> +        sizeof(DRM_PRIMARY_MINOR_NAME) - 1) == 0)
>> +        return DRM_NODE_PRIMARY;
>> +
>> +    if (strncmp(name, DRM_CONTROL_MINOR_NAME,
>> +        sizeof(DRM_CONTROL_MINOR_NAME ) - 1) == 0)
>> +        return DRM_NODE_CONTROL;
>> +
>> +    if (strncmp(name, DRM_RENDER_MINOR_NAME,
>> +        sizeof(DRM_RENDER_MINOR_NAME) - 1) == 0)
>> +        return DRM_NODE_RENDER;
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +static int drmParsePciDeviceInfo(const char *config,
>> +                                 drmPciDeviceInfoPtr device) {
>> +    if (config == NULL)
>> +        return -EINVAL;
>> +
>> +    device->vendor_id = config[0] | (config[1] << 8);
>> +    device->device_id = config[2] | (config[3] << 8);
>> +    device->revision_id = config[8];
>> +    device->subvendor_id = config[44] | (config[45] << 8);
>> +    device->subdevice_id = config[46] | (config[47] << 8);
>
> Why not reuse libpciaccess?
I fully agree that the implementation is not pretty, although...

Adding dependencies for optional new features doesn't seem too appealing, either. It will also save us some headaches when someone starts shipping libpciaccess.so with their binary game/program (just like libudev is today).

Would be great if we can use libudev but... just not yet.

If you have any other ideas/suggestions please fire away.

Thanks
Emi