Re: [PATCH] drm/i915: Small compaction of the engine init code

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, Jun 22, 2016 at 04:55:51PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> 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@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  /**
>   * 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?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx