[Intel-gfx] [PATCH] drm/i915: Infrastructure for supporting different GGTT views per object

Daniel Vetter daniel at ffwll.ch
Fri Nov 28 18:31:06 CET 2014


On Thu, Nov 27, 2014 at 02:52:44PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Things like reliable GGTT mappings and mirrored 2d-on-3d display will need
> to map objects into the same address space multiple times.
> 
> Added a GGTT view concept and linked it with the VMA to distinguish between
> multiple instances per address space.
> 
> New objects and GEM functions which do not take this new view as a parameter
> assume the default of zero (I915_GGTT_VIEW_NORMAL) which preserves the
> previous behaviour.
> 
> This now means that objects can have multiple VMA entries so the code which
> assumed there will only be one also had to be modified.
> 
> Alternative GGTT views are supposed to borrow DMA addresses from obj->pages
> which is DMA mapped on first VMA instantiation and unmapped on the last one
> going away.
> 
> v2:
>     * Removed per view special casing in i915_gem_ggtt_prepare /
>       finish_object in favour of creating and destroying DMA mappings
>       on first VMA instantiation and last VMA destruction. (Daniel Vetter)
>     * Simplified i915_vma_unbind which does not need to count the GGTT views.
>       (Daniel Vetter)
>     * Also moved obj->map_and_fenceable reset under the same check.
>     * Checkpatch cleanups.
> 
> Issue: VIZ-4544
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>

lgtm overall. Please find someone for detailed review for knowledge
sharing (so different geo/team).

A few comemnts and questions below still. Also could you please do a
follow-up patch which adds a DOC: section with a short exlanation of gtt
views and pulls it into the i915 docbook template? We need to start
somewhere with gem docs ...

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        |  5 +-
>  drivers/gpu/drm/i915/i915_drv.h            | 46 +++++++++++++++++--
>  drivers/gpu/drm/i915/i915_gem.c            | 73 ++++++++++++++++--------------
>  drivers/gpu/drm/i915/i915_gem_context.c    |  4 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c        | 61 +++++++++++++++++++------
>  drivers/gpu/drm/i915/i915_gem_gtt.h        | 22 ++++++++-
>  drivers/gpu/drm/i915/i915_gpu_error.c      |  8 +---
>  8 files changed, 159 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 319da61..1a9569f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -154,8 +154,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>  			seq_puts(m, " (pp");
>  		else
>  			seq_puts(m, " (g");
> -		seq_printf(m, "gtt offset: %08lx, size: %08lx)",
> -			   vma->node.start, vma->node.size);
> +		seq_printf(m, "gtt offset: %08lx, size: %08lx, type: %u)",
> +			   vma->node.start, vma->node.size,
> +			   vma->ggtt_view.type);
>  	}
>  	if (obj->stolen)
>  		seq_printf(m, " (stolen: %08lx)", obj->stolen->start);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c4f2cb6..6250a2c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2501,10 +2501,23 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
>  #define PIN_GLOBAL 0x4
>  #define PIN_OFFSET_BIAS 0x8
>  #define PIN_OFFSET_MASK (~4095)
> +int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
> +					  struct i915_address_space *vm,
> +					  uint32_t alignment,
> +					  uint64_t flags,
> +					  const struct i915_ggtt_view *view);
> +static inline
>  int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
>  				     struct i915_address_space *vm,
>  				     uint32_t alignment,
> -				     uint64_t flags);
> +				     uint64_t flags)
> +{
> +	return i915_gem_object_pin_view(obj, vm, alignment, flags,
> +						&i915_ggtt_view_normal);
> +}
> +
> +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
> +		   u32 flags);
>  int __must_check i915_vma_unbind(struct i915_vma *vma);
>  int i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
>  void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv);
> @@ -2656,18 +2669,43 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
>  
>  void i915_gem_restore_fences(struct drm_device *dev);
>  
> +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
> +				       struct i915_address_space *vm,
> +				       enum i915_ggtt_view_type view);
> +static inline
>  unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
> -				  struct i915_address_space *vm);
> +				  struct i915_address_space *vm)
> +{
> +	return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);
> +}
>  bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
>  bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
>  			struct i915_address_space *vm);
>  unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
>  				struct i915_address_space *vm);
> +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj,
> +					  struct i915_address_space *vm,
> +					  const struct i915_ggtt_view *view);
> +static inline
>  struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
> -				     struct i915_address_space *vm);
> +				     struct i915_address_space *vm)
> +{
> +	return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal);
> +}
> +
> +struct i915_vma *
> +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,
> +				       struct i915_address_space *vm,
> +				       const struct i915_ggtt_view *view);
> +
> +static inline
>  struct i915_vma *
>  i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
> -				  struct i915_address_space *vm);
> +				  struct i915_address_space *vm)
> +{
> +	return i915_gem_obj_lookup_or_create_vma_view(obj, vm,
> +						&i915_ggtt_view_normal);
> +}
>  
>  struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj);
>  static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 86cf428..6213c07 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2090,8 +2090,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
>  			/* For the unbound phase, this should be a no-op! */
>  			list_for_each_entry_safe(vma, v,
>  						 &obj->vma_list, vma_link)
> -				if (i915_vma_unbind(vma))
> -					break;
> +				i915_vma_unbind(vma);

Why drop the early break if a vma_unbind fails? Looks like a superflous
hunk to me.

>  
>  			if (i915_gem_object_put_pages(obj) == 0)
>  				count += obj->base.size >> PAGE_SHIFT;
> @@ -2292,17 +2291,14 @@ void i915_vma_move_to_active(struct i915_vma *vma,
>  static void
>  i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
>  {
> -	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> -	struct i915_address_space *vm;
>  	struct i915_vma *vma;
>  
>  	BUG_ON(obj->base.write_domain & ~I915_GEM_GPU_DOMAINS);
>  	BUG_ON(!obj->active);
>  
> -	list_for_each_entry(vm, &dev_priv->vm_list, global_link) {
> -		vma = i915_gem_obj_to_vma(obj, vm);
> -		if (vma && !list_empty(&vma->mm_list))
> -			list_move_tail(&vma->mm_list, &vm->inactive_list);
> +	list_for_each_entry(vma, &obj->vma_list, vma_link) {
> +		if (!list_empty(&vma->mm_list))
> +			list_move_tail(&vma->mm_list, &vma->vm->inactive_list);
>  	}
>  
>  	intel_fb_obj_flush(obj, true);
> @@ -3043,7 +3039,8 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	/* Throw away the active reference before moving to the unbound list */
>  	i915_gem_object_retire(obj);
>  
> -	if (i915_is_ggtt(vma->vm)) {
> +	if (i915_is_ggtt(vma->vm) &&
> +	    vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
>  		i915_gem_object_finish_gtt(obj);

If anyone adds subobject gtt mmap fault support we'll need to undo this
again, but makes sense for the current usecase. So imo ok either way right
now.

>  
>  		/* release the fence reg _after_ flushing */
> @@ -3057,7 +3054,8 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	vma->unbind_vma(vma);
>  
>  	list_del_init(&vma->mm_list);
> -	if (i915_is_ggtt(vma->vm))
> +	if (i915_is_ggtt(vma->vm) &&
> +	    vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)
>  		obj->map_and_fenceable = false;

Same here.

>  
>  	drm_mm_remove_node(&vma->node);
> @@ -3476,7 +3474,8 @@ static struct i915_vma *
>  i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>  			   struct i915_address_space *vm,
>  			   unsigned alignment,
> -			   uint64_t flags)
> +			   uint64_t flags,
> +			   const struct i915_ggtt_view *view)
>  {
>  	struct drm_device *dev = obj->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3526,7 +3525,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>  
>  	i915_gem_object_pin_pages(obj);
>  
> -	vma = i915_gem_obj_lookup_or_create_vma(obj, vm);
> +	vma = i915_gem_obj_lookup_or_create_vma_view(obj, vm, view);
>  	if (IS_ERR(vma))
>  		goto err_unpin;
>  
> @@ -3560,7 +3559,7 @@ search_free:
>  	list_add_tail(&vma->mm_list, &vm->inactive_list);
>  
>  	trace_i915_vma_bind(vma, flags);
> -	vma->bind_vma(vma, obj->cache_level,
> +	i915_vma_bind(vma, obj->cache_level,
>  		      flags & PIN_GLOBAL ? GLOBAL_BIND : 0);
>  
>  	return vma;
> @@ -3768,8 +3767,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
>  
>  		list_for_each_entry(vma, &obj->vma_list, vma_link)
>  			if (drm_mm_node_allocated(&vma->node))
> -				vma->bind_vma(vma, cache_level,
> -						vma->bound & GLOBAL_BIND);
> +				i915_vma_bind(vma, cache_level,
> +					      vma->bound & GLOBAL_BIND);
>  	}
>  
>  	list_for_each_entry(vma, &obj->vma_list, vma_link)
> @@ -4123,10 +4122,11 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags)
>  }
>  
>  int
> -i915_gem_object_pin(struct drm_i915_gem_object *obj,
> -		    struct i915_address_space *vm,
> -		    uint32_t alignment,
> -		    uint64_t flags)
> +i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
> +			 struct i915_address_space *vm,
> +			 uint32_t alignment,
> +			 uint64_t flags,
> +			 const struct i915_ggtt_view *view)
>  {
>  	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>  	struct i915_vma *vma;
> @@ -4142,7 +4142,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
>  	if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE))
>  		return -EINVAL;
>  
> -	vma = i915_gem_obj_to_vma(obj, vm);
> +	vma = i915_gem_obj_to_vma_view(obj, vm, view);
>  	if (vma) {
>  		if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT))
>  			return -EBUSY;
> @@ -4152,7 +4152,8 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
>  			     "bo is already pinned with incorrect alignment:"
>  			     " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d,"
>  			     " obj->map_and_fenceable=%d\n",
> -			     i915_gem_obj_offset(obj, vm), alignment,
> +			     i915_gem_obj_offset_view(obj, vm, view->type),
> +			     alignment,
>  			     !!(flags & PIN_MAPPABLE),
>  			     obj->map_and_fenceable);
>  			ret = i915_vma_unbind(vma);
> @@ -4165,13 +4166,14 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
>  
>  	bound = vma ? vma->bound : 0;
>  	if (vma == NULL || !drm_mm_node_allocated(&vma->node)) {
> -		vma = i915_gem_object_bind_to_vm(obj, vm, alignment, flags);
> +		vma = i915_gem_object_bind_to_vm(obj, vm, alignment,
> +						 flags, view);
>  		if (IS_ERR(vma))
>  			return PTR_ERR(vma);
>  	}
>  
>  	if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND))
> -		vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
> +		i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND);
>  
>  	if ((bound ^ vma->bound) & GLOBAL_BIND) {
>  		bool mappable, fenceable;
> @@ -4583,12 +4585,13 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> -struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
> -				     struct i915_address_space *vm)
> +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj,
> +					  struct i915_address_space *vm,
> +					  const struct i915_ggtt_view *view)
>  {
>  	struct i915_vma *vma;
>  	list_for_each_entry(vma, &obj->vma_list, vma_link)
> -		if (vma->vm == vm)
> +		if (vma->vm == vm && vma->ggtt_view.type == view->type)
>  			return vma;
>  
>  	return NULL;
> @@ -5272,8 +5275,9 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
>  }
>  
>  /* All the new VM stuff */
> -unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
> -				  struct i915_address_space *vm)
> +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
> +				       struct i915_address_space *vm,
> +				       enum i915_ggtt_view_type view)
>  {
>  	struct drm_i915_private *dev_priv = o->base.dev->dev_private;
>  	struct i915_vma *vma;
> @@ -5281,7 +5285,7 @@ unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
>  	WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base);
>  
>  	list_for_each_entry(vma, &o->vma_list, vma_link) {
> -		if (vma->vm == vm)
> +		if (vma->vm == vm && vma->ggtt_view.type == view)
>  			return vma->node.start;
>  
>  	}
> @@ -5430,9 +5434,12 @@ struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj)
>  {
>  	struct i915_vma *vma;
>  
> -	vma = list_first_entry(&obj->vma_list, typeof(*vma), vma_link);
> -	if (vma->vm != i915_obj_to_ggtt(obj))
> -		return NULL;
> +	list_for_each_entry(vma, &obj->vma_list, vma_link) {
> +		if (vma->vm != i915_obj_to_ggtt(obj))
> +			continue;
> +		if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)
> +			return vma;
> +	}

We fairly put the ggtt vma into the head of the list. Imo better to keep
the head slot reserved for the ggtt normal view, might be some random code
relying upon this.

>  
> -	return vma;
> +	return NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index d17ff43..4bf9f79 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -580,8 +580,8 @@ static int do_switch(struct intel_engine_cs *ring,
>  
>  	vma = i915_gem_obj_to_ggtt(to->legacy_hw_ctx.rcs_state);
>  	if (!(vma->bound & GLOBAL_BIND))
> -		vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level,
> -				GLOBAL_BIND);
> +		i915_vma_bind(vma, to->legacy_hw_ctx.rcs_state->cache_level,
> +			      GLOBAL_BIND);
>  
>  	if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
>  		hw_flags |= MI_RESTORE_INHIBIT;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index e1ed85a..e79caf9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -358,8 +358,8 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
>  	if (unlikely(IS_GEN6(dev) &&
>  	    reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
>  	    !(target_vma->bound & GLOBAL_BIND)))
> -		target_vma->bind_vma(target_vma, target_i915_obj->cache_level,
> -				GLOBAL_BIND);
> +		i915_vma_bind(target_vma, target_i915_obj->cache_level,
> +			      GLOBAL_BIND);
>  
>  	/* Validate that the target is in a valid r/w GPU domain */
>  	if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index bee5b0a..2144738 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -30,6 +30,8 @@
>  #include "i915_trace.h"
>  #include "intel_drv.h"
>  
> +const struct i915_ggtt_view i915_ggtt_view_normal;
> +
>  static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv);
>  static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
>  
> @@ -76,7 +78,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>  }
>  
>  
> -static void ppgtt_bind_vma(struct i915_vma *vma,
> +static void ppgtt_bind_vma(struct i915_vma *vma, struct sg_table *pages,
>  			   enum i915_cache_level cache_level,
>  			   u32 flags);
>  static void ppgtt_unbind_vma(struct i915_vma *vma);
> @@ -1200,7 +1202,7 @@ void  i915_ppgtt_release(struct kref *kref)
>  }
>  
>  static void
> -ppgtt_bind_vma(struct i915_vma *vma,
> +ppgtt_bind_vma(struct i915_vma *vma, struct sg_table *pages,
>  	       enum i915_cache_level cache_level,
>  	       u32 flags)
>  {
> @@ -1208,7 +1210,7 @@ ppgtt_bind_vma(struct i915_vma *vma,
>  	if (vma->obj->gt_ro)
>  		flags |= PTE_READ_ONLY;
>  
> -	vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start,
> +	vma->vm->insert_entries(vma->vm, pages, vma->node.start,
>  				cache_level, flags);
>  }
>  
> @@ -1343,7 +1345,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  		 * without telling our object about it. So we need to fake it.
>  		 */
>  		vma->bound &= ~GLOBAL_BIND;
> -		vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
> +		i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND);
>  	}
>  
>  
> @@ -1529,7 +1531,7 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm,
>  }
>  
>  
> -static void i915_ggtt_bind_vma(struct i915_vma *vma,
> +static void i915_ggtt_bind_vma(struct i915_vma *vma, struct sg_table *pages,
>  			       enum i915_cache_level cache_level,
>  			       u32 unused)
>  {
> @@ -1538,7 +1540,7 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma,
>  		AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
>  
>  	BUG_ON(!i915_is_ggtt(vma->vm));
> -	intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags);
> +	intel_gtt_insert_sg_entries(pages, entry, flags);
>  	vma->bound = GLOBAL_BIND;
>  }
>  
> @@ -1562,7 +1564,7 @@ static void i915_ggtt_unbind_vma(struct i915_vma *vma)
>  	intel_gtt_clear_range(first, size);
>  }
>  
> -static void ggtt_bind_vma(struct i915_vma *vma,
> +static void ggtt_bind_vma(struct i915_vma *vma, struct sg_table *pages,
>  			  enum i915_cache_level cache_level,
>  			  u32 flags)
>  {
> @@ -1588,7 +1590,7 @@ static void ggtt_bind_vma(struct i915_vma *vma,
>  	if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) {
>  		if (!(vma->bound & GLOBAL_BIND) ||
>  		    (cache_level != obj->cache_level)) {
> -			vma->vm->insert_entries(vma->vm, obj->pages,
> +			vma->vm->insert_entries(vma->vm, pages,
>  						vma->node.start,
>  						cache_level, flags);
>  			vma->bound |= GLOBAL_BIND;
> @@ -1600,7 +1602,7 @@ static void ggtt_bind_vma(struct i915_vma *vma,
>  	     (cache_level != obj->cache_level))) {
>  		struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt;
>  		appgtt->base.insert_entries(&appgtt->base,
> -					    vma->obj->pages,
> +					    pages,
>  					    vma->node.start,
>  					    cache_level, flags);
>  		vma->bound |= LOCAL_BIND;
> @@ -2165,7 +2167,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
>  }
>  
>  static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,
> -					      struct i915_address_space *vm)
> +					      struct i915_address_space *vm,
> +					      const struct i915_ggtt_view *view)
>  {
>  	struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);
>  	if (vma == NULL)
> @@ -2176,6 +2179,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,
>  	INIT_LIST_HEAD(&vma->exec_list);
>  	vma->vm = vm;
>  	vma->obj = obj;
> +	vma->ggtt_view = *view;
>  
>  	switch (INTEL_INFO(vm->dev)->gen) {
>  	case 9:
> @@ -2214,14 +2218,43 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,
>  }
>  
>  struct i915_vma *
> -i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
> -				  struct i915_address_space *vm)
> +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,
> +				       struct i915_address_space *vm,
> +				       const struct i915_ggtt_view *view)
>  {
>  	struct i915_vma *vma;
>  
> -	vma = i915_gem_obj_to_vma(obj, vm);
> +	vma = i915_gem_obj_to_vma_view(obj, vm, view);
>  	if (!vma)
> -		vma = __i915_gem_vma_create(obj, vm);
> +		vma = __i915_gem_vma_create(obj, vm, view);
>  
>  	return vma;
>  }
> +
> +static inline
> +struct sg_table *i915_ggtt_view_pages(struct i915_vma *vma)
> +{
> +	struct sg_table *pages = ERR_PTR(-EINVAL);
> +
> +	if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)
> +		pages = vma->obj->pages;
> +	else
> +		WARN(1, "Not implemented!\n");
> +
> +	return pages;
> +}
> +
> +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
> +		   u32 flags)
> +{
> +	struct sg_table *pages = i915_ggtt_view_pages(vma);
> +
> +	if (pages && !IS_ERR(pages)) {
> +		vma->bind_vma(vma, pages, cache_level, flags);
> +
> +		if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) {
> +			sg_free_table(pages);
> +			kfree(pages);
> +		}
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index d0562d0..3534727 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -109,7 +109,18 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t;
>  #define GEN8_PPAT_ELLC_OVERRIDE		(0<<2)
>  #define GEN8_PPAT(i, x)			((uint64_t) (x) << ((i) * 8))
>  
> +enum i915_ggtt_view_type {
> +	I915_GGTT_VIEW_NORMAL = 0,
> +};
> +
> +struct i915_ggtt_view {
> +	enum i915_ggtt_view_type type;
> +};
> +
> +extern const struct i915_ggtt_view i915_ggtt_view_normal;
> +
>  enum i915_cache_level;
> +
>  /**
>   * A VMA represents a GEM BO that is bound into an address space. Therefore, a
>   * VMA's presence cannot be guaranteed before binding, or after unbinding the
> @@ -129,6 +140,15 @@ struct i915_vma {
>  #define PTE_READ_ONLY	(1<<2)
>  	unsigned int bound : 4;
>  
> +	/**
> +	 * Support different GGTT views into the same object.
> +	 * This means there can be multiple VMA mappings per object and per VM.
> +	 * i915_ggtt_view_type is used to distinguish between those entries.
> +	 * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also
> +	 * assumed in GEM functions which take no ggtt view parameter.
> +	 */
> +	struct i915_ggtt_view ggtt_view;
> +
>  	/** This object's place on the active/inactive lists */
>  	struct list_head mm_list;
>  
> @@ -161,7 +181,7 @@ struct i915_vma {
>  	 * setting the valid PTE entries to a reserved scratch page. */
>  	void (*unbind_vma)(struct i915_vma *vma);
>  	/* Map an object into an address space with the given cache flags. */
> -	void (*bind_vma)(struct i915_vma *vma,
> +	void (*bind_vma)(struct i915_vma *vma, struct sg_table *pages,
>  			 enum i915_cache_level cache_level,
>  			 u32 flags);
>  };
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 89a2f3d..77f1bdc 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -717,10 +717,8 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err,
>  			break;
>  
>  		list_for_each_entry(vma, &obj->vma_list, vma_link)
> -			if (vma->vm == vm && vma->pin_count > 0) {
> +			if (vma->vm == vm && vma->pin_count > 0)
>  				capture_bo(err++, vma);
> -				break;

Not fully sure about this one, but can't hurt I guess.

> -			}
>  	}
>  
>  	return err - first;
> @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
>  
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
>  		list_for_each_entry(vma, &obj->vma_list, vma_link)
> -			if (vma->vm == vm && vma->pin_count > 0) {
> +			if (vma->vm == vm && vma->pin_count > 0)
>  				i++;
> -				break;
> -			}
>  	}
>  	error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx];
>  
> -- 
> 2.1.1
> 

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



More information about the Intel-gfx mailing list