Re: [PATCH] drm/i915: add more debug info for when atomic updates fail v3

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

On Tue, Sep 15, 2015 at 02:19:32PM -0700, Jesse Barnes wrote:
> I used these additional fields to track down the issue I saw on HSW.
> 
> v2: move debug fields into a substruct (Ville)
> v3: clean up debug code more (Ville)
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91579
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  8 ++++++--
>  drivers/gpu/drm/i915/intel_sprite.c | 25 +++++++++++++++++--------
>  2 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 02a755a..1df6ebf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -565,8 +565,12 @@ struct intel_crtc {
>  
>  	int scanline_offset;
>  
> -	unsigned start_vbl_count;
> -	ktime_t start_vbl_time;
> +	struct {
> +		unsigned start_vbl_count;
> +		ktime_t start_vbl_time;
> +		int min_vbl, max_vbl;
> +		int scanline_start;

A bit inconsistent with start_foo vs. foo_start.

But othwerwise looks sane, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	} debug;
>  
>  	struct intel_crtc_atomic_commit atomic;
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4d27243..7de121d 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -95,7 +95,6 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  	max = vblank_start - 1;
>  
>  	local_irq_disable();
> -	crtc->start_vbl_count = 0;
>  
>  	if (min <= 0 || max <= 0)
>  		return;
> @@ -134,11 +133,15 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  
>  	drm_crtc_vblank_put(&crtc->base);
>  
> -	crtc->start_vbl_time = ktime_get();
> -	crtc->start_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
> +	crtc->debug.min_vbl = min;
> +	crtc->debug.max_vbl = max;
> +	crtc->debug.scanline_start = scanline;
> +	crtc->debug.start_vbl_time = ktime_get();
> +	crtc->debug.start_vbl_count =
> +		dev->driver->get_vblank_counter(dev, pipe);
>  
>  	trace_i915_pipe_update_vblank_evaded(crtc, min, max,
> -					     crtc->start_vbl_count);
> +					     crtc->debug.start_vbl_count);
>  }
>  
>  /**
> @@ -154,6 +157,7 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	enum pipe pipe = crtc->pipe;
> +	int scanline_end = intel_get_crtc_scanline(crtc);
>  	u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
>  	ktime_t end_vbl_time = ktime_get();
>  
> @@ -161,10 +165,15 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
>  
>  	local_irq_enable();
>  
> -	if (crtc->start_vbl_count && crtc->start_vbl_count != end_vbl_count)
> -		DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us\n",
> -			  pipe_name(pipe), crtc->start_vbl_count, end_vbl_count,
> -			  ktime_us_delta(end_vbl_time, crtc->start_vbl_time));
> +	if (crtc->debug.start_vbl_count &&
> +	    crtc->debug.start_vbl_count != end_vbl_count) {
> +		DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n",
> +			  pipe_name(pipe), crtc->debug.start_vbl_count,
> +			  end_vbl_count,
> +			  ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time),
> +			  crtc->debug.min_vbl, crtc->debug.max_vbl,
> +			  crtc->debug.scanline_start, scanline_end);
> +	}
>  }
>  
>  static void
> -- 
> 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