[Intel-gfx] [PATCH 6/7] drm/i915: Store last context instead of the obj
Ben Widawsky
ben at bwidawsk.net
Fri Apr 5 01:41:53 CEST 2013
Storing the last context requires refcounting. Mika recently submitted
some refcounting patches which leverages our request mechanism. This is
insufficient for my needs because we want to know the last context even
if the request has ended, ie. doing the kref_put when a request is
finished isn't okay (unless we switch back to the default context, and
wait for the switch)
Context lifecycle works identically to the way the context object
lifecycle works.
As of now, I'm not making use of the context_status field. It seems like
it will be useful at least for debugging, but we can drop it if desired.
Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++
drivers/gpu/drm/i915/i915_gem.c | 2 +
drivers/gpu/drm/i915/i915_gem_context.c | 83 ++++++++++++++++++++++++---------
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
4 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 25cdade..3025b65 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -438,9 +438,12 @@ 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;
enum {
I915_CTX_INITIALIZED=1,
+ I915_CTX_FILE_CLOSED,
+ I915_CTX_DESTROY_IOCTL,
} status;
struct drm_i915_file_private *file_priv;
struct intel_ring_buffer *ring;
@@ -1666,6 +1669,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *gem_obj, int flags);
/* i915_gem_context.c */
+void ctx_release(struct kref *kref);
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);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8a2cbee..4097173 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1924,6 +1924,8 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
obj->fenced_gpu_access = false;
obj->active = 0;
+ if (obj->ctx)
+ kref_put(&obj->ctx->ref, ctx_release);
drm_gem_object_unreference(&obj->base);
WARN_ON(i915_verify_lists(dev));
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 41be2a5..f57c91a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -124,13 +124,25 @@ static int get_context_size(struct drm_device *dev)
static void do_destroy(struct i915_hw_context *ctx)
{
- if (ctx->file_priv)
+ /* Need to remove the idr if close/destroy was called while the context
+ * was active
+ */
+ if (ctx->status == I915_CTX_DESTROY_IOCTL)
idr_remove(&ctx->file_priv->context_idr, ctx->id);
drm_gem_object_unreference(&ctx->obj->base);
+ if (WARN_ON(kref_get_unless_zero(&ctx->ref)))
+ kref_put(&ctx->ref, ctx_release);
kfree(ctx);
}
+void ctx_release(struct kref *kref)
+{
+ struct i915_hw_context *ctx = container_of(kref,
+ struct i915_hw_context, ref);
+ do_destroy(ctx);
+}
+
static struct i915_hw_context *
create_hw_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
@@ -159,8 +171,10 @@ create_hw_context(struct drm_device *dev,
ctx->ring = &dev_priv->ring[RCS];
/* Default context will never have a file_priv */
- if (file_priv == NULL)
+ if (file_priv == NULL) {
+ kref_init(&ctx->ref);
return ctx;
+ }
ctx->file_priv = file_priv;
@@ -169,6 +183,7 @@ create_hw_context(struct drm_device *dev,
if (ret < 0)
goto err_out;
ctx->id = ret;
+ kref_init(&ctx->ref);
return ctx;
@@ -209,6 +224,8 @@ static int create_default_context(struct drm_i915_private *dev_priv)
if (ret)
goto err_destroy;
+ kref_get(&ctx->ref);
+
ret = do_switch(ctx);
if (ret)
goto err_unpin;
@@ -266,7 +283,8 @@ void i915_gem_context_fini(struct drm_device *dev)
i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj);
- do_destroy(dev_priv->ring[RCS].default_context);
+ while (!kref_put(&dev_priv->ring[RCS].default_context->ref,
+ ctx_release));
}
static int context_idr_cleanup(int id, void *p, void *data)
@@ -275,8 +293,11 @@ static int context_idr_cleanup(int id, void *p, void *data)
BUG_ON(id == DEFAULT_CONTEXT_ID);
- DRM_DEBUG_DRIVER("Context %d closed before destroy.\n", ctx->id);
- do_destroy(ctx);
+ if (ctx->status != I915_CTX_DESTROY_IOCTL) {
+ DRM_DEBUG_DRIVER("Context %d closed before destroy.\n", ctx->id);
+ kref_put(&ctx->ref, ctx_release);
+ }
+ ctx->status = I915_CTX_FILE_CLOSED;
return 0;
}
@@ -303,6 +324,9 @@ i915_gem_context_get(struct intel_ring_buffer *ring,
ctx = (struct i915_hw_context *)
idr_find(&file_priv->context_idr, id);
+ if (likely(ctx))
+ kref_get(&ctx->ref);
+
return ctx;
}
@@ -356,18 +380,20 @@ mi_set_context(struct intel_ring_buffer *ring,
static int do_switch(struct i915_hw_context *to)
{
struct intel_ring_buffer *ring = to->ring;
- struct drm_i915_gem_object *from_obj = ring->last_context_obj;
+ struct i915_hw_context *from_ctx = ring->last_context;
u32 hw_flags = 0;
int ret;
- BUG_ON(from_obj != NULL && from_obj->pin_count == 0);
+ BUG_ON(from_ctx != NULL && from_ctx->obj->pin_count == 0);
- if (from_obj == to->obj)
- return 0;
+ if (from_ctx == to) {
+ ret = 0;
+ goto err_out;
+ }
ret = i915_gem_object_pin(to->obj, CONTEXT_ALIGN, false, false);
if (ret)
- return ret;
+ goto err_out;
/* Clear this page out of any CPU caches for coherent swap-in/out. Note
* that thanks to write = false in this call and us not setting any gpu
@@ -377,7 +403,7 @@ static int do_switch(struct i915_hw_context *to)
ret = i915_gem_object_set_to_gtt_domain(to->obj, false);
if (ret) {
i915_gem_object_unpin(to->obj);
- return ret;
+ goto err_out;
}
if (!to->obj->has_global_gtt_mapping)
@@ -385,13 +411,13 @@ static int do_switch(struct i915_hw_context *to)
if (!to->status || is_default_context(to))
hw_flags |= MI_RESTORE_INHIBIT;
- else if (WARN_ON_ONCE(from_obj == to->obj)) /* not yet expected */
+ else if (WARN_ON_ONCE(from_ctx == to)) /* not yet expected */
hw_flags |= MI_FORCE_RESTORE;
ret = mi_set_context(ring, to, hw_flags);
if (ret) {
i915_gem_object_unpin(to->obj);
- return ret;
+ goto err_out;
}
/* The backing object for the context is done after switching to the
@@ -400,9 +426,9 @@ static int do_switch(struct i915_hw_context *to)
* is a bit suboptimal because the retiring can occur simply after the
* MI_SET_CONTEXT instead of when the next seqno has completed.
*/
- if (likely(from_obj)) {
- from_obj->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
- i915_gem_object_move_to_active(from_obj, ring);
+ if (likely(from_ctx)) {
+ from_ctx->obj->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+ i915_gem_object_move_to_active(from_ctx->obj, ring);
/* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
* whole damn pipeline, we don't need to explicitly mark the
* object dirty. The only exception is that the context must be
@@ -410,18 +436,22 @@ static int do_switch(struct i915_hw_context *to)
* able to defer doing this until we know the object would be
* swapped, but there is no way to do that yet.
*/
- from_obj->dirty = 1;
- BUG_ON(from_obj->ring != ring);
- i915_gem_object_unpin(from_obj);
+ from_ctx->obj->dirty = 1;
+ BUG_ON(from_ctx->obj->ring != ring);
+ i915_gem_object_unpin(from_ctx->obj);
- drm_gem_object_unreference(&from_obj->base);
+ drm_gem_object_unreference(&from_ctx->obj->base);
}
drm_gem_object_reference(&to->obj->base);
- ring->last_context_obj = to->obj;
+ ring->last_context = to;
to->status = I915_CTX_INITIALIZED;
return 0;
+
+err_out:
+ kref_put(&to->ref, ctx_release);
+ return ret;
}
/**
@@ -458,6 +488,9 @@ int i915_switch_context(struct intel_ring_buffer *ring,
BUG_ON(to_id == DEFAULT_CONTEXT_ID);
DRM_DEBUG_DRIVER("Couldn't find context %d\n", to_id);
return -ENOENT;
+ } else if (to->status == I915_CTX_DESTROY_IOCTL) {
+ BUG_ON(kref_put(&to->ref, ctx_release));
+ return -ENOENT;
}
return do_switch(to);
@@ -488,7 +521,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
return PTR_ERR(ctx);
args->ctx_id = ctx->id;
- DRM_DEBUG_DRIVER("HW context %d created\n", args->ctx_id);
+ DRM_DEBUG_DRIVER("HW context %d created (%p)\n", args->ctx_id, ctx);
return 0;
}
@@ -517,7 +550,11 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
return -ENOENT;
}
- do_destroy(ctx);
+ BUG_ON(kref_put(&ctx->ref, ctx_release));
+ ctx->status = I915_CTX_DESTROY_IOCTL;
+ /* NB: We can't remove from the idr yet because a user might create a
+ * new context, and we need to reserve this id */
+ kref_put(&ctx->ref, ctx_release);
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d66208c..dac1614 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -135,7 +135,7 @@ struct intel_ring_buffer {
*/
bool itlb_before_ctx_switch;
struct i915_hw_context *default_context;
- struct drm_i915_gem_object *last_context_obj;
+ struct i915_hw_context *last_context;
void *private;
};
--
1.8.2
More information about the Intel-gfx
mailing list