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

[Chris Wilson]

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>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre