[Intel-gfx] [PATCH 1/2] drm/i915: reference count for i915_hw_contexts

[Mika Kuoppala]

Enabling PPGTT and also the need to track which context was guilty of
gpu hang (arb robustness enabling) have put pressure for struct i915_hw_context
to be more than just a placeholder for hw context state.

In order to track object lifetime properly in a multi peer usage, add reference
counting for i915_hw_context.

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

v3 (Ben): Get rid of do_release() and handle refcounting more compactly.
(recommended by Chis)

v4: kref_* put inside static inlines (Daniel Vetter)
remove code duplication on freeing context (Chris Wilson)

v5: idr_remove and ctx->file_priv = NULL in destroy ioctl (Chris)
This actually will cause a problem if one destroys a context and later
refers to the idea of the context (multiple contexts may have the same
id, but only 1 will exist in the idr).

v6: Strip out the request related stuff. Reworded commit message.
Got rid of do_destroy and introduced i915_gem_context_release_handle,
suggested by Chris Wilson.

v7: idr_remove can't be called inside idr_for_each (Chris Wilson)

Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com> (v2)
Signed-off-by: Ben Widawsky <ben at bwidawsk.net> (v3)
Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com> (v4)
Signed-off-by: Ben Widawsky <ben at bwidawsk.net> (v5)
Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com> (v7)
---
 drivers/gpu/drm/i915/i915_drv.h         |   12 ++++++++++++
 drivers/gpu/drm/i915/i915_gem_context.c |   28 ++++++++++++++--------------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 14156f2..7f83be4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -452,6 +452,7 @@ struct i915_hw_ppgtt {
 /* 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;
@@ -1701,6 +1702,17 @@ 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);
+void i915_gem_context_free(struct kref *ctx_ref);
+static inline void i915_gem_context_reference(struct i915_hw_context *ctx)
+{
+	kref_get(&ctx->ref);
+}
+
+static inline void i915_gem_context_unreference(struct i915_hw_context *ctx)
+{
+	kref_put(&ctx->ref, i915_gem_context_free);
+}
+
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a1e8ecb..9e8c685 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -124,10 +124,10 @@ 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)
 {
-	if (ctx->file_priv)
-		idr_remove(&ctx->file_priv->context_idr, ctx->id);
+	struct i915_hw_context *ctx = container_of(ctx_ref,
+						   typeof(*ctx), ref);
 
 	drm_gem_object_unreference(&ctx->obj->base);
 	kfree(ctx);
@@ -145,6 +145,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);
@@ -169,18 +170,18 @@ create_hw_context(struct drm_device *dev,
 	if (file_priv == NULL)
 		return ctx;
 
-	ctx->file_priv = file_priv;
-
 	ret = idr_alloc(&file_priv->context_idr, ctx, DEFAULT_CONTEXT_ID + 1, 0,
 			GFP_KERNEL);
 	if (ret < 0)
 		goto err_out;
+
+	ctx->file_priv = file_priv;
 	ctx->id = ret;
 
 	return ctx;
 
 err_out:
-	do_destroy(ctx);
+	i915_gem_context_unreference(ctx);
 	return ERR_PTR(ret);
 }
 
@@ -226,7 +227,7 @@ static int create_default_context(struct drm_i915_private *dev_priv)
 err_unpin:
 	i915_gem_object_unpin(ctx->obj);
 err_destroy:
-	do_destroy(ctx);
+	i915_gem_context_unreference(ctx);
 	return ret;
 }
 
@@ -262,6 +263,7 @@ void i915_gem_context_init(struct drm_device *dev)
 void i915_gem_context_fini(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_hw_context *dctx = dev_priv->ring[RCS].default_context;
 
 	if (dev_priv->hw_contexts_disabled)
 		return;
@@ -271,9 +273,8 @@ void i915_gem_context_fini(struct drm_device *dev)
 	 * other code, leading to spurious errors. */
 	intel_gpu_reset(dev);
 
-	i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj);
-
-	do_destroy(dev_priv->ring[RCS].default_context);
+	i915_gem_object_unpin(dctx->obj);
+	i915_gem_context_unreference(dctx);
 }
 
 static int context_idr_cleanup(int id, void *p, void *data)
@@ -282,8 +283,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
 
 	BUG_ON(id == DEFAULT_CONTEXT_ID);
 
-	do_destroy(ctx);
-
+	i915_gem_context_unreference(ctx);
 	return 0;
 }
 
@@ -512,8 +512,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 		return -ENOENT;
 	}
 
-	do_destroy(ctx);
-
+	idr_remove(&ctx->file_priv->context_idr, ctx->id);
+	i915_gem_context_unreference(ctx);
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
-- 
1.7.9.5