Re: [PATCH 02/13] drm/i915: Implement command buffer parsing logic

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 29 Jan 2014, bradley.d.volkin@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.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center