diff mbox

[v2] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC

Message ID 1439311630-20029-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Aug. 11, 2015, 4:47 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we don't clflush on pin_to_display if the bo is already
UC/WT and is not in the CPU write domain. This causes problems with
pwrite since pwrite doesn't change the write domain, and it avoids
clflushing on UC/WT buffers on LLC platforms unless the buffer is
currently being scanned out.

Fix the problem by marking the cache dirty and adjusting
i915_gem_object_set_cache_level() to clflush when the cache is dirty
even if the cache_level doesn't change.

My last attempt [1] at fixing this via write domain frobbing was shot
down, but now with the cache_dirty flag we can do things in a nicer way.

[1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html

v2: Drop the I915_CACHE_NONE/WT checks from pwrite

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
Testcase: igt/kms_pwrite_crc
Testcase: igt/gem_pwrite_snooped
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Chris Wilson Aug. 11, 2015, 5:12 p.m. UTC | #1 
On Tue, Aug 11, 2015 at 07:47:10PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we don't clflush on pin_to_display if the bo is already
> UC/WT and is not in the CPU write domain. This causes problems with
> pwrite since pwrite doesn't change the write domain, and it avoids
> clflushing on UC/WT buffers on LLC platforms unless the buffer is
> currently being scanned out.
> 
> Fix the problem by marking the cache dirty and adjusting
> i915_gem_object_set_cache_level() to clflush when the cache is dirty
> even if the cache_level doesn't change.
> 
> My last attempt [1] at fixing this via write domain frobbing was shot
> down, but now with the cache_dirty flag we can do things in a nicer way.
> 
> [1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
> 
> v2: Drop the I915_CACHE_NONE/WT checks from pwrite
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
> Testcase: igt/kms_pwrite_crc
> Testcase: igt/gem_pwrite_snooped
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Daniel Vetter Aug. 12, 2015, 1:20 p.m. UTC | #2 
On Tue, Aug 11, 2015 at 06:12:04PM +0100, Chris Wilson wrote:
> On Tue, Aug 11, 2015 at 07:47:10PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we don't clflush on pin_to_display if the bo is already
> > UC/WT and is not in the CPU write domain. This causes problems with
> > pwrite since pwrite doesn't change the write domain, and it avoids
> > clflushing on UC/WT buffers on LLC platforms unless the buffer is
> > currently being scanned out.
> > 
> > Fix the problem by marking the cache dirty and adjusting
> > i915_gem_object_set_cache_level() to clflush when the cache is dirty
> > even if the cache_level doesn't change.
> > 
> > My last attempt [1] at fixing this via write domain frobbing was shot
> > down, but now with the cache_dirty flag we can do things in a nicer way.
> > 
> > [1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
> > 
> > v2: Drop the I915_CACHE_NONE/WT checks from pwrite
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
> > Testcase: igt/kms_pwrite_crc
> > Testcase: igt/gem_pwrite_snooped
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next, thanks for the patch.
-Daniel
Shuang He Aug. 15, 2015, 12:48 a.m. UTC | #3 
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7152
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                                  283/283              283/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..f828dc7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1005,12 +1005,14 @@  out:
 		if (!needs_clflush_after &&
 		    obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
 			if (i915_gem_clflush_object(obj, obj->pin_display))
-				i915_gem_chipset_flush(dev);
+				needs_clflush_after = true;
 		}
 	}
 
 	if (needs_clflush_after)
 		i915_gem_chipset_flush(dev);
+	else
+		obj->cache_dirty = true;
 
 	intel_fb_obj_flush(obj, false, ORIGIN_CPU);
 	return ret;
@@ -3639,10 +3641,10 @@  int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 {
 	struct drm_device *dev = obj->base.dev;
 	struct i915_vma *vma, *next;
-	int ret;
+	int ret = 0;
 
 	if (obj->cache_level == cache_level)
-		return 0;
+		goto out;
 
 	if (i915_gem_obj_is_pinned(obj)) {
 		DRM_DEBUG("can not change the cache level of pinned objects\n");
@@ -3687,6 +3689,7 @@  int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 		vma->node.color = cache_level;
 	obj->cache_level = cache_level;
 
+out:
 	if (obj->cache_dirty &&
 	    obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
 	    cpu_write_needs_clflush(obj)) {