Re: [PATCH v3] drm/i915/lrc: Update PDPx registers with lri commands

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Michel Thierry <michel.thierry@intel.com> writes:

> A safer way to update the PDPx registers is sending lri commands, added
> in the ring before the batchbuffer start. Otherwise, the ctx must be idle
> before trying to change anything (but the ring-tail) in the ctx image. An
> example where the ctx won't be idle is lite-restore.
>
> This patch depends on 5b7e4c9ce ("drm/i915/gtt: Mark TLBS dirty for gen8+").
>
> v2: Combine lri writes (and save 8 commands). (Mika)
> v3: Rebase after ring/req changes, and removed references to deprecated patches.
>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 42 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d527b7b..e87d74c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1379,6 +1379,34 @@ static int gen9_init_render_ring(struct intel_engine_cs *ring)
>  	return init_workarounds_ring(ring);
>  }
>  
> +static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
> +{
> +	struct i915_hw_ppgtt *ppgtt = req->ctx->ppgtt;
> +	struct intel_engine_cs *ring = req->ring;
> +	struct intel_ringbuffer *ringbuf = req->ringbuf;
> +	const int num_lri_cmds = GEN8_LEGACY_PDPES * 2;
> +	int i, ret;
> +
> +	ret = intel_logical_ring_begin(req, num_lri_cmds * 2 + 2);
> +	if (ret)
> +		return ret;
> +
> +	intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(num_lri_cmds));
> +	for (i = GEN8_LEGACY_PDPES - 1; i >= 0; i--) {
> +		const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
> +
> +		intel_logical_ring_emit(ringbuf, GEN8_RING_PDP_UDW(ring, i));
> +		intel_logical_ring_emit(ringbuf, upper_32_bits(pd_daddr));
> +		intel_logical_ring_emit(ringbuf, GEN8_RING_PDP_LDW(ring, i));
> +		intel_logical_ring_emit(ringbuf, lower_32_bits(pd_daddr));
> +	}
> +
> +	intel_logical_ring_emit(ringbuf, MI_NOOP);
> +	intel_logical_ring_advance(ringbuf);
> +
> +	return 0;
> +}
> +
>  static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  			      u64 offset, unsigned dispatch_flags)
>  {
> @@ -1386,6 +1414,20 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  	bool ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE);
>  	int ret;
>  
> +	/* Don't rely in hw updating PDPs, specially in lite-restore.
> +	 * Ideally, we should set Force PD Restore in ctx descriptor,
> +	 * but we can't. Force Restore would be a second option, but
> +	 * it is unsafe in case of lite-restore (because the ctx is
> +	 * not idle). */
> +	if (req->ctx->ppgtt &&
> +	    (intel_ring_flag(req->ring) & req->ctx->ppgtt->pd_dirty_rings)) {
> +		ret = intel_logical_ring_emit_pdps(req);
> +		if (ret)
> +			return ret;
> +
> +		req->ctx->ppgtt->pd_dirty_rings &= ~intel_ring_flag(req->ring);
> +	}
> +
>  	ret = intel_logical_ring_begin(req, 4);
>  	if (ret)
>  		return ret;
> -- 
> 2.4.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx