Re: [PATCH 3/3] drm/i915: check gtfifodbg after possibly failed writes

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Feb 07, 2012 at 04:21:50PM +0100, Ben Widawsky wrote:
> If we don't have a sufficient number of free entries in the FIFO, we
> proceed to do a write anyway. With this check we should have a clue if
> that write actually failed or not.
> 
> After some discussion with Daniel Vetter regarding his original
> complaint, we agreed upon this.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |   15 ++++++++++++---
>  drivers/gpu/drm/i915/i915_drv.h |    2 +-
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a7858a1..ff5c3c7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -413,8 +413,10 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  		dev_priv->display.force_wake_put(dev_priv);
>  }
>  
> -void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> +int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>  {
> +	int ret = 0;
> +
>  	if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
>  		int loop = 500;
>  		u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> @@ -422,10 +424,13 @@ void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>  			udelay(10);
>  			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
>  		}
> -		WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES);
> +		WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES &&
> +			++ret);

Bit a bikeshed comment, but I prefer explicit control flow instead of
playing tricks with the short-circuiting behaviour of &&. Mind if you can
change that, too?
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48