[Intel-gfx] [PATCH 5/5] drm/i915: tracepoints for semaphores

[Daniel Vetter]

On Thu, Sep 15, 2011 at 07:09:01PM -0700, Ben Widawsky wrote:
> The tracepoints give enough info to figure the updates and compares
> (document terminology for signals and waits) and dependencies therein of
> the semaphore mailboxes.
> 
> Here are arguments to perf to get interesting info (mostly copied from
> Chris):
> record -f -g -c 1 -e i915:intel_ringbuffer_add_request -e i915:intel_ringbuffer_consume_semaphore -a
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_trace.h       |   61 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    5 +++
>  2 files changed, 66 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index d623fef..47d55fd 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -410,6 +410,67 @@ TRACE_EVENT(i915_reg_rw,
>  		     (u32)(__entry->val >> 32))
>  );
>  
> +#define SEMAPHORE_REG_NAME(x) (\
> +	(x == RENDER_RING_BASE + 0x40) ? ("RVSYNC") : \
> +	((x == RENDER_RING_BASE + 0x44) ? ("RBSYNC") : \
> +	((x == GEN6_BSD_RING_BASE + 0x40) ? ("VRSYNC") : \
> +	((x == GEN6_BSD_RING_BASE + 0x44) ? ("VBSYNC") : \
> +	((x == BLT_RING_BASE + 0x40) ? ("BVSYNC") : \
> +	((x == BLT_RING_BASE + 0x44) ? ("BRSYNC") : \
> +	("UNKNOWN")))))))
> +
> +#define RING_NAME(x) (\
> +	(x == 1) ? ("rcs") : \
> +	((x == 2) ? ("vcs") : \
> +	((x == 4) ? ("bcs") : \
> +	("unk"))))

With the ring->id cleanup, that's now wrong. Also, the other trace events
only report the numeric ring id, so for consistency, I think you can just
drop this.

> +TRACE_EVENT(intel_ringbuffer_add_request,
> +	    TP_PROTO(struct intel_ring_buffer *ring,
> +		     u32 seqno,
> +		     u32 reg1,
> +		     u32 reg2),
> +	    TP_ARGS(ring, seqno, reg1, reg2),
> +	    TP_STRUCT__entry(
> +			     __field(int, id)
> +			     __field(u32, seqno)
> +			     __field(u32, reg1)
> +			     __field(u32, reg2)
> +			    ),
> +	    TP_fast_assign(
> +			   __entry->id = ring->id;
> +			   __entry->seqno = seqno;
> +			   __entry->reg1 = reg1;
> +			   __entry->reg2 = reg2;
> +			  ),
> +	    TP_printk("ring = %s, seqno = %u, signal mbox 1 = %d, signal mbox 2 = %d",
> +		      RING_NAME(__entry->id), __entry->seqno,
> +		      SEMAPHORE_REG_NAME(__entry->reg1),
> +		      SEMAPHORE_REG_NAME(__entry->reg2))
> +);
> +
> +TRACE_EVENT(intel_ringbuffer_consume_semaphore,
> +	    TP_PROTO(struct intel_ring_buffer *updater,
> +		     struct intel_ring_buffer *comparer,
> +		     u32 seqno),
> +	    TP_ARGS(updater, comparer, seqno),
> +	    TP_STRUCT__entry(
> +			     __field(int, src)
> +			     __field(int, dest)
> +			     __field(u32, seqno)
> +			    ),
> +	    TP_fast_assign(
> +			   __entry->src = updater->id;
> +			   __entry->dest = comparer->id;
> +			   __entry->seqno = seqno;
> +			  ),
> +	    /* It's not easy to macro the consuming mbox register */
> +	    TP_printk("signaller = %s, waiter = %s, seqno = %u",
> +		      RING_NAME(__entry->src),
> +		      RING_NAME(__entry->dest),
> +		      __entry->seqno)
> +);
> +
>  #endif /* _I915_TRACE_H_ */
>  
>  /* This part must be outside protection */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index cbf255e..6bf1dd9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -351,6 +351,9 @@ gen6_add_request(struct intel_ring_buffer *ring,
>  	intel_ring_advance(ring);
>  
>  	*result = seqno;
> +
> +	trace_intel_ringbuffer_add_request(ring, seqno, ring->signal_mbox[0],
> +					   ring->signal_mbox[1]);
>  	return 0;
>  }

We already have trace_i915_gem_request_add in i915_add_request which is
essentially giving out the same information (well, minus the hopefully
correct singal_mbox reg addresses). I think we can drop this one.

> @@ -375,6 +378,8 @@ intel_ring_sync(struct intel_ring_buffer *waiter,
>  	intel_ring_emit(waiter, MI_NOOP);
>  	intel_ring_advance(waiter);
>  
> +	trace_intel_ringbuffer_consume_semaphore(signaller, waiter, seqno);
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.6.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48