Re: Corruption in glxgears with Compiz

[Chris Wilson <chris@chris-wilson.co.uk>]

On Sat, 23 Oct 2010 05:07:57 +0100, Peter Clifton <pcjc2@cam.ac.uk> wrote:
> Although I don't doubt that it is incorrect for some reason. My logic
> was this.. the mm.flush_rings is supposed to be |='d with the object's
> ring->id if the ring is set on a given object.

Well the whole inter-ring flushing is decidedly suspect since we have no
synchronisation between rings, yet. However in this scenario, you are just
using one ring...

If an object is in a GPU domain and so requires a flush, it is attached to
a ring. However, if the object needs an invalidation it may not yet be
attached to the ring (and in any event the invalidation needs to be
performed on the pending ring). Ahah.

Note to self: flushes must be done on the from-ring before the semaphore
and invalidations on the to-ring after the semaphore.

Can you try this patch?

diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 9290f02..e7f27a5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3078,7 +3078,8 @@ i915_gem_object_set_to_cpu_domain(struct
drm_gem_object *o
bj, int write)
  *		drm_agp_chipset_flush
  */
 static void
-i915_gem_object_set_to_gpu_domain(struct drm_gem_object *obj)
+i915_gem_object_set_to_gpu_domain(struct drm_gem_object *obj,
+				  struct intel_ring_buffer *ring)
 {
 	struct drm_device		*dev = obj->dev;
 	struct drm_i915_private		*dev_priv = dev->dev_private;
@@ -3132,8 +3133,10 @@ i915_gem_object_set_to_gpu_domain(struct
drm_gem_object *
obj)
 
 	dev->invalidate_domains |= invalidate_domains;
 	dev->flush_domains |= flush_domains;
-	if (obj_priv->ring)
+	if (flush_domains & I915_GEM_GPU_DOMAINS)
 		dev_priv->mm.flush_rings |= obj_priv->ring->id;
+	if (invalidate_domains & I915_GEM_GPU_DOMAINS)
+		dev_priv->mm.flush_rings |= ring->id;
 
 	trace_i915_gem_object_change_domain(obj,
 					    old_read_domains,
@@ -3765,7 +3768,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
 		struct drm_gem_object *obj = object_list[i];
 
 		/* Compute new gpu domains and update invalidate/flush */
-		i915_gem_object_set_to_gpu_domain(obj);
+		i915_gem_object_set_to_gpu_domain(obj, ring);
 	}
 
 	if (dev->invalidate_domains | dev->flush_domains) {

-- 
Chris Wilson, Intel Open Source Technology Centre