[Intel-gfx] [PATCH 02/13] drm/i915: Implement command buffer parsing logic

[Daniel Vetter]

On Fri, Feb 07, 2014 at 03:58:46PM +0200, Jani Nikula wrote:
> On Wed, 29 Jan 2014, bradley.d.volkin at intel.com 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