[PATCH] drm: Provide drm_set_busid() fallback

[Daniel Vetter]

On Fri, Apr 11, 2014 at 8:30 PM, Thierry Reding
<thierry.reding at gmail.com> wrote:
> On Fri, Apr 11, 2014 at 07:43:18PM +0200, Daniel Vetter wrote:
>> On Fri, Apr 11, 2014 at 03:28:56PM +0200, Thierry Reding wrote:
> [...]
>> > -   ret = dev->driver->bus->set_busid(dev, master);
>> > -   if (ret)
>> > -           goto err;
>> > +   if (dev->driver->bus && dev->driver->bus->set_busid) {
>> > +           ret = dev->driver->bus->set_busid(dev, master);
>> > +           if (ret)
>> > +                   goto err;
>> > +   } else {
>>
>> Hm, my plan was actually to just provide a drm_dev_setunique to drivers so
>> that they can set whatever their userspace wants,
>
> So that would be going one step further and not call drm_set_busid() in
> the first place, but rather just let drivers statically assign unique at
> probe/load time? Yeah, that makes sense. It's not like the unique string
> is going to change at runtime, is it?

It's static, except for pci. There it can change depending upon the
drm version set by userspace.

>> and then have no set_busid implementation here at all for !pci.
>
> Perhaps for PCI there could simply be a common helper to initialize the
> unique string in the way it's currently generated by the PCI bus'
> implementation of .set_busid(). Even for PCI it's still a static string
> that's fixed at probe/load time, isn't it? And it isn't specific per
> device, only per drm_bus.

Well if you mean static string = something known at compile time put
into the .data section then no. At least for pci it's generated at
runtime in 2 different version. I think for everything else it's
actually static data (driver name usually).

>> Some userspace at least uses the unique thing to match for the driver,
>> so we need to do the usual bending over backwards to keep it
>> consistent.
>
> For new drivers and userspace it should be okay to just match on the
> driver name. Any differentiation on a per-device basis is probably
> better done using a GET_PARAM ioctl than by parsing a bus ID string.
>
> Or what is userspace doing with the bus ID in the first place?

libdrm expose an drmOpenByBusid, which is used by a lot of places to
match pci devices to drm drivers. And mesa/Xorg ... have big pci id
tables to match pci devices to userspace drivers. On non-pci it's all
different afaik.

>> The approach with a drm_set_unique helper would also make conversion of
>> existing platform and usb drivers easier.
>
> Yeah, I like that better than what this patch does. Is that something
> you have already queued up? If not I could take a stab at it.

Nope, not yet done really.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch