Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

Chris Wilson
2016-06-29T16:01:00Z

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.ursulin@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.ursulin@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 <ben@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.

-Chris

--
Chris Wilson, Intel Open Source Technology Centre