[Intel-gfx] [PATCH] drm/i915: Restore marking context objects as dirty on pinning

[Chris Wilson]

On Thu, Mar 23, 2017 at 09:41:30AM +0000, Tvrtko Ursulin wrote:
> 
> On 22/03/2017 20:59, Chris Wilson wrote:
> >Commit e8a9c58fcd9a ("drm/i915: Unify active context tracking between
> >legacy/execlists/guc") converted the legacy intel_ringbuffer submission
> >to the same context pinning mechanism as execlists - that is to pin the
> >context until the subsequent request is retired. Previously it used the
> >vma retirement of the context object to keep itself pinned until the
> >next request (after i915_vma_move_to_active()). In the conversion, I
> >missed that the vma retirement was also responsible for marking the
> >object as dirty. Mark the context object as dirty when pinning
> >(equivalent to execlists) which ensures that if the context is swapped
> >out due to mempressure or suspend/hibernation, when it is loaded back in
> >it does so with the previous state (and not all zero).
> >
> >Fixes: e8a9c58fcd9a ("drm/i915: Unify active context tracking between legacy/execlists/guc")
> >Reported-by: Dennis Gilmore <dennis at ausil.us>
> >Reported-by: Mathieu Marquer <mathieu.marquer at gmail.com>
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99993
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100181
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >Cc: <drm-intel-fixes at lists.freedesktop.org> # v4.11-rc1
> >---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >index 0ca5ea7a9407..62756eb2bd4a 100644
> >--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >@@ -1440,6 +1440,8 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine,
> > 		ret = context_pin(ctx);
> > 		if (ret)
> > 			goto error;
> >+
> >+		ce->state->obj->mm.dirty = true;
> > 	}
> >
> > 	/* The kernel context is only used as a placeholder for flushing the
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Could it be related to 99671, it also mentions suspend?

No, I do not believe so. The symptoms do not match (but that may just be
snb handling the failure differently) but more tellingly it started (or
at least based on the bug reports became more prevalent) before the
breakage was introduced.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre