diff mbox

[2/2] drm/i915: Include completed status in request tracepoints

Message ID 20180501134114.29216-2-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 1, 2018, 1:41 p.m. UTC
Include a bool to show whether the request is complete in every
tracepoint. This especially helps when tracing the flow of requests
through the HW.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_trace.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Tvrtko Ursulin May 1, 2018, 2:41 p.m. UTC | #1
On 01/05/2018 14:41, Chris Wilson wrote:
> Include a bool to show whether the request is complete in every
> tracepoint. This especially helps when tracing the flow of requests
> through the HW.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_trace.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 408827bf5d96..0122a22d6613 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -647,6 +647,7 @@ DECLARE_EVENT_CLASS(i915_request,
>   			     __field(u32, ctx)
>   			     __field(u32, seqno)
>   			     __field(u32, global)
> +			     __field(u32, completed)
>   			     ),
>   
>   	    TP_fast_assign(
> @@ -656,11 +657,12 @@ DECLARE_EVENT_CLASS(i915_request,
>   			   __entry->ctx = rq->fence.context;
>   			   __entry->seqno = rq->fence.seqno;
>   			   __entry->global = rq->global_seqno;
> +			   __entry->completed = i915_request_completed(rq);
>   			   ),
>   
> -	    TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u",
> +	    TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, completed?=%d",
>   		      __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx,
> -		      __entry->seqno, __entry->global)
> +		      __entry->seqno, __entry->global, __entry->completed)
>   );
>   
>   DEFINE_EVENT(i915_request, i915_request_add,
> 

Wouldn't i915_request_hw class be more interesting for things like the 
preemption bug?

And you mentioned you would like to see priority in there as well?

I am just thinking is completed status to all is not an overkill. Well, 
only from the angle of manually looking at the traces it may be too 
noisy. Not sure yet.

Regards,

Tvrtko
Chris Wilson May 1, 2018, 2:46 p.m. UTC | #2
Quoting Tvrtko Ursulin (2018-05-01 15:41:51)
> 
> On 01/05/2018 14:41, Chris Wilson wrote:
> > Include a bool to show whether the request is complete in every
> > tracepoint. This especially helps when tracing the flow of requests
> > through the HW.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_trace.h | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> > index 408827bf5d96..0122a22d6613 100644
> > --- a/drivers/gpu/drm/i915/i915_trace.h
> > +++ b/drivers/gpu/drm/i915/i915_trace.h
> > @@ -647,6 +647,7 @@ DECLARE_EVENT_CLASS(i915_request,
> >                            __field(u32, ctx)
> >                            __field(u32, seqno)
> >                            __field(u32, global)
> > +                          __field(u32, completed)
> >                            ),
> >   
> >           TP_fast_assign(
> > @@ -656,11 +657,12 @@ DECLARE_EVENT_CLASS(i915_request,
> >                          __entry->ctx = rq->fence.context;
> >                          __entry->seqno = rq->fence.seqno;
> >                          __entry->global = rq->global_seqno;
> > +                        __entry->completed = i915_request_completed(rq);
> >                          ),
> >   
> > -         TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u",
> > +         TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, completed?=%d",
> >                     __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx,
> > -                   __entry->seqno, __entry->global)
> > +                   __entry->seqno, __entry->global, __entry->completed)
> >   );
> >   
> >   DEFINE_EVENT(i915_request, i915_request_add,
> > 
> 
> Wouldn't i915_request_hw class be more interesting for things like the 
> preemption bug?

i915_request_in uses i915_request_hw
i915_request_out uses i915_request

I was looking at request_out, hence ^

> And you mentioned you would like to see priority in there as well?

That will be useful for in. So I guess out should be converted to use
request_hw.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 408827bf5d96..0122a22d6613 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -647,6 +647,7 @@  DECLARE_EVENT_CLASS(i915_request,
 			     __field(u32, ctx)
 			     __field(u32, seqno)
 			     __field(u32, global)
+			     __field(u32, completed)
 			     ),
 
 	    TP_fast_assign(
@@ -656,11 +657,12 @@  DECLARE_EVENT_CLASS(i915_request,
 			   __entry->ctx = rq->fence.context;
 			   __entry->seqno = rq->fence.seqno;
 			   __entry->global = rq->global_seqno;
+			   __entry->completed = i915_request_completed(rq);
 			   ),
 
-	    TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u",
+	    TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, completed?=%d",
 		      __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx,
-		      __entry->seqno, __entry->global)
+		      __entry->seqno, __entry->global, __entry->completed)
 );
 
 DEFINE_EVENT(i915_request, i915_request_add,