From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: Corruption in glxgears with Compiz Date: Sat, 23 Oct 2010 10:10:24 +0100 Message-ID: References: <1287751996.19922.4.camel@pcjc2lap> <849307$a3hfvt@azsmga001.ch.intel.com> <1287774644.17191.9.camel@pcjc2lap> <5b55a1$ien4q1@fmsmga002.fm.intel.com> <1287804946.9701.3.camel@pcjc2lap> <1287806877.2423.3.camel@pcjc2lap> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id D5E3A9E761 for ; Sat, 23 Oct 2010 02:10:27 -0700 (PDT) In-Reply-To: <1287806877.2423.3.camel@pcjc2lap> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Peter Clifton Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sat, 23 Oct 2010 05:07:57 +0100, Peter Clifton 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