On Fri, Feb 07, 2014 at 03:58:46PM +0200, Jani Nikula wrote: > On Wed, 29 Jan 2014, bradley.d.volkin@xxxxxxxxx wrote: > > +static int valid_reg(const u32 *table, int count, u32 addr) > > +{ > > + if (table && count != 0) { > > + int i; > > + > > + for (i = 0; i < count; i++) { > > + if (table[i] == addr) > > + return 1; > > + } > > + } > > You go to great lengths to validate the register tables are sorted, but > in the end you don't take advantage of this fact by bailing out early if > the lookup goes past the addr. > > Is this optimization the main reason for having the tables sorted, or > are there other reasons too (I couldn't find any)? > > I'm beginning to wonder if this is a premature optimization that adds > extra code. For master restricted registers you will always scan the > regular reg table completely first. Perhaps a better option would be to > have all registers in the same table, with a separate master flag, > ordered by how frequently they are expected to be used. We do want to > optimize for the happy day scenario. But maybe it's too early to tell. > > I'm inclined to ripping out the sort requirement and check, if the sole > purpose is optimization, for simplicity's sake. tbh I don't mind the sorting requirement, and iirc Brad has patches already for binary search. Once we start to rely on the sorting we can easily add a little functions which checks for that at ring initialization, so I also don't see any concerns wrt code fragility. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx