[PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

If these flags are on the object level it will be more difficult to allow
for multiple VMAs per object.

v2: Simplification and cleanup after code review comments (Chris Wilson).

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  2 +-
 drivers/gpu/drm/i915/i915_drv.h            |  2 --
 drivers/gpu/drm/i915/i915_gem.c            |  4 ++--
 drivers/gpu/drm/i915/i915_gem_context.c    | 10 +++++-----
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 +++------
 drivers/gpu/drm/i915/i915_gem_gtt.c        | 24 ++++++++++++------------
 drivers/gpu/drm/i915/i915_gem_gtt.h        |  8 ++++++--
 drivers/gpu/drm/i915/i915_gem_stolen.c     |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c      | 10 ++++++----
 9 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d8d08fc..6a15fb6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -118,7 +118,7 @@ static const char *get_tiling_flag(struct drm_i915_gem_object *obj)
 
 static inline const char *get_global_flag(struct drm_i915_gem_object *obj)
 {
-	return obj->has_global_gtt_mapping ? "g" : " ";
+	return i915_gem_obj_to_ggtt(obj) ? "g" : " ";
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ce3f01f..a830b85 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1965,8 +1965,6 @@ struct drm_i915_gem_object {
 	unsigned long gt_ro:1;
 	unsigned int cache_level:3;
 
-	unsigned int has_aliasing_ppgtt_mapping:1;
-	unsigned int has_global_gtt_mapping:1;
 	unsigned int has_dma_mapping:1;
 
 	unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index edab8f1..255a431 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3700,7 +3700,7 @@ 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,
-					      obj->has_global_gtt_mapping ? GLOBAL_BIND : 0);
+						vma->bound & GLOBAL_BIND);
 	}
 
 	list_for_each_entry(vma, &obj->vma_list, vma_link)
@@ -4096,7 +4096,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 			return PTR_ERR(vma);
 	}
 
-	if (flags & PIN_GLOBAL && !obj->has_global_gtt_mapping)
+	if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND))
 		vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
 
 	vma->pin_count++;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index bf41f41..5ff6e94 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -525,6 +525,7 @@ static int do_switch(struct intel_engine_cs *ring,
 	struct intel_context *from = ring->last_context;
 	u32 hw_flags = 0;
 	bool uninitialized = false;
+	struct i915_vma *vma;
 	int ret, i;
 
 	if (from != NULL && ring == &dev_priv->ring[RCS]) {
@@ -574,11 +575,10 @@ static int do_switch(struct intel_engine_cs *ring,
 	if (ret)
 		goto unpin_out;
 
-	if (!to->legacy_hw_ctx.rcs_state->has_global_gtt_mapping) {
-		struct i915_vma *vma = i915_gem_obj_to_vma(to->legacy_hw_ctx.rcs_state,
-							   &dev_priv->gtt.base);
-		vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level, GLOBAL_BIND);
-	}
+	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);
 
 	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 f8cc935..8d56d5b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -357,12 +357,9 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 	 * through the ppgtt for non_secure batchbuffers. */
 	if (unlikely(IS_GEN6(dev) &&
 	    reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
-	    !target_i915_obj->has_global_gtt_mapping)) {
-		struct i915_vma *vma =
-			list_first_entry(&target_i915_obj->vma_list,
-					 typeof(*vma), vma_link);
-		vma->bind_vma(vma, target_i915_obj->cache_level, GLOBAL_BIND);
-	}
+	    !(target_vma->bound & GLOBAL_BIND)))
+		target_vma->bind_vma(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 b80ab86..814a805 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1324,7 +1324,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 		 * Unfortunately above, we've just wiped out the mappings
 		 * without telling our object about it. So we need to fake it.
 		 */
-		obj->has_global_gtt_mapping = 0;
+		vma->bound &= ~GLOBAL_BIND;
 		vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
 	}
 
@@ -1521,7 +1521,7 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma,
 
 	BUG_ON(!i915_is_ggtt(vma->vm));
 	intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags);
-	vma->obj->has_global_gtt_mapping = 1;
+	vma->bound = GLOBAL_BIND;
 }
 
 static void i915_ggtt_clear_range(struct i915_address_space *vm,
@@ -1540,7 +1540,7 @@ static void i915_ggtt_unbind_vma(struct i915_vma *vma)
 	const unsigned int size = vma->obj->base.size >> PAGE_SHIFT;
 
 	BUG_ON(!i915_is_ggtt(vma->vm));
-	vma->obj->has_global_gtt_mapping = 0;
+	vma->bound = 0;
 	intel_gtt_clear_range(first, size);
 }
 
@@ -1568,24 +1568,24 @@ static void ggtt_bind_vma(struct i915_vma *vma,
 	 * flags. At all other times, the GPU will use the aliasing PPGTT.
 	 */
 	if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) {
-		if (!obj->has_global_gtt_mapping ||
+		if (!(vma->bound & GLOBAL_BIND) ||
 		    (cache_level != obj->cache_level)) {
 			vma->vm->insert_entries(vma->vm, obj->pages,
 						vma->node.start,
 						cache_level, flags);
-			obj->has_global_gtt_mapping = 1;
+			vma->bound |= GLOBAL_BIND;
 		}
 	}
 
 	if (dev_priv->mm.aliasing_ppgtt &&
-	    (!obj->has_aliasing_ppgtt_mapping ||
+	    (!(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->obj->pages,
 					    vma->node.start,
 					    cache_level, flags);
-		vma->obj->has_aliasing_ppgtt_mapping = 1;
+		vma->bound |= LOCAL_BIND;
 	}
 }
 
@@ -1595,21 +1595,21 @@ static void ggtt_unbind_vma(struct i915_vma *vma)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj = vma->obj;
 
-	if (obj->has_global_gtt_mapping) {
+	if (vma->bound & GLOBAL_BIND) {
 		vma->vm->clear_range(vma->vm,
 				     vma->node.start,
 				     obj->base.size,
 				     true);
-		obj->has_global_gtt_mapping = 0;
+		vma->bound &= ~GLOBAL_BIND;
 	}
 
-	if (obj->has_aliasing_ppgtt_mapping) {
+	if (vma->bound & LOCAL_BIND) {
 		struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt;
 		appgtt->base.clear_range(&appgtt->base,
 					 vma->node.start,
 					 obj->base.size,
 					 true);
-		obj->has_aliasing_ppgtt_mapping = 0;
+		vma->bound &= ~LOCAL_BIND;
 	}
 }
 
@@ -1687,7 +1687,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev,
 			DRM_DEBUG_KMS("Reservation failed: %i\n", ret);
 			return ret;
 		}
-		obj->has_global_gtt_mapping = 1;
+		vma->bound |= GLOBAL_BIND;
 	}
 
 	dev_priv->gtt.base.start = start;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index d5c14af..d0562d0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -123,6 +123,12 @@ struct i915_vma {
 	struct drm_i915_gem_object *obj;
 	struct i915_address_space *vm;
 
+	/** Flags and address space this VMA is bound to */
+#define GLOBAL_BIND	(1<<0)
+#define LOCAL_BIND	(1<<1)
+#define PTE_READ_ONLY	(1<<2)
+	unsigned int bound : 4;
+
 	/** This object's place on the active/inactive lists */
 	struct list_head mm_list;
 
@@ -155,8 +161,6 @@ 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. */
-#define GLOBAL_BIND (1<<0)
-#define PTE_READ_ONLY (1<<1)
 	void (*bind_vma)(struct i915_vma *vma,
 			 enum i915_cache_level cache_level,
 			 u32 flags);
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 85fda6b..c388918 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -533,7 +533,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 		}
 	}
 
-	obj->has_global_gtt_mapping = 1;
+	vma->bound |= GLOBAL_BIND;
 
 	list_add_tail(&obj->global_list, &dev_priv->mm.bound_list);
 	list_add_tail(&vma->mm_list, &ggtt->inactive_list);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 053d99e..80f0c80 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -567,6 +567,7 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
 			 struct i915_address_space *vm)
 {
 	struct drm_i915_error_object *dst;
+	struct i915_vma *vma = NULL;
 	int num_pages;
 	bool use_ggtt;
 	int i = 0;
@@ -587,16 +588,17 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
 		dst->gtt_offset = -1;
 
 	reloc_offset = dst->gtt_offset;
+	if (i915_is_ggtt(vm))
+		vma = i915_gem_obj_to_ggtt(src);
 	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
-		    i915_is_ggtt(vm) &&
-		    src->has_global_gtt_mapping &&
-		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
+		   vma && (vma->bound & GLOBAL_BIND) &&
+		   reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
 
 	/* Cannot access stolen address directly, try to use the aperture */
 	if (src->stolen) {
 		use_ggtt = true;
 
-		if (!src->has_global_gtt_mapping)
+		if (!(vma && vma->bound & GLOBAL_BIND))
 			goto unwind;
 
 		reloc_offset = i915_gem_obj_ggtt_offset(src);
-- 
2.1.1

[RFC] drm/i915: Prepare for the idea there can be multiple VMA per VM.

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

With multiple VMA per object and per address space we need some sort of
key to distinguish between then for which there is a new 'id' field.

Old API which does not know or care about this will use a default id.

I have tried to identify all places which assume there can only be one.
It compiles and runs the most basic stuff but this is more for
soliciting feedback and review.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c       | 27 +++++++++++++--------------
 drivers/gpu/drm/i915/i915_gem_gtt.c   |  6 ++++--
 drivers/gpu/drm/i915/i915_gem_gtt.h   |  7 +++++++
 drivers/gpu/drm/i915/i915_gpu_error.c |  8 ++------
 5 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6a15fb6..73d98cc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -156,8 +156,8 @@ 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, id: %u)",
+			   vma->node.start, vma->node.size, vma->id);
 	}
 	if (obj->stolen)
 		seq_printf(m, " (stolen: %08lx)", obj->stolen->start);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 255a431..7f283d8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2009,8 +2009,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
 			drm_gem_object_reference(&obj->base);
 
 			list_for_each_entry_safe(vma, v, &obj->vma_list, vma_link)
-				if (i915_vma_unbind(vma))
-					break;
+				i915_vma_unbind(vma);
 
 			if (i915_gem_object_put_pages(obj) == 0)
 				count += obj->base.size >> PAGE_SHIFT;
@@ -2211,17 +2210,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);
@@ -4493,7 +4489,7 @@ struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
 {
 	struct i915_vma *vma;
 	list_for_each_entry(vma, &obj->vma_list, vma_link)
-		if (vma->vm == vm)
+		if (vma->vm == vm && vma->id == VMA_ID_DEFAULT)
 			return vma;
 
 	return NULL;
@@ -5188,7 +5184,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->id == VMA_ID_DEFAULT)
 			return vma->node.start;
 
 	}
@@ -5336,9 +5332,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->id == VMA_ID_DEFAULT)
+			return vma;
+	}
 
-	return vma;
+	return NULL;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 814a805..e4ee2ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2127,7 +2127,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,
+					      unsigned int id)
 {
 	struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);
 	if (vma == NULL)
@@ -2138,6 +2139,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->id = id;
 
 	switch (INTEL_INFO(vm->dev)->gen) {
 	case 9:
@@ -2183,7 +2185,7 @@ i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
 
 	vma = i915_gem_obj_to_vma(obj, vm);
 	if (!vma)
-		vma = __i915_gem_vma_create(obj, vm);
+		vma = __i915_gem_vma_create(obj, vm, VMA_ID_DEFAULT);
 
 	return vma;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index d0562d0..66bc44b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -128,6 +128,13 @@ struct i915_vma {
 #define LOCAL_BIND	(1<<1)
 #define PTE_READ_ONLY	(1<<2)
 	unsigned int bound : 4;
+	/**
+	 * With multiple VMA mappings per object id distinguishes between
+	 * the ones in the same address space. The default one of zero is
+	 * for users of the legacy API.
+	 */
+#define VMA_ID_DEFAULT	(0)
+	unsigned int id;
 
 	/** This object's place on the active/inactive lists */
 	struct list_head mm_list;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 80f0c80..50f4294 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -719,10 +719,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;
-			}
 	}
 
 	return err - first;
@@ -1098,10 +1096,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

Re: [PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[Chris Wilson]

On Fri, Oct 24, 2014 at 12:42:33PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> If these flags are on the object level it will be more difficult to allow
> for multiple VMAs per object.
> 
> v2: Simplification and cleanup after code review comments (Chris Wilson).
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 053d99e..80f0c80 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -567,6 +567,7 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
>  			 struct i915_address_space *vm)
>  {
>  	struct drm_i915_error_object *dst;
> +	struct i915_vma *vma = NULL;
>  	int num_pages;
>  	bool use_ggtt;
>  	int i = 0;
> @@ -587,16 +588,17 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
>  		dst->gtt_offset = -1;
>  
>  	reloc_offset = dst->gtt_offset;
> +	if (i915_is_ggtt(vm))
> +		vma = i915_gem_obj_to_ggtt(src);
>  	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> -		    i915_is_ggtt(vm) &&
> -		    src->has_global_gtt_mapping &&
> -		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> +		   vma && (vma->bound & GLOBAL_BIND) &&
> +		   reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
>  
>  	/* Cannot access stolen address directly, try to use the aperture */
>  	if (src->stolen) {
>  		use_ggtt = true;
>  
> -		if (!src->has_global_gtt_mapping)
> +		if (!(vma && vma->bound & GLOBAL_BIND))
>  			goto unwind;

This looked odd as I have the vma being passed in:

static struct drm_i915_error_object *
i915_error_object_create(struct drm_i915_private *dev_priv,
                         struct i915_vma *vma);

but I presume the ppgtt error-capture is still outstanding.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[Daniel Vetter]

On Fri, Oct 24, 2014 at 01:09:08PM +0100, Chris Wilson wrote:
> On Fri, Oct 24, 2014 at 12:42:33PM +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > If these flags are on the object level it will be more difficult to allow
> > for multiple VMAs per object.
> > 
> > v2: Simplification and cleanup after code review comments (Chris Wilson).
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 053d99e..80f0c80 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -567,6 +567,7 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
> >  			 struct i915_address_space *vm)
> >  {
> >  	struct drm_i915_error_object *dst;
> > +	struct i915_vma *vma = NULL;
> >  	int num_pages;
> >  	bool use_ggtt;
> >  	int i = 0;
> > @@ -587,16 +588,17 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
> >  		dst->gtt_offset = -1;
> >  
> >  	reloc_offset = dst->gtt_offset;
> > +	if (i915_is_ggtt(vm))
> > +		vma = i915_gem_obj_to_ggtt(src);
> >  	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> > -		    i915_is_ggtt(vm) &&
> > -		    src->has_global_gtt_mapping &&
> > -		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> > +		   vma && (vma->bound & GLOBAL_BIND) &&
> > +		   reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> >  
> >  	/* Cannot access stolen address directly, try to use the aperture */
> >  	if (src->stolen) {
> >  		use_ggtt = true;
> >  
> > -		if (!src->has_global_gtt_mapping)
> > +		if (!(vma && vma->bound & GLOBAL_BIND))
> >  			goto unwind;
> 
> This looked odd as I have the vma being passed in:
> 
> static struct drm_i915_error_object *
> i915_error_object_create(struct drm_i915_private *dev_priv,
>                          struct i915_vma *vma);
> 
> but I presume the ppgtt error-capture is still outstanding.

Oh meh, this is probably one of those tasks that I've asked Jesse to close
in one of the "fuck this all" rage-quits in the past few months ... Indeed
both code and testcase seem to not have materialized afaics.

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

Re: [PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[Daniel Vetter]

On Fri, Oct 24, 2014 at 04:43:08PM +0200, Daniel Vetter wrote:
> On Fri, Oct 24, 2014 at 01:09:08PM +0100, Chris Wilson wrote:
> > On Fri, Oct 24, 2014 at 12:42:33PM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > 
> > > If these flags are on the object level it will be more difficult to allow
> > > for multiple VMAs per object.
> > > 
> > > v2: Simplification and cleanup after code review comments (Chris Wilson).
> > > 
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > index 053d99e..80f0c80 100644
> > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > @@ -567,6 +567,7 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
> > >  			 struct i915_address_space *vm)
> > >  {
> > >  	struct drm_i915_error_object *dst;
> > > +	struct i915_vma *vma = NULL;
> > >  	int num_pages;
> > >  	bool use_ggtt;
> > >  	int i = 0;
> > > @@ -587,16 +588,17 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
> > >  		dst->gtt_offset = -1;
> > >  
> > >  	reloc_offset = dst->gtt_offset;
> > > +	if (i915_is_ggtt(vm))
> > > +		vma = i915_gem_obj_to_ggtt(src);
> > >  	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> > > -		    i915_is_ggtt(vm) &&
> > > -		    src->has_global_gtt_mapping &&
> > > -		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> > > +		   vma && (vma->bound & GLOBAL_BIND) &&
> > > +		   reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> > >  
> > >  	/* Cannot access stolen address directly, try to use the aperture */
> > >  	if (src->stolen) {
> > >  		use_ggtt = true;
> > >  
> > > -		if (!src->has_global_gtt_mapping)
> > > +		if (!(vma && vma->bound & GLOBAL_BIND))
> > >  			goto unwind;
> > 
> > This looked odd as I have the vma being passed in:
> > 
> > static struct drm_i915_error_object *
> > i915_error_object_create(struct drm_i915_private *dev_priv,
> >                          struct i915_vma *vma);
> > 
> > but I presume the ppgtt error-capture is still outstanding.
> 
> Oh meh, this is probably one of those tasks that I've asked Jesse to close
> in one of the "fuck this all" rage-quits in the past few months ... Indeed
> both code and testcase seem to not have materialized afaics.
> 
> Jesse?

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

Re: [PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[Jesse Barnes]

On Fri, 24 Oct 2014 16:44:32 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Oct 24, 2014 at 04:43:08PM +0200, Daniel Vetter wrote:
> > On Fri, Oct 24, 2014 at 01:09:08PM +0100, Chris Wilson wrote:
> > > On Fri, Oct 24, 2014 at 12:42:33PM +0100, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > 
> > > > If these flags are on the object level it will be more difficult to allow
> > > > for multiple VMAs per object.
> > > > 
> > > > v2: Simplification and cleanup after code review comments (Chris Wilson).
> > > > 
> > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > index 053d99e..80f0c80 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > @@ -567,6 +567,7 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
> > > >  			 struct i915_address_space *vm)
> > > >  {
> > > >  	struct drm_i915_error_object *dst;
> > > > +	struct i915_vma *vma = NULL;
> > > >  	int num_pages;
> > > >  	bool use_ggtt;
> > > >  	int i = 0;
> > > > @@ -587,16 +588,17 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
> > > >  		dst->gtt_offset = -1;
> > > >  
> > > >  	reloc_offset = dst->gtt_offset;
> > > > +	if (i915_is_ggtt(vm))
> > > > +		vma = i915_gem_obj_to_ggtt(src);
> > > >  	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> > > > -		    i915_is_ggtt(vm) &&
> > > > -		    src->has_global_gtt_mapping &&
> > > > -		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> > > > +		   vma && (vma->bound & GLOBAL_BIND) &&
> > > > +		   reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> > > >  
> > > >  	/* Cannot access stolen address directly, try to use the aperture */
> > > >  	if (src->stolen) {
> > > >  		use_ggtt = true;
> > > >  
> > > > -		if (!src->has_global_gtt_mapping)
> > > > +		if (!(vma && vma->bound & GLOBAL_BIND))
> > > >  			goto unwind;
> > > 
> > > This looked odd as I have the vma being passed in:
> > > 
> > > static struct drm_i915_error_object *
> > > i915_error_object_create(struct drm_i915_private *dev_priv,
> > >                          struct i915_vma *vma);
> > > 
> > > but I presume the ppgtt error-capture is still outstanding.
> > 
> > Oh meh, this is probably one of those tasks that I've asked Jesse to close
> > in one of the "fuck this all" rage-quits in the past few months ... Indeed
> > both code and testcase seem to not have materialized afaics.
> > 
> > Jesse?
> 
> Actually cc Jesse ...

The only thing I'm aware of here is this task:
https://jira01.devtools.intel.com/browse/VIZ-4123

which Mika already closed.  If there's still some breakage, do we have
bugs open on them?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx