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@xxxxxxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxx> > Cc: Stéphane Marchesin <marcheu@xxxxxxxxxxxx> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530 > Cc: stable@xxxxxxxxxxxxxxx (for 3.10 only) > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html