[Intel-gfx] [PATCH 7/7] drm/i915: Broaden application of set-domain(GTT)

[Daniel Vetter]

On Mon, Nov 24, 2014 at 08:29:46AM -0800, Rodrigo Vivi wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
> 
> Previously, this was restricted to only operate on bound objects - to
> make pointer access through the GTT to the object coherent with writes
> to and from the GPU. A second usecase is drm_intel_bo_wait_rendering()
> which at present does not function unless the object also happens to
> be bound into the GGTT (on current systems that is becoming increasingly
> rare, especially for the typical requests from mesa). A third usecase is
> a future patch wishing to extend the coverage of the GTT domain to
> include objects not bound into the GGTT but still in its coherent cache
> domain. For the latter pair of requests, we need to operate on the
> object regardless of its bind state.
> 
> v2: After discussion with Akash, we came to the conclusion that the
> get-pages was required in order for accurate domain tracking in the
> corner cases (like the shrinker) and also useful for ensuring memory
> coherency with earlier cached CPU mmaps in case userspace uses exotic
> cache bypass (non-temporal) instructions.
> 
> v3: Fix the inactive object check.
> 
> Cc: Akash Goel <akash.goel at intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Reviewed-by: Akash Goel <akash.goel at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

This is part of the wc mmap series, you can probably drop this one here.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 43 ++++++++++++++++++-----------------------
>  1 file changed, 19 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1d58bc5..1af042b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -153,12 +153,6 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static inline bool
> -i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> -{
> -	return i915_gem_obj_bound_any(obj) && !obj->active;
> -}
> -
>  int
>  i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>  			    struct drm_file *file)
> @@ -1466,18 +1460,10 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		goto unref;
>  
> -	if (read_domains & I915_GEM_DOMAIN_GTT) {
> +	if (read_domains & I915_GEM_DOMAIN_GTT)
>  		ret = i915_gem_object_set_to_gtt_domain(obj, write_domain != 0);
> -
> -		/* Silently promote "you're not bound, there was nothing to do"
> -		 * to success, since the client was just asking us to
> -		 * make sure everything was done.
> -		 */
> -		if (ret == -EINVAL)
> -			ret = 0;
> -	} else {
> +	else
>  		ret = i915_gem_object_set_to_cpu_domain(obj, write_domain != 0);
> -	}
>  
>  unref:
>  	drm_gem_object_unreference(&obj->base);
> @@ -3685,15 +3671,10 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
>  int
>  i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  {
> -	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> -	struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
>  	uint32_t old_write_domain, old_read_domains;
> +	struct i915_vma *vma;
>  	int ret;
>  
> -	/* Not valid to be called on unbound objects. */
> -	if (vma == NULL)
> -		return -EINVAL;
> -
>  	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
>  		return 0;
>  
> @@ -3702,6 +3683,19 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  		return ret;
>  
>  	i915_gem_object_retire(obj);
> +
> +	/* Flush and acquire obj->pages so that we are coherent through
> +	 * direct access in memory with previous cached writes through
> +	 * shmemfs and that our cache domain tracking remains valid.
> +	 * For example, if the obj->filp was moved to swap without us
> +	 * being notified and releasing the pages, we would mistakenly
> +	 * continue to assume that the obj remained out of the CPU cached
> +	 * domain.
> +	 */
> +	ret = i915_gem_object_get_pages(obj);
> +	if (ret)
> +		return ret;
> +
>  	i915_gem_object_flush_cpu_write_domain(obj, false);
>  
>  	/* Serialise direct access to this object with the barriers for
> @@ -3733,9 +3727,10 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  					    old_write_domain);
>  
>  	/* And bump the LRU for this access */
> -	if (i915_gem_object_is_inactive(obj))
> +	vma = i915_gem_obj_to_ggtt(obj);
> +	if (vma && drm_mm_node_allocated(&vma->node) && !obj->active)
>  		list_move_tail(&vma->mm_list,
> -			       &dev_priv->gtt.base.inactive_list);
> +			       &to_i915(obj->base.dev)->gtt.base.inactive_list);
>  
>  	return 0;
>  }
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch