Commit dce3271b authored by Mika Kuoppala's avatar Mika Kuoppala Committed by Daniel Vetter

drm/i915: reference count for i915_hw_contexts

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: Ben Widawsky <ben@bwidawsk.net> (v5)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> (v7)
Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
[danvet: Squash sob lines, the patch ping-ponged between Ben and Mika
a bit ...]
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 3c3686cd
...@@ -452,6 +452,7 @@ struct i915_hw_ppgtt { ...@@ -452,6 +452,7 @@ struct i915_hw_ppgtt {
/* This must match up with the value previously used for execbuf2.rsvd1. */ /* This must match up with the value previously used for execbuf2.rsvd1. */
#define DEFAULT_CONTEXT_ID 0 #define DEFAULT_CONTEXT_ID 0
struct i915_hw_context { struct i915_hw_context {
struct kref ref;
int id; int id;
bool is_initialized; bool is_initialized;
struct drm_i915_file_private *file_priv; struct drm_i915_file_private *file_priv;
...@@ -1697,6 +1698,17 @@ void i915_gem_context_fini(struct drm_device *dev); ...@@ -1697,6 +1698,17 @@ void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file); void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct intel_ring_buffer *ring, int i915_switch_context(struct intel_ring_buffer *ring,
struct drm_file *file, int to_id); 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, int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
struct drm_file *file); struct drm_file *file);
int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
......
...@@ -124,10 +124,10 @@ static int get_context_size(struct drm_device *dev) ...@@ -124,10 +124,10 @@ static int get_context_size(struct drm_device *dev)
return ret; return ret;
} }
static void do_destroy(struct i915_hw_context *ctx) void i915_gem_context_free(struct kref *ctx_ref)
{ {
if (ctx->file_priv) struct i915_hw_context *ctx = container_of(ctx_ref,
idr_remove(&ctx->file_priv->context_idr, ctx->id); typeof(*ctx), ref);
drm_gem_object_unreference(&ctx->obj->base); drm_gem_object_unreference(&ctx->obj->base);
kfree(ctx); kfree(ctx);
...@@ -145,6 +145,7 @@ create_hw_context(struct drm_device *dev, ...@@ -145,6 +145,7 @@ create_hw_context(struct drm_device *dev,
if (ctx == NULL) if (ctx == NULL)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
kref_init(&ctx->ref);
ctx->obj = i915_gem_alloc_object(dev, dev_priv->hw_context_size); ctx->obj = i915_gem_alloc_object(dev, dev_priv->hw_context_size);
if (ctx->obj == NULL) { if (ctx->obj == NULL) {
kfree(ctx); kfree(ctx);
...@@ -169,18 +170,18 @@ create_hw_context(struct drm_device *dev, ...@@ -169,18 +170,18 @@ create_hw_context(struct drm_device *dev,
if (file_priv == NULL) if (file_priv == NULL)
return ctx; return ctx;
ctx->file_priv = file_priv;
ret = idr_alloc(&file_priv->context_idr, ctx, DEFAULT_CONTEXT_ID + 1, 0, ret = idr_alloc(&file_priv->context_idr, ctx, DEFAULT_CONTEXT_ID + 1, 0,
GFP_KERNEL); GFP_KERNEL);
if (ret < 0) if (ret < 0)
goto err_out; goto err_out;
ctx->file_priv = file_priv;
ctx->id = ret; ctx->id = ret;
return ctx; return ctx;
err_out: err_out:
do_destroy(ctx); i915_gem_context_unreference(ctx);
return ERR_PTR(ret); return ERR_PTR(ret);
} }
...@@ -226,7 +227,7 @@ static int create_default_context(struct drm_i915_private *dev_priv) ...@@ -226,7 +227,7 @@ static int create_default_context(struct drm_i915_private *dev_priv)
err_unpin: err_unpin:
i915_gem_object_unpin(ctx->obj); i915_gem_object_unpin(ctx->obj);
err_destroy: err_destroy:
do_destroy(ctx); i915_gem_context_unreference(ctx);
return ret; return ret;
} }
...@@ -262,6 +263,7 @@ void i915_gem_context_init(struct drm_device *dev) ...@@ -262,6 +263,7 @@ void i915_gem_context_init(struct drm_device *dev)
void i915_gem_context_fini(struct drm_device *dev) void i915_gem_context_fini(struct drm_device *dev)
{ {
struct drm_i915_private *dev_priv = dev->dev_private; 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) if (dev_priv->hw_contexts_disabled)
return; return;
...@@ -271,9 +273,8 @@ void i915_gem_context_fini(struct drm_device *dev) ...@@ -271,9 +273,8 @@ void i915_gem_context_fini(struct drm_device *dev)
* other code, leading to spurious errors. */ * other code, leading to spurious errors. */
intel_gpu_reset(dev); intel_gpu_reset(dev);
i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj); i915_gem_object_unpin(dctx->obj);
i915_gem_context_unreference(dctx);
do_destroy(dev_priv->ring[RCS].default_context);
} }
static int context_idr_cleanup(int id, void *p, void *data) 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) ...@@ -282,8 +283,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
BUG_ON(id == DEFAULT_CONTEXT_ID); BUG_ON(id == DEFAULT_CONTEXT_ID);
do_destroy(ctx); i915_gem_context_unreference(ctx);
return 0; return 0;
} }
...@@ -512,8 +512,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, ...@@ -512,8 +512,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
return -ENOENT; return -ENOENT;
} }
do_destroy(ctx); idr_remove(&ctx->file_priv->context_idr, ctx->id);
i915_gem_context_unreference(ctx);
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id); DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment