[PATCH 0/8] drm/i915/context

[PATCH 0/8] drm/i915/context

[Ben Widawsky]

This is pretty much the same as before except it addresses comments from
ickle and jbarnes.  

The params to tell userspace what is supported by the driver is now the
last commit and has been squashed in with a related change I made to the
params

All deinit chas been hanged to fini rename. Bad habit leftover from my
WinCE days.

Bad verbage: i915_context_handle_binding()->i915_context_bind_ppgtt()

Ben

[PATCH 1/8] drm/i915/context: basic implementation context ioctls

[Ben Widawsky]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 9461 bytes --]

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

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0ae6a7c..244536a 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -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 \
diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
new file mode 100644
index 0000000..0d140b8
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -0,0 +1,113 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *
+ */
+
+#include "drmP.h"
+#include "drm.h"
+#include "i915_drm.h"
+#include "intel_drv.h"
+
+static struct drm_i915_gem_context *
+i915_gem_lookup_ctx_id(struct drm_device *dev,
+		       uint32_t id)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	return idr_find(&dev_priv->i915_ctx_idr, id);
+}
+
+static void
+i915_gem_del_ctx_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,
+			  uint64_t aperture_size,
+			  struct drm_i915_gem_context **ctx_out)
+{
+	return -ENOMEM;
+}
+
+int
+i915_gem_ctx_create_ioctl(struct drm_device *dev, void *data,
+			  struct drm_file *file)
+{
+	struct drm_i915_gem_ctx_create *args = data;
+	struct drm_i915_gem_context *ctx = NULL;
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+	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 (ret) {
+		goto out;
+	}
+
+	ctx->bufs = drm_malloc_ab(slots + 1, sizeof(struct drm_gem_object*));
+	if (ctx->bufs == NULL) {
+		kfree(ctx);
+		drm_free_large(ctx->bufs);
+		ret = -ENOMEM;
+		goto out;
+	}
+	ctx->slot_count = slots;
+	for(i = 0; i < slots + 1; i++) {
+		ctx->bufs[i] = NULL;
+	}
+
+	list_add(&ctx->file_list, &file_priv->context.context_list);
+
+	args->ctx_id = ctx->ctx_id;
+out:
+	return ret;
+}
+
+int
+i915_gem_ctx_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;
+
+	ctx = i915_gem_lookup_ctx_id(dev, args->ctx_id);
+	if (ctx == NULL) {
+		return -EINVAL;
+	}
+
+	list_for_each_safe(pos, n, &file_priv->context.context_list) {
+		ctx_temp = list_entry(pos, struct drm_i915_gem_context, context_list);
+	}
+	i915_gem_del_ctx_id(dev, ctx);
+	kfree(ctx);
+
+	/* TODO: ring switch may be needed */
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 17bd766..91f062e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2249,6 +2249,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);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0149c6..28affdd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -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 {
@@ -1090,6 +1120,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,
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 0039f1f..692de60 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -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.
@@ -843,4 +848,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_ */
-- 
1.7.3.4


[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/8] drm/i915/context: context initialization/destruction

[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(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index 0d140b8..6284080 100644
--- a/drivers/gpu/drm/i915/i915_context.c
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -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 int i915_context_hw_fini(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_fini(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_fini(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);
+}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 91f062e..8dbf17c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2035,6 +2035,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:
@@ -2060,6 +2061,8 @@ int i915_driver_unload(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
+	i915_context_fini(dev);
+
 	spin_lock(&mchdev_lock);
 	i915_mch_dev = NULL;
 	spin_unlock(&mchdev_lock);
@@ -2159,6 +2162,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;
 }
@@ -2197,6 +2202,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);
 }
@@ -2249,8 +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),
+	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);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 28affdd..15635dd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -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;
@@ -1120,9 +1120,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);
@@ -1316,6 +1316,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_fini(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) \
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6218fa9..89adb9d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -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 */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 6d6fde8..68ebecf 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -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
-- 
1.7.3.4

[PATCH 3/8] drm/i915/context: whitespace cleanup, and warning cleanup

[Ben Widawsky]

---
 drivers/gpu/drm/i915/i915_context.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index 6284080..dec3b02 100644
--- a/drivers/gpu/drm/i915/i915_context.c
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -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_fini(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);
 }
-- 
1.7.3.4

[PATCH 4/8] drm/i915/context: minimal support for contexts in execbuffer2

[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(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index dec3b02..dac09be 100644
--- a/drivers/gpu/drm/i915/i915_context.c
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -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.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 15635dd..d21b125 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1320,6 +1320,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_fini(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])
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d2f445e..a60996d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -979,7 +979,9 @@ static int
 i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		       struct drm_file *file,
 		       struct drm_i915_gem_execbuffer2 *args,
-		       struct drm_i915_gem_exec_object2 *exec)
+		       struct drm_i915_gem_exec_object2 *exec,
+		       struct drm_i915_context_flag *ctx_flags,
+		       int flag_count)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct list_head objects;
@@ -999,7 +1001,15 @@ 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_flags, flag_count);
+		if (ret) {
+			if (ret == -EAGAIN)
+				DRM_DEBUG_DRIVER("Context resubmission required\n");
+			return ret;
+		}
+	}
 #if WATCH_EXEC
 	DRM_INFO("buffers_ptr %d buffer_count %d len %08x\n",
 		  (int) args->buffers_ptr, args->buffer_count, args->batch_len);
@@ -1299,7 +1309,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
 	exec2.cliprects_ptr = args->cliprects_ptr;
 	exec2.flags = I915_EXEC_RENDER;
 
-	ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list);
+	ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list,
+				     NULL, 0);
 	if (!ret) {
 		/* Copy the new buffer offsets back to the user's exec list. */
 		for (i = 0; i < args->buffer_count; i++)
@@ -1328,6 +1339,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
 {
 	struct drm_i915_gem_execbuffer2 *args = data;
 	struct drm_i915_gem_exec_object2 *exec2_list = NULL;
+	struct drm_i915_context_flag *flags = NULL;
+	uint32_t flag_count = 0;
 	int ret;
 
 #if WATCH_EXEC
@@ -1356,8 +1369,32 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
 		drm_free_large(exec2_list);
 		return -EFAULT;
 	}
+	if (EXECBUFFER2_FLAGS_PTR(args) && EXECBUFFER2_FLAGS_COUNT(argc)) {
+		flag_count = EXECBUFFER2_FLAGS_COUNT(argc);
+		flags = drm_malloc_ab(sizeof(*flags), flag_count);
+		if (flags == NULL) {
+			DRM_ERROR("allocation of flags failed\n");
+			drm_free_large(exec2_list);
+			/*  return -EAGAIN; */
+			return -ENOMEM;
+		}
 
-	ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list);
+		ret = copy_from_user(flags,
+				     (struct drm_context_flags __user *)
+				     (uintptr_t)EXECBUFFER2_FLAGS_PTR(args),
+				     sizeof(*flags) * flag_count);
+		if (ret != 0) {
+			DRM_ERROR("copy %d flags failed %d\n",
+				  flag_count, ret);
+			drm_free_large(flags);
+			drm_free_large(exec2_list);
+			return -EFAULT;
+		}
+		ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list,
+					     flags, flag_count);
+	} else
+		ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list,
+					     NULL, 0);
 	if (!ret) {
 		/* Copy the new buffer offsets back to the user's exec list. */
 		ret = copy_to_user((struct drm_i915_relocation_entry __user *)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 692de60..76b37f6 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -657,6 +657,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;
-- 
1.7.3.4

[PATCH 5/8] drm/i915/context: context validation for execbuffer2

[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            |   13 ++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   12 ++-
 include/drm/i915_drm.h                     |    5 +-
 4 files changed, 130 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index dac09be..4795caf 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_bind_ppgtt(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 d21b125..dc7b132 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)
@@ -1321,10 +1323,13 @@ extern void i915_context_init(struct drm_device *dev);
 extern void i915_context_fini(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_bind_ppgtt(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..356388a 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_bind_ppgtt(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 76b37f6..5605e97 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -667,8 +667,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

Re: [PATCH 5/8] drm/i915/context: context validation for execbuffer2

[Daniel Vetter]

On Wed, Feb 02, 2011 at 03:00:17PM -0800, Ben Widawsky wrote:
> 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.

Hm, this remark got me thinking (because currently with read/write domains
per reloc domain changes on the fly are super-simple): Why not drop the
whole associate/disassociate idea and require userspace to always submit a
full list of bos still used by this context (and their required
offsets)?

Upsides:
- No fiddling when reused bos change domains.
- No need to track stuff in the kernel, we only check the offsets.
- Userspace implementation sounds rather straightforward, too: If the
  aperture check fails, submit the batchbuffer and then store all bos
  bound to the current gl context (assuming a one-on-one hw context <-> gl
  context mapping) and their desired domains (also easy because the usage
  is known). Then on the next execbuffer use that stored array for the
  additionally required bos (your flag array).
- Improvements like ppgtt or (re-)pinning bos at the right place can still
  be added incrementally. After all, you already require userspace to be
  able to do a full context restore as part of your abi ...
Downsides:
- Might no be optimal. But given our constant fiddling with the
  batchbuffer submission, expecting to get this right on the first try is
  probably wishful thinking.

Just my 2 (constantly changing) cents on this. Anyway, you've thought much
more about this than me, so I expect you to shoot this down in a
half-sentence ... ;)

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 5/8] drm/i915/context: context validation for execbuffer2

[Ben Widawsky]

On Mon, Feb 14, 2011 at 09:21:22PM +0100, Daniel Vetter wrote:
> On Wed, Feb 02, 2011 at 03:00:17PM -0800, Ben Widawsky wrote:
> > 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.
> 
> Hm, this remark got me thinking (because currently with read/write domains
> per reloc domain changes on the fly are super-simple): Why not drop the
> whole associate/disassociate idea and require userspace to always submit a
> full list of bos still used by this context (and their required
> offsets)?
> 
The code has changed quite a bit since these patches:
http://cgit.freedesktop.org/~bwidawsk/drm-intel/

I have no problem with this. The complex part of the code has become
tracking when I can retire the context's backing object. As I see it,
your proposal is essentially an implicit associate (if it has an
offset), with no disassociate. In terms of the driver code to handle it,
it would just read through the flags instead of slots.

I had, and still have very little clue as to what clients of the
interface would like. I'm very scared of digging into mesa. A fear which
I am trying to get over presently. So if people think this is at least
as good, or better - then great!

I think what I'll do is fix the couple of outstanding bugs I'm aware
of, get a couple of better unit tests, do some kind of code review on
the result of that, start tinkering with mesa - and then decide which
one seems better. Unless others chime in and say they like one over the
other...

Your argument is pretty convincing. It seems the only advantage of slots
is you can save on the number of flags submitted per execbuffer call,
which is probably negligible since multiple people have said there are
relatively few buffers which we actually need to be tied to a context
anyway.

>   expecting to get this right on the first try is probably wishful
>   thinking.

This is my opinion as well, but I'm not sure everyone agrees.

> 
> Yours, Daniel

Thanks Daniel.
Ben

[PATCH 6/8] drm/i915/context: enable calling context_switch

[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_gem_execbuffer.c |   13 ++++++++-----
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    2 +-
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index 4795caf..1085797 100644
--- a/drivers/gpu/drm/i915/i915_context.c
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -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_bind_ppgtt(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 356388a..b2313f0 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -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) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 68ebecf..61525ba 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -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
-- 
1.7.3.4

[PATCH 7/8] drm/i915/context: Insert MI_SET_CONTEXT in ringbuffer context switch

[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        |   26 +++++++++++----
 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, 72 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_context.c b/drivers/gpu/drm/i915/i915_context.c
index 1085797..7f47563 100644
--- a/drivers/gpu/drm/i915/i915_context.c
+++ b/drivers/gpu/drm/i915/i915_context.c
@@ -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) {
@@ -265,17 +265,28 @@ static int i915_context_hw_fini(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);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index b2313f0..91133f3 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -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;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 89adb9d..4381730 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -750,15 +750,55 @@ render_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 	return 0;
 }
 
-static void
+static int
 render_ring_ctx_switch(struct intel_ring_buffer *ring,
 		       struct drm_i915_gem_context *ctx,
-		       uint32_t flags)
+		       u32 flags)
 {
+	struct drm_device *dev = ring->dev;
+	struct drm_gem_object *obj;
+	uint32_t ctx_switch_flags;
+	int ret = 0;
+
 	if (ring->last_context == ctx)
-		return;
+		return 0;
+
+	DRM_DEBUG_DRIVER("Context switch to %d\n",
+			 ctx->ctx_id);
 
 	ring->last_context = ctx;
+	obj = ctx->ctx_obj;
+
+	if (flags == I915_CONTEXT_SAVE_ONLY)
+		ctx_switch_flags = MI_RESTORE_INHIBIT;
+	else
+		ctx_switch_flags = 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	if (IS_IRONLAKE_D(dev) || IS_IRONLAKE_M(dev))
+		intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
+	else
+		intel_ring_emit(ring, MI_NOOP);
+
+	intel_ring_emit(ring, MI_SET_CONTEXT);
+	intel_ring_emit(ring, to_intel_bo(obj)->gtt_offset |
+			MI_MM_SPACE_GTT |
+			MI_SAVE_EXT_STATE_EN |
+			MI_RESTORE_EXT_STATE_EN |
+			ctx_switch_flags);
+
+	if (IS_IRONLAKE_D(dev) || IS_IRONLAKE_M(dev))
+		intel_ring_emit(ring, MI_SUSPEND_FLUSH);
+		/* TODO: we may need a NOOP here */
+	else
+		intel_ring_emit(ring, MI_NOOP);
+
+	intel_ring_advance(ring);
+
+	return 0;
 }
 
 static void cleanup_status_page(struct intel_ring_buffer *ring)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 61525ba..e8f8b6a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -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);
 
-- 
1.7.3.4

[PATCH 8/8] drm/i915/context: context switch, and PPGTT params

[Ben Widawsky]

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8dbf17c..1c1eecc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -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 = dev_priv->ctx_disable ? 0 : 1;
+		break;
+	case I915_PARAM_HAS_PPGTT:
+		value = HAS_PPGTT(dev);
+		break;
 	default:
 		DRM_DEBUG_DRIVER("Unknown parameter %d\n",
 				 param->param);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dc7b132..5324742 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -982,6 +982,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[];
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 5605e97..dbd0332 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -295,6 +295,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;
-- 
1.7.3.4

Re: [PATCH 0/8] drm/i915/context

[Chris Wilson]

I don't think we should be pinning contexts for their entire lifetime.
The page should only be pinned whilst it is being referenced by the
hardware, and it looks like you already parts of the infrastructure to
be able to track this. You will also need to track seqno and retire
contexts...

Being picky, there are still renames ;-) But it is much easier to see
what you are introducing in each step, thanks!

I'm not convinced by the locking strategy, there seems to be quite a few
holes left, and is something else that is much easier to make it correct
from the beginning and incrementally adjust.

And then there are lots of little changes to try and make the code
tidier... But those are probably best left for another day.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/8] drm/i915/context

[Ben Widawsky]

On Wed, Feb 02, 2011 at 11:29:37PM +0000, Chris Wilson wrote:
> I don't think we should be pinning contexts for their entire lifetime.
> The page should only be pinned whilst it is being referenced by the
> hardware, and it looks like you already parts of the infrastructure to
> be able to track this. You will also need to track seqno and retire
> contexts...
> 

I have given thought to not pinning the contexts multiple times, and
believe it is ultimately what we will end up doing.  However, the
current implementation is definitely the easier, and more stable way to
go about it. So I'd definitely prefer to keep this as is until the other
parts of the stack are functional.

We need to consider how many clients we will actually have at one time.
I have no idea.  If we're talking a common case of 2 or 3 contexts, the
overhead may not be worth it. Retiring of contexts is very simple the
way it is now too because it only happens voluntarily.

> I'm not convinced by the locking strategy, there seems to be quite a few
> holes left, and is something else that is much easier to make it correct
> from the beginning and incrementally adjust.

Yes, I still need to review this, and I totally agree that it must be
right the first time. Is there something in particular that
bothers you about it?

> 
> And then there are lots of little changes to try and make the code
> tidier... But those are probably best left for another day.
> -Chris

Thanks for the feedback.
Ben