[PATCH] Move the logic for matching devices out of probe routine.

[Aaron Plattner]

On Mon, Sep 14, 2009 at 09:03:41AM -0700, Mario Limonciello wrote:
> Hi Aaron:
> Aaron Plattner wrote:
> > On Sat, Sep 12, 2009 at 07:37:39AM -0700, Mario Limonciello wrote:
> > I'm somewhat baffled and saddened by the fact that we have a driver probe
> > routine when the driver is not allowed to probe the hardware and make
> > decisions based on things other than the PCI ID.  It seems like this really
> > ought to be fixed in the server.  That said, it does seem relatively easy
> > to just generate a ridiculously big table with all the current PCI IDs plus
> > anything that the NVIsSupported function would allow, as a workaround.
> >
> Yeah when I first looked at the problem I was in agreement that it
> seemed like something the server should support, but then when I saw
> that all the other drivers are doing it this way, the easier solution
> was to switch it over to a large table.  If  the server gets support for
> properly returning a probed card, then all the drivers would be able to
> change over.

Well, for some drivers, it does make sense to have a single static table.
E.g., the mach64 driver has 34 supported devices total and the chance of
adding new ones any time soon is low.  For the nv driver, the table is
much, much larger but probably still manageable.  However, I also have to
look out for the nvidia driver, where we really need to query the kernel
module to be completely sure whether a given device is supported or not.
This requires some amount of code to be executed at runtime, so a static
table is not really sufficient.  I suspect that the open-source drivers
that are moving to DRI/DRM-based kernel support are going to need similar
compatibility handshakes.  If the server can fall back gracefully when a
driver probe routine returns FALSE, then it allows the kernel modules to
trigger these fallbacks based on whatever criteria they choose, rather than
trying to encode hard and fast supported/not supported information into
something as limited as a PCI ID match table.

I think the match tables should be treated as an optimization, so that you
don't need to call every driver's probe routine on every device in the
system, rather than a comprehensive list of which devices are and are not
supported.

> > libpciaccess really ought to have had mask-based matches for device_id like
> > it does for device_class, but it's too late for that now.
> >
> > I do object to having a Perl script generate this information from a CSV
> > file.  If you absolutely must have the PCI ID table somewhere outside the
> > driver, the right way to do it is to generate the CSV file from a header
> > file using a C small helper program, not the other way around with Perl.
> >
> I by no means require a CSV file.  I implemented it this way because it
> makes updating two header files easier when a developer needs to update
> one file, run a script, and commit.  I see a couple of possible
> solutions here.  Let me know what you think is the best, and i'll go
> with that when I resubmit the series of patches with Gaetan's suggestions:

Okay.  There have been problems in the past with distributions trying to
parse the driver source to come up with a PCI ID-based match table and
getting it wrong.

> 1) Drop the perl script and csv, just submit the two header files. 
> They'll both have to be updated every time there is a new ID added.
> 2) Drop the perl script and csv, remove the code that outputs the name
> of all the supported devices when the driver gets loaded.  Instead add
> the name of every device into the comments section of a single header
> file.  That one header file will have to be updated every time.
> 3) Leave the perl script and csv, no changes.
> 4) Some other more specific suggestion you have.

I'd like to leave the driver as-is and instead work on improving the
server's driver selection process.  Does that sound reasonable to you?
Ideally, the server would be able to fall back even if a driver returns
TRUE for ProbeHardware and then later fails PreInit, but that's even more
ambitious.

There have been a number of suggestions on the list about how to configure
the server's driver selection process.  I'll try to round up some people at
XDC and see if we come up with a concrete plan, and then I'll try to free
up some time to help implement it.

Sincerely,
Aaron