[PATCH libdrm 2/3] xf86drm: Add USB support

[Emil Velikov]

On 2 January 2017 at 13:26, Thierry Reding <thierry.reding at gmail.com> wrote:
> On Sat, Dec 24, 2016 at 04:38:04PM +0000, Emil Velikov wrote:
>> Hi Thierry,
>>
>> On 23 December 2016 at 17:49, Thierry Reding <thierry.reding at gmail.com> wrote:
>> > Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
>> > infrastructure.
>> >
>> > Signed-off-by: Thierry Reding <thierry.reding at gmail.com>
>> > ---
>> > Note that this is completely untested because I don't have a UDL device
>> > for testing. I'm fairly confident that this will work, though, and it'd
>> > be nice to include it before the new platform and host1x busses because
>> > support for it existed in the kernel longer than for platform devices.
>> >
>> Functionality looks spot on, but I'm a bit hesitant to get this in
>> without any testing.
>> Can we please extend tests/drmdevice.c (separate patch?) as poke
>> someone on dri-devel/xorg-devel to give it a quick run ?
>
> I can do that.
>
>> > +static int drmParseUsbDeviceInfo(int maj, int min, drmUsbDeviceInfoPtr info)
>> > +{
>> > +    char path[PATH_MAX + 1], *value;
>> > +    unsigned int vendor, product;
>> > +    int ret;
>> > +
>> > +    snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device", maj, min);
>> > +
>> > +    value = sysfs_uevent_get(path, "PRODUCT");
>> > +    ret = sscanf(value, "%x/%x", &vendor, &product);
>> > +    free(value);
>> > +
>> > +    if (ret <= 0)
>> > +        return -errno;
>> > +
>> > +    info->vendor = vendor;
>> > +    info->product = product;
>> > +
>> Worth fetching bcdDevice as well ?
>
> This is currently only parsing the uevent file, which doesn't have
> bcdDevice. The only data that isn't currently being parsed is TYPE
> (bDeviceClass/bdeviceSubClass/bDeviceProtocol), not sure if we'd
> want that.
>
> I could of course read bcdDevice from the bcdDevice file.
>
It's up-to you really. The above was meant as food for thought.

> One thing that I realized the other day, and it's relevant to all of
> drmDevice, not just USB, is that we don't have a good way of preserving
> ABI compatibility. With the USB and platform/host1x bus patches we're
> not running into problems yet, but consider what would happen if we
> added more bus or device info. The drmDevice.businfo and
> drmDevice.deviceinfo unions could be growing beyond what they are now,
> causing applications written against old versions to potentially
> allocate too little memory.
>
> I wonder if that's something we need to care about. As long as we keep
> the bus and device info fixed after they've been added, we should be
> safe because the bus type will be unknown to earlier versions and hence
> cause the code to error out early. Technically we wouldn't be able to
> ever extend one type of bus or device info, though.
>
Afaict extending structs will be an issue, as one builds mesa/other
against new libdrm and then uses older at runtime.
Yes, things are not ideal on the topic, but it's something that's been
around for a while.

These days distros are pretty good at keeping the runtime version same
as (or greater than) the build-time one, so we should be safe.

Thanks
Emil