Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.
Date: Tue, 24 May 2016 10:22:01 +0200	[thread overview]
Message-ID: <20160524082201.GN27098@phenom.ffwll.local> (raw)
In-Reply-To: <1464010661-32674-3-git-send-email-maarten.lankhorst@linux.intel.com>

On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
> With nonblocking unpin there can be many cursor pins before they're
> cleared by the next page flip.
> 
> Fix this by extending pin_count to the full 32-bit to prevent a
> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: a6747b7304a9 ("drm/i915: Make unpin async.")
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 62be77cac5cd..1d43cc290f71 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -218,8 +218,8 @@ struct i915_vma {
>  	 *
>  	 * In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3
>  	 * bits with absolutely no headroom. So use 4 bits. */
> -	unsigned int pin_count:4;
> -#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf
> +	unsigned int pin_count;
> +#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT UINT_MAX

You need to read up on some of the history of this. The problem is that
some peeps have too many rt threads and managed to sufficiently stall our
unpin worker until we ran out of memory. Well, we would have if not for
this check here. The real fix should be to eventually stall for the unpin
workers to complete (like we do/did in the old pageflip code), or to have
an explicit (per-crtc probably) list of to-be-unpinned stuff, so that we
can process old unpins synchronously once they're completed. Just bumping
the limit so that no one notices that we leak pin counts like bad ain't a
fix ;-)
-Daniel

>  };
>  
>  struct i915_page_dma {
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx