diff mbox

[RFC,6/7] drm/i915: reference count for i915_hw_contexts

Message ID 1359986683-29788-7-git-send-email-mika.kuoppala@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala Feb. 4, 2013, 2:04 p.m. UTC
When using i915_hw_context pointer in request struct,
we need to use reference counts as context might be
released before request processing takes place.

v2: track i915_hw_context pointers instead of using ctx_ids
    (from Chris Wilson)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |    4 +++-
 drivers/gpu/drm/i915/i915_gem.c            |   14 ++++++++++-
 drivers/gpu/drm/i915/i915_gem_context.c    |   35 +++++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 4 files changed, 46 insertions(+), 9 deletions(-)

Comments

Ben Widawsky Feb. 15, 2013, 5:55 a.m. UTC | #1
On Mon, Feb 04, 2013 at 04:04:42PM +0200, Mika Kuoppala wrote:
> When using i915_hw_context pointer in request struct,
> we need to use reference counts as context might be
> released before request processing takes place.
> 
> v2: track i915_hw_context pointers instead of using ctx_ids
>     (from Chris Wilson)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

It's satisfying to me to see this given that Daniel killed my version of
this patch so long ago.

http://lists.freedesktop.org/archives/intel-gfx/2012-March/015519.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b0df678..e2f8ea6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -438,6 +438,7 @@  struct i915_reset_stats {
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_ID 0
 struct i915_hw_context {
+	struct kref ref;
 	int id;
 	bool is_initialized;
 	struct drm_i915_file_private *file_priv;
@@ -1688,7 +1689,8 @@  void i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
 int i915_switch_context(struct intel_ring_buffer *ring,
-			struct drm_file *file, int to_id);
+			struct drm_file *file, int to_id,
+			struct i915_hw_context **ctx_out);
 void i915_gem_context_free(struct kref *ctx_ref);
 int i915_gem_context_get_reset_stats(struct intel_ring_buffer *ring,
 				     struct drm_file *file,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9aad0aa..b304b06 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2036,6 +2036,10 @@  i915_add_request(struct intel_ring_buffer *ring,
 	request->tail = request_ring_position;
 	request->batch_obj = obj;
 	request->ctx = ctx;
+
+	if (request->ctx)
+		kref_get(&request->ctx->ref);
+
 	request->emitted_jiffies = jiffies;
 	was_empty = list_empty(&ring->request_list);
 	list_add_tail(&request->list, &ring->request_list);
@@ -2100,6 +2104,10 @@  static void i915_gem_reset_ring_lists(struct drm_i915_private *dev_priv,
 
 		list_del(&request->list);
 		i915_gem_request_remove_from_client(request);
+
+		if (request->ctx)
+			kref_put(&request->ctx->ref, i915_gem_context_free);
+
 		kfree(request);
 	}
 
@@ -2194,6 +2202,10 @@  i915_gem_retire_requests_ring(struct intel_ring_buffer *ring)
 
 		list_del(&request->list);
 		i915_gem_request_remove_from_client(request);
+
+		if (request->ctx)
+			kref_put(&request->ctx->ref, i915_gem_context_free);
+
 		kfree(request);
 	}
 
@@ -2516,7 +2528,7 @@  int i915_gpu_idle(struct drm_device *dev)
 
 	/* Flush everything onto the inactive list. */
 	for_each_ring(ring, dev_priv, i) {
-		ret = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID);
+		ret = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID, NULL);
 		if (ret)
 			return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8e0d50c..7e1fdb1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -124,7 +124,14 @@  static int get_context_size(struct drm_device *dev)
 	return ret;
 }
 
-static void do_destroy(struct i915_hw_context *ctx)
+void i915_gem_context_free(struct kref *ctx_ref)
+{
+	struct i915_hw_context *ctx = container_of(ctx_ref,
+						   typeof(*ctx), ref);
+	kfree(ctx);
+}
+
+static void do_release(struct i915_hw_context *ctx)
 {
 	struct drm_device *dev = ctx->obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -135,6 +142,11 @@  static void do_destroy(struct i915_hw_context *ctx)
 		BUG_ON(ctx != dev_priv->ring[RCS].default_context);
 
 	drm_gem_object_unreference(&ctx->obj->base);
+}
+
+static void do_destroy(struct i915_hw_context *ctx)
+{
+	do_release(ctx);
 	kfree(ctx);
 }
 
@@ -150,6 +162,7 @@  create_hw_context(struct drm_device *dev,
 	if (ctx == NULL)
 		return ERR_PTR(-ENOMEM);
 
+	kref_init(&ctx->ref);
 	ctx->obj = i915_gem_alloc_object(dev, dev_priv->hw_context_size);
 	if (ctx->obj == NULL) {
 		kfree(ctx);
@@ -294,8 +307,8 @@  static int context_idr_cleanup(int id, void *p, void *data)
 
 	BUG_ON(id == DEFAULT_CONTEXT_ID);
 
-	do_destroy(ctx);
-
+	do_release(ctx);
+	kref_put(&ctx->ref, i915_gem_context_free);
 	return 0;
 }
 
@@ -481,10 +494,12 @@  static int do_switch(struct i915_hw_context *to)
  */
 int i915_switch_context(struct intel_ring_buffer *ring,
 			struct drm_file *file,
-			int to_id)
+			int to_id,
+			struct i915_hw_context **ctx_out)
 {
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct i915_hw_context *to;
+	int ret;
 
 	if (dev_priv->hw_contexts_disabled)
 		return 0;
@@ -503,7 +518,14 @@  int i915_switch_context(struct intel_ring_buffer *ring,
 			return -ENOENT;
 	}
 
-	return do_switch(to);
+	ret = do_switch(to);
+	if (ret)
+		return ret;
+
+	if (ctx_out)
+		*ctx_out = to;
+
+	return 0;
 }
 
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
@@ -557,7 +579,8 @@  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 		return -ENOENT;
 	}
 
-	do_destroy(ctx);
+	do_release(ctx);
+	kref_put(&ctx->ref, i915_gem_context_free);
 
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 312683c..3b76317 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -999,7 +999,7 @@  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (ret)
 		goto err;
 
-	ret = i915_switch_context(ring, file, ctx_id);
+	ret = i915_switch_context(ring, file, ctx_id, &ctx);
 	if (ret)
 		goto err;