[Intel-gfx] [PATCH] drm/i915: correctly restore fences with objects attached

[Daniel Vetter]

On Wed, Jul 17, 2013 at 03:44:16PM +0100, Chris Wilson wrote:
> On Wed, Jul 17, 2013 at 02:51:28PM +0200, Daniel Vetter wrote:
> > To avoid stalls we delay tiling changes and especially hold of
> > committing the new fence state for as long as possible.
> > Synchronization points are in the execbuf code and in our gtt fault
> > handler.
> > 
> > Unfortunately we've missed that tricky detail when adding proper fence
> > restore code in
> > 
> > commit 19b2dbde5732170a03bd82cc8bd442cf88d856f7
> > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > Date:   Wed Jun 12 10:15:12 2013 +0100
> > 
> >     drm/i915: Restore fences after resume and GPU resets
> > 
> > The result was that we've restored fences for objects with no tiling,
> > since the object<->fence link still existed after resume. Now that
> > wouldn't have been too bad since any subsequent access would have
> > fixed things up, but if we've changed from tiled to untiled real havoc
> > happened:
> > 
> > The tiling stride is stored -1 in the fence register, so a stride of 0
> > resulted in all 1s in the top 32bits, and so a completely bogus fence
> > spanning everything from the start of the object to the top of the
> > GTT. The tell-tale in the register dumps looks like:
> > 
> >                  FENCE START 2: 0x0214d001
> >                  FENCE END 2: 0xfffff3ff
> > 
> > Bit 11 isn't set since the hw doesn't store it, even when writing all
> > 1s (at least on my snb here).
> > 
> > To prevent such a gaffle in the future add a sanity check for fences
> > with an untiled object attached in i915_gem_write_fence.
> > 
> > v2: Fix the WARN, spotted by Chris.
> > 
> > v3: Trying to reuse get_fences looked ugly and obfuscated the code.
> > Instead reuse update_fence and to make it really dtrt also move the
> > fence dirty state clearing into update_fence.
> > 
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Stéphane Marchesin <marcheu at chromium.org>
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530
> > Cc: stable at vger.kernel.org (for 3.10 only)
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> Sigh. I thought we were covered because before anything accessed this
> dirty object, the fence would have been rewritten. However, Daniel
> correctly points out that the stride==0 fence clobbers the entire GTT
> and so may be used by the hardware in preference to any other fence.
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Picked up for -fixes, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch