On Thu, Jun 23, 2016 at 12:46:42PM +0100, Tvrtko Ursulin wrote:
>
> On 23/06/16 12:25, Chris Wilson wrote:
> >On Thu, Jun 23, 2016 at 12:12:29PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >>
> >>Effectively removes one layer of indirection between the mask of
> >>possible engines and the engine constructors. Instead of spelling
> >>out in code the mapping of HAS_<engine> to constructors, makes
> >>more use of the recently added data driven approach by putting
> >>engine constructor vfuncs into the table as well.
> >>
> >>Effect is fewer lines of source and smaller binary.
> >>
> >>At the same time simplify the error handling since engine
> >>destructors can run on unitialized engines anyway.
> >>
> >>Similar approach could be done for legacy submission is wanted.
> >>
> >>v2: Removed ugly BUILD_BUG_ONs in favour of newly introduced
> >> ENGINE_MASK and HAS_ENGINE macros.
> >> Also removed the forward declarations by shuffling functions
> >> around.
> >>
> >>v3: Warn when logical_rings table does not contain enough data
> >> and disable the engines which could not be initialized.
> >> (Chris Wilson)
> >
> >I was happy with the BUILD_BUG suggestion :)
>
> I've changed my mind later. :)
>
> >>+ for (i = 0;
> >>+ i < I915_NUM_ENGINES && i < ARRAY_SIZE(logical_rings); i++) {
> >
> >HAS_ENGINE() == false if i >= I915_NUM_ENGINES
>
> Don't follow. :) Why is v3 not good enough?
Both (all three) is overkill.
I feel like HAS_ENGINE() should encompass i < I915_NUM_ENGINES quite
succinctly. For belt and braces,
WARN_ON(dev_priv->intel_info.rings_mask & -(1 << I915_NUM_ENGINES)));
for (i = 0; i < ARRAY_SIZE(); i++) {
if (!HAS_ENGINE(i))
continue;
if (!logical_rings[i].init)
continue;
ret = logical_rings[i].init(logical_rings_engine(i));
if (ret)
goto err;
mask |= ENGINE_MASK(i);
}
WARN_ON(mask != dev_priv->intel_info.rings_mask) ...
?
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
