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@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> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
