diff mbox

[04/13] drm/i915: Introduce i915_gem_object_finish_gtt()

Message ID 1302771827-26112-5-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson April 14, 2011, 9:03 a.m. UTC
Like its siblings finish_gpu(), this function clears the object from the
GTT domain forcing it to be trigger a domain invalidation should we ever
need to use via the GTT again.

Note that the most important side-effect of finishing the GTT domain
(aside from clearing the tracking read/write domains) is that it imposes
an memory barrier so that all accesses are complete before it returns,
which is important if you intend to be modifying translation tables
shortly afterwards. The second most important side-effect is that it
tears down the GTT mappings forcing a page-fault and invalidation on
next user access to the object.

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

Comments

Daniel Vetter April 14, 2011, 4:12 p.m. UTC | #1
On Thu, Apr 14, 2011 at 10:03:38AM +0100, Chris Wilson wrote:
> Like its siblings finish_gpu(), this function clears the object from the
> GTT domain forcing it to be trigger a domain invalidation should we ever
> need to use via the GTT again.
> 
> Note that the most important side-effect of finishing the GTT domain
> (aside from clearing the tracking read/write domains) is that it imposes
> an memory barrier so that all accesses are complete before it returns,
> which is important if you intend to be modifying translation tables
> shortly afterwards. The second most important side-effect is that it
> tears down the GTT mappings forcing a page-fault and invalidation on
> next user access to the object.

Our maze of cache handling functions, all alike is starting to get
annoying. Especially these finish functions which are essentially two-way
barriers and hence contain all the code that already exists in the from of
flush_foo_write_domain.

But every time I bang my head against this particular wall, the only thing
I can come up with is some abomination from hell. And I've been tossing
around ideas for the better part of a year already with no luck.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Chris Wilson April 14, 2011, 8:20 p.m. UTC | #2
On Thu, 14 Apr 2011 18:12:13 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> But every time I bang my head against this particular wall, the only thing
> I can come up with is some abomination from hell. And I've been tossing
> around ideas for the better part of a year already with no luck.

Hear, hear. My feelings exactly. At the moment, I think just having a
consistent set of interfaces is good enough.
-Chris
Keith Packard May 4, 2011, 4:47 p.m. UTC | #3
On Thu, 14 Apr 2011 10:03:38 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> +	obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
> +	obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;

I'll bet you want to modify the read_domain as well.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2781b40..5d3e69f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2151,6 +2151,30 @@  i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
 	return 0;
 }
 
+static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
+{
+	u32 old_write_domain, old_read_domains;
+
+	if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
+		return;
+
+	/* Act a barrier for all accesses through the GTT */
+	mb();
+
+	/* Force a pagefault for domain tracking on next user access */
+	i915_gem_release_mmap(obj);
+
+	old_read_domains = obj->base.read_domains;
+	old_write_domain = obj->base.write_domain;
+
+	obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
+	obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
+
+	trace_i915_gem_object_change_domain(obj,
+					    old_read_domains,
+					    old_write_domain);
+}
+
 /**
  * Unbinds an object from the GTT aperture.
  */
@@ -2175,8 +2199,7 @@  i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 	 * cause memory corruption through use-after-free.
 	 */
 
-	/* blow away mappings if mapped through GTT */
-	i915_gem_release_mmap(obj);
+	i915_gem_object_finish_gtt(obj);
 
 	/* Move the object to the CPU domain to ensure that
 	 * any possible CPU writes while it's not in the GTT