[Intel-gfx] [PATCH 02/20] drm/i915: Force PD restore on dirty ppGTTs

[Ville Syrjälä]

On Thu, May 21, 2015 at 05:37:30PM +0300, Mika Kuoppala wrote:
> Force page directory reload when ppgtt va->pa
> mapping has changed. Extend dirty rings mechanism
> for gen > 7 and use it to force pd restore in execlist
> mode when vm has been changed.
> 
> Some parts of execlist context update cleanup based on
> work by Chris Wilson.
> 
> v2: Add comment about lite restore (Chris)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 65 ++++++++++++++++++++--------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0413b8f..5ee2a8c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -264,9 +264,10 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
>  }
>  
>  static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
> -					 struct drm_i915_gem_object *ctx_obj)
> +					 struct intel_context *ctx)
>  {
>  	struct drm_device *dev = ring->dev;
> +	struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
>  	uint64_t desc;
>  	uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj);
>  
> @@ -284,6 +285,14 @@ static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
>  	 * signalling between Command Streamers */
>  	/* desc |= GEN8_CTX_FORCE_RESTORE; */
>  
> +	/* When performing a LiteRestore but with updated PD we need
> +	 * to force the GPU to reload the PD
> +	 */
> +	if (intel_ring_flag(ring) & ctx->ppgtt->pd_dirty_rings) {
> +		desc |= GEN8_CTX_FORCE_PD_RESTORE;

Wasn't there a hardware issue which basically meant you are not
allowed to actually set this bit?

Rafael had some details on that as far as I recall so adding cc...

> +		ctx->ppgtt->pd_dirty_rings &= ~intel_ring_flag(ring);
> +	}
> +
>  	/* WaEnableForceRestoreInCtxtDescForVCS:skl */
>  	if (IS_GEN9(dev) &&
>  	    INTEL_REVID(dev) <= SKL_REVID_B0 &&
> @@ -295,8 +304,8 @@ static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
>  }
>  
>  static void execlists_elsp_write(struct intel_engine_cs *ring,
> -				 struct drm_i915_gem_object *ctx_obj0,
> -				 struct drm_i915_gem_object *ctx_obj1)
> +				 struct intel_context *to0,
> +				 struct intel_context *to1)
>  {
>  	struct drm_device *dev = ring->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -304,14 +313,15 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  	uint32_t desc[4];
>  
>  	/* XXX: You must always write both descriptors in the order below. */
> -	if (ctx_obj1)
> -		temp = execlists_ctx_descriptor(ring, ctx_obj1);
> +	if (to1)
> +		temp = execlists_ctx_descriptor(ring, to1);
>  	else
>  		temp = 0;
> +
>  	desc[1] = (u32)(temp >> 32);
>  	desc[0] = (u32)temp;
>  
> -	temp = execlists_ctx_descriptor(ring, ctx_obj0);
> +	temp = execlists_ctx_descriptor(ring, to0);
>  	desc[3] = (u32)(temp >> 32);
>  	desc[2] = (u32)temp;
>  
> @@ -330,14 +340,20 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  	spin_unlock(&dev_priv->uncore.lock);
>  }
>  
> -static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
> -				    struct drm_i915_gem_object *ring_obj,
> -				    struct i915_hw_ppgtt *ppgtt,
> -				    u32 tail)
> +static void execlists_update_context(struct intel_engine_cs *ring,
> +				     struct intel_context *ctx,
> +				     u32 tail)
>  {
> +	struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
> +	struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
> +	struct drm_i915_gem_object *ring_obj = ringbuf->obj;
> +	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
>  	struct page *page;
>  	uint32_t *reg_state;
>  
> +	WARN_ON(!i915_gem_obj_is_pinned(ctx_obj));
> +	WARN_ON(!i915_gem_obj_is_pinned(ring_obj));
> +
>  	page = i915_gem_object_get_page(ctx_obj, 1);
>  	reg_state = kmap_atomic(page);
>  
> @@ -347,7 +363,7 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
>  	/* True PPGTT with dynamic page allocation: update PDP registers and
>  	 * point the unallocated PDPs to the scratch page
>  	 */
> -	if (ppgtt) {
> +	if (ppgtt && intel_ring_flag(ring) & ctx->ppgtt->pd_dirty_rings) {
>  		ASSIGN_CTX_PDP(ppgtt, reg_state, 3);
>  		ASSIGN_CTX_PDP(ppgtt, reg_state, 2);
>  		ASSIGN_CTX_PDP(ppgtt, reg_state, 1);
> @@ -355,36 +371,21 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
>  	}
>  
>  	kunmap_atomic(reg_state);
> -
> -	return 0;
>  }
>  
>  static void execlists_submit_contexts(struct intel_engine_cs *ring,
>  				      struct intel_context *to0, u32 tail0,
>  				      struct intel_context *to1, u32 tail1)
>  {
> -	struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
> -	struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
> -	struct drm_i915_gem_object *ctx_obj1 = NULL;
> -	struct intel_ringbuffer *ringbuf1 = NULL;
> -
> -	BUG_ON(!ctx_obj0);
> -	WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
> -	WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
> -
> -	execlists_update_context(ctx_obj0, ringbuf0->obj, to0->ppgtt, tail0);
> +	if (WARN_ON(to0 == NULL))
> +		return;
>  
> -	if (to1) {
> -		ringbuf1 = to1->engine[ring->id].ringbuf;
> -		ctx_obj1 = to1->engine[ring->id].state;
> -		BUG_ON(!ctx_obj1);
> -		WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
> -		WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
> +	execlists_update_context(ring, to0, tail0);
>  
> -		execlists_update_context(ctx_obj1, ringbuf1->obj, to1->ppgtt, tail1);
> -	}
> +	if (to1)
> +		execlists_update_context(ring, to1, tail1);
>  
> -	execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
> +	execlists_elsp_write(ring, to0, to1);
>  }
>  
>  static void execlists_context_unqueue(struct intel_engine_cs *ring)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC