Re: [Intel-gfx] [PATCH v3 03/16] drm/i915: reference count for i915_hw_contexts

2013-04-09 Thread Chris Wilson
On Thu, Apr 04, 2013 at 06:32:35PM +0300, Mika Kuoppala wrote:
 In preparation to do analysis of which context was
 guilty of gpu hung, store kreffed context pointer
 into request struct.
 
 This allows us to inspect contexts when gpu is reset
 even if those contexts would already be released
 by userspace.
 
 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)
 
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com (v2)
 Signed-off-by: Ben Widawsky b...@bwidawsk.net (v3)
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com (v4)
 ---
 @@ -511,7 +520,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device 
 *dev, void *data,
   return -ENOENT;
   }
  
 - do_destroy(ctx);
 + i915_gem_context_unreference(ctx);

This needs to remove the ctx from the idr as well as dropping the
reference associated with the id.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v3 03/16] drm/i915: reference count for i915_hw_contexts

2013-04-04 Thread Mika Kuoppala
In preparation to do analysis of which context was
guilty of gpu hung, store kreffed context pointer
into request struct.

This allows us to inspect contexts when gpu is reset
even if those contexts would already be released
by userspace.

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)

Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com (v2)
Signed-off-by: Ben Widawsky b...@bwidawsk.net (v3)
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com (v4)
---
 drivers/gpu/drm/i915/i915_drv.h|   20 ++--
 drivers/gpu/drm/i915/i915_gem.c|   27 ---
 drivers/gpu/drm/i915/i915_gem_context.c|   19 ++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |7 ---
 4 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e5e2083..3c85c1b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -456,6 +456,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;
@@ -1260,6 +1261,9 @@ struct drm_i915_gem_request {
/** Postion in the ringbuffer of the end of the request */
u32 tail;
 
+   /** Context related to this request */
+   struct i915_hw_context *ctx;
+
/** Time at which this request was emitted, in jiffies. */
unsigned long emitted_jiffies;
 
@@ -1646,9 +1650,10 @@ int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_idle(struct drm_device *dev);
 int _i915_add_request(struct intel_ring_buffer *ring,
  u32 *seqno,
- struct drm_file *file);
+ struct drm_file *file,
+ struct i915_hw_context *ctx);
 #define i915_add_request(ring, seqno) \
-   _i915_add_request(ring, seqno, NULL)
+   _i915_add_request(ring, seqno, NULL, NULL)
 int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
 uint32_t seqno);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
@@ -1692,6 +1697,17 @@ void i915_gem_context_close(struct drm_device *dev, 
struct drm_file *file);
 struct i915_hw_context * __must_check
 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.c b/drivers/gpu/drm/i915/i915_gem.c
index d3ce0a7..f586f9c4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2001,7 +2001,8 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 
 int _i915_add_request(struct intel_ring_buffer *ring,
  u32 *out_seqno,
- struct drm_file *file)
+ struct drm_file *file,
+ struct i915_hw_context *ctx)
 {
drm_i915_private_t *dev_priv = ring-dev-dev_private;
struct drm_i915_gem_request *request;
@@ -2041,6 +2042,11 @@ int _i915_add_request(struct intel_ring_buffer *ring,
request-seqno = intel_ring_get_seqno(ring);
request-ring = ring;
request-tail = request_ring_position;
+   request-ctx = ctx;
+
+   if (request-ctx)
+   i915_gem_context_reference(ctx);
+
request-emitted_jiffies = jiffies;
was_empty = list_empty(ring-request_list);
list_add_tail(request-list, ring-request_list);
@@ -2093,6 +2099,17 @@ i915_gem_request_remove_from_client(struct 
drm_i915_gem_request *request)
spin_unlock(file_priv-mm.lock);
 }
 
+static void i915_gem_free_request(struct drm_i915_gem_request *request)
+{
+   list_del(request-list);
+   i915_gem_request_remove_from_client(request);
+
+   if (request-ctx)
+   i915_gem_context_unreference(request-ctx);
+
+   kfree(request);
+}
+
 static void i915_gem_reset_ring_lists(struct drm_i915_private *dev_priv,
  struct intel_ring_buffer *ring)
 {
@@ -2103,9 +2120,7 @@ static void