Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec7adb6e authored by Joonas Lahtinen's avatar Joonas Lahtinen Committed by Daniel Vetter
Browse files

drm/i915: Do not use ggtt_view with (aliasing) PPGTT 



GGTT views are only applicable when dealing with GGTT. Change the code to
reject ggtt_view where it should not be used and require it when it should
be.

v2:
- Dropped _ppgtt_ infixes, allow both types to be passed
- Disregard other but normal views when no view is specified
- More checks that valid parameters are passed
- More readable error checking

v3:
- Prefer WARN_ONCE over BUG_ON when there is code path for failure

Signed-off-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
[danvet: Drop unecessary forward decl from earlier patch iterations.]
[danvet: Remove unused variable spotted by Tvrtko.]
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent aca5e361

drivers/gpu/drm/i915/i915_drv.h

+38 −62
Original line number Diff line number Diff line
@@ -2623,20 +2623,16 @@ 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,

int __must_check

i915_gem_object_pin(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,

		    uint64_t flags);

int __must_check

i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,

			 const struct i915_ggtt_view *view,

			 uint32_t alignment,

				     uint64_t flags)

{

	return i915_gem_object_pin_view(obj, vm, alignment, flags,

						&i915_ggtt_view_normal);

}

			 uint64_t flags);



int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,

		  u32 flags);
@@ -2800,60 +2796,46 @@ 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,

unsigned long

i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,

			      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)

unsigned long

i915_gem_obj_offset(struct drm_i915_gem_object *o,

		    struct i915_address_space *vm);

static inline unsigned long

i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)

{

	return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);

	return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL);

}



bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);

bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,

			     struct i915_address_space *vm,

bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,

				  enum i915_ggtt_view_type view);

static inline

bool i915_gem_obj_bound(struct drm_i915_gem_object *o,

			struct i915_address_space *vm)

{

	return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);

}

			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)

{

	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,

i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,

		    struct i915_address_space *vm);

struct i915_vma *

i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,

			  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)

{

	return i915_gem_obj_lookup_or_create_vma_view(obj, vm,

						&i915_ggtt_view_normal);

}

				  struct i915_address_space *vm);

struct i915_vma *

i915_gem_obj_lookup_or_create_ggtt_vma(struct drm_i915_gem_object *obj,

				       const struct i915_ggtt_view *view);



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) {

	struct i915_vma *vma;

	list_for_each_entry(vma, &obj->vma_list, vma_link)

		if (vma->pin_count > 0)

			return true;

	return false;

static inline struct i915_vma *

i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj)

{

	return i915_gem_obj_to_ggtt_view(obj, &i915_ggtt_view_normal);

}

bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj);



/* Some GGTT VM helpers */

#define i915_obj_to_ggtt(obj) \
@@ -2876,13 +2858,7 @@ i915_vm_to_ppgtt(struct i915_address_space *vm) 


static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *obj)

{

	return i915_gem_obj_bound(obj, i915_obj_to_ggtt(obj));

}



static inline unsigned long

i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *obj)

{

	return i915_gem_obj_offset(obj, i915_obj_to_ggtt(obj));

	return i915_gem_obj_ggtt_bound_view(obj, I915_GGTT_VIEW_NORMAL);

}



static inline unsigned long

drivers/gpu/drm/i915/i915_gem.c

+135 −39
Original line number Diff line number Diff line
@@ -3518,9 +3518,9 @@ static bool i915_gem_valid_gtt_space(struct i915_vma *vma, 
static struct i915_vma *

i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,

			   struct i915_address_space *vm,

			   const struct i915_ggtt_view *ggtt_view,

			   unsigned alignment,

			   uint64_t flags,

			   const struct i915_ggtt_view *view)

			   uint64_t flags)

{

	struct drm_device *dev = obj->base.dev;

	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3532,6 +3532,9 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, 
	struct i915_vma *vma;

	int ret;



	if(WARN_ON(i915_is_ggtt(vm) != !!ggtt_view))

		return ERR_PTR(-EINVAL);



	fence_size = i915_gem_get_gtt_size(dev,

					   obj->base.size,

					   obj->tiling_mode);
@@ -3570,7 +3573,9 @@ 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_view(obj, vm, view);

	vma = ggtt_view ? i915_gem_obj_lookup_or_create_ggtt_vma(obj, ggtt_view) :

			  i915_gem_obj_lookup_or_create_vma(obj, vm);



	if (IS_ERR(vma))

		goto err_unpin;
@@ -4167,12 +4172,12 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags) 
	return false;

}



int

i915_gem_object_pin_view(struct drm_i915_gem_object *obj,

static int

i915_gem_object_do_pin(struct drm_i915_gem_object *obj,

		       struct i915_address_space *vm,

		       const struct i915_ggtt_view *ggtt_view,

		       uint32_t alignment,

			 uint64_t flags,

			 const struct i915_ggtt_view *view)

		       uint64_t flags)

{

	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;

	struct i915_vma *vma;
@@ -4188,17 +4193,29 @@ i915_gem_object_pin_view(struct drm_i915_gem_object *obj, 
	if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE))

		return -EINVAL;



	vma = i915_gem_obj_to_vma_view(obj, vm, view);

	if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view))

		return -EINVAL;



	vma = ggtt_view ? i915_gem_obj_to_ggtt_view(obj, ggtt_view) :

			  i915_gem_obj_to_vma(obj, vm);



	if (IS_ERR(vma))

		return PTR_ERR(vma);



	if (vma) {

		if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT))

			return -EBUSY;



		if (i915_vma_misplaced(vma, alignment, flags)) {

			unsigned long offset;

			offset = ggtt_view ? i915_gem_obj_ggtt_offset_view(obj, ggtt_view->type) :

					     i915_gem_obj_offset(obj, vm);

			WARN(vma->pin_count,

			     "bo is already pinned with incorrect alignment:"

			     "bo is already pinned in %s with incorrect alignment:"

			     " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d,"

			     " obj->map_and_fenceable=%d\n",

			     i915_gem_obj_offset_view(obj, vm, view->type),

			     ggtt_view ? "ggtt" : "ppgtt",

			     offset,

			     alignment,

			     !!(flags & PIN_MAPPABLE),

			     obj->map_and_fenceable);
@@ -4212,8 +4229,8 @@ i915_gem_object_pin_view(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, view);

		vma = i915_gem_object_bind_to_vm(obj, vm, ggtt_view, alignment,

						 flags);

		if (IS_ERR(vma))

			return PTR_ERR(vma);

	}
@@ -4254,6 +4271,30 @@ i915_gem_object_pin_view(struct drm_i915_gem_object *obj, 
	return 0;

}



int

i915_gem_object_pin(struct drm_i915_gem_object *obj,

		    struct i915_address_space *vm,

		    uint32_t alignment,

		    uint64_t flags)

{

	return i915_gem_object_do_pin(obj, vm,

				      i915_is_ggtt(vm) ? &i915_ggtt_view_normal : NULL,

				      alignment, flags);

}



int

i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,

			 const struct i915_ggtt_view *view,

			 uint32_t alignment,

			 uint64_t flags)

{

	if (WARN_ONCE(!view, "no view specified"))

		return -EINVAL;



	return i915_gem_object_do_pin(obj, i915_obj_to_ggtt(obj), view,

				      alignment, flags);

}



void

i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)

{
@@ -4559,15 +4600,32 @@ 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_view(struct drm_i915_gem_object *obj,

					  struct i915_address_space *vm,

struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,

				     struct i915_address_space *vm)

{

	struct i915_vma *vma;

	list_for_each_entry(vma, &obj->vma_list, vma_link) {

		if (i915_is_ggtt(vma->vm) &&

		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)

			continue;

		if (vma->vm == vm)

			return vma;

	}

	return NULL;

}



struct i915_vma *i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,

					   const struct i915_ggtt_view *view)

{

	struct i915_address_space *ggtt = i915_obj_to_ggtt(obj);

	struct i915_vma *vma;



	if (WARN_ONCE(!view, "no view specified"))

		return ERR_PTR(-EINVAL);



	list_for_each_entry(vma, &obj->vma_list, vma_link)

		if (vma->vm == vm && vma->ggtt_view.type == view->type)

		if (vma->vm == ggtt && vma->ggtt_view.type == view->type)

			return vma;



	return NULL;

}
@@ -5176,9 +5234,9 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) 
}



/* All the new VM stuff */

unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,

				       struct i915_address_space *vm,

				       enum i915_ggtt_view_type view)

unsigned long

i915_gem_obj_offset(struct drm_i915_gem_object *o,

		    struct i915_address_space *vm)

{

	struct drm_i915_private *dev_priv = o->base.dev->dev_private;

	struct i915_vma *vma;
@@ -5186,23 +5244,57 @@ unsigned long i915_gem_obj_offset_view(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 && vma->ggtt_view.type == view)

		if (i915_is_ggtt(vma->vm) &&

		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)

			continue;

		if (vma->vm == vm)

			return vma->node.start;



	}



	WARN(1, "%s vma for this object not found.\n",

	     i915_is_ggtt(vm) ? "global" : "ppgtt");

	return -1;

}



bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,

			     struct i915_address_space *vm,

unsigned long

i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,

			      enum i915_ggtt_view_type view)

{

	struct i915_address_space *ggtt = i915_obj_to_ggtt(o);

	struct i915_vma *vma;



	list_for_each_entry(vma, &o->vma_list, vma_link)

		if (vma->vm == ggtt && vma->ggtt_view.type == view)

			return vma->node.start;



	WARN(1, "global vma for this object not found.\n");

	return -1;

}



bool i915_gem_obj_bound(struct drm_i915_gem_object *o,

			struct i915_address_space *vm)

{

	struct i915_vma *vma;



	list_for_each_entry(vma, &o->vma_list, vma_link) {

		if (i915_is_ggtt(vma->vm) &&

		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)

			continue;

		if (vma->vm == vm && drm_mm_node_allocated(&vma->node))

			return true;

	}



	return false;

}



bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,

				  enum i915_ggtt_view_type view)

{

	struct i915_address_space *ggtt = i915_obj_to_ggtt(o);

	struct i915_vma *vma;



	list_for_each_entry(vma, &o->vma_list, vma_link)

		if (vma->vm == vm &&

		if (vma->vm == ggtt &&

		    vma->ggtt_view.type == view &&

		    drm_mm_node_allocated(&vma->node))

			return true;
@@ -5231,10 +5323,13 @@ unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, 


	BUG_ON(list_empty(&o->vma_list));



	list_for_each_entry(vma, &o->vma_list, vma_link)

	list_for_each_entry(vma, &o->vma_list, vma_link) {

		if (i915_is_ggtt(vma->vm) &&

		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)

			continue;

		if (vma->vm == vm)

			return vma->node.size;



	}

	return 0;

}
@@ -5334,15 +5429,16 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr) 
	return NOTIFY_DONE;

}



struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj)

bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj)

{

	struct i915_address_space *ggtt = i915_obj_to_ggtt(obj);

	struct i915_vma *vma;



	list_for_each_entry(vma, &obj->vma_list, vma_link)

		if (vma->vm == ggtt &&

		    vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)

			return vma;



	return NULL;

	list_for_each_entry(vma, &obj->vma_list, vma_link) {

		if (i915_is_ggtt(vma->vm) &&

		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)

			continue;

		if (vma->pin_count > 0)

			return true;

	}

	return false;

}

drivers/gpu/drm/i915/i915_gem_gtt.c

+53 −18
Original line number Diff line number Diff line
@@ -67,8 +67,9 @@ 
 * i915_ggtt_view_type and struct i915_ggtt_view.

 *

 * A new flavour of core GEM functions which work with GGTT bound objects were

 * added with the _view suffix. They take the struct i915_ggtt_view parameter

 * encapsulating all metadata required to implement a view.

 * added with the _ggtt_ infix, and sometimes with _view postfix to avoid

 * renaming  in large amounts of code. They take the struct i915_ggtt_view

 * parameter encapsulating all metadata required to implement a view.

 *

 * As a helper for callers which are only interested in the normal view,

 * globally const i915_ggtt_view_normal singleton instance exists. All old core
@@ -1726,11 +1727,15 @@ static void ggtt_bind_vma(struct i915_vma *vma, 
	struct drm_device *dev = vma->vm->dev;

	struct drm_i915_private *dev_priv = dev->dev_private;

	struct drm_i915_gem_object *obj = vma->obj;

	struct sg_table *pages = obj->pages;



	/* Currently applicable only to VLV */

	if (obj->gt_ro)

		flags |= PTE_READ_ONLY;



	if (i915_is_ggtt(vma->vm))

		pages = vma->ggtt_view.pages;



	/* If there is no aliasing PPGTT, or the caller needs a global mapping,

	 * or we have a global mapping already but the cacheability flags have

	 * changed, set the global PTEs.
@@ -1745,7 +1750,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, vma->ggtt_view.pages,

			vma->vm->insert_entries(vma->vm, pages,

						vma->node.start,

						cache_level, flags);

			vma->bound |= GLOBAL_BIND;
@@ -1756,8 +1761,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, 
	    (!(vma->bound & LOCAL_BIND) ||

	     (cache_level != obj->cache_level))) {

		struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt;

		appgtt->base.insert_entries(&appgtt->base,

					    vma->ggtt_view.pages,

		appgtt->base.insert_entries(&appgtt->base, pages,

					    vma->node.start,

					    cache_level, flags);

		vma->bound |= LOCAL_BIND;
@@ -2331,23 +2335,28 @@ int i915_gem_gtt_init(struct drm_device *dev) 
	return 0;

}



static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,

static struct i915_vma *

__i915_gem_vma_create(struct drm_i915_gem_object *obj,

		      struct i915_address_space *vm,

					      const struct i915_ggtt_view *view)

		      const struct i915_ggtt_view *ggtt_view)

{

	struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);

	if (vma == NULL)

		return ERR_PTR(-ENOMEM);



	if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view))

		return ERR_PTR(-EINVAL);



	INIT_LIST_HEAD(&vma->vma_link);

	INIT_LIST_HEAD(&vma->mm_list);

	INIT_LIST_HEAD(&vma->exec_list);

	vma->vm = vm;

	vma->obj = obj;

	vma->ggtt_view = *view;



	if (INTEL_INFO(vm->dev)->gen >= 6) {

		if (i915_is_ggtt(vm)) {

			vma->ggtt_view = *ggtt_view;



			vma->unbind_vma = ggtt_unbind_vma;

			vma->bind_vma = ggtt_bind_vma;

		} else {
@@ -2356,6 +2365,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, 
		}

	} else {

		BUG_ON(!i915_is_ggtt(vm));

		vma->ggtt_view = *ggtt_view;

		vma->unbind_vma = i915_ggtt_unbind_vma;

		vma->bind_vma = i915_ggtt_bind_vma;

	}
@@ -2368,21 +2378,44 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, 
}



struct i915_vma *

i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,

				       struct i915_address_space *vm,

i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,

				  struct i915_address_space *vm)

{

	struct i915_vma *vma;



	vma = i915_gem_obj_to_vma(obj, vm);

	if (!vma)

		vma = __i915_gem_vma_create(obj, vm,

					    i915_is_ggtt(vm) ? &i915_ggtt_view_normal : NULL);



	return vma;

}



struct i915_vma *

i915_gem_obj_lookup_or_create_ggtt_vma(struct drm_i915_gem_object *obj,

				       const struct i915_ggtt_view *view)

{

	struct i915_address_space *ggtt = i915_obj_to_ggtt(obj);

	struct i915_vma *vma;



	vma = i915_gem_obj_to_vma_view(obj, vm, view);

	if (WARN_ON(!view))

		return ERR_PTR(-EINVAL);



	vma = i915_gem_obj_to_ggtt_view(obj, view);



	if (IS_ERR(vma))

		return vma;



	if (!vma)

		vma = __i915_gem_vma_create(obj, vm, view);

		vma = __i915_gem_vma_create(obj, ggtt, view);



	return vma;



}





static inline

int i915_get_vma_pages(struct i915_vma *vma)

int i915_get_ggtt_vma_pages(struct i915_vma *vma)

{

	if (vma->ggtt_view.pages)

		return 0;
@@ -2394,7 +2427,7 @@ int i915_get_vma_pages(struct i915_vma *vma) 
			  vma->ggtt_view.type);



	if (!vma->ggtt_view.pages) {

		DRM_ERROR("Failed to get pages for VMA view type %u!\n",

		DRM_ERROR("Failed to get pages for GGTT view type %u!\n",

			  vma->ggtt_view.type);

		return -EINVAL;

	}
@@ -2415,10 +2448,12 @@ int i915_get_vma_pages(struct i915_vma *vma) 
int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,

		  u32 flags)

{

	int ret = i915_get_vma_pages(vma);

	if (i915_is_ggtt(vma->vm)) {

		int ret = i915_get_ggtt_vma_pages(vma);



		if (ret)

			return ret;

	}



	vma->bind_vma(vma, cache_level, flags);