[Intel-gfx] [PATCH] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

Joonas Lahtinen joonas.lahtinen at linux.intel.com
Mon Mar 9 07:34:33 PDT 2015


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.

Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  91 ++++++++++++------------
 drivers/gpu/drm/i915/i915_gem.c     | 134 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/i915_gem_gtt.c |  41 ++++++++---
 3 files changed, 184 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd7651b..6bc67fb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2608,20 +2608,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,
-					  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)
-{
-	return i915_gem_object_pin_view(obj, vm, alignment, flags,
-						&i915_ggtt_view_normal);
-}
+int __must_check
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+		    struct i915_address_space *vm,
+		    uint32_t alignment,
+		    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);
 
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 		  u32 flags);
@@ -2785,41 +2781,55 @@ 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)
+static inline bool i915_is_ggtt(struct i915_address_space *vm);
+
+unsigned long
+i915_gem_obj_ppgtt_offset(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_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,
-			     enum i915_ggtt_view_type view);
-static inline
-bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
-			struct i915_address_space *vm)
+
+static inline unsigned long
+i915_gem_obj_offset(struct drm_i915_gem_object *o,
+		    struct i915_address_space *vm)
 {
-	return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);
+	if (i915_is_ggtt(vm))
+		return i915_gem_obj_ggtt_offset(o);
+	return i915_gem_obj_ppgtt_offset(o, vm);
 }
 
+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);
+bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
+			     enum i915_ggtt_view_type view);
+
 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,
+struct i915_vma *i915_gem_obj_to_ppgtt_vma(struct drm_i915_gem_object *obj,
+					   struct i915_address_space *vm);
+struct i915_vma *i915_gem_obj_to_ggtt_vma(struct drm_i915_gem_object *obj,
 					  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);
+	if (i915_is_ggtt(vm))
+		return i915_gem_obj_to_ggtt_vma(obj, &i915_ggtt_view_normal);
+	return i915_gem_obj_to_ppgtt_vma(obj, vm);
 }
 
 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_ppgtt_vma(struct drm_i915_gem_object *obj,
+					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);
 
 static inline
@@ -2827,8 +2837,9 @@ 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);
+	if (i915_is_ggtt(vm))
+		return i915_gem_obj_lookup_or_create_ggtt_vma(obj, &i915_ggtt_view_normal);
+	return i915_gem_obj_lookup_or_create_ppgtt_vma(obj, vm);
 }
 
 struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj);
@@ -2861,13 +2872,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
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3831cc0..d6edbef 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3517,9 +3517,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;
@@ -3531,6 +3531,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);
@@ -3569,7 +3572,11 @@ 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);
+	if (i915_is_ggtt(vm))
+		vma = i915_gem_obj_lookup_or_create_ggtt_vma(obj, ggtt_view);
+	else
+		vma = i915_gem_obj_lookup_or_create_ppgtt_vma(obj, vm);
+
 	if (IS_ERR(vma))
 		goto err_unpin;
 
@@ -4166,14 +4173,15 @@ 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,
-			 struct i915_address_space *vm,
-			 uint32_t alignment,
-			 uint64_t flags,
-			 const struct i915_ggtt_view *view)
+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)
 {
 	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
+	const bool is_ggtt = i915_is_ggtt(vm);
 	struct i915_vma *vma;
 	unsigned bound;
 	int ret;
@@ -4181,23 +4189,32 @@ i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
 	if (WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base))
 		return -ENODEV;
 
-	if (WARN_ON(flags & (PIN_GLOBAL | PIN_MAPPABLE) && !i915_is_ggtt(vm)))
+	if (WARN_ON(flags & (PIN_GLOBAL | PIN_MAPPABLE)))
 		return -EINVAL;
 
 	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(!is_ggtt ^ !!ggtt_view))
+		return -EINVAL;
+
+	vma = is_ggtt ? i915_gem_obj_to_ggtt_vma(obj, ggtt_view) :
+			i915_gem_obj_to_ppgtt_vma(obj, vm);
+
 	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 = is_ggtt ? i915_gem_obj_ggtt_offset_view(obj, ggtt_view->type) :
+					   i915_gem_obj_ppgtt_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),
+			     is_ggtt ? "ggtt" : "ppgtt",
+			     offset,
 			     alignment,
 			     !!(flags & PIN_MAPPABLE),
 			     obj->map_and_fenceable);
@@ -4211,8 +4228,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);
 	}
@@ -4253,6 +4270,27 @@ 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)
+{
+	const struct i915_ggtt_view *ggtt_view;
+	ggtt_view = i915_is_ggtt(vm) ? &i915_ggtt_view_normal : NULL;
+	return i915_gem_object_do_pin(obj, vm, ggtt_view, alignment, flags);
+}
+
+int
+i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
+			 const struct i915_ggtt_view *ggtt_view,
+			 uint32_t alignment,
+			 uint64_t flags)
+{
+	return i915_gem_object_do_pin(obj, i915_obj_to_ggtt(obj), ggtt_view,
+				      alignment, flags);
+}
+
 void
 i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)
 {
@@ -4558,15 +4596,24 @@ 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,
-					  const struct i915_ggtt_view *view)
+struct i915_vma *i915_gem_obj_to_ppgtt_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 (vma->vm == vm && vma->ggtt_view.type == view->type)
+		if (vma->vm == vm)
 			return vma;
+	return NULL;
+}
 
+struct i915_vma *i915_gem_obj_to_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;
+	list_for_each_entry(vma, &obj->vma_list, vma_link)
+		if (vma->vm == ggtt && vma->ggtt_view.type == view->type)
+			return vma;
 	return NULL;
 }
 
@@ -5161,33 +5208,62 @@ 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_ppgtt_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;
 
+	BUG_ON(vm == i915_obj_to_ggtt(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 (vma->vm == vm)
 			return vma->node.start;
+	}
+
+	WARN(1, "ppgtt vma for this object not found.\n");
+	return -1;
+}
+
+unsigned long
+i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+			      enum i915_ggtt_view_type view)
+{
+	struct drm_i915_private *dev_priv = o->base.dev->dev_private;
+	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, "%s vma for this object not found.\n",
-	     i915_is_ggtt(vm) ? "global" : "ppgtt");
+
+	WARN(1, "global vma for this object not found.\n");
 	return -1;
 }
 
-bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,
-			     struct i915_address_space *vm,
-			     enum i915_ggtt_view_type view)
+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 (vma->vm == vm &&
+		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 == ggtt &&
 		    vma->ggtt_view.type == view &&
 		    drm_mm_node_allocated(&vma->node))
 			return true;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2034f7c..eb46804 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1757,7 +1757,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->ggtt_view.pages,
+					    vma->obj->pages,
 					    vma->node.start,
 					    cache_level, flags);
 		vma->bound |= LOCAL_BIND;
@@ -2331,9 +2331,10 @@ 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,
-					      struct i915_address_space *vm,
-					      const struct i915_ggtt_view *view)
+static struct i915_vma *
+__i915_gem_vma_create(struct drm_i915_gem_object *obj,
+		      struct i915_address_space *vm,
+		      const struct i915_ggtt_view *ggtt_view)
 {
 	struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);
 	if (vma == NULL)
@@ -2344,18 +2345,23 @@ 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;
 
 	if (INTEL_INFO(vm->dev)->gen >= 6) {
 		if (i915_is_ggtt(vm)) {
+			BUG_ON(!ggtt_view);
+			vma->ggtt_view = *ggtt_view;
+
 			vma->unbind_vma = ggtt_unbind_vma;
 			vma->bind_vma = ggtt_bind_vma;
 		} else {
+			BUG_ON(ggtt_view);
 			vma->unbind_vma = ppgtt_unbind_vma;
 			vma->bind_vma = ppgtt_bind_vma;
 		}
 	} else {
 		BUG_ON(!i915_is_ggtt(vm));
+		BUG_ON(!ggtt_view);
+		vma->ggtt_view = *ggtt_view;
 		vma->unbind_vma = i915_ggtt_unbind_vma;
 		vma->bind_vma = i915_ggtt_bind_vma;
 	}
@@ -2368,23 +2374,38 @@ 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_ppgtt_vma(struct drm_i915_gem_object *obj,
+					struct i915_address_space *vm)
+{
+	struct i915_vma *vma;
+
+	vma = i915_gem_obj_to_ppgtt_vma(obj, vm);
+	if (!vma)
+		vma = __i915_gem_vma_create(obj, vm, 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);
+	vma = i915_gem_obj_to_ggtt_vma(obj, view);
 	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)
 {
-	if (vma->ggtt_view.pages)
+	if (!i915_is_ggtt(vma->vm) || vma->ggtt_view.pages)
 		return 0;
 
 	if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)
-- 
1.9.3





More information about the Intel-gfx mailing list