Skip to contentSkip to conversation

drm/1915/context (i915HW Context support, early)

PATCH In review Ben Widawsky 3 participants 12 messages Feb 1, 2011

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Ben Widawsky

author
#0 lore raw

cover drm/1915/context (i915HW Context support, early)

This patch adds the basic API for HW context manipulation. Right now only a very basic unit test is passing. I was hoping to get some eyes on this code while I look into porting some of mesa (and cleaning up my libdrm patches).

I tried my best to split up the patches logically, but I'm afraid there are still some fairly large ones.

Ben

author
#1 lore raw

PATCH 1/9 drm/i915/context: context switch, and PPGTT params

Adding parameters for userspace to query what features the driver/hardware supports. In the future context and ppgtt will always go together, but for now we will use hardware contexts without ppgtt since there are some issues with ppgtt on the current generation.

---
 drivers/gpu/drm/i915/i915_dma.c |    6 ++++++
 drivers/gpu/drm/i915/i915_drv.h |    3 +++
 include/drm/i915_drm.h          |    2 ++
 3 files changed, 11 insertions(+), 0 deletions(-)
drivers/gpu/drm/i915/i915_dma.c +6 -0
index 17bd766..bbaf60c 100644
@@ -771,6 +771,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_EXEC_CONSTANTS:
 		value = INTEL_INFO(dev)->gen >= 4;
 		break;
+	case I915_PARAM_HAS_CONTEXTS:
+		value = HAS_HW_CONTEXTS(dev);
+		break;
+	case I915_PARAM_HAS_PPGTT:
+		value = HAS_PPGTT(dev);
+		break;
 	default:
 		DRM_DEBUG_DRIVER("Unknown parameter %d\n",
 				 param->param);
drivers/gpu/drm/i915/i915_drv.h +3 -0
index a0149c6..caf0a28 100644
@@ -950,6 +950,9 @@ enum intel_chip_family {
 #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
 #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
 
+#define HAS_HW_CONTEXTS(dev)	(INTEL_INFO(dev)->gen >= 4)
+#define HAS_PPGTT(dev)		(INTEL_INFO(dev)->gen >= 7)
+
 #include "i915_trace.h"
 
 extern struct drm_ioctl_desc i915_ioctls[];
include/drm/i915_drm.h +2 -0
index 0039f1f..26759a8 100644
@@ -290,6 +290,8 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_RELAXED_FENCING	 12
 #define I915_PARAM_HAS_COHERENT_RINGS	 13
 #define I915_PARAM_HAS_EXEC_CONSTANTS	 14
+#define I915_PARAM_HAS_CONTEXTS		 15
+#define I915_PARAM_HAS_PPGTT		 16
 
 typedef struct drm_i915_getparam {
 	int param;
Signature · 1 line
1.7.3.4
#2 lore raw

Re: [PATCH 1/9] drm/i915/context: context switch, and PPGTT params

Quoted 5 lines · On Tue, 1 Feb 2011 10:16:18 -0800 Ben Widawsky <[email protected]> wrote:

Adding parameters for userspace to query what features the driver/hardware supports. In the future context and ppgtt will always go together, but for now we will use hardware contexts without ppgtt since there are some issues with ppgtt on the current generation.
---

Minor ordering nit pick: this patch should be the last one in the series, otherwise bisecting across it with a new userspace will be hard (kernel will come back with "yes I can handle hw contexts" but the code to support them isn't there yet).

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
author
#3 lore raw

PATCH 2/9 drm/i915/context: basic implementation context ioctls

In reply to Ben Widawsky

Added the minimal amount of code to enable the two ioctls used for creating and destroying contexts. Also added neccessary information in the structures to implement some basic operations the ioctls will have to perform.

A small whitespace fixup in the Makefile also made it in here.

---
 drivers/gpu/drm/i915/Makefile       |    3 +-
 drivers/gpu/drm/i915/i915_context.c |  113 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_dma.c     |    2 +
 drivers/gpu/drm/i915/i915_drv.h     |   34 +++++++++++
 include/drm/i915_drm.h              |   15 +++++
 5 files changed, 166 insertions(+), 1 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_context.c
drivers/gpu/drm/i915/Makefile +2 -1
index 0ae6a7c..244536a 100644
@@ -5,13 +5,14 @@
 ccflags-y := -Iinclude/drm
 i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
 	  i915_debugfs.o \
-          i915_suspend.o \
+	  i915_suspend.o \
 	  i915_gem.o \
 	  i915_gem_debug.o \
 	  i915_gem_evict.o \
 	  i915_gem_execbuffer.o \
 	  i915_gem_gtt.o \
 	  i915_gem_tiling.o \
+	  i915_context.o \
 	  i915_trace_points.o \
 	  intel_display.o \
 	  intel_crt.o \
drivers/gpu/drm/i915/i915_context.c new file +113 -0

113 lines not on this page · raw message

drivers/gpu/drm/i915/i915_dma.c +2 -0
index bbaf60c..f314b0f 100644
@@ -2255,6 +2255,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CTX_CREATE, i915_gem_ctx_create_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CTX_DESTROY, i915_gem_ctx_destroy_ioctl, DRM_UNLOCKED),
 };
 
 int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
drivers/gpu/drm/i915/i915_drv.h +34 -0
index caf0a28..60aec8f 100644
@@ -280,6 +280,13 @@ typedef struct drm_i915_private {
 	struct drm_i915_gem_object *pwrctx;
 	struct drm_i915_gem_object *renderctx;
 
+	/** Device specific context info */
+	uint32_t ctx_size;
+	struct drm_i915_gem_context *default_ctx;
+	struct spinlock i915_ctx_lock;
+	struct idr i915_ctx_idr;
+	bool ctx_disable;
+
 	struct resource mch_res;
 
 	unsigned int cpp;

@@ -848,6 +855,25 @@ struct drm_i915_gem_object {
 
 #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
 
+struct drm_i915_gem_context {
+	int ctx_id;
+	/** Pinned buffer for the HW context */
+	struct drm_gem_object *ctx_obj;
+	uint32_t aperture_size_mb;
+
+	/** TODO: rw lock? */
+	struct mutex slot_mtx;
+	struct drm_gem_object **bufs;
+	int slot_count;
+
+	/** This object's place on the per bo context list */
+	struct list_head context_list;
+
+	/** This object's place no the per file context list */
+	struct list_head file_list;
+};
+
+
 /**
  * Request queue structure.
  *

@@ -881,6 +907,10 @@ struct drm_i915_file_private {
 		struct spinlock lock;
 		struct list_head request_list;
 	} mm;
+	struct {
+		struct mutex mtx;
+		struct list_head context_list;
+	} context;
 };
 
 enum intel_chip_family {

@@ -1093,6 +1123,10 @@ int i915_gem_get_tiling(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
 int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
+int i915_gem_ctx_create_ioctl(struct drm_device *dev, void *data,
+			      struct drm_file *file);
+int i915_gem_ctx_destroy_ioctl(struct drm_device *dev, void *data,
+			       struct drm_file *file);
 void i915_gem_load(struct drm_device *dev);
 int i915_gem_init_object(struct drm_gem_object *obj);
 int __must_check i915_gem_flush_ring(struct drm_device *dev,
include/drm/i915_drm.h +15 -0
index 26759a8..58c4482 100644
@@ -198,6 +198,8 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_OVERLAY_PUT_IMAGE	0x27
 #define DRM_I915_OVERLAY_ATTRS	0x28
 #define DRM_I915_GEM_EXECBUFFER2	0x29
+#define DRM_I915_GEM_CTX_CREATE		0x2a
+#define DRM_I915_GEM_CTX_DESTROY	0x2b
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)

@@ -239,6 +241,9 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_MADVISE	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MADVISE, struct drm_i915_gem_madvise)
 #define DRM_IOCTL_I915_OVERLAY_PUT_IMAGE	DRM_IOW(DRM_COMMAND_BASE + DRM_IOCTL_I915_OVERLAY_ATTRS, struct drm_intel_overlay_put_image)
 #define DRM_IOCTL_I915_OVERLAY_ATTRS	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
+#define DRM_IOCTL_I915_GEM_CTX_CREATE	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CTX_CREATE, struct drm_i915_gem_ctx_create)
+#define DRM_IOCTL_I915_GEM_CTX_DESTROY	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CTX_DESTROY, struct drm_i915_gem_ctx_destroy)
+
 
 /* Allow drivers to submit batchbuffers directly to hardware, relying
  * on the security mechanisms provided by hardware.

@@ -845,4 +850,14 @@ struct drm_intel_overlay_attrs {
 	__u32 gamma5;
 };
 
+struct drm_i915_gem_ctx_create {
+	__s32 slot_count;
+	__u64 aperture_size;
+	__u32 ctx_id;
+};
+
+struct drm_i915_gem_ctx_destroy {
+	__u32 ctx_id;
+};
+
 #endif				/* _I915_DRM_H_ */
Signature · 1 line
1.7.3.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

#4 lore raw

Re: [PATCH 2/9] drm/i915/context: basic implementation context ioctls

Quoted 4 lines · Ben Widawsky

Added the minimal amount of code to enable the two ioctls used for creating and destroying contexts. Also added neccessary information in the structures to implement some basic operations the ioctls will have to perform.

Locking to be added at a later date? ;-)

Contexts should be per-file unless we really mean for them to have a global namespace, in which case we should explicitly export them with an flink_to style interface.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#5 lore raw

PATCH 3/9 drm/i915/context: context initialization/destruction

In reply to Ben Widawsky

This does most of the work needed to create, destroy, and cleanup-after contexts, but does not have the actual code to the do the context switching, or buffer association. This includes context id/name generation, context object allocation, calls hardware initialization functions, ringbuffer API for context switch, as well as calls from upper layers on init/deinit, and drm file close and open.

---
 drivers/gpu/drm/i915/i915_context.c     |  273 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_dma.c         |   10 +-
 drivers/gpu/drm/i915/i915_drv.h         |   11 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   14 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |    7 +
 5 files changed, 286 insertions(+), 29 deletions(-)
drivers/gpu/drm/i915/i915_context.c +250 -23
index 0d140b8..4e19636 100644
@@ -29,8 +29,30 @@
 #include "i915_drm.h"
 #include "intel_drv.h"
 
+static int
+i915_context_gen_id(struct drm_i915_private *dev_priv,
+		    struct drm_i915_gem_context *ctx)
+{
+	int ret, id;
+again:
+	if (idr_pre_get(&dev_priv->i915_ctx_idr, GFP_KERNEL) == 0) {
+		return -ENOMEM;
+	}
+
+	spin_lock(&dev_priv->i915_ctx_lock);
+	/*  Don't start at 0 to make finding non-context users easier */
+	ret = idr_get_new_above(&dev_priv->i915_ctx_idr, ctx, 1, &id);
+	if (ret == -EAGAIN) {
+		spin_unlock(&dev_priv->i915_ctx_lock);
+		goto again;
+	}
+	spin_unlock(&dev_priv->i915_ctx_lock);
+
+	return id;
+}
+
 static struct drm_i915_gem_context *
-i915_gem_lookup_ctx_id(struct drm_device *dev,
+i915_context_lookup_id(struct drm_device *dev,
 		       uint32_t id)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

@@ -38,23 +60,169 @@ i915_gem_lookup_ctx_id(struct drm_device *dev,
 }
 
 static void
-i915_gem_del_ctx_id(struct drm_device *dev,
+i915_context_del_id(struct drm_device *dev,
 		    struct drm_i915_gem_context *ctx)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	idr_remove(&dev_priv->i915_ctx_idr, ctx->ctx_id);
 }
 
-int
-i915_gem_alloc_hw_context(struct drm_device *dev,
+/**
+ * i915_context_alloc_backing_obj - Allocate and pin space in the global GTT for
+ * use by the HW to save, and restore context information.
+ */
+static struct drm_gem_object *
+i915_context_alloc_backing_obj(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	int ret;
+	size_t size = dev_priv->ctx_size;
+	if (!size) {
+		/*  XXX size is HW specific, this needs to be fixed!! */
+		size = 4096;
+		dev_priv->ctx_size = size;
+	}
+
+	obj = i915_gem_alloc_object(dev, size);
+
+	if (!obj) {
+		DRM_DEBUG("Failed to allocate context\n");
+		return NULL;
+	}
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = i915_gem_object_pin(obj, 0x1000, false);
+	if (ret) {
+		DRM_ERROR("Failed to pin context: %d\n", ret);
+		goto err_unref;
+	}
+
+	ret = i915_gem_object_set_to_gtt_domain(obj, 1);
+	if (ret) {
+		DRM_ERROR("failed to set domain on context: %d\n", ret);
+		goto err_unpin;
+	}
+	mutex_unlock(&dev->struct_mutex);
+
+	return &obj->base;
+
+err_unpin:
+	i915_gem_object_unpin(obj);
+err_unref:
+	drm_gem_object_unreference(&obj->base);
+	mutex_unlock(&dev->struct_mutex);
+	return NULL;
+}
+/**
+ * i915_context_hw_init - Create a valid context for the given object
+ * on the specified ring. This may need to be hardware dependent in the future,
+ * but for now just creates a CCID for the context, saves state, and then
+ * restores that stte.
+ * @dev: drm
+ * @obj: object where the context will be stored.
+ * @ring: ring to submit commands to. Render ring only for now.
+ */
+static void i915_context_hw_init(struct drm_device *dev,
+				 struct drm_i915_gem_context *ctx,
+				 struct intel_ring_buffer *ring)
+{
+	if (ring->context_switch == NULL)
+		return;
+
+	mutex_lock(&dev->struct_mutex);
+	if (intel_ring_begin(ring, 2)) {
+		mutex_unlock(&dev->struct_mutex);
+		return;
+	}
+	intel_ring_emit(ring, MI_FLUSH);
+	intel_ring_emit(ring, MI_NOOP | (1 << 22) | ctx->ctx_id);
+	intel_ring_advance(ring);
+	ring->context_switch(ring, ctx, I915_CONTEXT_SAVE_ONLY);
+	ring->context_switch(ring, ctx, I915_CONTEXT_NORMAL_SWITCH);
+	mutex_unlock(&dev->struct_mutex);
+}
+
+static void i915_context_hw_deinit(struct drm_device *dev,
+				   struct drm_i915_gem_context *ctx,
+				   struct intel_ring_buffer *ring)
+{
+	/* XXX We can prevent restoring contexts, but not saving them
+	 * so if we're going to take away our backing context object
+	 * of the last context, we have to switch now.
+	 */
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	if (ring->last_context == ctx && ctx != dev_priv->default_context) {
+		mutex_lock(&dev->struct_mutex);
+		ring->context_switch(ring, dev_priv->default_context,
+				     I915_CONTEXT_NORMAL_SWITCH);
+		mutex_unlock(&dev->struct_mutex);
+	}
+}
+
+static int
+i915_alloc_hw_context(struct drm_device *dev,
 			  uint64_t aperture_size,
 			  struct drm_i915_gem_context **ctx_out)
 {
-	return -ENOMEM;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_context *ctx;
+	int ret = 0;
+
+	ctx = kzalloc(sizeof(struct drm_i915_gem_context), GFP_KERNEL);
+	if (ctx == NULL)
+		return -ENOMEM;
+
+	ctx->ctx_id = i915_context_gen_id(dev_priv, ctx);
+	if (ctx->ctx_id < 0) {
+		ret = ctx->ctx_id;
+		goto out;
+	}
+
+	ctx->ctx_obj = i915_context_alloc_backing_obj(dev);
+	if (ctx->ctx_obj == NULL) {
+		ret = -ENOMEM;
+		goto id_out;
+	}
+
+	if (!aperture_size)
+		aperture_size = 256 << 20;
+	ctx->aperture_size_mb = aperture_size >> 20;
+
+	i915_context_hw_init(dev, ctx, &dev_priv->ring[RCS]);
+
+	mutex_init(&ctx->slot_mtx);
+	INIT_LIST_HEAD(&ctx->context_list);
+	INIT_LIST_HEAD(&ctx->file_list);
+
+	*ctx_out = ctx;
+	return ret;
+
+id_out:
+	i915_context_del_id(dev, ctx);
+out:
+	kfree(ctx);
+	return ret;
+}
+
+/**
+ * Destroy everything associated with a context but does not free the context's
+ * memory.
+ */
+static int
+i915_destroy_hw_context(struct drm_device *dev,
+		        struct drm_i915_gem_context *ctx)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	i915_context_hw_deinit(dev, ctx, (&dev_priv->ring[RCS]));
+	i915_gem_free_object(ctx->ctx_obj);
+	i915_context_del_id(dev, ctx);
+	return 0;
 }
 
 int
-i915_gem_ctx_create_ioctl(struct drm_device *dev, void *data,
+i915_context_create_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *file)
 {
 	struct drm_i915_gem_ctx_create *args = data;

@@ -63,19 +231,22 @@ i915_gem_ctx_create_ioctl(struct drm_device *dev, void *data,
 	int slots = args->slot_count;
 	int i, ret = 0;
 
-	/* TODO: sanitize aperture_size*/
-	ret = i915_gem_alloc_hw_context(dev, args->aperture_size, &ctx);
+	if(args->aperture_size & (args->aperture_size - 1))
+		return -EINVAL;
+
+	ret = i915_alloc_hw_context(dev, args->aperture_size, &ctx);
 	if (ret) {
-		goto out;
+		DRM_DEBUG_DRIVER("Couldn't allocate a HW context\n");
+		return -ENOMEM;
 	}
 
 	ctx->bufs = drm_malloc_ab(slots + 1, sizeof(struct drm_gem_object*));
 	if (ctx->bufs == NULL) {
+		i915_destroy_hw_context(dev, ctx);
 		kfree(ctx);
-		drm_free_large(ctx->bufs);
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
+
 	ctx->slot_count = slots;
 	for(i = 0; i < slots + 1; i++) {
 		ctx->bufs[i] = NULL;

@@ -84,30 +255,86 @@ i915_gem_ctx_create_ioctl(struct drm_device *dev, void *data,
 	list_add(&ctx->file_list, &file_priv->context.context_list);
 
 	args->ctx_id = ctx->ctx_id;
-out:
+
 	return ret;
 }
 
+static int
+do_context_destroy(struct drm_device *dev, struct drm_i915_gem_context *ctx)
+{
+	list_del(&ctx->file_list);
+	drm_free_large(ctx->bufs);
+	i915_destroy_hw_context(dev, ctx);
+	kfree(ctx);
+	return 0;
+}
+
 int
-i915_gem_ctx_destroy_ioctl(struct drm_device *dev, void *data,
+i915_context_destroy_ioctl(struct drm_device *dev, void *data,
 			   struct drm_file *file)
 {
 	struct drm_i915_gem_ctx_destroy *args = data;
-	struct drm_i915_gem_context *ctx, *ctx_temp;
-	struct list_head *pos, *n;
-	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_gem_context *ctx;
 
-	ctx = i915_gem_lookup_ctx_id(dev, args->ctx_id);
+	ctx = i915_context_lookup_id(dev, args->ctx_id);
 	if (ctx == NULL) {
 		return -EINVAL;
 	}
+	return do_context_destroy(dev, ctx);
+}
 
-	list_for_each_safe(pos, n, &file_priv->context.context_list) {
-		ctx_temp = list_entry(pos, struct drm_i915_gem_context, context_list);
+void i915_context_close(struct drm_device *dev, struct drm_file *file)
+{
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_gem_context *ctx, *next;
+	list_for_each_entry_safe(ctx, next, &file_priv->context.context_list,
+		file_list) {
+		do_context_destroy(dev, ctx);
 	}
-	i915_gem_del_ctx_id(dev, ctx);
-	kfree(ctx);
+}
 
-	/* TODO: ring switch may be needed */
+void i915_context_init(struct drm_device *dev)
+{
+	int ret;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	if (!HAS_HW_CONTEXTS(dev)) {
+		DRM_DEBUG_DRIVER("Context support disabled, device does not support\n");
+		dev_priv->ctx_disable = 1;
+		return;
+	}
+
+	idr_init(&dev_priv->i915_ctx_idr);
+	spin_lock_init(&dev_priv->i915_ctx_lock);
+	ret = i915_alloc_hw_context(dev, 0, &dev_priv->default_context);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Context support disabled [%d]\n", ret);
+		dev_priv->ctx_disable = 1;
+		idr_destroy(&dev_priv->i915_ctx_idr);
+	} else {
+		DRM_DEBUG_DRIVER("Context support enabled\n", ret);
+	}
+}
+
+static
+int i915_context_idr_cleanup(int id, void *p, void *data)
+{
+	struct drm_device *dev = (struct drm_device *)data;
+	struct drm_i915_gem_context *ctx = (struct drm_i915_gem_context *)p;
+	DRM_DEBUG_DRIVER("Destroying abandoned context %d\n", ctx->ctx_id);
+	do_context_destroy(dev, ctx);
 	return 0;
 }
+
+void i915_context_deinit(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	if (dev_priv->ctx_disable)
+		return;
+
+	i915_destroy_hw_context(dev, dev_priv->default_context);
+	spin_lock(&dev_priv->i915_ctx_lock);
+	idr_for_each(&dev_priv->i915_ctx_idr, i915_context_idr_cleanup, dev);
+	idr_remove_all(&dev_priv->i915_ctx_idr);
+	idr_destroy(&dev_priv->i915_ctx_idr);
+	spin_unlock(&dev_priv->i915_ctx_lock);
+}
drivers/gpu/drm/i915/i915_dma.c +8 -2
index f314b0f..b0acc0d 100644
@@ -2041,6 +2041,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	ips_ping_for_i915_load();
 
+	i915_context_init(dev);
 	return 0;
 
 out_gem_unload:

@@ -2066,6 +2067,8 @@ int i915_driver_unload(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
+	i915_context_deinit(dev);
+
 	spin_lock(&mchdev_lock);
 	i915_mch_dev = NULL;
 	spin_unlock(&mchdev_lock);

@@ -2165,6 +2168,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 
 	spin_lock_init(&file_priv->mm.lock);
 	INIT_LIST_HEAD(&file_priv->mm.request_list);
+	mutex_init(&file_priv->context.mtx);
+	INIT_LIST_HEAD(&file_priv->context.context_list);
 
 	return 0;
 }

@@ -2203,6 +2208,7 @@ void i915_driver_preclose(struct drm_device * dev, struct drm_file *file_priv)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	i915_gem_release(dev, file_priv);
+	i915_context_close(dev, file_priv);
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_mem_release(dev, file_priv, dev_priv->agp_heap);
 }

@@ -2255,8 +2261,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CTX_CREATE, i915_gem_ctx_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CTX_DESTROY, i915_gem_ctx_destroy_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CTX_CREATE, i915_context_create_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CTX_DESTROY, i915_context_destroy_ioctl, DRM_UNLOCKED),
 };
 
 int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
drivers/gpu/drm/i915/i915_drv.h +8 -3
index 60aec8f..695a0bf 100644
@@ -282,7 +282,7 @@ typedef struct drm_i915_private {
 
 	/** Device specific context info */
 	uint32_t ctx_size;
-	struct drm_i915_gem_context *default_ctx;
+	struct drm_i915_gem_context *default_context;
 	struct spinlock i915_ctx_lock;
 	struct idr i915_ctx_idr;
 	bool ctx_disable;

@@ -1123,9 +1123,9 @@ int i915_gem_get_tiling(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
 int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
-int i915_gem_ctx_create_ioctl(struct drm_device *dev, void *data,
+int i915_context_create_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *file);
-int i915_gem_ctx_destroy_ioctl(struct drm_device *dev, void *data,
+int i915_context_destroy_ioctl(struct drm_device *dev, void *data,
 			       struct drm_file *file);
 void i915_gem_load(struct drm_device *dev);
 int i915_gem_init_object(struct drm_gem_object *obj);

@@ -1319,6 +1319,11 @@ extern void intel_display_print_error_state(struct seq_file *m,
 					    struct intel_display_error_state *error);
 #endif
 
+/* context */
+extern void i915_context_init(struct drm_device *dev);
+extern void i915_context_deinit(struct drm_device *dev);
+extern void i915_context_close(struct drm_device *dev, struct drm_file *file);
+
 #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
 
 #define BEGIN_LP_RING(n) \
drivers/gpu/drm/i915/intel_ringbuffer.c +13 -1
index 6218fa9..89adb9d 100644
@@ -750,6 +750,17 @@ render_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 	return 0;
 }
 
+static void
+render_ring_ctx_switch(struct intel_ring_buffer *ring,
+		       struct drm_i915_gem_context *ctx,
+		       uint32_t flags)
+{
+	if (ring->last_context == ctx)
+		return;
+
+	ring->last_context = ctx;
+}
+
 static void cleanup_status_page(struct intel_ring_buffer *ring)
 {
 	drm_i915_private_t *dev_priv = ring->dev->dev_private;

@@ -1015,7 +1026,8 @@ static const struct intel_ring_buffer render_ring = {
 	.irq_get		= render_ring_get_irq,
 	.irq_put		= render_ring_put_irq,
 	.dispatch_execbuffer	= render_ring_dispatch_execbuffer,
-       .cleanup			= render_ring_cleanup,
+	.cleanup		= render_ring_cleanup,
+	.context_switch		= render_ring_ctx_switch,
 };
 
 /* ring buffer for bit-stream decoder */
drivers/gpu/drm/i915/intel_ringbuffer.h +7 -0
index 6d6fde8..68ebecf 100644
@@ -35,6 +35,9 @@ struct  intel_hw_status_page {
 #define I915_READ_SYNC_0(ring) I915_RING_READ(RING_SYNC_0((ring)->mmio_base))
 #define I915_READ_SYNC_1(ring) I915_RING_READ(RING_SYNC_1((ring)->mmio_base))
 
+#define I915_CONTEXT_NORMAL_SWITCH	0
+#define I915_CONTEXT_SAVE_ONLY		1
+
 struct  intel_ring_buffer {
 	const char	*name;
 	enum intel_ring_id {

@@ -76,6 +79,10 @@ struct  intel_ring_buffer {
 	int		(*dispatch_execbuffer)(struct intel_ring_buffer *ring,
 					       u32 offset, u32 length);
 	void		(*cleanup)(struct intel_ring_buffer *ring);
+	struct		drm_i915_gem_context *last_context;
+	void		(*context_switch)(struct intel_ring_buffer *ring,
+					  struct drm_i915_gem_context *ctx,
+					  uint32_t flags);
 
 	/**
 	 * List of objects currently involved in rendering from the
Signature · 1 line
1.7.3.4
author
#6 lore raw

PATCH 4/9 drm/i915/context: whitespace cleanup, and warning cleanup

In reply to Ben Widawsky
---
 drivers/gpu/drm/i915/i915_context.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/gpu/drm/i915/i915_context.c +6 -1
index 4e19636..90b7bf3 100644
@@ -115,6 +115,7 @@ err_unref:
 	mutex_unlock(&dev->struct_mutex);
 	return NULL;
 }
+
 /**
  * i915_context_hw_init - Create a valid context for the given object
  * on the specified ring. This may need to be hardware dependent in the future,

@@ -315,12 +316,14 @@ void i915_context_init(struct drm_device *dev)
 	}
 }
 
+static int warn_idr_cleanup = 0;
+
 static
 int i915_context_idr_cleanup(int id, void *p, void *data)
 {
 	struct drm_device *dev = (struct drm_device *)data;
 	struct drm_i915_gem_context *ctx = (struct drm_i915_gem_context *)p;
-	DRM_DEBUG_DRIVER("Destroying abandoned context %d\n", ctx->ctx_id);
+	WARN_ON_ONCE(warn_idr_cleanup);
 	do_context_destroy(dev, ctx);
 	return 0;
 }

@@ -333,8 +336,10 @@ void i915_context_deinit(struct drm_device *dev)
 
 	i915_destroy_hw_context(dev, dev_priv->default_context);
 	spin_lock(&dev_priv->i915_ctx_lock);
+	warn_idr_cleanup=1;
 	idr_for_each(&dev_priv->i915_ctx_idr, i915_context_idr_cleanup, dev);
 	idr_remove_all(&dev_priv->i915_ctx_idr);
 	idr_destroy(&dev_priv->i915_ctx_idr);
+	warn_idr_cleanup=0;
 	spin_unlock(&dev_priv->i915_ctx_lock);
 }
Signature · 1 line
1.7.3.4
author
#7 lore raw

PATCH 5/9 drm/i915/context: switch context support query to variable

In reply to Ben Widawsky
---
 drivers/gpu/drm/i915/i915_dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/gpu/drm/i915/i915_dma.c +1 -1
index b0acc0d..9d055ba 100644
@@ -772,7 +772,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
 		value = INTEL_INFO(dev)->gen >= 4;
 		break;
 	case I915_PARAM_HAS_CONTEXTS:
-		value = HAS_HW_CONTEXTS(dev);
+		value = dev_priv->ctx_disable ? 0 : 1;
 		break;
 	case I915_PARAM_HAS_PPGTT:
 		value = HAS_PPGTT(dev);
Signature · 1 line
1.7.3.4
author
#8 lore raw

PATCH 6/9 drm/i915/context: minimal support for contexts in execbuffer2

In reply to Ben Widawsky

This change doesn't do anything except take parameters from clients for context flag information, and pass it to a stubbed function to validate the context flag information.

Included here are the updates to the necessary functions and structures needed to handle contexts when execbuffer is called. It seems we are able to reuse the rsvd1 and rsvd2 fields and not need a new ioctl for this.

Renamed ctx to context in many places to avoid confusion with drm contexts which seem unrelated.

---
 drivers/gpu/drm/i915/i915_context.c        |   11 +++++--
 drivers/gpu/drm/i915/i915_drv.h            |    5 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   45 +++++++++++++++++++++++++--
 include/drm/i915_drm.h                     |   15 +++++++++
 4 files changed, 69 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_context.c +8 -3
index 90b7bf3..a486a9c 100644
@@ -59,14 +59,19 @@ i915_context_lookup_id(struct drm_device *dev,
 	return idr_find(&dev_priv->i915_ctx_idr, id);
 }
 
-static void
-i915_context_del_id(struct drm_device *dev,
-		    struct drm_i915_gem_context *ctx)
+static void i915_context_del_id(struct drm_device *dev,
+				struct drm_i915_gem_context *ctx)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	idr_remove(&dev_priv->i915_ctx_idr, ctx->ctx_id);
 }
 
+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;
+}
 /**
  * i915_context_alloc_backing_obj - Allocate and pin space in the global GTT for
  * use by the HW to save, and restore context information.
drivers/gpu/drm/i915/i915_drv.h +5 -0
index 695a0bf..48d4e6f 100644
@@ -1323,6 +1323,11 @@ extern void intel_display_print_error_state(struct seq_file *m,
 extern void i915_context_init(struct drm_device *dev);
 extern void i915_context_deinit(struct drm_device *dev);
 extern void i915_context_close(struct drm_device *dev, struct drm_file *file);
+struct drm_i915_context_flag;
+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);
 
 #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
drivers/gpu/drm/i915/i915_gem_execbuffer.c +41 -4

76 lines not on this page · raw message

include/drm/i915_drm.h +15 -0
index 58c4482..0ebce11 100644
@@ -659,6 +659,21 @@ struct drm_i915_gem_execbuffer2 {
 	__u64 rsvd2;
 };
 
+#define EXECBUFFER2_FLAGS_PTR(exec2) (exec2->rsvd1)
+#define EXECBUFFER2_FLAGS_COUNT(exec2)  ((uint32_t)(args->rsvd2>>32))
+#define EXECBUFFER2_CTX_ID(exec2) ((uint32_t)args->rsvd2 )
+
+struct drm_i915_context_flag {
+	__u8 slot;
+	__u32 handle;
+	__u64 offset;
+	__u32 read_domain;
+	__u32 write_domain;
+#define I915_CTX_ASSOC_BUF (1 << 0)
+#define I915_CTX_DISASSOC_BUF (1 << 1)
+	__u32 command;
+};
+
 struct drm_i915_gem_pin {
 	/** Handle of the buffer to be pinned. */
 	__u32 handle;
Signature · 1 line
1.7.3.4
author
#9 lore raw

PATCH 7/9 drm/i915/context: context validation for execbuffer2

In reply to Ben Widawsky

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(-)
drivers/gpu/drm/i915/i915_context.c +110 -1
index a486a9c..cba0452 100644
@@ -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.
drivers/gpu/drm/i915/i915_drv.h +3 -0
index 48d4e6f..3105fd6 100644
@@ -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])
drivers/gpu/drm/i915/i915_gem_execbuffer.c +8 -4
index a60996d..1051b1e 100644
@@ -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");
include/drm/i915_drm.h +3 -2
index 0ebce11..dbd0332 100644
@@ -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;
 };
Signature · 1 line
1.7.3.4
author
#10 lore raw

PATCH 8/9 drm/i915/context: enable calling context_switch

In reply to Ben Widawsky

Changed context_validation code to return a pointer to the context which was validated. This saved us a context id lookup later when we want to actually context switch. The downside is we can't differentiate a lost context (buffer moved) from a never-existed context. This seems okay to me for now.

Added a call from do_execbuffer to actually context_switch for the ringbuffer. Although context_switch is not yet implemented, this can hopefully prove we don't break the existing code.

---
 drivers/gpu/drm/i915/i915_context.c        |   14 ++++++++------
 drivers/gpu/drm/i915/i915_drv.h            |    9 +++++----
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   13 ++++++++-----
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    2 +-
 4 files changed, 22 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/i915_context.c +8 -6
index cba0452..4f255fb 100644
@@ -73,7 +73,8 @@ static void i915_context_del_id(struct drm_device *dev,
 	idr_remove(&dev_priv->i915_ctx_idr, ctx->ctx_id);
 }
 
-int i915_context_validate(struct drm_device *dev, struct drm_file *file,
+struct drm_i915_gem_context *
+i915_context_validate(struct drm_device *dev, struct drm_file *file,
 			  uint32_t ctx_id,
 			  struct drm_i915_context_flag *ctx_flag, int count)
 {

@@ -85,7 +86,7 @@ int i915_context_validate(struct drm_device *dev, struct drm_file *file,
 	ctx = i915_context_lookup_id(dev, ctx_id);
 	if (ctx == NULL) {
 		DRM_ERROR("Couldn't find context\n");
-		return -ENXIO;
+		return ctx;
 	}
 
 	if ((!count || !ctx_flag))

@@ -117,7 +118,7 @@ int i915_context_validate(struct drm_device *dev, struct drm_file *file,
 			}
 			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

@@ -163,13 +164,14 @@ out:
 			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);
+			ctx = NULL;
+			return ctx;
 		}
 	}
 
 	mutex_unlock(&ctx->slot_mtx);
-	return ret;
+	return ctx;
 }
 
 void i915_context_handle_binding(struct drm_i915_gem_object *obj)
drivers/gpu/drm/i915/i915_drv.h +5 -4
index 3105fd6..110e495 100644
@@ -1326,10 +1326,11 @@ extern void i915_context_init(struct drm_device *dev);
 extern void i915_context_deinit(struct drm_device *dev);
 extern void i915_context_close(struct drm_device *dev, struct drm_file *file);
 struct drm_i915_context_flag;
-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 struct drm_i915_gem_context *
+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])
drivers/gpu/drm/i915/i915_gem_execbuffer.c +8 -5
index 1051b1e..9d022fb 100644
@@ -990,6 +990,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	struct drm_i915_gem_object *batch_obj;
 	struct drm_clip_rect *cliprects = NULL;
 	struct intel_ring_buffer *ring;
+	struct drm_i915_gem_context *ctx;
 	u32 exec_start, exec_len;
 	u32 seqno;
 	u32 ctx_id;

@@ -1006,12 +1007,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	ctx_id = EXECBUFFER2_CTX_ID(args);
 	if (ctx_id) {
-		ret = i915_context_validate(dev, file, ctx_id,
+		ctx = i915_context_validate(dev, file, ctx_id,
 					    ctx_flags, flag_count);
-		if (ret) {
-			if (ret == -EIO)
-				DRM_DEBUG_DRIVER("Context resubmission required\n");
-			return ret;
+		if (!ctx) {
+			DRM_DEBUG_DRIVER("Context resubmission required\n");
+			return -EIO;
 		}
 	}
 #if WATCH_EXEC

@@ -1203,6 +1203,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		}
 	}
 
+	if (!dev_priv->ctx_disable)
+		ring->context_switch(ring, ctx, I915_CONTEXT_NORMAL_SWITCH);
+
 	exec_start = batch_obj->gtt_offset + args->batch_start_offset;
 	exec_len = args->batch_len;
 	if (cliprects) {
drivers/gpu/drm/i915/intel_ringbuffer.h +1 -1
index 68ebecf..61525ba 100644
@@ -82,7 +82,7 @@ struct  intel_ring_buffer {
 	struct		drm_i915_gem_context *last_context;
 	void		(*context_switch)(struct intel_ring_buffer *ring,
 					  struct drm_i915_gem_context *ctx,
-					  uint32_t flags);
+					  u32 flags);
 
 	/**
 	 * List of objects currently involved in rendering from the
Signature · 1 line
1.7.3.4
author
#11 lore raw

PATCH 9/9 drm/i915/context: Insert MI_SET_CONTEXT in ringbuffer context switch

In reply to Ben Widawsky

Added ringbuffer code to initiate context switch. The context switch can fail since intel_ring_begin() can fail, and I am not sure how to handle that yet. In most cases a context switch failure is pretty much fatal.

Modified the locking on deinit, which had a race (though results were not fatal, just slower).

Fixed execbuffer to pass default_context when there is no context specified by userspace.

---
 drivers/gpu/drm/i915/i915_context.c        |   28 ++++++++++++-----
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   13 +++++--
 drivers/gpu/drm/i915/intel_ringbuffer.c    |   46 ++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    2 +-
 4 files changed, 73 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/i915_context.c +20 -8
index 4f255fb..6674a9d 100644
@@ -81,7 +81,7 @@ i915_context_validate(struct drm_device *dev, struct drm_file *file,
 	struct drm_gem_object *obj;
 	struct drm_i915_gem_object *obj_priv;
 	struct drm_i915_gem_context *ctx;
-	int i, ret = 0;
+	int i;
 
 	ctx = i915_context_lookup_id(dev, ctx_id);
 	if (ctx == NULL) {

@@ -261,21 +261,32 @@ static void i915_context_hw_init(struct drm_device *dev,
 	mutex_unlock(&dev->struct_mutex);
 }
 
-static void i915_context_hw_deinit(struct drm_device *dev,
+static int i915_context_hw_deinit(struct drm_device *dev,
 				   struct drm_i915_gem_context *ctx,
 				   struct intel_ring_buffer *ring)
 {
+	int ret;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	/* XXX We can prevent restoring contexts, but not saving them
 	 * so if we're going to take away our backing context object
 	 * of the last context, we have to switch now.
 	 */
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	mutex_lock(&dev->struct_mutex);
 	if (ring->last_context == ctx && ctx != dev_priv->default_context) {
-		mutex_lock(&dev->struct_mutex);
-		ring->context_switch(ring, dev_priv->default_context,
-				     I915_CONTEXT_NORMAL_SWITCH);
-		mutex_unlock(&dev->struct_mutex);
+		DRM_DEBUG_DRIVER("Switching to default context\n");
+		ret = ring->context_switch(ring, dev_priv->default_context,
+					   I915_CONTEXT_NORMAL_SWITCH);
+		if (ret) {
+			DRM_ERROR("Couldn't switch back to default context\n");
+			goto out;
+		}
+		ring->last_context = dev_priv->default_context;
 	}
+
+out:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
 }
 
 static int

@@ -428,7 +439,8 @@ void i915_context_init(struct drm_device *dev)
 		dev_priv->ctx_disable = 1;
 		idr_destroy(&dev_priv->i915_ctx_idr);
 	} else {
-		DRM_DEBUG_DRIVER("Context support enabled\n", ret);
+		DRM_DEBUG_DRIVER("Default context = %p\n",
+		 to_intel_bo(dev_priv->default_context->ctx_obj)->gtt_offset);
 	}
 }
drivers/gpu/drm/i915/i915_gem_execbuffer.c +9 -4
index 9d022fb..c1d9a0a 100644
@@ -990,7 +990,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	struct drm_i915_gem_object *batch_obj;
 	struct drm_clip_rect *cliprects = NULL;
 	struct intel_ring_buffer *ring;
-	struct drm_i915_gem_context *ctx;
+	struct drm_i915_gem_context *ctx = NULL;
 	u32 exec_start, exec_len;
 	u32 seqno;
 	u32 ctx_id;

@@ -1013,7 +1013,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			DRM_DEBUG_DRIVER("Context resubmission required\n");
 			return -EIO;
 		}
-	}
+	} else
+		ctx = dev_priv->default_context;
+
 #if WATCH_EXEC
 	DRM_INFO("buffers_ptr %d buffer_count %d len %08x\n",
 		  (int) args->buffers_ptr, args->buffer_count, args->batch_len);

@@ -1203,8 +1205,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		}
 	}
 
-	if (!dev_priv->ctx_disable)
-		ring->context_switch(ring, ctx, I915_CONTEXT_NORMAL_SWITCH);
+	if (!dev_priv->ctx_disable) {
+		ret = ring->context_switch(ring, ctx, I915_CONTEXT_NORMAL_SWITCH);
+		if (ret)
+			goto err;
+	}
 
 	exec_start = batch_obj->gtt_offset + args->batch_start_offset;
 	exec_len = args->batch_len;
drivers/gpu/drm/i915/intel_ringbuffer.c +43 -3

58 lines not on this page · raw message

drivers/gpu/drm/i915/intel_ringbuffer.h +1 -1
index 61525ba..e8f8b6a 100644
@@ -80,7 +80,7 @@ struct  intel_ring_buffer {
 					       u32 offset, u32 length);
 	void		(*cleanup)(struct intel_ring_buffer *ring);
 	struct		drm_i915_gem_context *last_context;
-	void		(*context_switch)(struct intel_ring_buffer *ring,
+	int		(*context_switch)(struct intel_ring_buffer *ring,
 					  struct drm_i915_gem_context *ctx,
 					  u32 flags);
Signature · 1 line
1.7.3.4