[Intel-gfx] [PATCH 18/59] drm/i915: Split i915_ppgtt_init_hw() in half - generic and per ring

[Tomas Elf]

On 19/03/2015 12:30, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
>
> The i915_gem_init_hw() function calls a bunch of smaller initialisation
> functions. Multiple of which have generic sections and per ring sections. This
> means multiple passes are done over the rings. Each pass writes data to the ring
> which floats around in that ring's OLR until some random point in the future
> when an add_request() is done by some random other piece of code.
>
> This patch breaks i915_ppgtt_init_hw() in two with the per ring initialisation
> now being done in i915_ppgtt_init_ring(). The ring looping is now done at the
> top level in i915_gem_init_hw().
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c     |   25 +++++++++++++++++++------
>   drivers/gpu/drm/i915/i915_gem_gtt.c |   28 +++++++++++++++-------------
>   drivers/gpu/drm/i915/i915_gem_gtt.h |    1 +
>   3 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8677293..683fc80 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4883,19 +4883,32 @@ i915_gem_init_hw(struct drm_device *dev)
>   	 */
>   	init_unused_rings(dev);
>
> +	ret = i915_ppgtt_init_hw(dev);
> +	if (ret) {
> +		DRM_ERROR("PPGTT enable HW failed %d\n", ret);
> +		goto out;
> +	}
> +
> +	/* Need to do basic initialisation of all rings first: */
>   	for_each_ring(ring, dev_priv, i) {
>   		ret = ring->init_hw(ring);
>   		if (ret)
>   			goto out;
>   	}
>
> -	for (i = 0; i < NUM_L3_SLICES(dev); i++)
> -		i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> +	/* Now it is safe to go back round and do everything else: */
> +	for_each_ring(ring, dev_priv, i) {
> +		if (ring->id == RCS) {
> +			for (i = 0; i < NUM_L3_SLICES(dev); i++)
> +				i915_gem_l3_remap(ring, i);
> +		}
>
> -	ret = i915_ppgtt_init_hw(dev);
> -	if (ret && ret != -EIO) {
> -		DRM_ERROR("PPGTT enable failed %d\n", ret);
> -		i915_gem_cleanup_ringbuffer(dev);
> +		ret = i915_ppgtt_init_ring(ring);
> +		if (ret && ret != -EIO) {
> +			DRM_ERROR("PPGTT enable ring #%d failed %d\n", i, ret);
> +			i915_gem_cleanup_ringbuffer(dev);
> +			goto out;
> +		}
>   	}
>
>   	ret = i915_gem_context_enable(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d8ff1a8..83076d7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1280,11 +1280,6 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>
>   int i915_ppgtt_init_hw(struct drm_device *dev)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_engine_cs *ring;
> -	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> -	int i, ret = 0;
> -
>   	/* In the case of execlists, PPGTT is enabled by the context descriptor
>   	 * and the PDPs are contained within the context itself.  We don't
>   	 * need to do anything here. */
> @@ -1303,16 +1298,23 @@ int i915_ppgtt_init_hw(struct drm_device *dev)
>   	else
>   		MISSING_CASE(INTEL_INFO(dev)->gen);
>
> -	if (ppgtt) {
> -		for_each_ring(ring, dev_priv, i) {
> -			ret = ppgtt->switch_mm(ppgtt, ring);
> -			if (ret != 0)
> -				return ret;
> -		}
> -	}
> +	return 0;
> +}
>
> -	return ret;
> +int i915_ppgtt_init_ring(struct intel_engine_cs *ring)
> +{
> +	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> +
> +	if (i915.enable_execlists)
> +		return 0;
> +
> +	if (!ppgtt)
> +		return 0;
> +
> +	return ppgtt->switch_mm(ppgtt, ring);
>   }
> +
>   struct i915_hw_ppgtt *
>   i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index c9e93f5..2941fbb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -311,6 +311,7 @@ void i915_global_gtt_cleanup(struct drm_device *dev);
>
>   int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
>   int i915_ppgtt_init_hw(struct drm_device *dev);
> +int i915_ppgtt_init_ring(struct intel_engine_cs *ring);
>   void i915_ppgtt_release(struct kref *kref);
>   struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev,
>   					struct drm_i915_file_private *fpriv);
>

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

Thanks,
Tomas