[RFC 0/8] Tracepoints cleanup and improvements for requests

[RFC 0/8] Tracepoints cleanup and improvements for requests

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

During some performance analysis I have noticed that currently our trace points
are in a bit of a mess, especially the ones relating to requests.

This series attempts to tidy some of that and also adds some new trace points.

These new trace points are usable to analyze the engine utilization and also
look at how the scheduler resolves dependencies and fences, how much time
requests spend being blocked, and so on.

With some post-processing the new trace points can be used to generate request
and engine timeline graphs like for example the on on this URL:

https://people.freedesktop.org/~tursulin/trace-demo.html

If people find this interesting we could also merge the post-processing tool
to IGT. Although ideally, someone more versed in GUI development could pick up
on the general idea an implement something better than the not very scalable
or precise JS library I have used to prototype this.

Current way of getting to the graphs is essentially:

perf record -e <list-of-events> -a -c 1 binary-to-be-traced
perf script | trace.pl >output.html

Tvrtko

Tvrtko Ursulin (8):
  drm/i915/tracepoints: Tidy request event class
  drm/i915/tracepoints: Adjust i915_gem_ring_dispatch
  drm/i915/tracepoints: Tidy i915_gem_request_wait_begin
  drm/i915/tracepoints: Remove unused i915_gem_request_complete
  drm/i915/tracepoints: Add request submit and execute tracepoints
  drm/i915/tracepoints: Rename i915_gem_request_notify
  drm/i915/tracepoints: Add backend level request in and out tracepoints
  drm/i915/tracepoints: Add hw_id to context tracepoints

 drivers/gpu/drm/i915/Kconfig.debug         |  11 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   4 +-
 drivers/gpu/drm/i915/i915_gem_request.c    |   2 +
 drivers/gpu/drm/i915/i915_guc_submission.c |   2 +
 drivers/gpu/drm/i915/i915_irq.c            |   2 +-
 drivers/gpu/drm/i915/i915_trace.h          | 122 ++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_lrc.c           |   4 +
 7 files changed, 124 insertions(+), 23 deletions(-)

-- 
2.7.4

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

[RFC 1/8] drm/i915/tracepoints: Tidy request event class

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

At the moment only the global seqno is logged which is not set
until the request is ready for submission.

Add the per-contex seqno and the context hardware id which are
both interesting data points.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 4461df5a94fe..d890e4b03902 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -555,18 +555,23 @@ DECLARE_EVENT_CLASS(i915_gem_request,
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
+			     __field(u32, ctx)
 			     __field(u32, ring)
 			     __field(u32, seqno)
+			     __field(u32, global)
 			     ),
 
 	    TP_fast_assign(
 			   __entry->dev = req->i915->drm.primary->index;
+			   __entry->ctx = req->ctx->hw_id;
 			   __entry->ring = req->engine->id;
-			   __entry->seqno = req->global_seqno;
+			   __entry->seqno = req->fence.seqno;
+			   __entry->global = req->global_seqno;
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u",
-		      __entry->dev, __entry->ring, __entry->seqno)
+	    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, global=%u",
+		      __entry->dev, __entry->ring, __entry->ctx, __entry->seqno,
+		      __entry->global)
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
-- 
2.7.4

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

[RFC 2/8] drm/i915/tracepoints: Adjust i915_gem_ring_dispatch

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Rename it to i915_gem_request_queue and fix the logged info
equivalent to the i915_gem_request even class. Also moved it
a bit further apart from the i915_gem_request_add tracepoint
since they otherwise provide similar information too close in
time.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
 drivers/gpu/drm/i915/i915_trace.h          | 11 +++++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c66e90571031..3cb7b0a3dd38 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1492,8 +1492,6 @@ execbuf_submit(struct i915_execbuffer_params *params,
 	if (ret)
 		return ret;
 
-	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
-
 	i915_gem_execbuffer_move_to_active(vmas, params->request);
 
 	return 0;
@@ -1803,6 +1801,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	params->dispatch_flags          = dispatch_flags;
 	params->ctx                     = ctx;
 
+	trace_i915_gem_request_queue(params->request, dispatch_flags);
+
 	ret = execbuf_submit(params, args, &eb->vmas);
 err_request:
 	__i915_add_request(params->request, ret == 0);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index d890e4b03902..79162c369812 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -503,13 +503,14 @@ TRACE_EVENT(i915_gem_ring_sync_to,
 		      __entry->seqno)
 );
 
-TRACE_EVENT(i915_gem_ring_dispatch,
+TRACE_EVENT(i915_gem_request_queue,
 	    TP_PROTO(struct drm_i915_gem_request *req, u32 flags),
 	    TP_ARGS(req, flags),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
 			     __field(u32, ring)
+			     __field(u32, ctx)
 			     __field(u32, seqno)
 			     __field(u32, flags)
 			     ),
@@ -517,13 +518,15 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 	    TP_fast_assign(
 			   __entry->dev = req->i915->drm.primary->index;
 			   __entry->ring = req->engine->id;
-			   __entry->seqno = req->global_seqno;
+			   __entry->ctx = req->ctx->hw_id;
+			   __entry->seqno = req->fence.seqno;
 			   __entry->flags = flags;
 			   dma_fence_enable_sw_signaling(&req->fence);
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
-		      __entry->dev, __entry->ring, __entry->seqno, __entry->flags)
+	    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, flags=%x",
+		      __entry->dev, __entry->ring, __entry->ctx, __entry->seqno,
+		      __entry->flags)
 );
 
 TRACE_EVENT(i915_gem_ring_flush,
-- 
2.7.4

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

[RFC 3/8] drm/i915/tracepoints: Tidy i915_gem_request_wait_begin

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Provide the same information as the other request even classes.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 79162c369812..cbdbe7d6f5ef 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -619,7 +619,9 @@ TRACE_EVENT(i915_gem_request_wait_begin,
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
 			     __field(u32, ring)
+			     __field(u32, ctx)
 			     __field(u32, seqno)
+			     __field(u32, global)
 			     __field(bool, blocking)
 			     ),
 
@@ -632,14 +634,16 @@ TRACE_EVENT(i915_gem_request_wait_begin,
 	    TP_fast_assign(
 			   __entry->dev = req->i915->drm.primary->index;
 			   __entry->ring = req->engine->id;
-			   __entry->seqno = req->global_seqno;
+			   __entry->ctx = req->ctx->hw_id;
+			   __entry->seqno = req->fence.seqno;
+			   __entry->global = req->global_seqno;
 			   __entry->blocking =
 				     mutex_is_locked(&req->i915->drm.struct_mutex);
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u, blocking=%s",
-		      __entry->dev, __entry->ring,
-		      __entry->seqno, __entry->blocking ?  "yes (NB)" : "no")
+	    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, global=%u, blocking=%s",
+		      __entry->dev, __entry->ring, __entry->ctx, __entry->seqno,
+		      __entry->global, __entry->blocking ?  "yes (NB)" : "no")
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_wait_end,
-- 
2.7.4

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

[RFC 4/8] drm/i915/tracepoints: Remove unused i915_gem_request_complete

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Tracepoint is not used and won't be suitable for its replacement.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index cbdbe7d6f5ef..f08ccac0d959 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -607,11 +607,6 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,
 	    TP_ARGS(req)
 );
 
-DEFINE_EVENT(i915_gem_request, i915_gem_request_complete,
-	    TP_PROTO(struct drm_i915_gem_request *req),
-	    TP_ARGS(req)
-);
-
 TRACE_EVENT(i915_gem_request_wait_begin,
 	    TP_PROTO(struct drm_i915_gem_request *req),
 	    TP_ARGS(req),
-- 
2.7.4

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

[RFC 5/8] drm/i915/tracepoints: Add request submit and execute tracepoints

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

These new tracepoints are emitted once the request is ready to
be submitted to the GPU and once the request is about to
be submitted to the GPU, respectively.

Former condition triggers as soon as all the fences and
dependencies have been resolved, and the latter once the
backend is about to submit it to the GPU.

New tracepoint are enabled via the new
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option which is disabled
by default to alleviate the performance impact concerns.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/Kconfig.debug      | 11 +++++++++++
 drivers/gpu/drm/i915/i915_gem_request.c |  2 ++
 drivers/gpu/drm/i915/i915_trace.h       | 24 ++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 597648c7a645..4fba5e1356a0 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -58,3 +58,14 @@ config DRM_I915_SW_FENCE_DEBUG_OBJECTS
           Recommended for driver developers only.
 
           If in doubt, say "N".
+
+config DRM_I915_LOW_LEVEL_TRACEPOINTS
+        bool "Enable low level request tracing events"
+        depends on DRM_I915
+        default n
+        help
+          Choose this option to turn on low level request tracing events.
+          This provides the ability to precisely monitor engine utilisation
+          and also analyze the request dependency resolving timeline.
+
+          If in doubt, say "N".
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..4a88b8ea01db 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -449,6 +449,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 
 	switch (state) {
 	case FENCE_COMPLETE:
+		trace_i915_gem_request_submit(request);
 		request->engine->submit_request(request);
 		break;
 
@@ -468,6 +469,7 @@ execute_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 
 	switch (state) {
 	case FENCE_COMPLETE:
+		trace_i915_gem_request_execute(request);
 		break;
 
 	case FENCE_FREE:
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f08ccac0d959..1fe1417f8c4d 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -582,6 +582,30 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
 	    TP_ARGS(req)
 );
 
+#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS)
+DEFINE_EVENT(i915_gem_request, i915_gem_request_submit,
+	     TP_PROTO(struct drm_i915_gem_request *req),
+	     TP_ARGS(req)
+);
+
+DEFINE_EVENT(i915_gem_request, i915_gem_request_execute,
+	     TP_PROTO(struct drm_i915_gem_request *req),
+	     TP_ARGS(req)
+);
+#else
+#if !defined(TRACE_HEADER_MULTI_READ)
+static inline void
+trace_i915_gem_request_submit(struct drm_i915_gem_request *req)
+{
+}
+
+static inline void
+trace_i915_gem_request_execute(struct drm_i915_gem_request *req)
+{
+}
+#endif
+#endif
+
 TRACE_EVENT(i915_gem_request_notify,
 	    TP_PROTO(struct intel_engine_cs *engine),
 	    TP_ARGS(engine),
-- 
2.7.4

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

[RFC 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

i915_gem_ring_notify is more appropriate since we do not have
the request information at this point, but it is simply a
signal from the engine that some request has been completed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c   | 2 +-
 drivers/gpu/drm/i915/i915_trace.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0ff75f2282fa..e16da1873be2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1035,7 +1035,7 @@ static void notify_ring(struct intel_engine_cs *engine)
 {
 	set_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted);
 	if (intel_engine_wakeup(engine))
-		trace_i915_gem_request_notify(engine);
+		trace_i915_gem_ring_notify(engine);
 }
 
 static void vlv_c0_read(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 1fe1417f8c4d..d24b89d0e3ab 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -606,7 +606,7 @@ trace_i915_gem_request_execute(struct drm_i915_gem_request *req)
 #endif
 #endif
 
-TRACE_EVENT(i915_gem_request_notify,
+TRACE_EVENT(i915_gem_ring_notify,
 	    TP_PROTO(struct intel_engine_cs *engine),
 	    TP_ARGS(engine),
 
-- 
2.7.4

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

[RFC 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Two new tracepoints placed at the call sites where requests are
actually passed to the GPU enable userspace to track engine
utilisation.

These tracepoints are only enabled when the
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option is enabled.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c |  2 ++
 drivers/gpu/drm/i915/i915_trace.h          | 49 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           |  4 +++
 3 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 8ced9e26f075..beec88a30347 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -518,6 +518,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
 		POSTING_READ_FW(GUC_STATUS);
 
+	trace_i915_gem_request_in(rq, 0);
+
 	b_ret = guc_ring_doorbell(client);
 
 	client->submissions[engine_id] += 1;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index d24b89d0e3ab..18dd21653a80 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -582,6 +582,35 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
 	    TP_ARGS(req)
 );
 
+DECLARE_EVENT_CLASS(i915_gem_request_hw,
+		    TP_PROTO(struct drm_i915_gem_request *req,
+			     unsigned int port),
+		    TP_ARGS(req, port),
+
+		    TP_STRUCT__entry(
+				     __field(u32, dev)
+				     __field(u32, ring)
+				     __field(u32, seqno)
+				     __field(u32, global_seqno)
+				     __field(u32, ctx)
+				     __field(u32, port)
+				    ),
+
+		    TP_fast_assign(
+			           __entry->dev = req->i915->drm.primary->index;
+			           __entry->ring = req->engine->id;
+			           __entry->ctx = req->ctx->hw_id;
+			           __entry->seqno = req->fence.seqno;
+			           __entry->global_seqno = req->global_seqno;
+			           __entry->port = port;
+			          ),
+
+		    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, global_seqno=%u, port=%u",
+			      __entry->dev, __entry->ring, __entry->ctx,
+			      __entry->seqno, __entry->global_seqno,
+			      __entry->port)
+);
+
 #if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS)
 DEFINE_EVENT(i915_gem_request, i915_gem_request_submit,
 	     TP_PROTO(struct drm_i915_gem_request *req),
@@ -592,6 +621,16 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_execute,
 	     TP_PROTO(struct drm_i915_gem_request *req),
 	     TP_ARGS(req)
 );
+
+DEFINE_EVENT(i915_gem_request_hw, i915_gem_request_in,
+	     TP_PROTO(struct drm_i915_gem_request *req, unsigned int port),
+	     TP_ARGS(req, port)
+);
+
+DEFINE_EVENT(i915_gem_request_hw, i915_gem_request_out,
+	     TP_PROTO(struct drm_i915_gem_request *req, unsigned int port),
+	     TP_ARGS(req, port)
+);
 #else
 #if !defined(TRACE_HEADER_MULTI_READ)
 static inline void
@@ -603,6 +642,16 @@ static inline void
 trace_i915_gem_request_execute(struct drm_i915_gem_request *req)
 {
 }
+
+static inline void
+trace_i915_gem_request_in(struct drm_i915_gem_request *req, unsigned int port)
+{
+}
+
+static inline void
+trace_i915_gem_request_out(struct drm_i915_gem_request *req, unsigned int port)
+{
+}
 #endif
 #endif
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index bee9d565b8f3..85581a05657b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -505,6 +505,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		cursor->priotree.priority = INT_MAX;
 
 		__i915_gem_request_submit(cursor);
+		trace_i915_gem_request_in(cursor, port - engine->execlist_port);
 		last = cursor;
 		submit = true;
 	}
@@ -561,6 +562,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_private *dev_priv = engine->i915;
+	unsigned int portidx = 0;
 
 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
 
@@ -597,6 +599,8 @@ static void intel_lrc_irq_handler(unsigned long data)
 				execlists_context_status_change(port[0].request,
 								INTEL_CONTEXT_SCHEDULE_OUT);
 
+				trace_i915_gem_request_out(port[0].request,
+							   portidx++);
 				i915_gem_request_put(port[0].request);
 				port[0] = port[1];
 				memset(&port[1], 0, sizeof(port[1]));
-- 
2.7.4

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

[RFC 8/8] drm/i915/tracepoints: Add hw_id to context tracepoints

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It is useful to provide this info to match the one provided
in the request tracepoints.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 18dd21653a80..e4b702063aa6 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -849,17 +849,19 @@ DECLARE_EVENT_CLASS(i915_context,
 	TP_STRUCT__entry(
 			__field(u32, dev)
 			__field(struct i915_gem_context *, ctx)
+			__field(u32, hw_id)
 			__field(struct i915_address_space *, vm)
 	),
 
 	TP_fast_assign(
+			__entry->dev = ctx->i915->drm.primary->index;
 			__entry->ctx = ctx;
+			__entry->hw_id = ctx->hw_id;
 			__entry->vm = ctx->ppgtt ? &ctx->ppgtt->base : NULL;
-			__entry->dev = ctx->i915->drm.primary->index;
 	),
 
-	TP_printk("dev=%u, ctx=%p, ctx_vm=%p",
-		  __entry->dev, __entry->ctx, __entry->vm)
+	TP_printk("dev=%u, ctx=%p, ctx_vm=%p, hw_id=%u",
+		  __entry->dev, __entry->ctx, __entry->vm, __entry->hw_id)
 )
 
 DEFINE_EVENT(i915_context, i915_context_create,
-- 
2.7.4

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

Re: [RFC 3/8] drm/i915/tracepoints: Tidy i915_gem_request_wait_begin

[Chris Wilson]

On Fri, Jan 27, 2017 at 12:01:22PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Provide the same information as the other request even classes.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_trace.h | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 79162c369812..cbdbe7d6f5ef 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -619,7 +619,9 @@ TRACE_EVENT(i915_gem_request_wait_begin,
>  	    TP_STRUCT__entry(
>  			     __field(u32, dev)
>  			     __field(u32, ring)
> +			     __field(u32, ctx)
>  			     __field(u32, seqno)
> +			     __field(u32, global)
>  			     __field(bool, blocking)
>  			     ),
>  
> @@ -632,14 +634,16 @@ TRACE_EVENT(i915_gem_request_wait_begin,
>  	    TP_fast_assign(
>  			   __entry->dev = req->i915->drm.primary->index;
>  			   __entry->ring = req->engine->id;
> -			   __entry->seqno = req->global_seqno;
> +			   __entry->ctx = req->ctx->hw_id;
> +			   __entry->seqno = req->fence.seqno;
> +			   __entry->global = req->global_seqno;
>  			   __entry->blocking =
>  				     mutex_is_locked(&req->i915->drm.struct_mutex);

We now have accurate information for this field from
(flags & I915_WAIT_LOCKED). Let's fix that up as well and drop the NB.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Chris Wilson]

On Fri, Jan 27, 2017 at 12:01:25PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> i915_gem_ring_notify is more appropriate since we do not have
> the request information at this point, but it is simply a
> signal from the engine that some request has been completed.

I was going to suggest intel_engine_notify.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Chris Wilson]

On Fri, Jan 27, 2017 at 12:01:26PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Two new tracepoints placed at the call sites where requests are
> actually passed to the GPU enable userspace to track engine
> utilisation.
> 
> These tracepoints are only enabled when the
> DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option is enabled.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c |  2 ++
>  drivers/gpu/drm/i915/i915_trace.h          | 49 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c           |  4 +++
>  3 files changed, 55 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 8ced9e26f075..beec88a30347 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -518,6 +518,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
>  	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
>  		POSTING_READ_FW(GUC_STATUS);
>  
> +	trace_i915_gem_request_in(rq, 0);

But how to get out for guc? We could do a similar in for ringbuffer.
The original way I used it was in == dispatch, out == ring-notify, which
is why trace_i915_gem_ring_dispatch enabled the signaling.

Hmm. Following on from that we can add the out tracepoint as a
fence-callback. For the moment, I'd drop guc/legacy
trace_i915_gem_request_in and we will try something more magical. Though
once we do enable the fake GuC scheduler we will have an appropriate
place to drop an trace_i915_gem_request_out. Just leaving ringbuffer as
the odd one out, but for who arguably the in/out logic is not as
important.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 2/8] drm/i915/tracepoints: Adjust i915_gem_ring_dispatch

[Chris Wilson]

On Fri, Jan 27, 2017 at 12:01:21PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Rename it to i915_gem_request_queue and fix the logged info
> equivalent to the i915_gem_request even class. Also moved it
> a bit further apart from the i915_gem_request_add tracepoint
> since they otherwise provide similar information too close in
> time.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
>  drivers/gpu/drm/i915/i915_trace.h          | 11 +++++++----
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index c66e90571031..3cb7b0a3dd38 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1492,8 +1492,6 @@ execbuf_submit(struct i915_execbuffer_params *params,
>  	if (ret)
>  		return ret;
>  
> -	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
> -
>  	i915_gem_execbuffer_move_to_active(vmas, params->request);
>  
>  	return 0;
> @@ -1803,6 +1801,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  	params->dispatch_flags          = dispatch_flags;
>  	params->ctx                     = ctx;
>  
> +	trace_i915_gem_request_queue(params->request, dispatch_flags);
> +
>  	ret = execbuf_submit(params, args, &eb->vmas);
>  err_request:
>  	__i915_add_request(params->request, ret == 0);
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index d890e4b03902..79162c369812 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -503,13 +503,14 @@ TRACE_EVENT(i915_gem_ring_sync_to,
>  		      __entry->seqno)
>  );
>  
> -TRACE_EVENT(i915_gem_ring_dispatch,
> +TRACE_EVENT(i915_gem_request_queue,
>  	    TP_PROTO(struct drm_i915_gem_request *req, u32 flags),
>  	    TP_ARGS(req, flags),
>  
>  	    TP_STRUCT__entry(
>  			     __field(u32, dev)
>  			     __field(u32, ring)
> +			     __field(u32, ctx)
>  			     __field(u32, seqno)
>  			     __field(u32, flags)
>  			     ),
> @@ -517,13 +518,15 @@ TRACE_EVENT(i915_gem_ring_dispatch,
>  	    TP_fast_assign(
>  			   __entry->dev = req->i915->drm.primary->index;
>  			   __entry->ring = req->engine->id;
> -			   __entry->seqno = req->global_seqno;
> +			   __entry->ctx = req->ctx->hw_id;
> +			   __entry->seqno = req->fence.seqno;
>  			   __entry->flags = flags;
>  			   dma_fence_enable_sw_signaling(&req->fence);

In keeping with the better tracking for in/out, this hack to enable the
notify irq here is no longer required.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/8] drm/i915/tracepoints: Remove unused i915_gem_request_complete

[Chris Wilson]

On Fri, Jan 27, 2017 at 12:01:23PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Tracepoint is not used and won't be suitable for its replacement.

Indeed, superseded by trace_dma_fence_signaled.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Tracepoints cleanup and improvements for requests

[Patchwork]

== Series Details ==

Series: Tracepoints cleanup and improvements for requests
URL   : https://patchwork.freedesktop.org/series/18676/
State : failure

== Summary ==

  LD      drivers/usb/storage/built-in.o
  CC [M]  drivers/gpu/drm/i915/gvt/render.o
  CC [M]  drivers/gpu/drm/i915/intel_lpe_audio.o
  LD      kernel/built-in.o
  LD      drivers/pci/pcie/aer/aerdriver.o
  LD      drivers/pci/pcie/aer/built-in.o
  LD      drivers/pci/pcie/built-in.o
  LD      net/packet/built-in.o
  LD      drivers/pci/built-in.o
  LD      drivers/scsi/scsi_mod.o
  LD      drivers/tty/serial/8250/8250.o
  LD      sound/built-in.o
  LD      drivers/gpu/drm/drm.o
  LD      drivers/iommu/built-in.o
  LD [M]  drivers/usb/serial/usbserial.o
  LD      drivers/acpi/acpica/acpi.o
  LD      drivers/thermal/thermal_sys.o
  LD      drivers/thermal/built-in.o
  LD      drivers/acpi/acpica/built-in.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
  LD      drivers/usb/gadget/libcomposite.o
  LD      drivers/acpi/built-in.o
  LD [M]  drivers/mmc/core/mmc_block.o
  LD      drivers/mmc/built-in.o
  LD      drivers/video/fbdev/core/fb.o
  LD      drivers/video/fbdev/core/built-in.o
  LD      drivers/spi/built-in.o
In file included from ./include/trace/define_trace.h:95:0,
                 from drivers/gpu/drm/i915/i915_trace.h:912,
                 from drivers/gpu/drm/i915/i915_trace_points.c:12:
./include/trace/trace_events.h:331:37: error: ‘trace_event_type_funcs_i915_gem_request_hw’ defined but not used [-Werror=unused-variable]
 static struct trace_event_functions trace_event_type_funcs_##call = { \
                                     ^
drivers/gpu/drm/i915/./i915_trace.h:585:1: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
 DECLARE_EVENT_CLASS(i915_gem_request_hw,
 ^
In file included from ./include/trace/define_trace.h:95:0,
                 from drivers/gpu/drm/i915/i915_trace.h:912,
                 from drivers/gpu/drm/i915/i915_trace_points.c:12:
./include/trace/trace_events.h:726:13: error: ‘print_fmt_i915_gem_request_hw’ defined but not used [-Werror=unused-variable]
 static char print_fmt_##call[] = print;     \
             ^
drivers/gpu/drm/i915/./i915_trace.h:585:1: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
 DECLARE_EVENT_CLASS(i915_gem_request_hw,
 ^
  LD      drivers/tty/serial/8250/8250_base.o
  LD      drivers/tty/serial/8250/built-in.o
  LD      drivers/usb/gadget/udc/udc-core.o
  LD      drivers/usb/gadget/udc/built-in.o
  LD      drivers/usb/gadget/built-in.o
  LD      drivers/tty/serial/built-in.o
  LD      drivers/scsi/sd_mod.o
  LD      drivers/video/fbdev/built-in.o
  LD      drivers/scsi/built-in.o
  AR      lib/lib.a
  EXPORTS lib/lib-ksyms.o
  LD      fs/btrfs/btrfs.o
  LD      lib/built-in.o
  LD      drivers/video/console/built-in.o
  LD      net/xfrm/built-in.o
  LD      drivers/video/built-in.o
  LD      drivers/usb/core/usbcore.o
  LD      fs/btrfs/built-in.o
  LD      drivers/usb/core/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  LD      arch/x86/kernel/cpu/built-in.o
  LD      arch/x86/kernel/built-in.o
  LD      net/ipv4/built-in.o
  LD      drivers/tty/vt/built-in.o
  LD      drivers/tty/built-in.o
  LD      net/ipv6/ipv6.o
  LD      drivers/md/md-mod.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      drivers/md/built-in.o
  LD      arch/x86/built-in.o
  LD      net/ipv6/built-in.o
cc1: all warnings being treated as errors
  LD      drivers/usb/host/xhci-hcd.o
scripts/Makefile.build:293: recipe for target 'drivers/gpu/drm/i915/i915_trace_points.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_trace_points.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  LD      fs/ext4/ext4.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      fs/ext4/built-in.o
  LD      fs/built-in.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD      net/core/built-in.o
  LD      net/built-in.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
scripts/Makefile.build:551: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:551: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:551: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:988: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [RFC 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Tvrtko Ursulin]

On 27/01/2017 12:20, Chris Wilson wrote:
> On Fri, Jan 27, 2017 at 12:01:25PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> i915_gem_ring_notify is more appropriate since we do not have
>> the request information at this point, but it is simply a
>> signal from the engine that some request has been completed.
>
> I was going to suggest intel_engine_notify.

Yeah I know, but i915_trace.h seemed like a last remaining bastion of 
rings so I narrowly tipped to stick with it. But can change it just as well.

Regards,

Tvrtko

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

Re: [RFC 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Tvrtko Ursulin]

On 27/01/2017 12:27, Chris Wilson wrote:
> On Fri, Jan 27, 2017 at 12:01:26PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Two new tracepoints placed at the call sites where requests are
>> actually passed to the GPU enable userspace to track engine
>> utilisation.
>>
>> These tracepoints are only enabled when the
>> DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option is enabled.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  2 ++
>>  drivers/gpu/drm/i915/i915_trace.h          | 49 ++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_lrc.c           |  4 +++
>>  3 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 8ced9e26f075..beec88a30347 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -518,6 +518,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
>>  	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
>>  		POSTING_READ_FW(GUC_STATUS);
>>
>> +	trace_i915_gem_request_in(rq, 0);
>
> But how to get out for guc? We could do a similar in for ringbuffer.
> The original way I used it was in == dispatch, out == ring-notify, which
> is why trace_i915_gem_ring_dispatch enabled the signaling.

That was my thinking as well.

> Hmm. Following on from that we can add the out tracepoint as a
> fence-callback. For the moment, I'd drop guc/legacy
> trace_i915_gem_request_in and we will try something more magical. Though
> once we do enable the fake GuC scheduler we will have an appropriate
> place to drop an trace_i915_gem_request_out. Just leaving ringbuffer as
> the odd one out, but for who arguably the in/out logic is not as
> important.

Fence signalled is very lazy if no one is listening, no? So until the 
GUC backend I thought request_in and deriving the _out from _notify in 
userspace would be OK. Meaning enable signalling stays until then.

Regards,

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

Re: [RFC 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Chris Wilson]

On Fri, Jan 27, 2017 at 01:59:54PM +0000, Tvrtko Ursulin wrote:
> On 27/01/2017 12:27, Chris Wilson wrote:
> >Hmm. Following on from that we can add the out tracepoint as a
> >fence-callback. For the moment, I'd drop guc/legacy
> >trace_i915_gem_request_in and we will try something more magical. Though
> >once we do enable the fake GuC scheduler we will have an appropriate
> >place to drop an trace_i915_gem_request_out. Just leaving ringbuffer as
> >the odd one out, but for who arguably the in/out logic is not as
> >important.
> 
> Fence signalled is very lazy if no one is listening, no? So until
> the GUC backend I thought request_in and deriving the _out from
> _notify in userspace would be OK. Meaning enable signalling stays
> until then.

It's lazy unless we use fence_add_callback(). I was thinking of some
magic inside the trace_request_in macro to add something like
fence_add_callback(this_fence, __trace_request_out);

It still has the problem request_out doesn't work unless request_in is
also being watched, but it has the advantage of being the same for all
generators (i.e. more convenient for userspace).

But as it seems limited to ringbuffer, we may consider it no longer
required?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Tvrtko Ursulin]

On 27/01/2017 14:07, Chris Wilson wrote:
> On Fri, Jan 27, 2017 at 01:59:54PM +0000, Tvrtko Ursulin wrote:
>> On 27/01/2017 12:27, Chris Wilson wrote:
>>> Hmm. Following on from that we can add the out tracepoint as a
>>> fence-callback. For the moment, I'd drop guc/legacy
>>> trace_i915_gem_request_in and we will try something more magical. Though
>>> once we do enable the fake GuC scheduler we will have an appropriate
>>> place to drop an trace_i915_gem_request_out. Just leaving ringbuffer as
>>> the odd one out, but for who arguably the in/out logic is not as
>>> important.
>>
>> Fence signalled is very lazy if no one is listening, no? So until
>> the GUC backend I thought request_in and deriving the _out from
>> _notify in userspace would be OK. Meaning enable signalling stays
>> until then.
>
> It's lazy unless we use fence_add_callback(). I was thinking of some
> magic inside the trace_request_in macro to add something like
> fence_add_callback(this_fence, __trace_request_out);
>
> It still has the problem request_out doesn't work unless request_in is
> also being watched, but it has the advantage of being the same for all
> generators (i.e. more convenient for userspace).
>
> But as it seems limited to ringbuffer, we may consider it no longer
> required?

You mean bank on the GUC backend getting in soon? Yeah, that sounds 
reasonable. I'll drop the sw signalling from notify then.

Regards,

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

Re: [RFC 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Chris Wilson]

On Fri, Jan 27, 2017 at 02:18:06PM +0000, Tvrtko Ursulin wrote:
> 
> On 27/01/2017 14:07, Chris Wilson wrote:
> >On Fri, Jan 27, 2017 at 01:59:54PM +0000, Tvrtko Ursulin wrote:
> >>On 27/01/2017 12:27, Chris Wilson wrote:
> >>>Hmm. Following on from that we can add the out tracepoint as a
> >>>fence-callback. For the moment, I'd drop guc/legacy
> >>>trace_i915_gem_request_in and we will try something more magical. Though
> >>>once we do enable the fake GuC scheduler we will have an appropriate
> >>>place to drop an trace_i915_gem_request_out. Just leaving ringbuffer as
> >>>the odd one out, but for who arguably the in/out logic is not as
> >>>important.
> >>
> >>Fence signalled is very lazy if no one is listening, no? So until
> >>the GUC backend I thought request_in and deriving the _out from
> >>_notify in userspace would be OK. Meaning enable signalling stays
> >>until then.
> >
> >It's lazy unless we use fence_add_callback(). I was thinking of some
> >magic inside the trace_request_in macro to add something like
> >fence_add_callback(this_fence, __trace_request_out);
> >
> >It still has the problem request_out doesn't work unless request_in is
> >also being watched, but it has the advantage of being the same for all
> >generators (i.e. more convenient for userspace).
> >
> >But as it seems limited to ringbuffer, we may consider it no longer
> >required?
> 
> You mean bank on the GUC backend getting in soon? Yeah, that sounds
> reasonable. I'll drop the sw signalling from notify then.

Considering Joonas asks quite regularly "are we nearly there yet", I
think so.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/8] drm/i915/tracepoints: Adjust i915_gem_ring_dispatch

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Rename it to i915_gem_request_queue and fix the logged info
equivalent to the i915_gem_request even class. Also moved it
a bit further apart from the i915_gem_request_add tracepoint
since they otherwise provide similar information too close in
time.

v2: Remove sw fence singalling. We will rely on the soon to
    come GuC scheduling backend to enable that. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
 drivers/gpu/drm/i915/i915_trace.h          | 12 +++++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 91c2393199a3..53c9dbab0cdd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1496,8 +1496,6 @@ execbuf_submit(struct i915_execbuffer_params *params,
 	if (ret)
 		return ret;
 
-	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
-
 	i915_gem_execbuffer_move_to_active(vmas, params->request);
 
 	return 0;
@@ -1842,6 +1840,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	params->dispatch_flags          = dispatch_flags;
 	params->ctx                     = ctx;
 
+	trace_i915_gem_request_queue(params->request, dispatch_flags);
+
 	ret = execbuf_submit(params, args, &eb->vmas);
 err_request:
 	__i915_add_request(params->request, ret == 0);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index d890e4b03902..95574229eea0 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -503,13 +503,14 @@ TRACE_EVENT(i915_gem_ring_sync_to,
 		      __entry->seqno)
 );
 
-TRACE_EVENT(i915_gem_ring_dispatch,
+TRACE_EVENT(i915_gem_request_queue,
 	    TP_PROTO(struct drm_i915_gem_request *req, u32 flags),
 	    TP_ARGS(req, flags),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
 			     __field(u32, ring)
+			     __field(u32, ctx)
 			     __field(u32, seqno)
 			     __field(u32, flags)
 			     ),
@@ -517,13 +518,14 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 	    TP_fast_assign(
 			   __entry->dev = req->i915->drm.primary->index;
 			   __entry->ring = req->engine->id;
-			   __entry->seqno = req->global_seqno;
+			   __entry->ctx = req->ctx->hw_id;
+			   __entry->seqno = req->fence.seqno;
 			   __entry->flags = flags;
-			   dma_fence_enable_sw_signaling(&req->fence);
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
-		      __entry->dev, __entry->ring, __entry->seqno, __entry->flags)
+	    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, flags=%x",
+		      __entry->dev, __entry->ring, __entry->ctx, __entry->seqno,
+		      __entry->flags)
 );
 
 TRACE_EVENT(i915_gem_ring_flush,
-- 
2.7.4

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

[PATCH v2 3/8] drm/i915/tracepoints: Tidy i915_gem_request_wait_begin

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Provide the same information as the other request event classes.

v2: Pass in flags so we can properly report the blocking status.
    (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c |  2 +-
 drivers/gpu/drm/i915/i915_trace.h       | 21 +++++++++++++--------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..823bc2a5bb7f 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1077,7 +1077,7 @@ long i915_wait_request(struct drm_i915_gem_request *req,
 	if (!timeout)
 		return -ETIME;
 
-	trace_i915_gem_request_wait_begin(req);
+	trace_i915_gem_request_wait_begin(req, flags);
 
 	if (!i915_sw_fence_done(&req->execute)) {
 		timeout = __i915_request_wait_for_execute(req, flags, timeout);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 95574229eea0..0523732c1307 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -612,13 +612,16 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_complete,
 );
 
 TRACE_EVENT(i915_gem_request_wait_begin,
-	    TP_PROTO(struct drm_i915_gem_request *req),
-	    TP_ARGS(req),
+	    TP_PROTO(struct drm_i915_gem_request *req, unsigned int flags),
+	    TP_ARGS(req, flags),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
 			     __field(u32, ring)
+			     __field(u32, ctx)
 			     __field(u32, seqno)
+			     __field(u32, global)
+			     __field(unsigned int, flags)
 			     __field(bool, blocking)
 			     ),
 
@@ -631,14 +634,16 @@ TRACE_EVENT(i915_gem_request_wait_begin,
 	    TP_fast_assign(
 			   __entry->dev = req->i915->drm.primary->index;
 			   __entry->ring = req->engine->id;
-			   __entry->seqno = req->global_seqno;
-			   __entry->blocking =
-				     mutex_is_locked(&req->i915->drm.struct_mutex);
+			   __entry->ctx = req->ctx->hw_id;
+			   __entry->seqno = req->fence.seqno;
+			   __entry->global = req->global_seqno;
+			   __entry->flags = flags;
+			   __entry->blocking = flags & I915_WAIT_LOCKED;
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u, blocking=%s",
-		      __entry->dev, __entry->ring,
-		      __entry->seqno, __entry->blocking ?  "yes (NB)" : "no")
+	    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=%x",
+		      __entry->dev, __entry->ring, __entry->ctx, __entry->seqno,
+		      __entry->global, __entry->blocking, __entry->flags)
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_wait_end,
-- 
2.7.4

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

[PATCH v2 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

i915_gem_ring_notify is more appropriate since we do not have
the request information at this point, but it is simply a
signal from the engine that some request has been completed.

v2:
  * Always trace and log if there were any waiters.
  * Rename to intel_engine_notify. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c   |  6 ++++--
 drivers/gpu/drm/i915/i915_trace.h | 13 ++++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0ff75f2282fa..30e6c2475ced 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1033,9 +1033,11 @@ static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
 
 static void notify_ring(struct intel_engine_cs *engine)
 {
+	bool waiters;
+
 	set_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted);
-	if (intel_engine_wakeup(engine))
-		trace_i915_gem_request_notify(engine);
+	waiters = intel_engine_wakeup(engine);
+	trace_intel_engine_notify(engine, waiters);
 }
 
 static void vlv_c0_read(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index ab81f1ef8350..7615cce74e0a 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -605,24 +605,27 @@ trace_i915_gem_request_execute(struct drm_i915_gem_request *req)
 #endif
 #endif
 
-TRACE_EVENT(i915_gem_request_notify,
-	    TP_PROTO(struct intel_engine_cs *engine),
-	    TP_ARGS(engine),
+TRACE_EVENT(intel_engine_notify,
+	    TP_PROTO(struct intel_engine_cs *engine, bool waiters),
+	    TP_ARGS(engine, waiters),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
 			     __field(u32, ring)
 			     __field(u32, seqno)
+			     __field(bool, waiters)
 			     ),
 
 	    TP_fast_assign(
 			   __entry->dev = engine->i915->drm.primary->index;
 			   __entry->ring = engine->id;
 			   __entry->seqno = intel_engine_get_seqno(engine);
+			   __entry->waiters = waiters;
 			   ),
 
-	    TP_printk("dev=%u, ring=%u, seqno=%u",
-		      __entry->dev, __entry->ring, __entry->seqno)
+	    TP_printk("dev=%u, ring=%u, seqno=%u, waiters=%u",
+		      __entry->dev, __entry->ring, __entry->seqno,
+		      __entry->waiters)
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,
-- 
2.7.4

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

[PATCH v2 7/8] drm/i915/tracepoints: Add backend level request in and out tracepoints

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Two new tracepoints placed at the call sites where requests are
actually passed to the GPU enable userspace to track engine
utilisation.

These tracepoints are only enabled when the
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option is enabled.

v2: Fix compilation with !CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c |  2 ++
 drivers/gpu/drm/i915/i915_trace.h          | 49 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           |  4 +++
 3 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 8ced9e26f075..beec88a30347 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -518,6 +518,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
 		POSTING_READ_FW(GUC_STATUS);
 
+	trace_i915_gem_request_in(rq, 0);
+
 	b_ret = guc_ring_doorbell(client);
 
 	client->submissions[engine_id] += 1;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 7615cce74e0a..2a272c199137 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -591,6 +591,45 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_execute,
 	     TP_PROTO(struct drm_i915_gem_request *req),
 	     TP_ARGS(req)
 );
+
+DECLARE_EVENT_CLASS(i915_gem_request_hw,
+		    TP_PROTO(struct drm_i915_gem_request *req,
+			     unsigned int port),
+		    TP_ARGS(req, port),
+
+		    TP_STRUCT__entry(
+				     __field(u32, dev)
+				     __field(u32, ring)
+				     __field(u32, seqno)
+				     __field(u32, global_seqno)
+				     __field(u32, ctx)
+				     __field(u32, port)
+				    ),
+
+		    TP_fast_assign(
+			           __entry->dev = req->i915->drm.primary->index;
+			           __entry->ring = req->engine->id;
+			           __entry->ctx = req->ctx->hw_id;
+			           __entry->seqno = req->fence.seqno;
+			           __entry->global_seqno = req->global_seqno;
+			           __entry->port = port;
+			          ),
+
+		    TP_printk("dev=%u, ring=%u, ctx=%u, seqno=%u, global_seqno=%u, port=%u",
+			      __entry->dev, __entry->ring, __entry->ctx,
+			      __entry->seqno, __entry->global_seqno,
+			      __entry->port)
+);
+
+DEFINE_EVENT(i915_gem_request_hw, i915_gem_request_in,
+	     TP_PROTO(struct drm_i915_gem_request *req, unsigned int port),
+	     TP_ARGS(req, port)
+);
+
+DEFINE_EVENT(i915_gem_request_hw, i915_gem_request_out,
+	     TP_PROTO(struct drm_i915_gem_request *req, unsigned int port),
+	     TP_ARGS(req, port)
+);
 #else
 #if !defined(TRACE_HEADER_MULTI_READ)
 static inline void
@@ -602,6 +641,16 @@ static inline void
 trace_i915_gem_request_execute(struct drm_i915_gem_request *req)
 {
 }
+
+static inline void
+trace_i915_gem_request_in(struct drm_i915_gem_request *req, unsigned int port)
+{
+}
+
+static inline void
+trace_i915_gem_request_out(struct drm_i915_gem_request *req, unsigned int port)
+{
+}
 #endif
 #endif
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0e99d53d5523..8aae8a375edc 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -475,6 +475,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		cursor->priotree.priority = INT_MAX;
 
 		__i915_gem_request_submit(cursor);
+		trace_i915_gem_request_in(cursor, port - engine->execlist_port);
 		last = cursor;
 		submit = true;
 	}
@@ -531,6 +532,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_private *dev_priv = engine->i915;
+	unsigned int portidx = 0;
 
 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
 
@@ -567,6 +569,8 @@ static void intel_lrc_irq_handler(unsigned long data)
 				execlists_context_status_change(port[0].request,
 								INTEL_CONTEXT_SCHEDULE_OUT);
 
+				trace_i915_gem_request_out(port[0].request,
+							   portidx++);
 				i915_gem_request_put(port[0].request);
 				port[0] = port[1];
 				memset(&port[1], 0, sizeof(port[1]));
-- 
2.7.4

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

Re: [PATCH v2 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Chris Wilson]

On Mon, Jan 30, 2017 at 06:07:29PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> i915_gem_ring_notify is more appropriate since we do not have
> the request information at this point, but it is simply a
> signal from the engine that some request has been completed.
> 
> v2:
>   * Always trace and log if there were any waiters.
Agreed, I'd sketched that change, well to always emit the tracepoint when
the interrupt fired. Whether it was in vain is icing on the cake - I've
some sketches of keeping the user-interrupt alive until the following rq
to short-circuit some ping-ponging or irq enable state.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Tracepoints cleanup and improvements for requests (rev5)

[Patchwork]

== Series Details ==

Series: Tracepoints cleanup and improvements for requests (rev5)
URL   : https://patchwork.freedesktop.org/series/18676/
State : success

== Summary ==

Series 18676v5 Tracepoints cleanup and improvements for requests
https://patchwork.freedesktop.org/api/1.0/series/18676/revisions/5/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                dmesg-warn -> PASS       (fi-snb-2520m)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                dmesg-warn -> PASS       (fi-snb-2520m)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a:
                dmesg-warn -> PASS       (fi-snb-2520m)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:78   pass:65   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:223  dwarn:0   dfail:0   fail:2   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:221  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

123d798c350471aba7e0625c154c6d9e395756c8 drm-tip: 2017y-01m-30d-21h-14m-37s UTC integration manifest
76609c5 drm/i915/tracepoints: Add hw_id to context tracepoints
e6bea66 drm/i915/tracepoints: Add backend level request in and out tracepoints
593c326 drm/i915/tracepoints: Rename i915_gem_request_notify
90db18b drm/i915/tracepoints: Add request submit and execute tracepoints
c8695c6 drm/i915/tracepoints: Remove unused i915_gem_request_complete
cec9de6 drm/i915/tracepoints: Tidy i915_gem_request_wait_begin
cbf06b5 drm/i915/tracepoints: Adjust i915_gem_ring_dispatch
b542e3c drm/i915/tracepoints: Tidy request event class

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3647/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Tvrtko Ursulin]

On 30/01/2017 19:44, Chris Wilson wrote:
> On Mon, Jan 30, 2017 at 06:07:29PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> i915_gem_ring_notify is more appropriate since we do not have
>> the request information at this point, but it is simply a
>> signal from the engine that some request has been completed.
>>
>> v2:
>>   * Always trace and log if there were any waiters.
> Agreed, I'd sketched that change, well to always emit the tracepoint when
> the interrupt fired. Whether it was in vain is icing on the cake - I've
> some sketches of keeping the user-interrupt alive until the following rq
> to short-circuit some ping-ponging or irq enable state.

Shall we progress this series or leave it for some future convenient time?

Regards,

Tvrtko

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

Re: [PATCH v2 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Chris Wilson]

On Mon, Feb 20, 2017 at 03:58:12PM +0000, Tvrtko Ursulin wrote:
> 
> On 30/01/2017 19:44, Chris Wilson wrote:
> >On Mon, Jan 30, 2017 at 06:07:29PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>i915_gem_ring_notify is more appropriate since we do not have
> >>the request information at this point, but it is simply a
> >>signal from the engine that some request has been completed.
> >>
> >>v2:
> >>  * Always trace and log if there were any waiters.
> >Agreed, I'd sketched that change, well to always emit the tracepoint when
> >the interrupt fired. Whether it was in vain is icing on the cake - I've
> >some sketches of keeping the user-interrupt alive until the following rq
> >to short-circuit some ping-ponging or irq enable state.
> 
> Shall we progress this series or leave it for some future convenient time?

iirc, it marries well with the patch (on list now) to keep the interrupt
alive for an extra tick. Now's as good as time as any to push the
improvements to rq tracing.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 6/8] drm/i915/tracepoints: Rename i915_gem_request_notify

[Tvrtko Ursulin]

On 20/02/2017 16:07, Chris Wilson wrote:
> On Mon, Feb 20, 2017 at 03:58:12PM +0000, Tvrtko Ursulin wrote:
>>
>> On 30/01/2017 19:44, Chris Wilson wrote:
>>> On Mon, Jan 30, 2017 at 06:07:29PM +0000, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> i915_gem_ring_notify is more appropriate since we do not have
>>>> the request information at this point, but it is simply a
>>>> signal from the engine that some request has been completed.
>>>>
>>>> v2:
>>>>  * Always trace and log if there were any waiters.
>>> Agreed, I'd sketched that change, well to always emit the tracepoint when
>>> the interrupt fired. Whether it was in vain is icing on the cake - I've
>>> some sketches of keeping the user-interrupt alive until the following rq
>>> to short-circuit some ping-ponging or irq enable state.
>>
>> Shall we progress this series or leave it for some future convenient time?
>
> iirc, it marries well with the patch (on list now) to keep the interrupt
> alive for an extra tick. Now's as good as time as any to push the
> improvements to rq tracing.

Cool, patches just need some r-b's then. :) Let me see if they still 
apply and resend the series.

Regards,

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