diff mbox

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

Message ID 20170221091350.14605-5-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin Feb. 21, 2017, 9:13 a.m. UTC 
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(+)

Comments

Chris Wilson Feb. 21, 2017, 9:41 a.m. UTC | #1 
On Tue, Feb 21, 2017 at 09:13:47AM +0000, Tvrtko Ursulin wrote:
> @@ -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;

Move to __i915_gem_request_submit(). (One less complication for me).
I guess you are thinking of changing its name to
__i915_gem_request_execute().
-Chris
Tvrtko Ursulin Feb. 21, 2017, 10:14 a.m. UTC | #2 
On 21/02/2017 09:41, Chris Wilson wrote:
> On Tue, Feb 21, 2017 at 09:13:47AM +0000, Tvrtko Ursulin wrote:
>> @@ -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;
>
> Move to __i915_gem_request_submit(). (One less complication for me).

Ack.

> I guess you are thinking of changing its name to
> __i915_gem_request_execute().

No, why? The idea is i915_gem_request_submit -> "ready for execution / 
dependencies resolved" and i915_gem_request_execute -> "about to be 
submitted to execution backend".

Regards,

Tvrtko
Chris Wilson Feb. 21, 2017, 10:23 a.m. UTC | #3 
On Tue, Feb 21, 2017 at 10:14:12AM +0000, Tvrtko Ursulin wrote:
> 
> 
> On 21/02/2017 09:41, Chris Wilson wrote:
> >On Tue, Feb 21, 2017 at 09:13:47AM +0000, Tvrtko Ursulin wrote:
> >>@@ -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;
> >
> >Move to __i915_gem_request_submit(). (One less complication for me).
> 
> Ack.
> 
> >I guess you are thinking of changing its name to
> >__i915_gem_request_execute().
> 
> No, why? The idea is i915_gem_request_submit -> "ready for execution
> / dependencies resolved" and i915_gem_request_execute -> "about to
> be submitted to execution backend".

Right. Only "i915_gem_request_execute" is currently called
__i915_gem_request_submit().

add_request ->
  [deps complete] ->
    submit_notify ->
      backend->submit() ->
	[some time passes] ->
	  __i915_gem_request_submit() 

You've convinced me that __i915_gem_request_execute is a step forward
from __i915_gem_request_submit.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 68ff072f8b76..e091809a9a9e 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -76,3 +76,14 @@  config DRM_I915_SELFTEST
 	  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 543cef57972b..9a2b5c544c9d 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 7d941ac890b9..ec523fdde01b 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -477,6 +477,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),