[Intel-gfx] [CI 2/2] drm/i915: Initialize legacy semaphores from engine hw id indexed array

[Chris Wilson]

On Wed, Aug 17, 2016 at 04:10:00PM +0100, Tvrtko Ursulin wrote:
> 
> On 17/08/16 15:44, Chris Wilson wrote:
> >On Wed, Aug 17, 2016 at 03:36:51PM +0100, Tvrtko Ursulin wrote:
> >>On 17/08/16 11:05, Chris Wilson wrote:
> >>>On Wed, Aug 17, 2016 at 10:57:34AM +0100, Tvrtko Ursulin wrote:
> >>>>
> >>>>On 17/08/16 10:41, Chris Wilson wrote:
> >>>>>On Wed, Aug 17, 2016 at 10:34:18AM +0100, Tvrtko Ursulin wrote:
> >>>>>>Or add an initialized engine array to dev_priv, in addition to the
> >>>>>>existing map for best of both worlds.
> >>>>>
> >>>>>We have the ring_mask which already tells us that mapping, so I think
> >>>>>the second array is overkill.
> >>>>
> >>>>Yes, I said "in addition to the existing map". In addition we could
> >>>>have an array of only initialized engines to avoid any skipping at
> >>>>runtime. Since iterators end with intel_engine_initialized anyway.
> >>>
> >>>And I'm saying we already have that information in ring_mask.
> >>
> >>The ffs smarts you've been developing? I am not sure devising very
> >>smart macros and expanding all that code to all the call sites is
> >>that great. It is effectively just re-implementing arrays at runtime
> >>using CPU instructions.
> >>
> >>What would be the big deal of just building the array when engines
> >>are initialized for simplicity? Just the allure of getting away with
> >>no iterator variable?
> >
> >It's just the redundant information. We definitely want the (sparse)
> >id->engine lookup table just for convenience in execbuf and friends.
> >Given that, having a second array is overkill. A list may be a
> >reasonable compromise, and I guess pushing for using an ffs iterator
> >where it matters (where we know we have a sparse set).
> 
> Don't get what you consider a list vs array? I was talking about an
> contiguous array of engine pointers (only initialized ones).
> 
> Can't see that it is redundant or overkill since it is not that
> uncommon to have two separate data structure/indices pointing the
> the same thing for ease of manipulation/use.
> 
> As I said, you build the list once at init time, or you build it
> effectively at the every iterator site. When you call ffs() you make
> the CPU do the skipping just on a lower level than the current C
> code does.

All I am saying is that makes having the dense array of engines
redundant.

There are only a few places where iterating over the engines is anything
other than a rare event (init, reset, etc) and where we want a sparse
set we typically have a mask of engines to iterate over already. In that
regard just the list of engines would suffice to make the common iterator
very simple.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre