[6/7] drm/i915: Updated request structure tracing

Commit Message

John Harrison Jan. 8, 2016, 6:47 p.m. UTC

From: John Harrison <[e-mail]>

Added the '_complete' trace event which occurs when a fence/request is
signaled as complete. Also moved the notify event from the IRQ handler
code to inside the notify function itself.

v3: Added the current ring seqno to the notify trace point.

v5: Line wrapping to keep the style checker happy.

For: VIZ-5190
Signed-off-by: John Harrison <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem.c   |  9 +++++++--
 drivers/gpu/drm/i915/i915_irq.c   |  2 --
 drivers/gpu/drm/i915/i915_trace.h | 14 +++++++++-----
 3 files changed, 16 insertions(+), 9 deletions(-)

Comments

Chris Wilson Jan. 8, 2016, 10:16 p.m. UTC | #1

On Fri, Jan 08, 2016 at 06:47:27PM +0000, [e-mail] wrote:
> From: John Harrison <[e-mail]>
> 
> Added the '_complete' trace event which occurs when a fence/request is
> signaled as complete. Also moved the notify event from the IRQ handler
> code to inside the notify function itself.

No. It was actually from the previous patch, but we do not query the current
sequence invoking a forcewake dance from inside the interrupt handler.
We did that years ago and realised our mistake very quickly.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 96cafab..ef03e4e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2816,13 +2816,16 @@  void i915_gem_request_notify(struct intel_engine_cs *ring, bool fence_locked)
 	unsigned long flags;
 	u32 seqno;
 
-	if (list_empty(&ring->fence_signal_list))
+	if (list_empty(&ring->fence_signal_list)) {
+		trace_i915_gem_request_notify(ring, 0);
 		return;
+	}
 
 	if (!fence_locked)
 		spin_lock_irqsave(&ring->fence_lock, flags);
 
 	seqno = ring->get_seqno(ring, false);
+	trace_i915_gem_request_notify(ring, seqno);
 
 	list_for_each_entry_safe(req, req_next, &ring->fence_signal_list, signal_link) {
 		if (!req->cancelled) {
@@ -2836,8 +2839,10 @@  void i915_gem_request_notify(struct intel_engine_cs *ring, bool fence_locked)
 		 */
 		list_del_init(&req->signal_link);
 
-		if (!req->cancelled)
+		if (!req->cancelled) {
 			fence_signal_locked(&req->fence);
+			trace_i915_gem_request_complete(req);
+		}
 
 		if (req->irq_enabled) {
 			req->ring->irq_put(req->ring);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 74f8552..d280e05 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -979,8 +979,6 @@  static void notify_ring(struct intel_engine_cs *ring)
 	if (!intel_ring_initialized(ring))
 		return;
 
-	trace_i915_gem_request_notify(ring);
-
 	i915_gem_request_notify(ring, false);
 
 	wake_up_all(&ring->irq_queue);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 04fe849..b3ae894 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -561,23 +561,27 @@  DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
 );
 
 TRACE_EVENT(i915_gem_request_notify,
-	    TP_PROTO(struct intel_engine_cs *ring),
-	    TP_ARGS(ring),
+	    TP_PROTO(struct intel_engine_cs *ring, uint32_t seqno),
+	    TP_ARGS(ring, seqno),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
 			     __field(u32, ring)
 			     __field(u32, seqno)
+			     __field(bool, is_empty)
 			     ),
 
 	    TP_fast_assign(
 			   __entry->dev = ring->dev->primary->index;
 			   __entry->ring = ring->id;
-			   __entry->seqno = ring->get_seqno(ring, false);
+			   __entry->seqno = seqno;
+			   __entry->is_empty =
+					list_empty(&ring->fence_signal_list);
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u",
-		      __entry->dev, __entry->ring, __entry->seqno)
+	    TP_printk("dev=%u, ring=%u, seqno=%u, empty=%d",
+		      __entry->dev, __entry->ring, __entry->seqno,
+		      __entry->is_empty)
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,