All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/6] drm/i915: Remove change_domain tracepoint
@ 2017-02-22  9:41 Chris Wilson
  2017-02-22  9:41 ` [PATCH v3 2/6] drm/i915: Move cpu_cache_is_coherent() to header Chris Wilson
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Chris Wilson @ 2017-02-22  9:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: matthew.auld

The change_domain tracepoint has been inaccurate for a few years - it
doesn't fully capture the domains, especially with userspace bypassing
them. It is defunct, misleading and time to be removed.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c            | 30 ------------------------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 ---
 drivers/gpu/drm/i915/i915_trace.h          | 24 ------------------------
 3 files changed, 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d93032875f28..9c5f091c771f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3214,9 +3214,6 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)
 	intel_fb_obj_flush(obj, false, write_origin(obj, I915_GEM_DOMAIN_GTT));
 
 	obj->base.write_domain = 0;
-	trace_i915_gem_object_change_domain(obj,
-					    obj->base.read_domains,
-					    I915_GEM_DOMAIN_GTT);
 }
 
 /** Flushes the CPU write domain for the object if it's dirty. */
@@ -3230,9 +3227,6 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj)
 	intel_fb_obj_flush(obj, false, ORIGIN_CPU);
 
 	obj->base.write_domain = 0;
-	trace_i915_gem_object_change_domain(obj,
-					    obj->base.read_domains,
-					    I915_GEM_DOMAIN_CPU);
 }
 
 /**
@@ -3246,7 +3240,6 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj)
 int
 i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 {
-	uint32_t old_write_domain, old_read_domains;
 	int ret;
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
@@ -3284,9 +3277,6 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 	if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
 		mb();
 
-	old_write_domain = obj->base.write_domain;
-	old_read_domains = obj->base.read_domains;
-
 	/* It should now be out of any other write domains, and we can update
 	 * the domain values for our changes.
 	 */
@@ -3298,10 +3288,6 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 		obj->mm.dirty = true;
 	}
 
-	trace_i915_gem_object_change_domain(obj,
-					    old_read_domains,
-					    old_write_domain);
-
 	i915_gem_object_unpin_pages(obj);
 	return 0;
 }
@@ -3538,7 +3524,6 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 				     const struct i915_ggtt_view *view)
 {
 	struct i915_vma *vma;
-	u32 old_read_domains, old_write_domain;
 	int ret;
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
@@ -3603,19 +3588,12 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 		intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
 	}
 
-	old_write_domain = obj->base.write_domain;
-	old_read_domains = obj->base.read_domains;
-
 	/* It should now be out of any other write domains, and we can update
 	 * the domain values for our changes.
 	 */
 	obj->base.write_domain = 0;
 	obj->base.read_domains |= I915_GEM_DOMAIN_GTT;
 
-	trace_i915_gem_object_change_domain(obj,
-					    old_read_domains,
-					    old_write_domain);
-
 	return vma;
 
 err_unpin_display:
@@ -3651,7 +3629,6 @@ i915_gem_object_unpin_from_display_plane(struct i915_vma *vma)
 int
 i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
 {
-	uint32_t old_write_domain, old_read_domains;
 	int ret;
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
@@ -3670,9 +3647,6 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
 
 	i915_gem_object_flush_gtt_write_domain(obj);
 
-	old_write_domain = obj->base.write_domain;
-	old_read_domains = obj->base.read_domains;
-
 	/* Flush the CPU cache if it's still invalid. */
 	if ((obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0) {
 		i915_gem_clflush_object(obj, false);
@@ -3693,10 +3667,6 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
 		obj->base.write_domain = I915_GEM_DOMAIN_CPU;
 	}
 
-	trace_i915_gem_object_change_domain(obj,
-					    old_read_domains,
-					    old_write_domain);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index dfed503301a6..6fb29832bc63 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1317,8 +1317,6 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 
 	list_for_each_entry(vma, vmas, exec_list) {
 		struct drm_i915_gem_object *obj = vma->obj;
-		u32 old_read = obj->base.read_domains;
-		u32 old_write = obj->base.write_domain;
 
 		obj->base.write_domain = obj->base.pending_write_domain;
 		if (obj->base.write_domain)
@@ -1329,7 +1327,6 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 
 		i915_vma_move_to_active(vma, req, vma->exec_entry->flags);
 		eb_export_fence(obj, req, vma->exec_entry->flags);
-		trace_i915_gem_object_change_domain(obj, old_read, old_write);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 0c2ed1289bfc..2dbef3147a60 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -175,30 +175,6 @@ TRACE_EVENT(i915_vma_unbind,
 		      __entry->obj, __entry->offset, __entry->size, __entry->vm)
 );
 
-TRACE_EVENT(i915_gem_object_change_domain,
-	    TP_PROTO(struct drm_i915_gem_object *obj, u32 old_read, u32 old_write),
-	    TP_ARGS(obj, old_read, old_write),
-
-	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
-			     __field(u32, read_domains)
-			     __field(u32, write_domain)
-			     ),
-
-	    TP_fast_assign(
-			   __entry->obj = obj;
-			   __entry->read_domains = obj->base.read_domains | (old_read << 16);
-			   __entry->write_domain = obj->base.write_domain | (old_write << 16);
-			   ),
-
-	    TP_printk("obj=%p, read=%02x=>%02x, write=%02x=>%02x",
-		      __entry->obj,
-		      __entry->read_domains >> 16,
-		      __entry->read_domains & 0xffff,
-		      __entry->write_domain >> 16,
-		      __entry->write_domain & 0xffff)
-);
-
 TRACE_EVENT(i915_gem_object_pwrite,
 	    TP_PROTO(struct drm_i915_gem_object *obj, u32 offset, u32 len),
 	    TP_ARGS(obj, offset, len),
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 3/6] drm/i915: Amalgamate flushing of display objects
@ 2017-09-12 22:11 Yaroslav Shabalin
  2017-09-14 10:48 ` Yaroslav Shabalin
  2017-09-14 10:57 ` Chris Wilson
  0 siblings, 2 replies; 17+ messages in thread
From: Yaroslav Shabalin @ 2017-09-12 22:11 UTC (permalink / raw)
  To: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2426 bytes --]

Hi!

I would like to report a problem which I believe is connected to changes
introduced in this patch (
https://lists.freedesktop.org/archives/intel-gfx/2017-February/120777.html).
Specifically the problem is: screen freezes for 0.5 - 5 seconds (mostly for
2-3 seconds) when PSR is enabled (i915.enable_psr=1 kernel boot option).
Freezes are more frequent with rich, dynamic screen content. The most
reliable way to reproduce it, I discovered so far, is to start Gnome "Disk
Usage Analyzer", analyze some catalog and move mouse pointer over color
rectangles for a while. Usually that way I get 3-5 freezes in a minute.
During normal PC usage freezes are happening less frequently, but still
annoyingly. For the first time this problem appeared after update to kernel
4.12 and still there in recent 4.13 too.
I've looked into it a little and seems that mentioned patch is the one that
brought it in. Git bisect indicates it as the first bad commit. After
realizing that I tried to partly revert changes in the patch and found that
following change:

 	/* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
-	if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) {
-		i915_gem_clflush_object(obj, true);
-		intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
-	}
+	__i915_gem_object_flush_to_display(obj);
+	intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);

when reverted solves the problem completely. One of the main
difference is that in old version intel_fb_obj_flush was called
conditionally and now it is called every time. So I surrounded the
call with "if" just to check and it worked:

/* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
-	if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) {
-		i915_gem_clflush_object(obj, true);
-		intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
-	}
+	__i915_gem_object_flush_to_display(obj);
+	if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) {
+	  intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
+ }

 The above code was just a guess based on some logical insight.
Although it eliminates the freezes, I don't really know if the change
in calling intel_fb_obj_flush wasn't made intentionally. So could you
please look at that case?

If you need any help in reproducing problem conditions or further
diagnostic information, please let me know.


Thank you in advance,

Yaroslav Shabalin

[-- Attachment #1.2: Type: text/html, Size: 2904 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-09-14 12:27 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-22  9:41 [PATCH v3 1/6] drm/i915: Remove change_domain tracepoint Chris Wilson
2017-02-22  9:41 ` [PATCH v3 2/6] drm/i915: Move cpu_cache_is_coherent() to header Chris Wilson
2017-02-22 11:02   ` Joonas Lahtinen
2017-02-22  9:41 ` [PATCH v3 3/6] drm/i915: Amalgamate flushing of display objects Chris Wilson
2017-02-22  9:54   ` Chris Wilson
2017-02-22 10:57   ` Joonas Lahtinen
2017-02-22  9:41 ` [PATCH v3 4/6] drm/i915: Skip clflushes for all non-page backed objects Chris Wilson
2017-02-22  9:41 ` [PATCH v3 5/6] drm/i915: Perform object clflushing asynchronously Chris Wilson
2017-02-22 11:01   ` Joonas Lahtinen
2017-02-22  9:41 ` [PATCH v3 6/6] drm/i915: Remove 'retire' parameter from intel_fb_obj_flush Chris Wilson
2017-02-22 11:04   ` Joonas Lahtinen
2017-02-22 10:22 ` ✓ Fi.CI.BAT: success for series starting with [v3,1/6] drm/i915: Remove change_domain tracepoint Patchwork
2017-02-22 10:30 ` [PATCH v3 1/6] " Joonas Lahtinen
2017-09-12 22:11 [PATCH v3 3/6] drm/i915: Amalgamate flushing of display objects Yaroslav Shabalin
2017-09-14 10:48 ` Yaroslav Shabalin
2017-09-14 10:57 ` Chris Wilson
2017-09-14 12:27   ` Yaroslav Shabalin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.