diff mbox

[17/30] drm/i915: Do not clflush snooped objects

Message ID 1302640318-23165-18-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson April 12, 2011, 8:31 p.m. UTC 
Rely on the GPU snooping into the CPU cache for appropriately bound
objects on MI_FLUSH. Or perhaps one day we will have a cache-coherent
CPU/GPU package...

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/i915_gem.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Daniel Vetter April 13, 2011, 4:04 p.m. UTC | #1 
On Tue, Apr 12, 2011 at 09:31:45PM +0100, Chris Wilson wrote:
> +	/* If the GPU is snooping the contents of the CPU cache,
> +	 * we do not need to clear the CPU cache lines. Instead we need
> +	 * to be sure to flush/invalidate the RENDER cache when the contents
> +	 * must be refreshed.
> +	 */

The "Instead we need to be sure to flush the RENDER cache ..." is a bit
confusing to me: There's no instead, we have to do this always when
writing with the cpu. Besides this minor comment-nitpick it makes sense.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Chris Wilson April 13, 2011, 5:34 p.m. UTC | #2 
On Wed, 13 Apr 2011 18:04:38 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Apr 12, 2011 at 09:31:45PM +0100, Chris Wilson wrote:
> > +	/* If the GPU is snooping the contents of the CPU cache,
> > +	 * we do not need to clear the CPU cache lines. Instead we need
> > +	 * to be sure to flush/invalidate the RENDER cache when the contents
> > +	 * must be refreshed.
> > +	 */
> 
> The "Instead we need to be sure to flush the RENDER cache ..." is a bit
> confusing to me: There's no instead, we have to do this always when
> writing with the cpu. Besides this minor comment-nitpick it makes sense.

I changed the comment to:

/* If the GPU is snooping the contents of the CPU cache,
 * we do not need to manually clear the CPU cache lines.  Instead,
 * the caches are only snooped when the render cache is
 * flushed/invalidated.  As we always have to emit invalidations
 * and flushes when moving into and out of the RENDER domain, correct
 * snooping behaviour occurs naturally as the result of our domain
 * tracking.
 */
-Chris
Daniel Vetter April 13, 2011, 8:47 p.m. UTC | #3 
On Wed, Apr 13, 2011 at 06:34:57PM +0100, Chris Wilson wrote:
> I changed the comment to:
> 
> /* If the GPU is snooping the contents of the CPU cache,
>  * we do not need to manually clear the CPU cache lines.  Instead,
>  * the caches are only snooped when the render cache is
>  * flushed/invalidated.  As we always have to emit invalidations
>  * and flushes when moving into and out of the RENDER domain, correct
>  * snooping behaviour occurs naturally as the result of our domain
>  * tracking.
>  */
Perfect!
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 264bec8..fa483d8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2878,6 +2878,14 @@  i915_gem_clflush_object(struct drm_i915_gem_object *obj)
 	if (obj->pages == NULL)
 		return;
 
+	/* If the GPU is snooping the contents of the CPU cache,
+	 * we do not need to clear the CPU cache lines. Instead we need
+	 * to be sure to flush/invalidate the RENDER cache when the contents
+	 * must be refreshed.
+	 */
+	if (obj->cache_level != I915_CACHE_NONE)
+		return;
+
 	trace_i915_gem_object_clflush(obj);
 
 	drm_clflush_pages(obj->pages, obj->base.size / PAGE_SIZE);