[ 79/99] drm/i915: correctly restore fences with objects attached

From: Greg Kroah-Hartman
Date: Fri Aug 02 2013 - 06:38:55 EST

3.10-stable review patch. If anyone has any objections, please let me know.


From: Daniel Vetter <daniel.vetter@xxxxxxxx>

commit 94a335dba34ff47cad3d6d0c29b452d43a1be3c8 upstream.

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

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

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.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: StÃphane Marchesin <marcheu@xxxxxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Tested-by: Matthew Garrett <matthew.garrett@xxxxxxxxxx>
Tested-by: BjÃrn Bidar <theodorstormgrade@xxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2138,7 +2138,17 @@ void i915_gem_restore_fences(struct drm_

for (i = 0; i < dev_priv->num_fence_regs; i++) {
struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
- i915_gem_write_fence(dev, i, reg->obj);
+ /*
+ * Commit delayed tiling changes if we have an object still
+ * attached to the fence, otherwise just clear the fence.
+ */
+ if (reg->obj) {
+ i915_gem_object_update_fence(reg->obj, reg,
+ reg->obj->tiling_mode);
+ } else {
+ i915_gem_write_fence(dev, i, NULL);
+ }

@@ -2676,6 +2686,10 @@ static void i915_gem_write_fence(struct
if (i915_gem_object_needs_mb(dev_priv->fence_regs[reg].obj))

+ WARN(obj && (!obj->stride || !obj->tiling_mode),
+ "bogus fence setup with stride: 0x%x, tiling mode: %i\n",
+ obj->stride, obj->tiling_mode);
switch (INTEL_INFO(dev)->gen) {
case 7:
case 6:
@@ -2735,6 +2749,7 @@ static void i915_gem_object_update_fence
fence->obj = NULL;
+ obj->fence_dirty = false;

static int
@@ -2864,7 +2879,6 @@ i915_gem_object_get_fence(struct drm_i91
return 0;

i915_gem_object_update_fence(obj, reg, enable);
- obj->fence_dirty = false;

return 0;

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/