Re: [PATCH] drm/i915: cleanup pipe_update trace functions with new crtc debug info

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 15, 2015 at 02:20:18PM -0700, Jesse Barnes wrote:
> Use the new debug info in the intel_crtc struct in these functions
> rather than passing them as args.
> 
> Requested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_trace.h   | 18 +++++++++---------
>  drivers/gpu/drm/i915/intel_sprite.c |  5 ++---
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index e6b5c74..b48da2d 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -17,8 +17,8 @@
>  /* pipe updates */
>  
>  TRACE_EVENT(i915_pipe_update_start,
> -	    TP_PROTO(struct intel_crtc *crtc, u32 min, u32 max),
> -	    TP_ARGS(crtc, min, max),
> +	    TP_PROTO(struct intel_crtc *crtc),
> +	    TP_ARGS(crtc),
>  
>  	    TP_STRUCT__entry(
>  			     __field(enum pipe, pipe)
> @@ -33,8 +33,8 @@ TRACE_EVENT(i915_pipe_update_start,
>  			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
>  										       crtc->pipe);
>  			   __entry->scanline = intel_get_crtc_scanline(crtc);
> -			   __entry->min = min;
> -			   __entry->max = max;
> +			   __entry->min = crtc->debug.min_vbl;
> +			   __entry->max = crtc->debug.max_vbl;

Hmm. I think you actually moved the min/max_vbl asignment to happen
after this tracepoint is called. So this is busted.

With that part fixed you can slap on a
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  			   ),
>  
>  	    TP_printk("pipe %c, frame=%u, scanline=%u, min=%u, max=%u",
> @@ -43,8 +43,8 @@ TRACE_EVENT(i915_pipe_update_start,
>  );
>  
>  TRACE_EVENT(i915_pipe_update_vblank_evaded,
> -	    TP_PROTO(struct intel_crtc *crtc, u32 min, u32 max, u32 frame),
> -	    TP_ARGS(crtc, min, max, frame),
> +	    TP_PROTO(struct intel_crtc *crtc),
> +	    TP_ARGS(crtc),
>  
>  	    TP_STRUCT__entry(
>  			     __field(enum pipe, pipe)
> @@ -56,10 +56,10 @@ TRACE_EVENT(i915_pipe_update_vblank_evaded,
>  
>  	    TP_fast_assign(
>  			   __entry->pipe = crtc->pipe;
> -			   __entry->frame = frame;
> +			   __entry->frame = crtc->debug.start_vbl_count;
>  			   __entry->scanline = intel_get_crtc_scanline(crtc);

We already have the scanline in the debug struct, so we could use it
here instead if reading it out again.

Same could be done for the i915_pipe_update_end tracepoint, but there we'd
have to pass it in obviously since we don't keep the end values in the
struct.

But these two are purely optional ideas. Feel free to ignore them if you
wish.

> -			   __entry->min = min;
> -			   __entry->max = max;
> +			   __entry->min = crtc->debug.min_vbl;
> +			   __entry->max = crtc->debug.max_vbl;
>  			   ),
>  
>  	    TP_printk("pipe %c, frame=%u, scanline=%u, min=%u, max=%u",
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 7de121d..4452390 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -102,7 +102,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  	if (WARN_ON(drm_crtc_vblank_get(&crtc->base)))
>  		return;
>  
> -	trace_i915_pipe_update_start(crtc, min, max);
> +	trace_i915_pipe_update_start(crtc);
>  
>  	for (;;) {
>  		/*
> @@ -140,8 +140,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  	crtc->debug.start_vbl_count =
>  		dev->driver->get_vblank_counter(dev, pipe);
>  
> -	trace_i915_pipe_update_vblank_evaded(crtc, min, max,
> -					     crtc->debug.start_vbl_count);
> +	trace_i915_pipe_update_vblank_evaded(crtc);
>  }
>  
>  /**
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx