From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 02/13] drm/i915: Implement command buffer parsing logic Date: Tue, 11 Feb 2014 20:21:52 +0200 Message-ID: <87ha8533y7.fsf@intel.com> 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> <20140207144548.GX17001@phenom.ffwll.local> <20140211181209.GA17499@bdvolkin-ubuntu-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id D7750FAA6E for ; Tue, 11 Feb 2014 10:22:54 -0800 (PST) In-Reply-To: <20140211181209.GA17499@bdvolkin-ubuntu-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: "Volkin, Bradley D" , Daniel Vetter Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Tue, 11 Feb 2014, "Volkin, Bradley D" wrote: > On Fri, Feb 07, 2014 at 06:45:48AM -0800, Daniel Vetter wrote: >> 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. > > Sorry for the delayed response. The background here is that I originally > just had the tables sorted with a comment to say as much. The idea was that > if the linear search became an issue, switching algorithms would be easier. > Chris suggested just moving to bsearch and checking that the tables are sorted > as part of the v1 series review. I implemented bsearch and found that the perf > change was the same to slightly worse. So I added the sorting check and kept > the linear search until we have better data. Ok. For the linear search I think you could add the check if you've iterated past the register and bail out early, and gather the data with that. BR, Jani. > > Thanks, > Brad > >> -Daniel >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center