[PATCH 2/2] xfree86: Support autoloading of non-PCI drivers

[Aaron Plattner]

On 02/12/2014 11:09 AM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Wed, Feb 12, 2014 at 05:28:56PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 02/12/2014 05:11 PM, Thierry Reding wrote:
>>> Drivers for non-PCI devices cannot be autoloaded because there is no
>>> mechanism in place to correlate the device to the driver name.
>>>
>>> This patch adds a simple table that maps the name of the Linux kernel
>>> driver name to a list of DDX module names. Matching modules will be
>>> added to the list of drivers that X tries to load for a given device.
>>>
>>> Signed-off-by: Thierry Reding <treding at nvidia.com>
>>> ---
>>>   hw/xfree86/common/xf86platformBus.c | 41 +++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 41 insertions(+)
>>>
>>> diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
>>> index 5875a91363cc..16c9bb6c1dbc 100644
>>> --- a/hw/xfree86/common/xf86platformBus.c
>>> +++ b/hw/xfree86/common/xf86platformBus.c
>>> @@ -176,6 +176,41 @@ xf86_check_platform_slot(const struct xf86_platform_device *pd)
>>>       return TRUE;
>>>   }
>>>
>>> +#ifdef __linux__
>>
>> I'm not a big fan of this, why not put most of this in:
>>
>> hw/xfree86/os-support/linux/lnx_platform.c
>>
>> And make xf86platformBus.c simple call a helper for this
>> from hw/xfree86/os-support/linux/lnx_platform.c ?
>
> I can easily move the table and the function below into lnx_platform.c
> and only call it from here if that's preferred. This really should've
> been marked RFC, since it's not clear to me if this is what we want. It
> works and isn't all that ugly, so I thought I'd post it to get early
> feedback if this is a good direction.
>
> Rob, Dave and I discussed this on IRC yesterday and Dave said that
> having a lookup table like this was really the only sane thing to do.

When I suggested something similar a while ago for trying to autoload 
the nvidia_drv.so driver, it was suggested that it would be better to 
have a driver match section that could be placed into an xorg.conf.d 
snippet, rather than hard-coding this table in the server.  IIRC there's 
something similar for input devices already.

The idea would be that xf86-video-opentegra would install a 
/usr/share/X11/xorg.conf.d/opentegra.conf that would consist of 
something like this:

Section "OutputClass"
   MatchPlatformDriver "tegra"
   Driver "opentegra"
EndSection

or something.  Does that sound reasonable?

-- Aaron

> Thierry
>
>>> +static const struct xf86platformDriver {
>>> +    const char *name;
>>> +    const char **modules;
>>> +} xf86platformDrivers[] = {
>>> +    {
>>> +        .name = "tegra",
>>> +        .modules = (const char *[]) {
>>> +            "opentegra",
>>> +            NULL
>>> +        },
>>> +    },
>>> +    { },
>>> +};
>>> +
>>> +static int xf86platformDriverList(char *name, char *matches[], int nmatches)
>>> +{
>>> +    const struct xf86platformDriver *driver = xf86platformDrivers;
>>> +    int i = 0;
>>> +
>>> +    while (driver->name) {
>>> +        if (strcmp(driver->name, name) == 0) {
>>> +            for (i = 0; driver->modules[i] && (i < nmatches); i++)
>>> +                matches[i] = xstrdup(driver->modules[i]);
>>> +
>>> +            break;
>>> +        }
>>> +
>>> +        driver++;
>>> +    }
>>> +
>>> +    return i;
>>> +}
>>> +#endif
>>> +
>>>   /**
>>>    *  @return The numbers of found devices that match with the current system
>>>    *  drivers.
>>> @@ -208,6 +243,12 @@ xf86PlatformMatchDriver(char *matches[], int nmatches)
>>>               if ((info != NULL) && (j < nmatches)) {
>>>                   j += xf86VideoPtrToDriverList(info, &(matches[j]), nmatches - j);
>>>               }
>>> +#ifdef __linux__
>>> +            else if (j < nmatches) {
>>> +                char *name = xf86_get_platform_attrib(i, ODEV_ATTRIB_DRIVER);
>>> +                j += xf86platformDriverList(name, &matches[j], nmatches - j);
>>> +            }
>>> +#endif
>>>           }
>>>       }
>>>       return j;
>>>