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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel