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

[Thierry Reding <thierry.reding@gmail.com>]

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

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@gmail.com> wrote:
> > Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
> > infrastructure.
> >
> > Signed-off-by: Thierry Reding <thierry.reding@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.

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.

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