[Intel-gfx] [PATCH 7/9] drm/i915/context: context validation for execbuffer2

Ben Widawsky bwidawsk at gmail.com
Tue Feb 1 19:16:24 CET 2011


Adds the support for actually doing something with buffer validation for
the context when submitted via execbuffer2. When a set of context flags
are submitted in the execbuffer call, the code is able to handle the
commands. It will also go through the existing buffers associated with
the context and make sure they are still present. The big thing missing
is if the client wants to change the domains of buffers which have
already been associated. In this case, the client must disassociate and
then re-associate with the proper domains.

Fixed up some small issues which hooks the main gem code the context
code to do buffer association and validation.

As a result of this, the unit test posted on the mailing list should
behave as expected.
---
 drivers/gpu/drm/i915/i915_context.c        |  111 +++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drv.h            |    3 +
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   12 ++-
 include/drm/i915_drm.h                     |    5 +-
 4 files changed, 124 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index a486a9c..cba0452 100644
--- a/drivers/gpu/drm/i915/i915_context.c
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -29,6 +29,13 @@
 #include "i915_drm.h"
 #include "intel_drv.h"
 
+#define PPGTT_NEEDS_RELOC	(1ULL << 63)
+#define PPGTT_USER_DEFINED	(1ULL << 62)
+#define PPGTT_RELOCATED		(1ULL << 61)
+#define PPGTT_ADDR(x)		(x & 0xFFFFFFFFFFULL)
+
+#define PPGTT_PINNED(x) ((uint64_t)x & (PPGTT_USER_DEFINED | PPGTT_RELOCATED))
+
 static int
 i915_context_gen_id(struct drm_i915_private *dev_priv,
 		    struct drm_i915_gem_context *ctx)
@@ -70,8 +77,110 @@ int i915_context_validate(struct drm_device *dev, struct drm_file *file,
 			  uint32_t ctx_id,
 			  struct drm_i915_context_flag *ctx_flag, int count)
 {
-	return 0;
+	struct drm_gem_object *obj;
+	struct drm_i915_gem_object *obj_priv;
+	struct drm_i915_gem_context *ctx;
+	int i, ret = 0;
+
+	ctx = i915_context_lookup_id(dev, ctx_id);
+	if (ctx == NULL) {
+		DRM_ERROR("Couldn't find context\n");
+		return -ENXIO;
+	}
+
+	if ((!count || !ctx_flag))
+		goto out;
+
+	for (i = 0; i < count; i++) {
+		struct drm_i915_context_flag *flag = &ctx_flag[i];
+		if (flag->slot >= ctx->slot_count) {
+			DRM_ERROR("Context command for invalid slot\n");
+			continue;
+		}
+		if (flag->command & (flag->command - 1)) {
+			DRM_ERROR("More than one command per context flag is not allowed\n");
+			continue;
+		}
+
+		switch (flag->command) {
+		case I915_CTX_ASSOC_BUF:
+			mutex_lock(&ctx->slot_mtx);
+			if (ctx->bufs[flag->slot] != NULL) {
+				DRM_DEBUG("Overwriting buffer slot without "
+					  "disassociating\n");
+			}
+			obj = drm_gem_object_lookup(dev, file, flag->handle);
+			if (obj == NULL) {
+				DRM_ERROR("Couldn't find object\n");
+				mutex_unlock(&ctx->slot_mtx);
+				continue;
+			}
+			obj_priv = to_intel_bo(obj);
+			if (flag->offset && HAS_PPGTT(dev)) {
+				/* 
+				 * No need to check for overlaps because this is
+				 * in their local GTT so they can only screw up
+				 * themselves. But do check serious violations
+				 */
+				if (flag->offset + obj->size >= 1ULL << 40) {
+					mutex_unlock(&ctx->slot_mtx);
+					continue;
+				}
+				obj_priv->ppgtt_offset = flag->offset | PPGTT_USER_DEFINED;
+			} else
+				obj_priv->ppgtt_offset = PPGTT_NEEDS_RELOC;
+
+			ctx->bufs[flag->slot] = obj;
+			mutex_unlock(&ctx->slot_mtx);
+			break;
+		case I915_CTX_DISASSOC_BUF:
+			mutex_lock(&ctx->slot_mtx);
+			ctx->bufs[flag->slot] = NULL;
+			mutex_unlock(&ctx->slot_mtx);
+			break;
+		case I915_CTX_DOMAIN_BUF:
+			DRM_ERROR("Changing domains is not yet supported\n");
+			break;
+		default:
+			DRM_ERROR("Unknown slot command\n");
+		}
+	}
+
+out:
+	mutex_lock(&ctx->slot_mtx);
+	/* Go through all slots to make sure everything is sane. */
+	for (i = 0; i < ctx->slot_count; i++) {
+		uint64_t ppgtt_offset;
+		if (ctx->bufs[i] == NULL)
+			continue;
+		obj_priv = to_intel_bo(ctx->bufs[i]);
+		if (obj_priv->ppgtt_offset == PPGTT_NEEDS_RELOC)
+			continue;
+
+		ppgtt_offset = PPGTT_ADDR(obj_priv->ppgtt_offset);
+		if (PPGTT_PINNED(obj_priv->ppgtt_offset) &&
+		    ppgtt_offset != obj_priv->gtt_offset) {
+			DRM_DEBUG_DRIVER("Context associated buffer has moved"
+					 " %p->%p\n",
+					 ppgtt_offset, obj_priv->gtt_offset);
+			ret = -EIO;
+			break;
+		}
+	}
+
+	mutex_unlock(&ctx->slot_mtx);
+	return ret;
 }
+
+void i915_context_handle_binding(struct drm_i915_gem_object *obj)
+{
+	if (obj->ppgtt_offset == PPGTT_NEEDS_RELOC) {
+		obj->ppgtt_offset = obj->gtt_offset;
+		obj->ppgtt_offset |= PPGTT_RELOCATED;
+	}
+}
+
+
 /**
  * i915_context_alloc_backing_obj - Allocate and pin space in the global GTT for
  * use by the HW to save, and restore context information.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48d4e6f..3105fd6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -851,6 +851,8 @@ struct drm_i915_gem_object {
 	 * reaches 0, dev_priv->pending_flip_queue will be woken up.
 	 */
 	atomic_t pending_flip;
+
+	uint64_t ppgtt_offset;
 };
 
 #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
@@ -1328,6 +1330,7 @@ extern int i915_context_validate(struct drm_device *dev,
 				 struct drm_file *file, uint32_t ctx_id,
 				 struct drm_i915_context_flag *ctx_flag,
 				 int count);
+extern void i915_context_handle_binding(struct drm_i915_gem_object *obj);
 
 #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a60996d..1051b1e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -589,6 +589,7 @@ i915_gem_execbuffer_reserve(struct intel_ring_buffer *ring,
 			}
 
 			entry->offset = obj->gtt_offset;
+			i915_context_handle_binding(obj);
 		}
 
 		/* Decrement pin count for bound objects */
@@ -991,6 +992,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	struct intel_ring_buffer *ring;
 	u32 exec_start, exec_len;
 	u32 seqno;
+	u32 ctx_id;
 	int ret, mode, i;
 
 	if (!i915_gem_check_execbuffer(args)) {
@@ -1001,11 +1003,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	ret = validate_exec_list(exec, args->buffer_count);
 	if (ret)
 		return ret;
-	if (ctx_flags) {
-		ret = i915_context_validate(dev, file, EXECBUFFER2_CTX_ID(args),
+
+	ctx_id = EXECBUFFER2_CTX_ID(args);
+	if (ctx_id) {
+		ret = i915_context_validate(dev, file, ctx_id,
 					    ctx_flags, flag_count);
 		if (ret) {
-			if (ret == -EAGAIN)
+			if (ret == -EIO)
 				DRM_DEBUG_DRIVER("Context resubmission required\n");
 			return ret;
 		}
@@ -1370,7 +1374,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
 		return -EFAULT;
 	}
 	if (EXECBUFFER2_FLAGS_PTR(args) && EXECBUFFER2_FLAGS_COUNT(argc)) {
-		flag_count = EXECBUFFER2_FLAGS_COUNT(argc);
+		flag_count = EXECBUFFER2_FLAGS_COUNT(args);
 		flags = drm_malloc_ab(sizeof(*flags), flag_count);
 		if (flags == NULL) {
 			DRM_ERROR("allocation of flags failed\n");
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 0ebce11..dbd0332 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -669,8 +669,9 @@ struct drm_i915_context_flag {
 	__u64 offset;
 	__u32 read_domain;
 	__u32 write_domain;
-#define I915_CTX_ASSOC_BUF (1 << 0)
-#define I915_CTX_DISASSOC_BUF (1 << 1)
+#define I915_CTX_ASSOC_BUF	(1 << 0)
+#define I915_CTX_DISASSOC_BUF	(1 << 1)
+#define I915_CTX_DOMAIN_BUF	(1 << 2)
 	__u32 command;
 };
 
-- 
1.7.3.4




More information about the Intel-gfx mailing list