On 22/06/16 17:10, Chris Wilson wrote:
On Wed, Jun 22, 2016 at 04:55:51PM +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.
Yup, long term plan is to reduce as much as the needless duplication
between the two/three (and kill of the dev_priv->gt.init_rings and
friends). Muttering was even afoot to seperate the legacy submission
code from the ring handling.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
/**
* intel_logical_rings_init() - allocate, populate and init the Engine Command Streamers
* @dev: DRM device.
*
- * This function inits the engines for an Execlists submission style (the equivalent in the
- * legacy ringbuffer submission world would be i915_gem_init_engines). It does it only for
- * those engines that are present in the hardware.
+ * This function inits the engines for an Execlists submission style (the
+ * equivalent in the legacy ringbuffer submission world would be
+ * i915_gem_init_engines). It does it only for those engines that are present in
+ * the hardware.
*
* Return: non-zero if the initialization failed.
*/
int intel_logical_rings_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *engine;
+ unsigned int i;
int ret;
- ret = logical_render_ring_init(dev);
- if (ret)
- return ret;
+ BUILD_BUG_ON((1 << RCS) != RENDER_RING);
+ BUILD_BUG_ON((1 << BCS) != BLT_RING);
+ BUILD_BUG_ON((1 << VCS) != BSD_RING);
+ BUILD_BUG_ON((1 << VCS2) != BSD2_RING);
+ BUILD_BUG_ON((1 << VECS) != VEBOX_RING);
Heh, isn't that the very definition of those in the header.
Planning for some array compaction?
No I was trying to protect against someone changing the definitions of
RENDER_RING & co since the loop below this depends on that. Maybe it was
too paranoid. Or maybe better, I could add HAS_ENGINE(id) and cement
that in one place instead of this many BUILD_BUG_ONs.
I'll respin anyway to remove forward decls which can be avoided with
some reshuffle.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
