On 23/06/16 13:11, Chris Wilson wrote:
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)));
I don't think this works - you meant testing that bits higher than
BIT(I915_NUM_ENGINES) were not set in ring_mask?
And it probably belongs somewhere else, in common code which initializes
intel_device_info I think.
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) ...
?
I like it, will resend when we clarify the above. Also solves one more
issue than the previous versions which is a potentially sparse
logical_rings array if engine ids get renumbered.
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
