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

Mika Kuoppala
2015-06-26T14:45:43Z

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