Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> An issue encountered with switching mm on gen7 is that the GPU likes to
> hang (with the VS unit busy) when told to force restore the current
> context. We can simply workaround this by substituting the
> MI_FORCE_RESTORE flag with a round-trip through the kernel_context,
> forcing the context to be saved and restored; thereby reloading the
> PP_DIR registers and updating the modified page directory!
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Matthew Auld <matthew.william.auld@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 30 ++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 65811e2fa7da..332d97bc5c27 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1458,6 +1458,7 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
> (HAS_LEGACY_SEMAPHORES(i915) && IS_GEN7(i915)) ?
> INTEL_INFO(i915)->num_rings - 1 :
> 0;
> + bool force_restore = false;
> int len;
> u32 *cs;
>
> @@ -1471,6 +1472,12 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
> len = 4;
> if (IS_GEN7(i915))
> len += 2 + (num_rings ? 4*num_rings + 6 : 0);
> + if (flags & MI_FORCE_RESTORE) {
> + GEM_BUG_ON(flags & MI_RESTORE_INHIBIT);
> + flags &= ~MI_FORCE_RESTORE;
> + force_restore = true;
> + len += 2;
> + }
>
> cs = intel_ring_begin(rq, len);
> if (IS_ERR(cs))
> @@ -1496,6 +1503,20 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
> }
>
> *cs++ = MI_NOOP;
> + if (force_restore) {
> + /*
> + * The HW doesn't handle being told to restore the current
> + * context very well. Quite often it likes goes to go off and
> + * sulk, especially when it is meant to be reloading PP_DIR.
> + * A very simple fix to force the reload is to simply switch
> + * away from the current context and back again.
> + */
> + *cs++ = MI_SET_CONTEXT;
> + *cs++ = i915_ggtt_offset(to_intel_context(i915->kernel_context,
> + engine)->state) |
> + MI_MM_SPACE_GTT |
> + MI_RESTORE_INHIBIT;
Why inhibit? You dont really switch to kernel but rather overwrite
current with kernel ctx.
-Mika
> + }
> *cs++ = MI_SET_CONTEXT;
> *cs++ = i915_ggtt_offset(rq->hw_context->state) | flags;
> /*
> @@ -1585,11 +1606,14 @@ static int switch_context(struct i915_request *rq)
>
> to_mm->pd_dirty_rings &= ~intel_engine_flag(engine);
> engine->legacy_active_ppgtt = to_mm;
> - hw_flags = MI_FORCE_RESTORE;
> +
> + if (to_ctx == from_ctx) {
> + hw_flags = MI_FORCE_RESTORE;
> + from_ctx = NULL;
> + }
> }
>
> - if (rq->hw_context->state &&
> - (to_ctx != from_ctx || hw_flags & MI_FORCE_RESTORE)) {
> + if (rq->hw_context->state && to_ctx != from_ctx) {
> GEM_BUG_ON(engine->id != RCS);
>
> /*
> --
> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
