On Wed, Jun 29, 2016 at 05:14:11PM +0100, Tvrtko Ursulin wrote: > > On 29/06/16 17:00, Chris Wilson wrote: > >On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote: > >> > >>On 29/06/16 16:34, Chris Wilson wrote: > >>>On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote: > >>>>From: Tvrtko Ursulin <tvrtko.ursu...@intel.com> > >>>> > >>>>Replace per-engine initialization with a common half-programatic, > >>>>half-data driven code for ease of maintenance and compactness. > >>>> > >>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com> > >>> > >>>This is the biggest pill to swallow (since our 5x5 table is only > >>>sparsely populated), but it looks correct, and more importantly easier to > >>>read. > >> > >>Yeah I was out of ideas on how to improve it. Fresh mind needed to > >>try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map > >>to bits and registers respectively, and write it as a function. > > > >It's actually a very simple cyclic function based on register > >offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings. > > > >(The only real challenge is picking the direction.) > > > >commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484 > >Author: Ben Widawsky <b...@bwidawsk.net> > >Date: Wed Sep 14 20:32:47 2011 -0700 > > > > drm/i915: Dumb down the semaphore logic > > > > While I think the previous code is correct, it was hard to follow and > > hard to debug. Since we already have a ring abstraction, might as well > > use it to handle the semaphore updates and compares. > > Should I try to go back to that then? Since I am not too happy with > the sparse table... > > This has passed CI so we could merge some of it if that would help > your series, or wait until I rework this patch.
The rule of thumb is incremental improvements tell a better story and should be easier to find a misstep. (My personal experience says the longer I play with a patch the larger it gets...) In short, you've already consolidated a lot of duplication in the vfuncs that will make my life easier (after some rebasing joy). Anything more is just icing on the cake. :) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx