[Intel-gfx] [PATCH v3] drm/i915: Simplify and fix object to display tracking

[Joonas Lahtinen]

On ma, 2015-04-13 at 11:50 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Purpose of this tracking is to know when to flush the cache between
> the CPU and the non-coherent display engine. Prior to:
> 
>    commit 121920faf2ccce9aa66a7e2588415c9647b66104
>    Author: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>    Date:   Mon Mar 23 11:10:37 2015 +0000
> 
>        drm/i915/skl: Query display address through a wrapper
> 
> This worked by a mix of direct flag manipulation and checking for
> existence of a pinned GGTT VMA.
> 
> With the introduction of rotated display mappings this approach is
> no longer correct.
> 
> New simpler approach is to just keep this count over calls which pin
> and unpin objects to and from display, at the slight cost of extra
> space in every bo.
> 
> (Inspired and extracted code from a larger rework by Chris Wilson.)
> 
> v2: Remove the limit since it is not well defined. (Chris Wilson, Ville Syrjälä)
> v3: Commit message corrections. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com> (v3)

> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  3 ++-
>  drivers/gpu/drm/i915/i915_gem.c | 30 ++++++------------------------
>  2 files changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4ef320c..37abd58 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1969,7 +1969,6 @@ struct drm_i915_gem_object {
>  	 */
>  	unsigned int fault_mappable:1;
>  	unsigned int pin_mappable:1;
> -	unsigned int pin_display:1;
>  
>  	/*
>  	 * Is the object to be mapped as read-only to the GPU
> @@ -1983,6 +1982,8 @@ struct drm_i915_gem_object {
>  
>  	unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;
>  
> +	unsigned int pin_display;
> +
>  	struct sg_table *pages;
>  	int pages_pin_count;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3a91365..8b75dab 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3849,24 +3849,6 @@ unlock:
>  	return ret;
>  }
>  
> -static bool is_pin_display(struct drm_i915_gem_object *obj)
> -{
> -	struct i915_vma *vma;
> -
> -	vma = i915_gem_obj_to_ggtt(obj);
> -	if (!vma)
> -		return false;
> -
> -	/* There are 2 sources that pin objects:
> -	 *   1. The display engine (scanouts, sprites, cursors);
> -	 *   2. Reservations for execbuffer;
> -	 *
> -	 * We can ignore reservations as we hold the struct_mutex and
> -	 * are only called outside of the reservation path.
> -	 */
> -	return vma->pin_count;
> -}
> -
>  /*
>   * Prepare buffer for display plane (scanout, cursors, etc).
>   * Can be called from an uninterruptible phase (modesetting) and allows
> @@ -3879,7 +3861,6 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  				     const struct i915_ggtt_view *view)
>  {
>  	u32 old_read_domains, old_write_domain;
> -	bool was_pin_display;
>  	int ret;
>  
>  	if (pipelined != i915_gem_request_get_ring(obj->last_read_req)) {
> @@ -3891,8 +3872,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  	/* Mark the pin_display early so that we account for the
>  	 * display coherency whilst setting up the cache domains.
>  	 */
> -	was_pin_display = obj->pin_display;
> -	obj->pin_display = true;
> +	obj->pin_display++;
>  
>  	/* The display engine is not coherent with the LLC cache on gen6.  As
>  	 * a result, we make sure that the pinning that is about to occur is
> @@ -3936,8 +3916,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  	return 0;
>  
>  err_unpin_display:
> -	WARN_ON(was_pin_display != is_pin_display(obj));
> -	obj->pin_display = was_pin_display;
> +	obj->pin_display--;
>  	return ret;
>  }
>  
> @@ -3945,9 +3924,12 @@ void
>  i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj,
>  					 const struct i915_ggtt_view *view)
>  {
> +	if (WARN_ON(obj->pin_display == 0))
> +		return;
> +
>  	i915_gem_object_ggtt_unpin_view(obj, view);
>  
> -	obj->pin_display = is_pin_display(obj);
> +	obj->pin_display--;
>  }
>  
>  int