Re: Corruption in glxgears with Compiz

[Peter Clifton <pcjc2@cam.ac.uk>]

On Sat, 2010-10-23 at 04:35 +0100, Peter Clifton wrote:

> Lost of bisecting and backporting later.. and I've identified the bad
> commit:
> 
> 9220434a8768902cd9cf248709972678b74aa8c1 drm/i915: Only emit a flush
> request on the active ring.

A minimal fix is this:

commit 78342e8fd01614ac0507db1f9c3e0522f4da3c14
Author: Peter Clifton <pcjc2@cam.ac.uk>
Date:   Sat Oct 23 04:00:21 2010 +0100

    Attempted fix

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9290f02..868a399 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3759,7 +3759,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
         */
        dev->invalidate_domains = 0;
        dev->flush_domains = 0;
-       dev_priv->mm.flush_rings = 0;
+       dev_priv->mm.flush_rings = ring->id;
 
        for (i = 0; i < args->buffer_count; i++) {
                struct drm_gem_object *obj = object_list[i];




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.

But we transfer objects to GPU domain before we actually put them in the
ring, therefore it never gets set.

So this patch just dumps the execbuffer ring into the list of rings to
be flushed. I guess that might be wrong.. perhaps we don't always need
to flush that ring unless an object in it gets reused.. anyway, I'm not
that familiar with GEM internals, and it is gone 5AM here. Still.. I
think I can claim I've nailed the offending commit at least.

Hopefully someone can come up with a sensible patch and explain to me
how this stuff works ;)


Best wishes,

-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)
Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)