Skip to content

Commit

Permalink
drm/i915: reference count for i915_hw_contexts
Browse files Browse the repository at this point in the history
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: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
[danvet: Squash sob lines, the patch ping-ponged between Ben and Mika
a bit ...]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
  • Loading branch information
Mika Kuoppala authored and Daniel Vetter committed Apr 30, 2013
1 parent 3c3686c commit dce3271
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
12 changes: 12 additions & 0 deletions drivers/gpu/drm/i915/i915_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
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,
Expand Down
28 changes: 14 additions & 14 deletions drivers/gpu/drm/i915/i915_gem_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit dce3271

Please sign in to comment.