drm/i915: Fix obj->map_and_fenceable across tiling changes
authorChris Wilson <chris@chris-wilson.co.uk>
Thu, 6 Nov 2014 08:40:35 +0000 (08:40 +0000)
committerJani Nikula <jani.nikula@intel.com>
Tue, 11 Nov 2014 09:04:48 +0000 (11:04 +0200)
As obj->map_and_fenceable computation has changed to only be set when
the object is bound inside the global GTT (and is suitable aligned to a
fence region) we need to accommodate those changes when the tiling is
adjusted. The easiest solution is to unbind from the global GTT if we
are currently fenceable, but will not be after the tiling change.

The bug has been exposed by

commit f8fcadba218fe6d23b2e353fea1cf0a4be4c9454
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Oct 31 13:53:52 2014 +0000

    drm/i915: Only mark as map-and-fenceable when bound into the GGTT

which tried to fix an oversight from

commit e6a844687cf929ec053c7578d5ecc794a8a6c5cf
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Aug 11 12:00:12 2014 +0200

    drm/i915: Force CPU relocations if not GTT mapped

which changed the handling of obj->map_and_fenceable.

Note that the alignment check is a vestige from our attempts to reduce
the alignment requirements of tiled but unfenced buffers on
gen2/3. Also, that was when unbinding from the GTT meant UC writes and
clflushing, so we went to great pains to avoid such.

That leaves the actual bug of setting map_and_fenceable to true if we're
not bound to ggtt, which violates the change introduced in the above
patch. Unbinding in that case really looks like the simplest and safest
option, we have to do it anyway.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85896
Testcase: igt/gem_concurrent_blit/gttX*
Tested-by: huax.lu@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Valtteri Rantala <valtteri.rantala@intel.com>
[Jani: amend commit message per input from Daniel and bisect result from
Valtteri]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
drivers/gpu/drm/i915/i915_gem_tiling.c

index 2cefb597df6dc92d446557442073d5c998843a30..2b1eaa29ada448e2b6ec8b1d070482d7ee278933 100644 (file)
@@ -364,22 +364,9 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
                 * has to also include the unfenced register the GPU uses
                 * whilst executing a fenced command for an untiled object.
                 */
-
-               obj->map_and_fenceable =
-                       !i915_gem_obj_ggtt_bound(obj) ||
-                       (i915_gem_obj_ggtt_offset(obj) +
-                        obj->base.size <= dev_priv->gtt.mappable_end &&
-                        i915_gem_object_fence_ok(obj, args->tiling_mode));
-
-               /* Rebind if we need a change of alignment */
-               if (!obj->map_and_fenceable) {
-                       u32 unfenced_align =
-                               i915_gem_get_gtt_alignment(dev, obj->base.size,
-                                                           args->tiling_mode,
-                                                           false);
-                       if (i915_gem_obj_ggtt_offset(obj) & (unfenced_align - 1))
-                               ret = i915_gem_object_ggtt_unbind(obj);
-               }
+               if (obj->map_and_fenceable &&
+                   !i915_gem_object_fence_ok(obj, args->tiling_mode))
+                       ret = i915_gem_object_ggtt_unbind(obj);
 
                if (ret == 0) {
                        obj->fence_dirty =