From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH 02/13] drm/i915: Implement command buffer
 parsing logic
Date: Fri, 7 Feb 2014 15:45:48 +0100
Message-ID: <20140207144548.GX17001@phenom.ffwll.local>
References: <1385484699-51596-1-git-send-email-bradley.d.volkin@intel.com>
	<1391032514-19136-1-git-send-email-bradley.d.volkin@intel.com>
	<1391032514-19136-3-git-send-email-bradley.d.volkin@intel.com>
	<8761or10tl.fsf@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
Received: from mail-ea0-f177.google.com (mail-ea0-f177.google.com
	[209.85.215.177])
	by gabe.freedesktop.org (Postfix) with ESMTP id 6CB24FBAB7
	for <intel-gfx@lists.freedesktop.org>;
	Fri,  7 Feb 2014 06:45:52 -0800 (PST)
Received: by mail-ea0-f177.google.com with SMTP id n15so1618057ead.22
	for <intel-gfx@lists.freedesktop.org>;
	Fri, 07 Feb 2014 06:45:51 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <8761or10tl.fsf@intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces@lists.freedesktop.org
Errors-To: intel-gfx-bounces@lists.freedesktop.org
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Fri, Feb 07, 2014 at 03:58:46PM +0200, Jani Nikula wrote:
> 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.

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