[PATCH 4/4 v3] drm/i915: miscellaneous tiny tweaks to GEM object->dirty

From: Dave Gordon <david.s.gordon@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 4/4 v3] drm/i915: miscellaneous tiny tweaks to GEM object->dirty
Date: Thu, 10 Dec 2015 18:51:26 +0000	[thread overview]
Message-ID: <1449773486-30822-5-git-send-email-david.s.gordon@intel.com> (raw)
In-Reply-To: <1449773486-30822-1-git-send-email-david.s.gordon@intel.com>

This patch covers a couple more places where a GEM object is (or may be)
modified by means of CPU writes, and should therefore be marked dirty to
ensure that the changes are not lost in the event that the object is
evicted under memory pressure.

One is in i915_gem_begin_cpu_access(); after this call, the GEM object may
be written to by the caller (which may not be part of the i915 driver e.g.
udl). We must therefore assume that the object is dirty hereafter if
the caller has asked for write access.

Another is in copy_batch(); the destination object is obviously dirty
after being written, but failing to mark it doesn't cause a bug at
present, because the object is page-pinned at this point, and should
remain either page- pinned or GTT-pinned until it's retired, at which
point its content can be discarded. However, if the lifecycle of shadow
batches is ever changed (e.g. by the introduction of a GPU scheduler)
this might no longer be true, so it's safer to mark it correctly (this
introduces no overhead if the buffer is never swappable). It also makes
the content cycle clearer:

	---allocate-->
	[empty buffer acquired from pool]
	---fill-->
	[valid buffer full of unsaved data]
	---use-->
	[buffer full of unsaved but unwanted data]
	--retire-->
	[purgeable buffer returned to pool]
	... repeat ...

The last change here is just for consistency; since 'dirty' has been
declared as an (unsigned int) bitfield, let's not treat it as a bool.
Maybe it should be a byte instead?

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 3 +++
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 3 +++
 drivers/gpu/drm/i915/intel_lrc.c       | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 814d894..81a4fa2 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -946,6 +946,9 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
 
 	memcpy(dst, src, batch_len);
 
+	/* After writing on the dest_obj, its backing store is out-of-date */
+	dest_obj->dirty = 1;
+
 unmap_src:
 	vunmap(src_base);
 unpin_src:
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index e9c2bfd..5eb7887 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -208,6 +208,9 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
 		return ret;
 
 	ret = i915_gem_object_set_to_cpu_domain(obj, write);
+	if (write)
+		obj->dirty = 1;
+
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ceccecc..c7520b7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1030,7 +1030,7 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
 	if (ret)
 		goto unpin_ctx_obj;
 
-	ctx_obj->dirty = true;
+	ctx_obj->dirty = 1;
 
 	/* Invalidate GuC TLB. */
 	if (i915.enable_guc_submission)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
