[Intel-gfx] [PATCH libdrm v2 1/5] intel: add generic functions to check PCI ID

[Chris Wilson]

Quoting Lucas De Marchi (2018-08-31 17:06:01)
> On Fri, Aug 31, 2018 at 09:16:23AM +0100, Chris Wilson wrote:
> > Quoting Lucas De Marchi (2018-08-29 01:35:28)
> > > +bool intel_is_genx(unsigned int devid, int gen)
> > > +{
> > > +       const struct pci_device *p,
> > > +                 *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> > > +
> > > +       for (p = pciids; p < pend; p++) {
> > > +               /* PCI IDs are sorted */
> > > +               if (p->gen < gen)
> > > +                       break;
> > 
> > If we have lots of gen with lots of subids, a binary search for gen
> > would be sensible. However, do we need this function? Do we not just
> > convert everyone over to a lookup of pci-id on entry?
> 
> in some places we need the single IS_GEN9(). The advantage of using this
> function rather than intel_get_genx() is that it can be faster due to
> stopping here, or doing a binary search as you pointed out.
> With intel_get_genx we don't have this.  IS_GEN9() is may be called in
> non-initialization code paths, so IMO its worth.
> 
> What we *can* do here instead is: guarantee all codepaths will occur
> after the call to drm_intel_bufmgr_gem_init() then remove all macros and
> just implement a single function that checks the "cached value".

That would be similar to how we handle elsewhere. But there's no need to
jump there in one series.

> > Idle thought
> > #ifdef SELFTEST
> > int main(void)
> > {
> >       /* check pci-ids are ordered by gen */
> > }
> > #endif
> 
> $ git grep SELFTEST
> $
> 
> you do know this is a patch for libdrm, right?

Wouldn't be that hard to add a check_PROGRAMS target with a -DSELFTEST.
It was just an idle thought if you cared to improve the standard of a
stagnant library. It might as well retire with grace ;)
-Chris