Re: [PATCH 18/53] drm/i915: Moved the for_each_ring loop outside of i915_gem_context_enable()

[Tomas Elf <tomas.elf@intel.com>]

On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The start of day context initialisation code in i915_gem_context_enable() loops
> over each ring and calls the legacy switch context or the execlist init context
> code as appropriate.
>
> This patch moves the ring looping out of that function in to the top level
> caller i915_gem_init_hw(). This means the a single pass can be made over all
> rings doing the PPGTT, L3 remap and context initialisation of each ring
> altogether.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h         |    2 +-
>   drivers/gpu/drm/i915/i915_gem.c         |   18 ++++++++++-------
>   drivers/gpu/drm/i915/i915_gem_context.c |   32 +++++++++++--------------------
>   3 files changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bfd7b47..653c82d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2989,7 +2989,7 @@ int __must_check i915_gem_context_init(struct drm_device *dev);
>   void i915_gem_context_fini(struct drm_device *dev);
>   void i915_gem_context_reset(struct drm_device *dev);
>   int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
> -int i915_gem_context_enable(struct drm_i915_private *dev_priv);
> +int i915_gem_context_enable(struct intel_engine_cs *ring);
>   void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
>   int i915_switch_context(struct intel_engine_cs *ring,
>   			struct intel_context *to);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9bc60d7..5850991 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4844,6 +4844,8 @@ i915_gem_init_hw(struct drm_device *dev)
>   	 */
>   	init_unused_rings(dev);
>
> +	BUG_ON(!dev_priv->ring[RCS].default_context);
> +
>   	ret = i915_ppgtt_init_hw(dev);
>   	if (ret) {
>   		DRM_ERROR("PPGTT enable HW failed %d\n", ret);
> @@ -4859,6 +4861,8 @@ i915_gem_init_hw(struct drm_device *dev)
>
>   	/* Now it is safe to go back round and do everything else: */
>   	for_each_ring(ring, dev_priv, i) {
> +		WARN_ON(!ring->default_context);
> +
>   		if (ring->id == RCS) {
>   			for (i = 0; i < NUM_L3_SLICES(dev); i++)
>   				i915_gem_l3_remap(ring, i);
> @@ -4870,17 +4874,17 @@ i915_gem_init_hw(struct drm_device *dev)
>   			i915_gem_cleanup_ringbuffer(dev);
>   			return ret;
>   		}
> -	}
>
> -	ret = i915_gem_context_enable(dev_priv);
> -	if (ret && ret != -EIO) {
> -		DRM_ERROR("Context enable failed %d\n", ret);
> -		i915_gem_cleanup_ringbuffer(dev);
> +		ret = i915_gem_context_enable(ring);
> +		if (ret && ret != -EIO) {
> +			DRM_ERROR("Context enable ring #%d failed %d\n", i, ret);
> +			i915_gem_cleanup_ringbuffer(dev);
>
> -		return ret;
> +			return ret;
> +		}
>   	}
>
> -	return ret;
> +	return 0;
>   }
>
>   int i915_gem_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8603bf4..dd83d61 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -403,32 +403,22 @@ void i915_gem_context_fini(struct drm_device *dev)
>   	i915_gem_context_unreference(dctx);
>   }
>
> -int i915_gem_context_enable(struct drm_i915_private *dev_priv)
> +int i915_gem_context_enable(struct intel_engine_cs *ring)
>   {
> -	struct intel_engine_cs *ring;
> -	int ret, i;
> -
> -	BUG_ON(!dev_priv->ring[RCS].default_context);
> +	int ret;
>
>   	if (i915.enable_execlists) {
> -		for_each_ring(ring, dev_priv, i) {
> -			if (ring->init_context) {
> -				ret = ring->init_context(ring,
> -						ring->default_context);
> -				if (ret) {
> -					DRM_ERROR("ring init context: %d\n",
> -							ret);
> -					return ret;
> -				}
> -			}
> -		}
> +		if (ring->init_context == NULL)
> +			return 0;
>
> +		ret = ring->init_context(ring, ring->default_context);
>   	} else
> -		for_each_ring(ring, dev_priv, i) {
> -			ret = i915_switch_context(ring, ring->default_context);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = i915_switch_context(ring, ring->default_context);
> +
> +	if (ret) {
> +		DRM_ERROR("ring init context: %d\n", ret);
> +		return ret;
> +	}
>
>   	return 0;
>   }
>

Reviewed-by: Tomas Elf <tomas.elf@intel.com>

Thanks,
Tomas

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx