Re: [PATCH] drm/i915: correctly restore fences with objects attached

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]