[Intel-gfx] [PATCH] drm/i915: Don't let a device flush to prepare buffers clear new write_domains.

[Chris Wilson]

On Thu, 2009-02-19 at 15:25 -0800, Eric Anholt wrote:
> The problem was that object_set_to_gpu_domain would set the new write_domains
> that are getting set by this batchbuffer, then the accumulated flushes required
> for all the objects in preparation for this batchbuffer were posted, and the
> brand new write domain would get cleared by the flush being posted.  Instead,
> hang on to the new (or old if we're not changing it) value and set it after
> the flush is queued.

I've spent a bit of time looking over this and can see the problem you
need to solve and why this fixes it. AFAICS, the true problem here lies
with the batching of the flush for the exec buffer objects, and even
after this patch, that is still not very clear from the code. I'd like
to see a i915_gem_exec_objects_set_to_gpu_domains() (or some such) that
combined the 3 stages in execbuffer that handle the updating of the
write_domain across the pre-exec flush.

Whilst looking at the domain checking, I think one case is missing
during the validation. AIUI, a write object must have read_domains ==
write_domain.

>From eda7943bbc9acdea6dbd0d8e1b80067651f25757 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Fri, 20 Feb 2009 11:10:33 +0000
Subject: [PATCH 2/3] drm/i915: Check conflicting read/write domains.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cd9fa13..6d2cf53 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2427,6 +2427,20 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj,
 		target_obj->pending_read_domains |= reloc.read_domains;
 		target_obj->pending_write_domain |= reloc.write_domain;
 
+		if (target_obj->pending_write_domain &&
+		    target_obj->pending_write_domain != target_obj->pending_read_domains) {
+			DRM_ERROR("Write domain conflicts with read domains: "
+				  "obj %p target %d offset %d "
+				  "write %08x, read %08x\n",
+				  obj, reloc.target_handle,
+				  (int) reloc.offset,
+				  target_obj->pending_write_domain,
+				  target_obj->pending_read_domains);
+			drm_gem_object_unreference(target_obj);
+			i915_gem_object_unpin(obj);
+			return -EINVAL;
+		}
+
 		/* If the relocation already has the right value in it, no
 		 * more work needs to be done.
 		 */
-- 
1.6.0.4