All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization
@ 2016-05-22 13:02 Chris Wilson
  2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
                   ` (11 more replies)
  0 siblings, 12 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

During cleanup we have to synchronise with the async task we are using
to initialise and register our fbdev. Currently, we are using a full
synchronisation on the global domain, but we can restrict this to just
synchronising up to our task if we remember our cookie.

v2: async_synchronize_cookie() takes an exclusive upper bound, to
synchronize with our task we have to pass in the next cookie.
v3: Drop premature disregarding of the active cookie (we need to wait
until the task is complete before continuing in the teardown).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_fbdev.c | 29 ++++++++++++++++-------------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0741b2d3aa65..68751aee52cc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -159,6 +159,7 @@ struct intel_framebuffer {
 struct intel_fbdev {
 	struct drm_fb_helper helper;
 	struct intel_framebuffer *fb;
+	async_cookie_t cookie;
 	int preferred_bpp;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 99e27530e264..bfb2fd291e4e 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -538,8 +538,7 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.fb_probe = intelfb_create,
 };
 
-static void intel_fbdev_destroy(struct drm_device *dev,
-				struct intel_fbdev *ifbdev)
+static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
 {
 	/* We rely on the object-free to release the VMA pinning for
 	 * the info->screen_base mmaping. Leaking the VMA is simpler than
@@ -554,12 +553,14 @@ static void intel_fbdev_destroy(struct drm_device *dev,
 	if (ifbdev->fb) {
 		drm_framebuffer_unregister_private(&ifbdev->fb->base);
 
-		mutex_lock(&dev->struct_mutex);
+		mutex_lock(&ifbdev->helper.dev->struct_mutex);
 		intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
-		mutex_unlock(&dev->struct_mutex);
+		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
 
 		drm_framebuffer_remove(&ifbdev->fb->base);
 	}
+
+	kfree(ifbdev);
 }
 
 /*
@@ -736,32 +737,34 @@ int intel_fbdev_init(struct drm_device *dev)
 
 static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
 {
-	struct drm_i915_private *dev_priv = data;
-	struct intel_fbdev *ifbdev = dev_priv->fbdev;
+	struct intel_fbdev *ifbdev = data;
 
 	/* Due to peculiar init order wrt to hpd handling this is separate. */
 	if (drm_fb_helper_initial_config(&ifbdev->helper,
 					 ifbdev->preferred_bpp))
-		intel_fbdev_fini(dev_priv->dev);
+		intel_fbdev_fini(ifbdev->helper.dev);
 }
 
 void intel_fbdev_initial_config_async(struct drm_device *dev)
 {
-	async_schedule(intel_fbdev_initial_config, to_i915(dev));
+	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
+
+	ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev);
 }
 
 void intel_fbdev_fini(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	if (!dev_priv->fbdev)
+	struct intel_fbdev *ifbdev = dev_priv->fbdev;
+
+	if (!ifbdev)
 		return;
 
 	flush_work(&dev_priv->fbdev_suspend_work);
+	if (ifbdev->cookie && !current_is_async())
+		async_synchronize_cookie(ifbdev->cookie + 1);
 
-	if (!current_is_async())
-		async_synchronize_full();
-	intel_fbdev_destroy(dev, dev_priv->fbdev);
-	kfree(dev_priv->fbdev);
+	intel_fbdev_destroy(ifbdev);
 	dev_priv->fbdev = NULL;
 }
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 02/12] drm/i915: Rename struct intel_context
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:09   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Our goal is to rename the anonymous per-engine struct beneath the
current intel_context. However, after a lively debate resolving around
the confusion between intel_context_engine and intel_engine_context, the
realisation is that the two structs target different users. The outer
struct is API / user facing, and so carries the higher level GEM
information. The inner struct is hw facing. Thus we want to name the
inner struct intel_context and the outer one i915_gem_context. As the
first step, we need to rename the current struct:

	s/struct intel_context/struct i915_gem_context/

which fits much better with its constructors already conveying the
i915_gem_context prefix!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 10 +++---
 drivers/gpu/drm/i915/i915_drv.h            | 22 ++++++-------
 drivers/gpu/drm/i915/i915_gem.c            |  8 ++---
 drivers/gpu/drm/i915/i915_gem_context.c    | 52 +++++++++++++++---------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++---
 drivers/gpu/drm/i915/i915_guc_submission.c | 12 +++----
 drivers/gpu/drm/i915/i915_sysfs.c          |  2 +-
 drivers/gpu/drm/i915/i915_trace.h          | 12 +++----
 drivers/gpu/drm/i915/intel_guc.h           |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c           | 22 ++++++-------
 drivers/gpu/drm/i915/intel_lrc.h           | 10 +++---
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  4 +--
 12 files changed, 84 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4c6b48dbd6e2..ae28e6e9d603 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,7 +199,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 		seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
 }
 
-static void describe_ctx(struct seq_file *m, struct intel_context *ctx)
+static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
 {
 	seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
 	seq_putc(m, ctx->remap_slice ? 'R' : 'r');
@@ -2000,7 +2000,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *engine;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	enum intel_engine_id id;
 	int ret;
 
@@ -2046,7 +2046,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
 }
 
 static void i915_dump_lrc_obj(struct seq_file *m,
-			      struct intel_context *ctx,
+			      struct i915_gem_context *ctx,
 			      struct intel_engine_cs *engine)
 {
 	struct page *page;
@@ -2094,7 +2094,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *engine;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	if (!i915.enable_execlists) {
@@ -2274,7 +2274,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
 
 static int per_file_ctx(int id, void *ptr, void *data)
 {
-	struct intel_context *ctx = ptr;
+	struct i915_gem_context *ctx = ptr;
 	struct seq_file *m = data;
 	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e2abd9eb352b..8380102bbdd8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -831,7 +831,7 @@ struct i915_ctx_hang_stats {
 
 #define CONTEXT_NO_ZEROMAP (1<<0)
 /**
- * struct intel_context - as the name implies, represents a context.
+ * struct i915_gem_context - as the name implies, represents a context.
  * @ref: reference count.
  * @user_handle: userspace tracking identity for this context.
  * @remap_slice: l3 row remapping information.
@@ -849,7 +849,7 @@ struct i915_ctx_hang_stats {
  * Contexts are memory images used by the hardware to store copies of their
  * internal state.
  */
-struct intel_context {
+struct i915_gem_context {
 	struct kref ref;
 	int user_handle;
 	uint8_t remap_slice;
@@ -1710,7 +1710,7 @@ struct i915_execbuffer_params {
 	uint64_t                        batch_obj_vm_offset;
 	struct intel_engine_cs *engine;
 	struct drm_i915_gem_object      *batch_obj;
-	struct intel_context            *ctx;
+	struct i915_gem_context            *ctx;
 	struct drm_i915_gem_request     *request;
 };
 
@@ -2013,7 +2013,7 @@ struct drm_i915_private {
 		void (*stop_engine)(struct intel_engine_cs *engine);
 	} gt;
 
-	struct intel_context *kernel_context;
+	struct i915_gem_context *kernel_context;
 
 	/* perform PHY state sanity checks? */
 	bool chv_phy_assert[2];
@@ -2381,7 +2381,7 @@ struct drm_i915_gem_request {
 	 * i915_gem_request_free() will then decrement the refcount on the
 	 * context.
 	 */
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	struct intel_ringbuffer *ringbuf;
 
 	/**
@@ -2393,7 +2393,7 @@ struct drm_i915_gem_request {
 	 * we keep the previous context pinned until the following (this)
 	 * request is retired.
 	 */
-	struct intel_context *previous_context;
+	struct i915_gem_context *previous_context;
 
 	/** Batch buffer related to this request if any (used for
 	    error state dump only) */
@@ -2437,7 +2437,7 @@ struct drm_i915_gem_request {
 
 struct drm_i915_gem_request * __must_check
 i915_gem_request_alloc(struct intel_engine_cs *engine,
-		       struct intel_context *ctx);
+		       struct i915_gem_context *ctx);
 void i915_gem_request_free(struct kref *req_ref);
 int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
 				   struct drm_file *file);
@@ -3417,22 +3417,22 @@ void i915_gem_context_reset(struct drm_device *dev);
 int i915_gem_context_open(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 drm_i915_gem_request *req);
-struct intel_context *
+struct i915_gem_context *
 i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
 void i915_gem_context_free(struct kref *ctx_ref);
 struct drm_i915_gem_object *
 i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
-static inline void i915_gem_context_reference(struct intel_context *ctx)
+static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
 {
 	kref_get(&ctx->ref);
 }
 
-static inline void i915_gem_context_unreference(struct intel_context *ctx)
+static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
 {
 	kref_put(&ctx->ref, i915_gem_context_free);
 }
 
-static inline bool i915_gem_context_is_default(const struct intel_context *c)
+static inline bool i915_gem_context_is_default(const struct i915_gem_context *c)
 {
 	return c->user_handle == DEFAULT_CONTEXT_HANDLE;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c4af57ada378..728b66911840 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2689,7 +2689,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
 }
 
 static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
-				   const struct intel_context *ctx)
+				   const struct i915_gem_context *ctx)
 {
 	unsigned long elapsed;
 
@@ -2714,7 +2714,7 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
 }
 
 static void i915_set_reset_status(struct drm_i915_private *dev_priv,
-				  struct intel_context *ctx,
+				  struct i915_gem_context *ctx,
 				  const bool guilty)
 {
 	struct i915_ctx_hang_stats *hs;
@@ -2742,7 +2742,7 @@ void i915_gem_request_free(struct kref *req_ref)
 
 static inline int
 __i915_gem_request_alloc(struct intel_engine_cs *engine,
-			 struct intel_context *ctx,
+			 struct i915_gem_context *ctx,
 			 struct drm_i915_gem_request **req_out)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
@@ -2818,7 +2818,7 @@ err:
  */
 struct drm_i915_gem_request *
 i915_gem_request_alloc(struct intel_engine_cs *engine,
-		       struct intel_context *ctx)
+		       struct i915_gem_context *ctx)
 {
 	struct drm_i915_gem_request *req;
 	int err;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2aedd188473d..8484da26b5d4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -134,7 +134,7 @@ static int get_context_size(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-static void i915_gem_context_clean(struct intel_context *ctx)
+static void i915_gem_context_clean(struct i915_gem_context *ctx)
 {
 	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
 	struct i915_vma *vma, *next;
@@ -151,7 +151,7 @@ static void i915_gem_context_clean(struct intel_context *ctx)
 
 void i915_gem_context_free(struct kref *ctx_ref)
 {
-	struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+	struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
 
 	trace_i915_context_free(ctx);
 
@@ -234,12 +234,12 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
 	return 0;
 }
 
-static struct intel_context *
+static struct i915_gem_context *
 __create_hw_context(struct drm_device *dev,
 		    struct drm_i915_file_private *file_priv)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -296,12 +296,12 @@ err_out:
  * context state of the GPU for applications that don't utilize HW contexts, as
  * well as an idle case.
  */
-static struct intel_context *
+static struct i915_gem_context *
 i915_gem_create_context(struct drm_device *dev,
 			struct drm_i915_file_private *file_priv)
 {
 	const bool is_global_default_ctx = file_priv == NULL;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret = 0;
 
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -352,7 +352,7 @@ err_destroy:
 	return ERR_PTR(ret);
 }
 
-static void i915_gem_context_unpin(struct intel_context *ctx,
+static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 				   struct intel_engine_cs *engine)
 {
 	if (i915.enable_execlists) {
@@ -369,7 +369,7 @@ void i915_gem_context_reset(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (i915.enable_execlists) {
-		struct intel_context *ctx;
+		struct i915_gem_context *ctx;
 
 		list_for_each_entry(ctx, &dev_priv->context_list, link)
 			intel_lr_context_reset(dev_priv, ctx);
@@ -381,7 +381,7 @@ void i915_gem_context_reset(struct drm_device *dev)
 int i915_gem_context_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 
 	/* Init should only be called once per module load. Eventually the
 	 * restriction on the context_disabled check can be loosened. */
@@ -449,7 +449,7 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
 void i915_gem_context_fini(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_context *dctx = dev_priv->kernel_context;
+	struct i915_gem_context *dctx = dev_priv->kernel_context;
 
 	if (dctx->legacy_hw_ctx.rcs_state)
 		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
@@ -462,7 +462,7 @@ void i915_gem_context_fini(struct drm_device *dev)
 
 static int context_idr_cleanup(int id, void *p, void *data)
 {
-	struct intel_context *ctx = p;
+	struct i915_gem_context *ctx = p;
 
 	i915_gem_context_unreference(ctx);
 	return 0;
@@ -471,7 +471,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
 int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 
 	idr_init(&file_priv->context_idr);
 
@@ -495,12 +495,12 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
 	idr_destroy(&file_priv->context_idr);
 }
 
-struct intel_context *
+struct i915_gem_context *
 i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
 {
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 
-	ctx = (struct intel_context *)idr_find(&file_priv->context_idr, id);
+	ctx = idr_find(&file_priv->context_idr, id);
 	if (!ctx)
 		return ERR_PTR(-ENOENT);
 
@@ -641,7 +641,7 @@ static int remap_l3(struct drm_i915_gem_request *req, int slice)
 
 static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
 				   struct intel_engine_cs *engine,
-				   struct intel_context *to)
+				   struct i915_gem_context *to)
 {
 	if (to->remap_slice)
 		return false;
@@ -658,7 +658,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
 static bool
 needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
 		  struct intel_engine_cs *engine,
-		  struct intel_context *to)
+		  struct i915_gem_context *to)
 {
 	if (!ppgtt)
 		return false;
@@ -683,7 +683,7 @@ needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
 
 static bool
 needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
-		   struct intel_context *to,
+		   struct i915_gem_context *to,
 		   u32 hw_flags)
 {
 	if (!ppgtt)
@@ -700,10 +700,10 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
 
 static int do_rcs_switch(struct drm_i915_gem_request *req)
 {
-	struct intel_context *to = req->ctx;
+	struct i915_gem_context *to = req->ctx;
 	struct intel_engine_cs *engine = req->engine;
 	struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
-	struct intel_context *from;
+	struct i915_gem_context *from;
 	u32 hw_flags;
 	int ret, i;
 
@@ -859,7 +859,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
 
 	if (engine->id != RCS ||
 	    req->ctx->legacy_hw_ctx.rcs_state == NULL) {
-		struct intel_context *to = req->ctx;
+		struct i915_gem_context *to = req->ctx;
 		struct i915_hw_ppgtt *ppgtt =
 			to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
 
@@ -897,7 +897,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_gem_context_create *args = data;
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	if (!contexts_enabled(dev))
@@ -926,7 +926,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_gem_context_destroy *args = data;
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	if (args->pad != 0)
@@ -958,7 +958,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	struct drm_i915_gem_context_param *args = data;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	ret = i915_mutex_lock_interruptible(dev);
@@ -1001,7 +1001,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	struct drm_i915_gem_context_param *args = data;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	ret = i915_mutex_lock_interruptible(dev);
@@ -1047,7 +1047,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_reset_stats *args = data;
 	struct i915_ctx_hang_stats *hs;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	int ret;
 
 	if (args->flags || args->pad)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a54a243ccaac..e61b92d7b0bc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -716,7 +716,7 @@ eb_vma_misplaced(struct i915_vma *vma)
 static int
 i915_gem_execbuffer_reserve(struct intel_engine_cs *engine,
 			    struct list_head *vmas,
-			    struct intel_context *ctx,
+			    struct i915_gem_context *ctx,
 			    bool *need_relocs)
 {
 	struct drm_i915_gem_object *obj;
@@ -828,7 +828,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 				  struct intel_engine_cs *engine,
 				  struct eb_vmas *eb,
 				  struct drm_i915_gem_exec_object2 *exec,
-				  struct intel_context *ctx)
+				  struct i915_gem_context *ctx)
 {
 	struct drm_i915_gem_relocation_entry *reloc;
 	struct i915_address_space *vm;
@@ -1065,11 +1065,11 @@ validate_exec_list(struct drm_device *dev,
 	return 0;
 }
 
-static struct intel_context *
+static struct i915_gem_context *
 i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 			  struct intel_engine_cs *engine, const u32 ctx_id)
 {
-	struct intel_context *ctx = NULL;
+	struct i915_gem_context *ctx = NULL;
 	struct i915_ctx_hang_stats *hs;
 
 	if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
@@ -1430,7 +1430,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	struct drm_i915_gem_object *batch_obj;
 	struct drm_i915_gem_exec_object2 shadow_exec_entry;
 	struct intel_engine_cs *engine;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	struct i915_address_space *vm;
 	struct i915_execbuffer_params params_master; /* XXX: will be removed later */
 	struct i915_execbuffer_params *params = &params_master;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 169242a8adff..a292672f36d5 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -360,7 +360,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
 	struct drm_i915_gem_object *client_obj = client->client_obj;
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct intel_engine_cs *engine;
-	struct intel_context *ctx = client->owner;
+	struct i915_gem_context *ctx = client->owner;
 	struct guc_context_desc desc;
 	struct sg_table *sg;
 	enum intel_engine_id id;
@@ -426,7 +426,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
 	desc.wq_size = client->wq_size;
 
 	/*
-	 * XXX: Take LRCs from an existing intel_context if this is not an
+	 * XXX: Take LRCs from an existing context if this is not an
 	 * IsKMDCreatedContext client
 	 */
 	desc.desc_private = (uintptr_t)client;
@@ -677,7 +677,7 @@ static void guc_client_free(struct drm_device *dev,
  */
 static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
 						uint32_t priority,
-						struct intel_context *ctx)
+						struct i915_gem_context *ctx)
 {
 	struct i915_guc_client *client;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -915,7 +915,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_context *ctx = dev_priv->kernel_context;
+	struct i915_gem_context *ctx = dev_priv->kernel_context;
 	struct i915_guc_client *client;
 
 	/* client for execbuf submission */
@@ -966,7 +966,7 @@ int intel_guc_suspend(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	u32 data[3];
 
 	if (!i915.enable_guc_submission)
@@ -992,7 +992,7 @@ int intel_guc_resume(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	u32 data[3];
 
 	if (!i915.enable_guc_submission)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 37b6444b8e22..02507bfc8def 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -203,7 +203,7 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
 	struct drm_minor *dminor = dev_to_drm_minor(dev);
 	struct drm_device *drm_dev = dminor->dev;
 	struct drm_i915_private *dev_priv = drm_dev->dev_private;
-	struct intel_context *ctx;
+	struct i915_gem_context *ctx;
 	u32 *temp = NULL; /* Just here to make handling failures easy */
 	int slice = (int)(uintptr_t)attr->private;
 	int ret;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 20b2e4039792..6768db032f84 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -734,12 +734,12 @@ DEFINE_EVENT(i915_ppgtt, i915_ppgtt_release,
  * the context.
  */
 DECLARE_EVENT_CLASS(i915_context,
-	TP_PROTO(struct intel_context *ctx),
+	TP_PROTO(struct i915_gem_context *ctx),
 	TP_ARGS(ctx),
 
 	TP_STRUCT__entry(
 			__field(u32, dev)
-			__field(struct intel_context *, ctx)
+			__field(struct i915_gem_context *, ctx)
 			__field(struct i915_address_space *, vm)
 	),
 
@@ -754,12 +754,12 @@ DECLARE_EVENT_CLASS(i915_context,
 )
 
 DEFINE_EVENT(i915_context, i915_context_create,
-	TP_PROTO(struct intel_context *ctx),
+	TP_PROTO(struct i915_gem_context *ctx),
 	TP_ARGS(ctx)
 );
 
 DEFINE_EVENT(i915_context, i915_context_free,
-	TP_PROTO(struct intel_context *ctx),
+	TP_PROTO(struct i915_gem_context *ctx),
 	TP_ARGS(ctx)
 );
 
@@ -771,13 +771,13 @@ DEFINE_EVENT(i915_context, i915_context_free,
  * called only if full ppgtt is enabled.
  */
 TRACE_EVENT(switch_mm,
-	TP_PROTO(struct intel_engine_cs *engine, struct intel_context *to),
+	TP_PROTO(struct intel_engine_cs *engine, struct i915_gem_context *to),
 
 	TP_ARGS(engine, to),
 
 	TP_STRUCT__entry(
 			__field(u32, ring)
-			__field(struct intel_context *, to)
+			__field(struct i915_gem_context *, to)
 			__field(struct i915_address_space *, vm)
 			__field(u32, dev)
 	),
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 9d79c4c3e256..c9c757ef003f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -55,7 +55,7 @@ struct drm_i915_gem_request;
 struct i915_guc_client {
 	struct drm_i915_gem_object *client_obj;
 	void *client_base;		/* first page (only) of above	*/
-	struct intel_context *owner;
+	struct i915_gem_context *owner;
 	struct intel_guc *guc;
 	uint32_t priority;
 	uint32_t ctx_index;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index affca6d5ce7e..426dc90aceed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -231,9 +231,9 @@ enum {
 /* Typical size of the average request (2 pipecontrols and a MI_BB) */
 #define EXECLISTS_REQUEST_SIZE 64 /* bytes */
 
-static int execlists_context_deferred_alloc(struct intel_context *ctx,
+static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 					    struct intel_engine_cs *engine);
-static int intel_lr_context_pin(struct intel_context *ctx,
+static int intel_lr_context_pin(struct i915_gem_context *ctx,
 				struct intel_engine_cs *engine);
 
 /**
@@ -315,7 +315,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
  *    bits 55-63:    group ID, currently unused and set to 0
  */
 static void
-intel_lr_context_descriptor_update(struct intel_context *ctx,
+intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 				   struct intel_engine_cs *engine)
 {
 	u64 desc;
@@ -330,7 +330,7 @@ intel_lr_context_descriptor_update(struct intel_context *ctx,
 	ctx->engine[engine->id].lrc_desc = desc;
 }
 
-uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
+uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
 				     struct intel_engine_cs *engine)
 {
 	return ctx->engine[engine->id].lrc_desc;
@@ -932,7 +932,7 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
 	return 0;
 }
 
-static int intel_lr_context_pin(struct intel_context *ctx,
+static int intel_lr_context_pin(struct i915_gem_context *ctx,
 				struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = ctx->i915;
@@ -988,7 +988,7 @@ err:
 	return ret;
 }
 
-void intel_lr_context_unpin(struct intel_context *ctx,
+void intel_lr_context_unpin(struct i915_gem_context *ctx,
 			    struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *ctx_obj;
@@ -2049,7 +2049,7 @@ logical_ring_setup(struct drm_device *dev, enum intel_engine_id id)
 static int
 logical_ring_init(struct intel_engine_cs *engine)
 {
-	struct intel_context *dctx = engine->i915->kernel_context;
+	struct i915_gem_context *dctx = engine->i915->kernel_context;
 	int ret;
 
 	ret = i915_cmd_parser_init_ring(engine);
@@ -2273,7 +2273,7 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
 }
 
 static int
-populate_lr_context(struct intel_context *ctx,
+populate_lr_context(struct i915_gem_context *ctx,
 		    struct drm_i915_gem_object *ctx_obj,
 		    struct intel_engine_cs *engine,
 		    struct intel_ringbuffer *ringbuf)
@@ -2421,7 +2421,7 @@ populate_lr_context(struct intel_context *ctx,
  * takes care of the bits that are LRC related: the per-engine backing
  * objects and the logical ringbuffer.
  */
-void intel_lr_context_free(struct intel_context *ctx)
+void intel_lr_context_free(struct i915_gem_context *ctx)
 {
 	int i;
 
@@ -2489,7 +2489,7 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
  *
  * Return: non-zero on error.
  */
-static int execlists_context_deferred_alloc(struct intel_context *ctx,
+static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 					    struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *ctx_obj;
@@ -2539,7 +2539,7 @@ error_deref_obj:
 }
 
 void intel_lr_context_reset(struct drm_i915_private *dev_priv,
-			    struct intel_context *ctx)
+			    struct i915_gem_context *ctx)
 {
 	struct intel_engine_cs *engine;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 1afba0331dc6..eb2e1d157fed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -99,16 +99,18 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
 #define LRC_PPHWSP_PN	(LRC_GUCSHR_PN + 1)
 #define LRC_STATE_PN	(LRC_PPHWSP_PN + 1)
 
-void intel_lr_context_free(struct intel_context *ctx);
+struct i915_gem_context;
+
+void intel_lr_context_free(struct i915_gem_context *ctx);
 uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
-void intel_lr_context_unpin(struct intel_context *ctx,
+void intel_lr_context_unpin(struct i915_gem_context *ctx,
 			    struct intel_engine_cs *engine);
 
 struct drm_i915_private;
 
 void intel_lr_context_reset(struct drm_i915_private *dev_priv,
-			    struct intel_context *ctx);
-uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
+			    struct i915_gem_context *ctx);
+uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
 				     struct intel_engine_cs *engine);
 
 /* Execlists */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 929e7b4af2a4..b33c876fed20 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -119,7 +119,7 @@ struct intel_ringbuffer {
 	u32 last_retired_head;
 };
 
-struct	intel_context;
+struct i915_gem_context;
 struct drm_i915_reg_table;
 
 /*
@@ -310,7 +310,7 @@ struct intel_engine_cs {
 
 	wait_queue_head_t irq_queue;
 
-	struct intel_context *last_context;
+	struct i915_gem_context *last_context;
 
 	struct intel_ring_hangcheck hangcheck;
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
  2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:11   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Markup the functions that require the caller to hold struct_mutex with
lockdep_assert_held(). In the hopefully not-too-distant future we will
split the struct_mutex up, and in doing so we need to be sure that we
know what it protects - here the lockdep annotations are invaluable.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8380102bbdd8..48a222ddd8b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3429,6 +3429,7 @@ static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
 
 static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
 {
+	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
 	kref_put(&ctx->ref, i915_gem_context_free);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8484da26b5d4..af747f14522e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
 {
 	struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
 
+	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
 	trace_i915_context_free(ctx);
 
 	if (i915.enable_execlists)
@@ -181,6 +182,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
 	struct drm_i915_gem_object *obj;
 	int ret;
 
+	lockdep_assert_held(&dev->struct_mutex);
+
 	obj = i915_gem_object_create(dev, size);
 	if (IS_ERR(obj))
 		return obj;
@@ -368,6 +371,8 @@ void i915_gem_context_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lockdep_assert_held(&dev->struct_mutex);
+
 	if (i915.enable_execlists) {
 		struct i915_gem_context *ctx;
 
@@ -433,6 +438,8 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
 
+	lockdep_assert_held(&dev_priv->dev->struct_mutex);
+
 	for_each_engine(engine, dev_priv) {
 		if (engine->last_context == NULL)
 			continue;
@@ -451,6 +458,8 @@ void i915_gem_context_fini(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_gem_context *dctx = dev_priv->kernel_context;
 
+	lockdep_assert_held(&dev->struct_mutex);
+
 	if (dctx->legacy_hw_ctx.rcs_state)
 		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
 
@@ -491,6 +500,8 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 
+	lockdep_assert_held(&dev->struct_mutex);
+
 	idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
 	idr_destroy(&file_priv->context_idr);
 }
@@ -500,6 +511,8 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
 {
 	struct i915_gem_context *ctx;
 
+	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
+
 	ctx = idr_find(&file_priv->context_idr, id);
 	if (!ctx)
 		return ERR_PTR(-ENOENT);
@@ -852,10 +865,9 @@ unpin_out:
 int i915_switch_context(struct drm_i915_gem_request *req)
 {
 	struct intel_engine_cs *engine = req->engine;
-	struct drm_i915_private *dev_priv = req->i915;
 
 	WARN_ON(i915.enable_execlists);
-	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+	lockdep_assert_held(&req->i915->dev->struct_mutex);
 
 	if (engine->id != RCS ||
 	    req->ctx->legacy_hw_ctx.rcs_state == NULL) {
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get()
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
  2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
  2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:15   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

i915_gem_context_get() is a very simple wrapper around idr_find(), so
simple that it would be smaller to do the lookup inline. Also we use the
verb 'lookup' to return a pointer from a handle, freeing 'get' to imply
obtaining a reference to the context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            | 17 +++++++++++++++--
 drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++------------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48a222ddd8b3..e4e3614335ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3417,11 +3417,24 @@ void i915_gem_context_reset(struct drm_device *dev);
 int i915_gem_context_open(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 drm_i915_gem_request *req);
-struct i915_gem_context *
-i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
 void i915_gem_context_free(struct kref *ctx_ref);
 struct drm_i915_gem_object *
 i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
+
+static inline struct i915_gem_context *
+i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
+{
+	struct i915_gem_context *ctx;
+
+	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
+
+	ctx = idr_find(&file_priv->context_idr, id);
+	if (!ctx)
+		return ERR_PTR(-ENOENT);
+
+	return ctx;
+}
+
 static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
 {
 	kref_get(&ctx->ref);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index af747f14522e..aa329175f73a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -506,20 +506,6 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
 	idr_destroy(&file_priv->context_idr);
 }
 
-struct i915_gem_context *
-i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
-{
-	struct i915_gem_context *ctx;
-
-	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
-
-	ctx = idr_find(&file_priv->context_idr, id);
-	if (!ctx)
-		return ERR_PTR(-ENOENT);
-
-	return ctx;
-}
-
 static inline int
 mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
 {
@@ -951,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	ctx = i915_gem_context_get(file_priv, args->ctx_id);
+	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
 	if (IS_ERR(ctx)) {
 		mutex_unlock(&dev->struct_mutex);
 		return PTR_ERR(ctx);
@@ -977,7 +963,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	ctx = i915_gem_context_get(file_priv, args->ctx_id);
+	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
 	if (IS_ERR(ctx)) {
 		mutex_unlock(&dev->struct_mutex);
 		return PTR_ERR(ctx);
@@ -1020,7 +1006,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	ctx = i915_gem_context_get(file_priv, args->ctx_id);
+	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
 	if (IS_ERR(ctx)) {
 		mutex_unlock(&dev->struct_mutex);
 		return PTR_ERR(ctx);
@@ -1072,7 +1058,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	ctx = i915_gem_context_get(file->driver_priv, args->ctx_id);
+	ctx = i915_gem_context_lookup(file->driver_priv, args->ctx_id);
 	if (IS_ERR(ctx)) {
 		mutex_unlock(&dev->struct_mutex);
 		return PTR_ERR(ctx);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e61b92d7b0bc..84d990331abd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1075,7 +1075,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 	if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
 		return ERR_PTR(-EINVAL);
 
-	ctx = i915_gem_context_get(file->driver_priv, ctx_id);
+	ctx = i915_gem_context_lookup(file->driver_priv, ctx_id);
 	if (IS_ERR(ctx))
 		return ctx;
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (2 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:17   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

As these are wrappers around kref_get/kref_put() it is preferable to
follow the naming convention and use the same verb get/put in our
wrapper names for manipulating a reference to the context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |  6 ++++--
 drivers/gpu/drm/i915/i915_gem.c            |  7 +++----
 drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++++++++------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  6 +++---
 drivers/gpu/drm/i915/intel_lrc.c           |  4 ++--
 5 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e4e3614335ec..0a3442fcd93e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3435,12 +3435,14 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
 	return ctx;
 }
 
-static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
+static inline struct i915_gem_context *
+i915_gem_context_get(struct i915_gem_context *ctx)
 {
 	kref_get(&ctx->ref);
+	return ctx;
 }
 
-static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
+static inline void i915_gem_context_put(struct i915_gem_context *ctx)
 {
 	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
 	kref_put(&ctx->ref, i915_gem_context_free);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 728b66911840..d7ae030e5a0b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1418,7 +1418,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 					       request->engine);
 	}
 
-	i915_gem_context_unreference(request->ctx);
+	i915_gem_context_put(request->ctx);
 	i915_gem_request_unreference(request);
 }
 
@@ -2775,8 +2775,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
 	req->i915 = dev_priv;
 	req->engine = engine;
 	req->reset_counter = reset_counter;
-	req->ctx  = ctx;
-	i915_gem_context_reference(req->ctx);
+	req->ctx = i915_gem_context_get(ctx);
 
 	/*
 	 * Reserve space in the ring buffer for all the commands required to
@@ -2798,7 +2797,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
 	return 0;
 
 err_ctx:
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 err:
 	kmem_cache_free(dev_priv->requests, req);
 	return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index aa329175f73a..a4c6377eea32 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -290,7 +290,7 @@ __create_hw_context(struct drm_device *dev,
 	return ctx;
 
 err_out:
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	return ERR_PTR(ret);
 }
 
@@ -351,7 +351,7 @@ err_unpin:
 		i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
 err_destroy:
 	idr_remove(&file_priv->context_idr, ctx->user_handle);
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	return ERR_PTR(ret);
 }
 
@@ -363,7 +363,7 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 	} else {
 		if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
 			i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
-		i915_gem_context_unreference(ctx);
+		i915_gem_context_put(ctx);
 	}
 }
 
@@ -463,7 +463,7 @@ void i915_gem_context_fini(struct drm_device *dev)
 	if (dctx->legacy_hw_ctx.rcs_state)
 		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
 
-	i915_gem_context_unreference(dctx);
+	i915_gem_context_put(dctx);
 	dev_priv->kernel_context = NULL;
 
 	ida_destroy(&dev_priv->context_hw_ida);
@@ -473,7 +473,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
 {
 	struct i915_gem_context *ctx = p;
 
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	return 0;
 }
 
@@ -785,10 +785,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 
 		/* obj is kept alive until the next request by its active ref */
 		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
-		i915_gem_context_unreference(from);
+		i915_gem_context_put(from);
 	}
-	i915_gem_context_reference(to);
-	engine->last_context = to;
+	engine->last_context = i915_gem_context_get(to);
 
 	/* GEN8 does *not* require an explicit reload if the PDPs have been
 	 * setup, and we do not wish to move them.
@@ -873,10 +872,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
 		}
 
 		if (to != engine->last_context) {
-			i915_gem_context_reference(to);
 			if (engine->last_context)
-				i915_gem_context_unreference(engine->last_context);
-			engine->last_context = to;
+				i915_gem_context_put(engine->last_context);
+			engine->last_context = i915_gem_context_get(to);
 		}
 
 		return 0;
@@ -944,7 +942,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	}
 
 	idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 84d990331abd..14628ce91273 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1492,7 +1492,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		goto pre_mutex_err;
 	}
 
-	i915_gem_context_reference(ctx);
+	i915_gem_context_get(ctx);
 
 	if (ctx->ppgtt)
 		vm = &ctx->ppgtt->base;
@@ -1503,7 +1503,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	eb = eb_create(args);
 	if (eb == NULL) {
-		i915_gem_context_unreference(ctx);
+		i915_gem_context_put(ctx);
 		mutex_unlock(&dev->struct_mutex);
 		ret = -ENOMEM;
 		goto pre_mutex_err;
@@ -1647,7 +1647,7 @@ err_batch_unpin:
 
 err:
 	/* the request owns the ref now */
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	eb_destroy(eb);
 
 	mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 426dc90aceed..9a55478e23ac 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -966,7 +966,6 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (ret)
 		goto unpin_map;
 
-	i915_gem_context_reference(ctx);
 	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
 	intel_lr_context_descriptor_update(ctx, engine);
 	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
@@ -977,6 +976,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (i915.enable_guc_submission)
 		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
 
+	i915_gem_context_get(ctx);
 	return 0;
 
 unpin_map:
@@ -1009,7 +1009,7 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
 	ctx->engine[engine->id].lrc_desc = 0;
 	ctx->engine[engine->id].lrc_reg_state = NULL;
 
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 }
 
 static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (3 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:26   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

We want to give a name to the currently anonymous per-engine struct
inside the context, so that we can assign it to a local variable and
save clumsy typing. The name we have chosen is intel_context as it
reflects the HW facing portion of the context state (the logical context
state, the registers, the ringbuffer etc).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |  2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++---
 drivers/gpu/drm/i915/intel_lrc.c           | 90 +++++++++++++++---------------
 3 files changed, 51 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0a3442fcd93e..e6735dc9eeb2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -869,7 +869,7 @@ struct i915_gem_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	struct {
+	struct intel_context {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
 		int pin_count;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a292672f36d5..c29c1d19764f 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -363,7 +363,6 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
 	struct i915_gem_context *ctx = client->owner;
 	struct guc_context_desc desc;
 	struct sg_table *sg;
-	enum intel_engine_id id;
 	u32 gfx_addr;
 
 	memset(&desc, 0, sizeof(desc));
@@ -373,10 +372,10 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
 	desc.priority = client->priority;
 	desc.db_id = client->doorbell_id;
 
-	for_each_engine_id(engine, dev_priv, id) {
+	for_each_engine(engine, dev_priv) {
+		struct intel_context *ce = &ctx->engine[engine->id];
 		struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
 		struct drm_i915_gem_object *obj;
-		uint64_t ctx_desc;
 
 		/* TODO: We have a design issue to be solved here. Only when we
 		 * receive the first batch, we know which engine is used by the
@@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
 		 * for now who owns a GuC client. But for future owner of GuC
 		 * client, need to make sure lrc is pinned prior to enter here.
 		 */
-		obj = ctx->engine[id].state;
-		if (!obj)
+		if (!ce->state)
 			break;	/* XXX: continue? */
 
-		ctx_desc = intel_lr_context_descriptor(ctx, engine);
-		lrc->context_desc = (u32)ctx_desc;
+		lrc->context_desc = lower_32_bits(ce->lrc_desc);
 
 		/* The state page is after PPHWSP */
-		gfx_addr = i915_gem_obj_ggtt_offset(obj);
+		gfx_addr = i915_gem_obj_ggtt_offset(ce->state);
 		lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
 		lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
 				(engine->guc_id << GUC_ELC_ENGINE_OFFSET);
 
-		obj = ctx->engine[id].ringbuf->obj;
+		obj = ce->ringbuf->obj;
 		gfx_addr = i915_gem_obj_ggtt_offset(obj);
 
 		lrc->ring_begin = gfx_addr;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9a55478e23ac..4b7c9680b097 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -300,7 +300,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
  * 					  descriptor for a pinned context
  *
  * @ctx: Context to work on
- * @ring: Engine the descriptor will be used with
+ * @engine: Engine the descriptor will be used with
  *
  * The context descriptor encodes various attributes of a context,
  * including its GTT address and some flags. Because it's fairly
@@ -318,16 +318,17 @@ static void
 intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 				   struct intel_engine_cs *engine)
 {
+	struct intel_context *ce = &ctx->engine[engine->id];
 	u64 desc;
 
 	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
 
 	desc = engine->ctx_desc_template;			/* bits  0-11 */
-	desc |= ctx->engine[engine->id].lrc_vma->node.start +	/* bits 12-31 */
-	       LRC_PPHWSP_PN * PAGE_SIZE;
+	desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
+								/* bits 12-31 */
 	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
 
-	ctx->engine[engine->id].lrc_desc = desc;
+	ce->lrc_desc = desc;
 }
 
 uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
@@ -674,6 +675,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
 int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
+	struct intel_context *ce = &request->ctx->engine[engine->id];
 	int ret;
 
 	/* Flush enough space to reduce the likelihood of waiting after
@@ -682,13 +684,13 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
 	 */
 	request->reserved_space += EXECLISTS_REQUEST_SIZE;
 
-	if (request->ctx->engine[engine->id].state == NULL) {
+	if (!ce->state) {
 		ret = execlists_context_deferred_alloc(request->ctx, engine);
 		if (ret)
 			return ret;
 	}
 
-	request->ringbuf = request->ctx->engine[engine->id].ringbuf;
+	request->ringbuf = ce->ringbuf;
 
 	if (i915.enable_guc_submission) {
 		/*
@@ -711,12 +713,12 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
 	if (ret)
 		goto err_unpin;
 
-	if (!request->ctx->engine[engine->id].initialised) {
+	if (!ce->initialised) {
 		ret = engine->init_context(request);
 		if (ret)
 			goto err_unpin;
 
-		request->ctx->engine[engine->id].initialised = true;
+		ce->initialised = true;
 	}
 
 	/* Note that after this point, we have committed to using
@@ -936,24 +938,22 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 				struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = ctx->i915;
-	struct drm_i915_gem_object *ctx_obj;
-	struct intel_ringbuffer *ringbuf;
+	struct intel_context *ce = &ctx->engine[engine->id];
 	void *vaddr;
 	u32 *lrc_reg_state;
 	int ret;
 
 	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
 
-	if (ctx->engine[engine->id].pin_count++)
+	if (ce->pin_count++)
 		return 0;
 
-	ctx_obj = ctx->engine[engine->id].state;
-	ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-			PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	ret = i915_gem_obj_ggtt_pin(ce->state, GEN8_LR_CONTEXT_ALIGN,
+				    PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
 	if (ret)
 		goto err;
 
-	vaddr = i915_gem_object_pin_map(ctx_obj);
+	vaddr = i915_gem_object_pin_map(ce->state);
 	if (IS_ERR(vaddr)) {
 		ret = PTR_ERR(vaddr);
 		goto unpin_ctx_obj;
@@ -961,16 +961,16 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 
 	lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
 
-	ringbuf = ctx->engine[engine->id].ringbuf;
-	ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ringbuf);
+	ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ce->ringbuf);
 	if (ret)
 		goto unpin_map;
 
-	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
+	ce->lrc_vma = i915_gem_obj_to_ggtt(ce->state);
 	intel_lr_context_descriptor_update(ctx, engine);
-	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
-	ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
-	ctx_obj->dirty = true;
+
+	lrc_reg_state[CTX_RING_BUFFER_START+1] = ce->ringbuf->vma->node.start;
+	ce->lrc_reg_state = lrc_reg_state;
+	ce->state->dirty = true;
 
 	/* Invalidate GuC TLB. */
 	if (i915.enable_guc_submission)
@@ -980,34 +980,33 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	return 0;
 
 unpin_map:
-	i915_gem_object_unpin_map(ctx_obj);
+	i915_gem_object_unpin_map(ce->state);
 unpin_ctx_obj:
-	i915_gem_object_ggtt_unpin(ctx_obj);
+	i915_gem_object_ggtt_unpin(ce->state);
 err:
-	ctx->engine[engine->id].pin_count = 0;
+	ce->pin_count = 0;
 	return ret;
 }
 
 void intel_lr_context_unpin(struct i915_gem_context *ctx,
 			    struct intel_engine_cs *engine)
 {
-	struct drm_i915_gem_object *ctx_obj;
+	struct intel_context *ce = &ctx->engine[engine->id];
 
 	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
-	GEM_BUG_ON(ctx->engine[engine->id].pin_count == 0);
+	GEM_BUG_ON(ce->pin_count == 0);
 
-	if (--ctx->engine[engine->id].pin_count)
+	if (--ce->pin_count)
 		return;
 
-	intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
+	intel_unpin_ringbuffer_obj(ce->ringbuf);
 
-	ctx_obj = ctx->engine[engine->id].state;
-	i915_gem_object_unpin_map(ctx_obj);
-	i915_gem_object_ggtt_unpin(ctx_obj);
+	i915_gem_object_unpin_map(ce->state);
+	i915_gem_object_ggtt_unpin(ce->state);
 
-	ctx->engine[engine->id].lrc_vma = NULL;
-	ctx->engine[engine->id].lrc_desc = 0;
-	ctx->engine[engine->id].lrc_reg_state = NULL;
+	ce->lrc_vma = NULL;
+	ce->lrc_desc = 0;
+	ce->lrc_reg_state = NULL;
 
 	i915_gem_context_put(ctx);
 }
@@ -2493,12 +2492,13 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 					    struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *ctx_obj;
+	struct intel_context *ce = &ctx->engine[engine->id];
 	uint32_t context_size;
 	struct intel_ringbuffer *ringbuf;
 	int ret;
 
 	WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
-	WARN_ON(ctx->engine[engine->id].state);
+	WARN_ON(ce->state);
 
 	context_size = round_up(intel_lr_context_size(engine), 4096);
 
@@ -2523,9 +2523,9 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 		goto error_ringbuf;
 	}
 
-	ctx->engine[engine->id].ringbuf = ringbuf;
-	ctx->engine[engine->id].state = ctx_obj;
-	ctx->engine[engine->id].initialised = engine->init_context == NULL;
+	ce->ringbuf = ringbuf;
+	ce->state = ctx_obj;
+	ce->initialised = engine->init_context == NULL;
 
 	return 0;
 
@@ -2533,8 +2533,8 @@ error_ringbuf:
 	intel_ringbuffer_free(ringbuf);
 error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
-	ctx->engine[engine->id].ringbuf = NULL;
-	ctx->engine[engine->id].state = NULL;
+	ce->ringbuf = NULL;
+	ce->state = NULL;
 	return ret;
 }
 
@@ -2544,10 +2544,8 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
 	struct intel_engine_cs *engine;
 
 	for_each_engine(engine, dev_priv) {
-		struct drm_i915_gem_object *ctx_obj =
-				ctx->engine[engine->id].state;
-		struct intel_ringbuffer *ringbuf =
-				ctx->engine[engine->id].ringbuf;
+		struct intel_context *ce = &ctx->engine[engine->id];
+		struct drm_i915_gem_object *ctx_obj = ce->state;
 		void *vaddr;
 		uint32_t *reg_state;
 
@@ -2566,7 +2564,7 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
 
 		i915_gem_object_unpin_map(ctx_obj);
 
-		ringbuf->head = 0;
-		ringbuf->tail = 0;
+		ce->ringbuf->head = 0;
+		ce->ringbuf->tail = 0;
 	}
 }
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (4 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:33   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Rather than have every context ask "am I owned by the kernel? pin!",
move that logic into the creator of the kernel context, in order to
improve code comprehension.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 52 +++++++++++++++------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a4c6377eea32..51e83a79ab36 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -303,9 +303,7 @@ static struct i915_gem_context *
 i915_gem_create_context(struct drm_device *dev,
 			struct drm_i915_file_private *file_priv)
 {
-	const bool is_global_default_ctx = file_priv == NULL;
 	struct i915_gem_context *ctx;
-	int ret = 0;
 
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
 
@@ -313,30 +311,14 @@ i915_gem_create_context(struct drm_device *dev,
 	if (IS_ERR(ctx))
 		return ctx;
 
-	if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
-		/* We may need to do things with the shrinker which
-		 * require us to immediately switch back to the default
-		 * context. This can cause a problem as pinning the
-		 * default context also requires GTT space which may not
-		 * be available. To avoid this we always pin the default
-		 * context.
-		 */
-		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
-					    get_context_alignment(to_i915(dev)), 0);
-		if (ret) {
-			DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
-			goto err_destroy;
-		}
-	}
-
 	if (USES_FULL_PPGTT(dev)) {
 		struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
 
-		if (IS_ERR_OR_NULL(ppgtt)) {
+		if (IS_ERR(ppgtt)) {
 			DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
 					 PTR_ERR(ppgtt));
-			ret = PTR_ERR(ppgtt);
-			goto err_unpin;
+			i915_gem_context_put(ctx);
+			return ERR_CAST(ppgtt);
 		}
 
 		ctx->ppgtt = ppgtt;
@@ -345,14 +327,6 @@ i915_gem_create_context(struct drm_device *dev,
 	trace_i915_context_create(ctx);
 
 	return ctx;
-
-err_unpin:
-	if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
-		i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
-err_destroy:
-	idr_remove(&file_priv->context_idr, ctx->user_handle);
-	i915_gem_context_put(ctx);
-	return ERR_PTR(ret);
 }
 
 static void i915_gem_context_unpin(struct i915_gem_context *ctx,
@@ -426,6 +400,26 @@ int i915_gem_context_init(struct drm_device *dev)
 		return PTR_ERR(ctx);
 	}
 
+	if (ctx->legacy_hw_ctx.rcs_state) {
+		int ret;
+
+		/* We may need to do things with the shrinker which
+		 * require us to immediately switch back to the default
+		 * context. This can cause a problem as pinning the
+		 * default context also requires GTT space which may not
+		 * be available. To avoid this we always pin the default
+		 * context.
+		 */
+		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+					    get_context_alignment(dev_priv), 0);
+		if (ret) {
+			DRM_ERROR("Failed to pinned default global context (error %d)\n",
+				  ret);
+			i915_gem_context_put(ctx);
+			return ret;
+		}
+	}
+
 	dev_priv->kernel_context = ctx;
 
 	DRM_DEBUG_DRIVER("%s context support initialized\n",
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (5 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:42   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Print the context's owner (via the pid under file_priv) under debugfs.

Note that since this was originally introducing
dev_priv->kernel_context, there are a couple of leftover minor chunks.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ae28e6e9d603..945fe4710b37 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
 			continue;
 
 		seq_printf(m, "HW context %u ", ctx->hw_id);
+		if (IS_ERR(ctx->file_priv)) {
+			seq_puts(m, "(deleted) ");
+		} else if (ctx->file_priv) {
+			struct pid *pid = ctx->file_priv->file->pid;
+			struct task_struct *task;
+
+			task = get_pid_task(pid, PIDTYPE_PID);
+			if (task) {
+				seq_printf(m, "(%s [%d]) ",
+					   task->comm, task->pid);
+				put_task_struct(task);
+			}
+		} else
+			seq_puts(m, "(kernel) ");
+
 		describe_ctx(m, ctx);
-		if (ctx == dev_priv->kernel_context)
-			seq_printf(m, "(kernel context) ");
 
 		if (i915.enable_execlists) {
 			seq_putc(m, '\n');
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (6 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23  9:45   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Just move the kernel_context memboer of drm_i915_private next to the
engines it is associated with.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            | 3 +--
 drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e6735dc9eeb2..d3d2538d17e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1760,6 +1760,7 @@ struct drm_i915_private {
 	wait_queue_head_t gmbus_wait_queue;
 
 	struct pci_dev *bridge_dev;
+	struct i915_gem_context *kernel_context;
 	struct intel_engine_cs engine[I915_NUM_ENGINES];
 	struct drm_i915_gem_object *semaphore_obj;
 	uint32_t last_seqno, next_seqno;
@@ -2013,8 +2014,6 @@ struct drm_i915_private {
 		void (*stop_engine)(struct intel_engine_cs *engine);
 	} gt;
 
-	struct i915_gem_context *kernel_context;
-
 	/* perform PHY state sanity checks? */
 	bool chv_phy_assert[2];
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index c29c1d19764f..78b70526c197 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -912,11 +912,12 @@ int i915_guc_submission_enable(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
-	struct i915_gem_context *ctx = dev_priv->kernel_context;
 	struct i915_guc_client *client;
 
 	/* client for execbuf submission */
-	client = guc_client_alloc(dev, GUC_CTX_PRIORITY_KMD_NORMAL, ctx);
+	client = guc_client_alloc(dev,
+				  GUC_CTX_PRIORITY_KMD_NORMAL,
+				  dev_priv->kernel_context);
 	if (!client) {
 		DRM_ERROR("Failed to create execbuf guc_client\n");
 		return -ENOMEM;
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 10/12] drm/i915: Merge legacy+execlists context structs
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (7 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23 10:26   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

struct intel_context contains two substructs, one for the legacy RCS and
one for every execlists engine. Since legacy RCS is a subset of the
execlists engine support, just combine the two substructs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 41 +++++-------------
 drivers/gpu/drm/i915/i915_drv.h         |  7 ---
 drivers/gpu/drm/i915/i915_gem_context.c | 75 +++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_lrc.c        | 26 ------------
 drivers/gpu/drm/i915/intel_lrc.h        |  1 -
 5 files changed, 55 insertions(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 945fe4710b37..30cb26fe2fa9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,13 +199,6 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 		seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
 }
 
-static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
-{
-	seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
-	seq_putc(m, ctx->remap_slice ? 'R' : 'r');
-	seq_putc(m, ' ');
-}
-
 static int i915_gem_object_list_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -2001,7 +1994,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *engine;
 	struct i915_gem_context *ctx;
-	enum intel_engine_id id;
 	int ret;
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -2009,10 +2001,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		return ret;
 
 	list_for_each_entry(ctx, &dev_priv->context_list, link) {
-		if (!i915.enable_execlists &&
-		    ctx->legacy_hw_ctx.rcs_state == NULL)
-			continue;
-
 		seq_printf(m, "HW context %u ", ctx->hw_id);
 		if (IS_ERR(ctx->file_priv)) {
 			seq_puts(m, "(deleted) ");
@@ -2029,25 +2017,18 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		} else
 			seq_puts(m, "(kernel) ");
 
-		describe_ctx(m, ctx);
+		seq_putc(m, ctx->remap_slice ? 'R' : 'r');
+		seq_putc(m, '\n');
 
-		if (i915.enable_execlists) {
+		for_each_engine(engine, dev_priv) {
+			struct intel_context *ce = &ctx->engine[engine->id];
+			seq_printf(m, "%s: ", engine->name);
+			seq_putc(m, ce->initialised ? 'I' : 'i');
+			if (ce->state)
+				describe_obj(m, ce->state);
+			if (ce->ringbuf)
+				describe_ctx_ringbuf(m, ce->ringbuf);
 			seq_putc(m, '\n');
-			for_each_engine_id(engine, dev_priv, id) {
-				struct drm_i915_gem_object *ctx_obj =
-					ctx->engine[id].state;
-				struct intel_ringbuffer *ringbuf =
-					ctx->engine[id].ringbuf;
-
-				seq_printf(m, "%s: ", engine->name);
-				if (ctx_obj)
-					describe_obj(m, ctx_obj);
-				if (ringbuf)
-					describe_ctx_ringbuf(m, ringbuf);
-				seq_putc(m, '\n');
-			}
-		} else {
-			describe_obj(m, ctx->legacy_hw_ctx.rcs_state);
 		}
 
 		seq_putc(m, '\n');
@@ -2062,10 +2043,10 @@ static void i915_dump_lrc_obj(struct seq_file *m,
 			      struct i915_gem_context *ctx,
 			      struct intel_engine_cs *engine)
 {
+	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
 	struct page *page;
 	uint32_t *reg_state;
 	int j;
-	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
 	unsigned long ggtt_offset = 0;
 
 	seq_printf(m, "CONTEXT: %s %u\n", engine->name, ctx->hw_id);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d3d2538d17e8..259a0ee7a601 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -862,13 +862,6 @@ struct i915_gem_context {
 	/* Unique identifier for this context, used by the hw for tracking */
 	unsigned hw_id;
 
-	/* Legacy ring buffer submission */
-	struct {
-		struct drm_i915_gem_object *rcs_state;
-		bool initialized;
-	} legacy_hw_ctx;
-
-	/* Execlists */
 	struct intel_context {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 51e83a79ab36..9847235997b9 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -152,13 +152,11 @@ static void i915_gem_context_clean(struct i915_gem_context *ctx)
 void i915_gem_context_free(struct kref *ctx_ref)
 {
 	struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+	int i;
 
 	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
 	trace_i915_context_free(ctx);
 
-	if (i915.enable_execlists)
-		intel_lr_context_free(ctx);
-
 	/*
 	 * This context is going away and we need to remove all VMAs still
 	 * around. This is to handle imported shared objects for which
@@ -168,8 +166,19 @@ void i915_gem_context_free(struct kref *ctx_ref)
 
 	i915_ppgtt_put(ctx->ppgtt);
 
-	if (ctx->legacy_hw_ctx.rcs_state)
-		drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
+	for (i = 0; i < I915_NUM_ENGINES; i++) {
+		struct intel_context *ce = &ctx->engine[i];
+
+		if (!ce->state)
+			continue;
+
+		WARN_ON(ce->pin_count);
+		if (ce->ringbuf)
+			intel_ringbuffer_free(ce->ringbuf);
+
+		drm_gem_object_unreference(&ce->state->base);
+	}
+
 	list_del(&ctx->link);
 
 	ida_simple_remove(&ctx->i915->context_hw_ida, ctx->hw_id);
@@ -266,7 +275,7 @@ __create_hw_context(struct drm_device *dev,
 			ret = PTR_ERR(obj);
 			goto err_out;
 		}
-		ctx->legacy_hw_ctx.rcs_state = obj;
+		ctx->engine[RCS].state = obj;
 	}
 
 	/* Default context will never have a file_priv */
@@ -335,8 +344,11 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 	if (i915.enable_execlists) {
 		intel_lr_context_unpin(ctx, engine);
 	} else {
-		if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
-			i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+		struct intel_context *ce = &ctx->engine[engine->id];
+
+		if (ce->state)
+			i915_gem_object_ggtt_unpin(ce->state);
+
 		i915_gem_context_put(ctx);
 	}
 }
@@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
 		return PTR_ERR(ctx);
 	}
 
-	if (ctx->legacy_hw_ctx.rcs_state) {
+	if (ctx->engine[RCS].state) {
 		int ret;
 
 		/* We may need to do things with the shrinker which
@@ -410,7 +422,7 @@ int i915_gem_context_init(struct drm_device *dev)
 		 * be available. To avoid this we always pin the default
 		 * context.
 		 */
-		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+		ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
 					    get_context_alignment(dev_priv), 0);
 		if (ret) {
 			DRM_ERROR("Failed to pinned default global context (error %d)\n",
@@ -435,15 +447,17 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
 	lockdep_assert_held(&dev_priv->dev->struct_mutex);
 
 	for_each_engine(engine, dev_priv) {
-		if (engine->last_context == NULL)
-			continue;
+		if (engine->last_context) {
+			i915_gem_context_unpin(engine->last_context, engine);
+			engine->last_context = NULL;
+		}
 
-		i915_gem_context_unpin(engine->last_context, engine);
-		engine->last_context = NULL;
+		/* Force the GPU state to be reinitialised on enabling */
+		dev_priv->kernel_context->engine[engine->id].initialised =
+			engine->init_context == NULL;
 	}
 
 	/* Force the GPU state to be reinitialised on enabling */
-	dev_priv->kernel_context->legacy_hw_ctx.initialized = false;
 	dev_priv->kernel_context->remap_slice = ALL_L3_SLICES(dev_priv);
 }
 
@@ -454,8 +468,8 @@ void i915_gem_context_fini(struct drm_device *dev)
 
 	lockdep_assert_held(&dev->struct_mutex);
 
-	if (dctx->legacy_hw_ctx.rcs_state)
-		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+	if (!i915.enable_execlists && dctx->engine[RCS].state)
+		i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
 
 	i915_gem_context_put(dctx);
 	dev_priv->kernel_context = NULL;
@@ -562,7 +576,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
 	intel_ring_emit(engine, MI_NOOP);
 	intel_ring_emit(engine, MI_SET_CONTEXT);
 	intel_ring_emit(engine,
-			i915_gem_obj_ggtt_offset(req->ctx->legacy_hw_ctx.rcs_state) |
+			i915_gem_obj_ggtt_offset(req->ctx->engine[RCS].state) |
 			flags);
 	/*
 	 * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
@@ -639,7 +653,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
 	if (to->remap_slice)
 		return false;
 
-	if (!to->legacy_hw_ctx.initialized)
+	if (!to->engine[RCS].initialised)
 		return false;
 
 	if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
@@ -704,7 +718,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 		return 0;
 
 	/* Trying to pin first makes error handling easier. */
-	ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
+	ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
 				    get_context_alignment(engine->i915),
 				    0);
 	if (ret)
@@ -727,7 +741,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 	 *
 	 * XXX: We need a real interface to do this instead of trickery.
 	 */
-	ret = i915_gem_object_set_to_gtt_domain(to->legacy_hw_ctx.rcs_state, false);
+	ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state, false);
 	if (ret)
 		goto unpin_out;
 
@@ -742,7 +756,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 			goto unpin_out;
 	}
 
-	if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+	if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
 		/* NB: If we inhibit the restore, the context is not allowed to
 		 * die because future work may end up depending on valid address
 		 * space. This means we must enforce that a page table load
@@ -766,8 +780,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 	 * MI_SET_CONTEXT instead of when the next seqno has completed.
 	 */
 	if (from != NULL) {
-		from->legacy_hw_ctx.rcs_state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
-		i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state), req);
+		from->engine[RCS].state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+		i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->engine[RCS].state), req);
 		/* 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
@@ -775,10 +789,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 		 * able to defer doing this until we know the object would be
 		 * swapped, but there is no way to do that yet.
 		 */
-		from->legacy_hw_ctx.rcs_state->dirty = 1;
+		from->engine[RCS].state->dirty = 1;
 
 		/* obj is kept alive until the next request by its active ref */
-		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+		i915_gem_object_ggtt_unpin(from->engine[RCS].state);
 		i915_gem_context_put(from);
 	}
 	engine->last_context = i915_gem_context_get(to);
@@ -812,19 +826,19 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 		to->remap_slice &= ~(1<<i);
 	}
 
-	if (!to->legacy_hw_ctx.initialized) {
+	if (!to->engine[RCS].initialised) {
 		if (engine->init_context) {
 			ret = engine->init_context(req);
 			if (ret)
 				return ret;
 		}
-		to->legacy_hw_ctx.initialized = true;
+		to->engine[RCS].initialised = true;
 	}
 
 	return 0;
 
 unpin_out:
-	i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
+	i915_gem_object_ggtt_unpin(to->engine[RCS].state);
 	return ret;
 }
 
@@ -848,8 +862,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
 	WARN_ON(i915.enable_execlists);
 	lockdep_assert_held(&req->i915->dev->struct_mutex);
 
-	if (engine->id != RCS ||
-	    req->ctx->legacy_hw_ctx.rcs_state == NULL) {
+	if (!req->ctx->engine[engine->id].state) {
 		struct i915_gem_context *to = req->ctx;
 		struct i915_hw_ppgtt *ppgtt =
 			to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4b7c9680b097..558f069cd6d8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2413,31 +2413,6 @@ populate_lr_context(struct i915_gem_context *ctx,
 }
 
 /**
- * intel_lr_context_free() - free the LRC specific bits of a context
- * @ctx: the LR context to free.
- *
- * The real context freeing is done in i915_gem_context_free: this only
- * takes care of the bits that are LRC related: the per-engine backing
- * objects and the logical ringbuffer.
- */
-void intel_lr_context_free(struct i915_gem_context *ctx)
-{
-	int i;
-
-	for (i = I915_NUM_ENGINES; --i >= 0; ) {
-		struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
-		struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
-
-		if (!ctx_obj)
-			continue;
-
-		WARN_ON(ctx->engine[i].pin_count);
-		intel_ringbuffer_free(ringbuf);
-		drm_gem_object_unreference(&ctx_obj->base);
-	}
-}
-
-/**
  * intel_lr_context_size() - return the size of the context for an engine
  * @ring: which engine to find the context size for
  *
@@ -2497,7 +2472,6 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 	struct intel_ringbuffer *ringbuf;
 	int ret;
 
-	WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
 	WARN_ON(ce->state);
 
 	context_size = round_up(intel_lr_context_size(engine), 4096);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index eb2e1d157fed..a8db42a9c50f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -101,7 +101,6 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
 
 struct i915_gem_context;
 
-void intel_lr_context_free(struct i915_gem_context *ctx);
 uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
 void intel_lr_context_unpin(struct i915_gem_context *ctx,
 			    struct intel_engine_cs *engine);
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 11/12] drm/i915: Rearrange i915_gem_context
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (8 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-23 10:27   ` Tvrtko Ursulin
  2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
  2016-05-22 13:33 ` ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization Patchwork
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Pack the integers and related types together inside the struct.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 259a0ee7a601..3b5cf15d85ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -829,7 +829,6 @@ struct i915_ctx_hang_stats {
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_HANDLE 0
 
-#define CONTEXT_NO_ZEROMAP (1<<0)
 /**
  * struct i915_gem_context - as the name implies, represents a context.
  * @ref: reference count.
@@ -851,28 +850,31 @@ struct i915_ctx_hang_stats {
  */
 struct i915_gem_context {
 	struct kref ref;
-	int user_handle;
-	uint8_t remap_slice;
 	struct drm_i915_private *i915;
-	int flags;
 	struct drm_i915_file_private *file_priv;
-	struct i915_ctx_hang_stats hang_stats;
 	struct i915_hw_ppgtt *ppgtt;
 
+	struct i915_ctx_hang_stats hang_stats;
+
 	/* Unique identifier for this context, used by the hw for tracking */
 	unsigned hw_id;
+	uint32_t user_handle;
+	unsigned long flags;
+#define CONTEXT_NO_ZEROMAP		(1<<0)
 
 	struct intel_context {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
-		int pin_count;
 		struct i915_vma *lrc_vma;
-		u64 lrc_desc;
 		uint32_t *lrc_reg_state;
+		u64 lrc_desc;
+		int pin_count;
 		bool initialised;
 	} engine[I915_NUM_ENGINES];
 
 	struct list_head link;
+
+	uint8_t remap_slice;
 };
 
 enum fb_op_origin {
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (9 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
  2016-05-24  8:13   ` Daniel Vetter
  2016-05-22 13:33 ` ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization Patchwork
  11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 30cb26fe2fa9..3d14eb3215e1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
 	print_file_stats(m, "[k]batch pool", stats);
 }
 
+static int per_file_ctx_stats(int id, void *ptr, void *data)
+{
+	struct i915_gem_context *ctx = ptr;
+	int n;
+
+	for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
+		if (ctx->engine[n].state)
+			per_file_stats(0, ctx->engine[n].state, data);
+		if (ctx->engine[n].ringbuf)
+			per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
+	}
+
+	return 0;
+}
+
+static void print_context_stats(struct seq_file *m,
+				struct drm_i915_private *dev_priv)
+{
+	struct file_stats stats;
+	struct drm_file *file;
+
+	memset(&stats, 0, sizeof(stats));
+
+	if (dev_priv->kernel_context)
+		per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
+
+	list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
+		struct drm_i915_file_private *fpriv = file->driver_priv;
+		idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
+	}
+
+	print_file_stats(m, "[k]contexts", stats);
+}
+
 #define count_vmas(list, member) do { \
 	list_for_each_entry(vma, list, member) { \
 		size += i915_gem_obj_total_ggtt_size(vma->obj); \
@@ -521,6 +555,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 
 	seq_putc(m, '\n');
 	print_batch_pool_stats(m, dev_priv);
+	print_context_stats(m, dev_priv);
 
 	mutex_unlock(&dev->struct_mutex);
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization
  2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
                   ` (10 preceding siblings ...)
  2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
@ 2016-05-22 13:33 ` Patchwork
  11 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2016-05-22 13:33 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization
URL   : https://patchwork.freedesktop.org/series/7525/
State : failure

== Summary ==

Series 7525v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7525/revisions/1/mbox

Test gem_exec_gttfill:
        Subgroup basic:
                skip       -> FAIL       (ro-bdw-i7-5557U)
                skip       -> PASS       (fi-bdw-i7-5557u)
                skip       -> FAIL       (ro-bdw-i7-5600u)
                skip       -> PASS       (fi-hsw-i7-4770r)
                skip       -> FAIL       (ro-bdw-i5-5250u)
                skip       -> PASS       (fi-skl-i7-6700k)
                skip       -> PASS       (ro-hsw-i7-4770r)
                skip       -> PASS       (fi-hsw-i7-4770k)
                skip       -> FAIL       (ro-skl-i7-6700hq)
                skip       -> PASS       (ro-ivb2-i7-3770)
                skip       -> PASS       (ro-byt-n2820)

fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:2   skip:38 
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:209  pass:171  dwarn:0   dfail:0   fail:1   skip:37 
ro-bdw-i7-5557U  total:209  pass:196  dwarn:0   dfail:0   fail:1   skip:12 
ro-bdw-i7-5600u  total:209  pass:179  dwarn:0   dfail:0   fail:2   skip:28 
ro-bsw-n3050     total:209  pass:167  dwarn:0   dfail:0   fail:3   skip:39 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:209  pass:146  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl-i7-6700hq total:204  pass:182  dwarn:0   dfail:0   fail:1   skip:21 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
fi-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_960/

f1eaed1 drm-intel-nightly: 2016y-05m-20d-14h-35m-29s UTC integration manifest
83aedbd drm/i915: Show context objects in debugfs/i915_gem_objects
40af450 drm/i915: Rearrange i915_gem_context
3cb2b9f drm/i915: Merge legacy+execlists context structs
efc48e4 drm/i915: Put the kernel_context in drm_i915_private next to its friends
21b66dc drm/i915: Show i915_gem_context owner in debugfs
74e3400 drm/i915: Move pinning of dev_priv->kernel_context into its creator
3a68060 drm/i915: Name the inner most per-engine intel_context struct
59da8ff drm/i915: Rename i915_gem_context_reference/unreference()
185cd9c drm/i915: Rename and inline i915_gem_context_get()
1ce9e14 drm/i915: Apply lockdep annotations to i915_gem_context.c
35e3a4b drm/i915: Rename struct intel_context
2522d87 drm/i915/fbdev: Limit the global async-domain synchronization

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 02/12] drm/i915: Rename struct intel_context
  2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-23  9:09   ` Tvrtko Ursulin
  0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:09 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> Our goal is to rename the anonymous per-engine struct beneath the
> current intel_context. However, after a lively debate resolving around
> the confusion between intel_context_engine and intel_engine_context, the
> realisation is that the two structs target different users. The outer
> struct is API / user facing, and so carries the higher level GEM
> information. The inner struct is hw facing. Thus we want to name the
> inner struct intel_context and the outer one i915_gem_context. As the
> first step, we need to rename the current struct:
>
> 	s/struct intel_context/struct i915_gem_context/
>
> which fits much better with its constructors already conveying the
> i915_gem_context prefix!

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c        | 10 +++---
>   drivers/gpu/drm/i915/i915_drv.h            | 22 ++++++-------
>   drivers/gpu/drm/i915/i915_gem.c            |  8 ++---
>   drivers/gpu/drm/i915/i915_gem_context.c    | 52 +++++++++++++++---------------
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 12 +++----
>   drivers/gpu/drm/i915/i915_sysfs.c          |  2 +-
>   drivers/gpu/drm/i915/i915_trace.h          | 12 +++----
>   drivers/gpu/drm/i915/intel_guc.h           |  2 +-
>   drivers/gpu/drm/i915/intel_lrc.c           | 22 ++++++-------
>   drivers/gpu/drm/i915/intel_lrc.h           | 10 +++---
>   drivers/gpu/drm/i915/intel_ringbuffer.h    |  4 +--
>   12 files changed, 84 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 4c6b48dbd6e2..ae28e6e9d603 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -199,7 +199,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>   		seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
>   }
>
> -static void describe_ctx(struct seq_file *m, struct intel_context *ctx)
> +static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
>   {
>   	seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
>   	seq_putc(m, ctx->remap_slice ? 'R' : 'r');
> @@ -2000,7 +2000,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_engine_cs *engine;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	enum intel_engine_id id;
>   	int ret;
>
> @@ -2046,7 +2046,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   }
>
>   static void i915_dump_lrc_obj(struct seq_file *m,
> -			      struct intel_context *ctx,
> +			      struct i915_gem_context *ctx,
>   			      struct intel_engine_cs *engine)
>   {
>   	struct page *page;
> @@ -2094,7 +2094,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_engine_cs *engine;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	if (!i915.enable_execlists) {
> @@ -2274,7 +2274,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
>
>   static int per_file_ctx(int id, void *ptr, void *data)
>   {
> -	struct intel_context *ctx = ptr;
> +	struct i915_gem_context *ctx = ptr;
>   	struct seq_file *m = data;
>   	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e2abd9eb352b..8380102bbdd8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -831,7 +831,7 @@ struct i915_ctx_hang_stats {
>
>   #define CONTEXT_NO_ZEROMAP (1<<0)
>   /**
> - * struct intel_context - as the name implies, represents a context.
> + * struct i915_gem_context - as the name implies, represents a context.
>    * @ref: reference count.
>    * @user_handle: userspace tracking identity for this context.
>    * @remap_slice: l3 row remapping information.
> @@ -849,7 +849,7 @@ struct i915_ctx_hang_stats {
>    * Contexts are memory images used by the hardware to store copies of their
>    * internal state.
>    */
> -struct intel_context {
> +struct i915_gem_context {
>   	struct kref ref;
>   	int user_handle;
>   	uint8_t remap_slice;
> @@ -1710,7 +1710,7 @@ struct i915_execbuffer_params {
>   	uint64_t                        batch_obj_vm_offset;
>   	struct intel_engine_cs *engine;
>   	struct drm_i915_gem_object      *batch_obj;
> -	struct intel_context            *ctx;
> +	struct i915_gem_context            *ctx;
>   	struct drm_i915_gem_request     *request;
>   };
>
> @@ -2013,7 +2013,7 @@ struct drm_i915_private {
>   		void (*stop_engine)(struct intel_engine_cs *engine);
>   	} gt;
>
> -	struct intel_context *kernel_context;
> +	struct i915_gem_context *kernel_context;
>
>   	/* perform PHY state sanity checks? */
>   	bool chv_phy_assert[2];
> @@ -2381,7 +2381,7 @@ struct drm_i915_gem_request {
>   	 * i915_gem_request_free() will then decrement the refcount on the
>   	 * context.
>   	 */
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	struct intel_ringbuffer *ringbuf;
>
>   	/**
> @@ -2393,7 +2393,7 @@ struct drm_i915_gem_request {
>   	 * we keep the previous context pinned until the following (this)
>   	 * request is retired.
>   	 */
> -	struct intel_context *previous_context;
> +	struct i915_gem_context *previous_context;
>
>   	/** Batch buffer related to this request if any (used for
>   	    error state dump only) */
> @@ -2437,7 +2437,7 @@ struct drm_i915_gem_request {
>
>   struct drm_i915_gem_request * __must_check
>   i915_gem_request_alloc(struct intel_engine_cs *engine,
> -		       struct intel_context *ctx);
> +		       struct i915_gem_context *ctx);
>   void i915_gem_request_free(struct kref *req_ref);
>   int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
>   				   struct drm_file *file);
> @@ -3417,22 +3417,22 @@ void i915_gem_context_reset(struct drm_device *dev);
>   int i915_gem_context_open(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 drm_i915_gem_request *req);
> -struct intel_context *
> +struct i915_gem_context *
>   i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
>   void i915_gem_context_free(struct kref *ctx_ref);
>   struct drm_i915_gem_object *
>   i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
> -static inline void i915_gem_context_reference(struct intel_context *ctx)
> +static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
>   {
>   	kref_get(&ctx->ref);
>   }
>
> -static inline void i915_gem_context_unreference(struct intel_context *ctx)
> +static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
>   {
>   	kref_put(&ctx->ref, i915_gem_context_free);
>   }
>
> -static inline bool i915_gem_context_is_default(const struct intel_context *c)
> +static inline bool i915_gem_context_is_default(const struct i915_gem_context *c)
>   {
>   	return c->user_handle == DEFAULT_CONTEXT_HANDLE;
>   }
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c4af57ada378..728b66911840 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2689,7 +2689,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
>   }
>
>   static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
> -				   const struct intel_context *ctx)
> +				   const struct i915_gem_context *ctx)
>   {
>   	unsigned long elapsed;
>
> @@ -2714,7 +2714,7 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
>   }
>
>   static void i915_set_reset_status(struct drm_i915_private *dev_priv,
> -				  struct intel_context *ctx,
> +				  struct i915_gem_context *ctx,
>   				  const bool guilty)
>   {
>   	struct i915_ctx_hang_stats *hs;
> @@ -2742,7 +2742,7 @@ void i915_gem_request_free(struct kref *req_ref)
>
>   static inline int
>   __i915_gem_request_alloc(struct intel_engine_cs *engine,
> -			 struct intel_context *ctx,
> +			 struct i915_gem_context *ctx,
>   			 struct drm_i915_gem_request **req_out)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> @@ -2818,7 +2818,7 @@ err:
>    */
>   struct drm_i915_gem_request *
>   i915_gem_request_alloc(struct intel_engine_cs *engine,
> -		       struct intel_context *ctx)
> +		       struct i915_gem_context *ctx)
>   {
>   	struct drm_i915_gem_request *req;
>   	int err;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 2aedd188473d..8484da26b5d4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -134,7 +134,7 @@ static int get_context_size(struct drm_i915_private *dev_priv)
>   	return ret;
>   }
>
> -static void i915_gem_context_clean(struct intel_context *ctx)
> +static void i915_gem_context_clean(struct i915_gem_context *ctx)
>   {
>   	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
>   	struct i915_vma *vma, *next;
> @@ -151,7 +151,7 @@ static void i915_gem_context_clean(struct intel_context *ctx)
>
>   void i915_gem_context_free(struct kref *ctx_ref)
>   {
> -	struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
> +	struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
>
>   	trace_i915_context_free(ctx);
>
> @@ -234,12 +234,12 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
>   	return 0;
>   }
>
> -static struct intel_context *
> +static struct i915_gem_context *
>   __create_hw_context(struct drm_device *dev,
>   		    struct drm_i915_file_private *file_priv)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> @@ -296,12 +296,12 @@ err_out:
>    * context state of the GPU for applications that don't utilize HW contexts, as
>    * well as an idle case.
>    */
> -static struct intel_context *
> +static struct i915_gem_context *
>   i915_gem_create_context(struct drm_device *dev,
>   			struct drm_i915_file_private *file_priv)
>   {
>   	const bool is_global_default_ctx = file_priv == NULL;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret = 0;
>
>   	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
> @@ -352,7 +352,7 @@ err_destroy:
>   	return ERR_PTR(ret);
>   }
>
> -static void i915_gem_context_unpin(struct intel_context *ctx,
> +static void i915_gem_context_unpin(struct i915_gem_context *ctx,
>   				   struct intel_engine_cs *engine)
>   {
>   	if (i915.enable_execlists) {
> @@ -369,7 +369,7 @@ void i915_gem_context_reset(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>
>   	if (i915.enable_execlists) {
> -		struct intel_context *ctx;
> +		struct i915_gem_context *ctx;
>
>   		list_for_each_entry(ctx, &dev_priv->context_list, link)
>   			intel_lr_context_reset(dev_priv, ctx);
> @@ -381,7 +381,7 @@ void i915_gem_context_reset(struct drm_device *dev)
>   int i915_gem_context_init(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>
>   	/* Init should only be called once per module load. Eventually the
>   	 * restriction on the context_disabled check can be loosened. */
> @@ -449,7 +449,7 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
>   void i915_gem_context_fini(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_context *dctx = dev_priv->kernel_context;
> +	struct i915_gem_context *dctx = dev_priv->kernel_context;
>
>   	if (dctx->legacy_hw_ctx.rcs_state)
>   		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
> @@ -462,7 +462,7 @@ void i915_gem_context_fini(struct drm_device *dev)
>
>   static int context_idr_cleanup(int id, void *p, void *data)
>   {
> -	struct intel_context *ctx = p;
> +	struct i915_gem_context *ctx = p;
>
>   	i915_gem_context_unreference(ctx);
>   	return 0;
> @@ -471,7 +471,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
>   int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
>   {
>   	struct drm_i915_file_private *file_priv = file->driver_priv;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>
>   	idr_init(&file_priv->context_idr);
>
> @@ -495,12 +495,12 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
>   	idr_destroy(&file_priv->context_idr);
>   }
>
> -struct intel_context *
> +struct i915_gem_context *
>   i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
>   {
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>
> -	ctx = (struct intel_context *)idr_find(&file_priv->context_idr, id);
> +	ctx = idr_find(&file_priv->context_idr, id);
>   	if (!ctx)
>   		return ERR_PTR(-ENOENT);
>
> @@ -641,7 +641,7 @@ static int remap_l3(struct drm_i915_gem_request *req, int slice)
>
>   static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
>   				   struct intel_engine_cs *engine,
> -				   struct intel_context *to)
> +				   struct i915_gem_context *to)
>   {
>   	if (to->remap_slice)
>   		return false;
> @@ -658,7 +658,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
>   static bool
>   needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
>   		  struct intel_engine_cs *engine,
> -		  struct intel_context *to)
> +		  struct i915_gem_context *to)
>   {
>   	if (!ppgtt)
>   		return false;
> @@ -683,7 +683,7 @@ needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
>
>   static bool
>   needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
> -		   struct intel_context *to,
> +		   struct i915_gem_context *to,
>   		   u32 hw_flags)
>   {
>   	if (!ppgtt)
> @@ -700,10 +700,10 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
>
>   static int do_rcs_switch(struct drm_i915_gem_request *req)
>   {
> -	struct intel_context *to = req->ctx;
> +	struct i915_gem_context *to = req->ctx;
>   	struct intel_engine_cs *engine = req->engine;
>   	struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
> -	struct intel_context *from;
> +	struct i915_gem_context *from;
>   	u32 hw_flags;
>   	int ret, i;
>
> @@ -859,7 +859,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
>
>   	if (engine->id != RCS ||
>   	    req->ctx->legacy_hw_ctx.rcs_state == NULL) {
> -		struct intel_context *to = req->ctx;
> +		struct i915_gem_context *to = req->ctx;
>   		struct i915_hw_ppgtt *ppgtt =
>   			to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
>
> @@ -897,7 +897,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>   {
>   	struct drm_i915_gem_context_create *args = data;
>   	struct drm_i915_file_private *file_priv = file->driver_priv;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	if (!contexts_enabled(dev))
> @@ -926,7 +926,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>   {
>   	struct drm_i915_gem_context_destroy *args = data;
>   	struct drm_i915_file_private *file_priv = file->driver_priv;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	if (args->pad != 0)
> @@ -958,7 +958,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>   {
>   	struct drm_i915_file_private *file_priv = file->driver_priv;
>   	struct drm_i915_gem_context_param *args = data;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	ret = i915_mutex_lock_interruptible(dev);
> @@ -1001,7 +1001,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>   {
>   	struct drm_i915_file_private *file_priv = file->driver_priv;
>   	struct drm_i915_gem_context_param *args = data;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	ret = i915_mutex_lock_interruptible(dev);
> @@ -1047,7 +1047,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct drm_i915_reset_stats *args = data;
>   	struct i915_ctx_hang_stats *hs;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	int ret;
>
>   	if (args->flags || args->pad)
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index a54a243ccaac..e61b92d7b0bc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -716,7 +716,7 @@ eb_vma_misplaced(struct i915_vma *vma)
>   static int
>   i915_gem_execbuffer_reserve(struct intel_engine_cs *engine,
>   			    struct list_head *vmas,
> -			    struct intel_context *ctx,
> +			    struct i915_gem_context *ctx,
>   			    bool *need_relocs)
>   {
>   	struct drm_i915_gem_object *obj;
> @@ -828,7 +828,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
>   				  struct intel_engine_cs *engine,
>   				  struct eb_vmas *eb,
>   				  struct drm_i915_gem_exec_object2 *exec,
> -				  struct intel_context *ctx)
> +				  struct i915_gem_context *ctx)
>   {
>   	struct drm_i915_gem_relocation_entry *reloc;
>   	struct i915_address_space *vm;
> @@ -1065,11 +1065,11 @@ validate_exec_list(struct drm_device *dev,
>   	return 0;
>   }
>
> -static struct intel_context *
> +static struct i915_gem_context *
>   i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
>   			  struct intel_engine_cs *engine, const u32 ctx_id)
>   {
> -	struct intel_context *ctx = NULL;
> +	struct i915_gem_context *ctx = NULL;
>   	struct i915_ctx_hang_stats *hs;
>
>   	if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
> @@ -1430,7 +1430,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>   	struct drm_i915_gem_object *batch_obj;
>   	struct drm_i915_gem_exec_object2 shadow_exec_entry;
>   	struct intel_engine_cs *engine;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	struct i915_address_space *vm;
>   	struct i915_execbuffer_params params_master; /* XXX: will be removed later */
>   	struct i915_execbuffer_params *params = &params_master;
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 169242a8adff..a292672f36d5 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -360,7 +360,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>   	struct drm_i915_gem_object *client_obj = client->client_obj;
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   	struct intel_engine_cs *engine;
> -	struct intel_context *ctx = client->owner;
> +	struct i915_gem_context *ctx = client->owner;
>   	struct guc_context_desc desc;
>   	struct sg_table *sg;
>   	enum intel_engine_id id;
> @@ -426,7 +426,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>   	desc.wq_size = client->wq_size;
>
>   	/*
> -	 * XXX: Take LRCs from an existing intel_context if this is not an
> +	 * XXX: Take LRCs from an existing context if this is not an
>   	 * IsKMDCreatedContext client
>   	 */
>   	desc.desc_private = (uintptr_t)client;
> @@ -677,7 +677,7 @@ static void guc_client_free(struct drm_device *dev,
>    */
>   static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
>   						uint32_t priority,
> -						struct intel_context *ctx)
> +						struct i915_gem_context *ctx)
>   {
>   	struct i915_guc_client *client;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -915,7 +915,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
> -	struct intel_context *ctx = dev_priv->kernel_context;
> +	struct i915_gem_context *ctx = dev_priv->kernel_context;
>   	struct i915_guc_client *client;
>
>   	/* client for execbuf submission */
> @@ -966,7 +966,7 @@ int intel_guc_suspend(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	u32 data[3];
>
>   	if (!i915.enable_guc_submission)
> @@ -992,7 +992,7 @@ int intel_guc_resume(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	u32 data[3];
>
>   	if (!i915.enable_guc_submission)
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 37b6444b8e22..02507bfc8def 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -203,7 +203,7 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
>   	struct drm_minor *dminor = dev_to_drm_minor(dev);
>   	struct drm_device *drm_dev = dminor->dev;
>   	struct drm_i915_private *dev_priv = drm_dev->dev_private;
> -	struct intel_context *ctx;
> +	struct i915_gem_context *ctx;
>   	u32 *temp = NULL; /* Just here to make handling failures easy */
>   	int slice = (int)(uintptr_t)attr->private;
>   	int ret;
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 20b2e4039792..6768db032f84 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -734,12 +734,12 @@ DEFINE_EVENT(i915_ppgtt, i915_ppgtt_release,
>    * the context.
>    */
>   DECLARE_EVENT_CLASS(i915_context,
> -	TP_PROTO(struct intel_context *ctx),
> +	TP_PROTO(struct i915_gem_context *ctx),
>   	TP_ARGS(ctx),
>
>   	TP_STRUCT__entry(
>   			__field(u32, dev)
> -			__field(struct intel_context *, ctx)
> +			__field(struct i915_gem_context *, ctx)
>   			__field(struct i915_address_space *, vm)
>   	),
>
> @@ -754,12 +754,12 @@ DECLARE_EVENT_CLASS(i915_context,
>   )
>
>   DEFINE_EVENT(i915_context, i915_context_create,
> -	TP_PROTO(struct intel_context *ctx),
> +	TP_PROTO(struct i915_gem_context *ctx),
>   	TP_ARGS(ctx)
>   );
>
>   DEFINE_EVENT(i915_context, i915_context_free,
> -	TP_PROTO(struct intel_context *ctx),
> +	TP_PROTO(struct i915_gem_context *ctx),
>   	TP_ARGS(ctx)
>   );
>
> @@ -771,13 +771,13 @@ DEFINE_EVENT(i915_context, i915_context_free,
>    * called only if full ppgtt is enabled.
>    */
>   TRACE_EVENT(switch_mm,
> -	TP_PROTO(struct intel_engine_cs *engine, struct intel_context *to),
> +	TP_PROTO(struct intel_engine_cs *engine, struct i915_gem_context *to),
>
>   	TP_ARGS(engine, to),
>
>   	TP_STRUCT__entry(
>   			__field(u32, ring)
> -			__field(struct intel_context *, to)
> +			__field(struct i915_gem_context *, to)
>   			__field(struct i915_address_space *, vm)
>   			__field(u32, dev)
>   	),
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 9d79c4c3e256..c9c757ef003f 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -55,7 +55,7 @@ struct drm_i915_gem_request;
>   struct i915_guc_client {
>   	struct drm_i915_gem_object *client_obj;
>   	void *client_base;		/* first page (only) of above	*/
> -	struct intel_context *owner;
> +	struct i915_gem_context *owner;
>   	struct intel_guc *guc;
>   	uint32_t priority;
>   	uint32_t ctx_index;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index affca6d5ce7e..426dc90aceed 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -231,9 +231,9 @@ enum {
>   /* Typical size of the average request (2 pipecontrols and a MI_BB) */
>   #define EXECLISTS_REQUEST_SIZE 64 /* bytes */
>
> -static int execlists_context_deferred_alloc(struct intel_context *ctx,
> +static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   					    struct intel_engine_cs *engine);
> -static int intel_lr_context_pin(struct intel_context *ctx,
> +static int intel_lr_context_pin(struct i915_gem_context *ctx,
>   				struct intel_engine_cs *engine);
>
>   /**
> @@ -315,7 +315,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
>    *    bits 55-63:    group ID, currently unused and set to 0
>    */
>   static void
> -intel_lr_context_descriptor_update(struct intel_context *ctx,
> +intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
>   				   struct intel_engine_cs *engine)
>   {
>   	u64 desc;
> @@ -330,7 +330,7 @@ intel_lr_context_descriptor_update(struct intel_context *ctx,
>   	ctx->engine[engine->id].lrc_desc = desc;
>   }
>
> -uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> +uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
>   				     struct intel_engine_cs *engine)
>   {
>   	return ctx->engine[engine->id].lrc_desc;
> @@ -932,7 +932,7 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
>   	return 0;
>   }
>
> -static int intel_lr_context_pin(struct intel_context *ctx,
> +static int intel_lr_context_pin(struct i915_gem_context *ctx,
>   				struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = ctx->i915;
> @@ -988,7 +988,7 @@ err:
>   	return ret;
>   }
>
> -void intel_lr_context_unpin(struct intel_context *ctx,
> +void intel_lr_context_unpin(struct i915_gem_context *ctx,
>   			    struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_gem_object *ctx_obj;
> @@ -2049,7 +2049,7 @@ logical_ring_setup(struct drm_device *dev, enum intel_engine_id id)
>   static int
>   logical_ring_init(struct intel_engine_cs *engine)
>   {
> -	struct intel_context *dctx = engine->i915->kernel_context;
> +	struct i915_gem_context *dctx = engine->i915->kernel_context;
>   	int ret;
>
>   	ret = i915_cmd_parser_init_ring(engine);
> @@ -2273,7 +2273,7 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
>   }
>
>   static int
> -populate_lr_context(struct intel_context *ctx,
> +populate_lr_context(struct i915_gem_context *ctx,
>   		    struct drm_i915_gem_object *ctx_obj,
>   		    struct intel_engine_cs *engine,
>   		    struct intel_ringbuffer *ringbuf)
> @@ -2421,7 +2421,7 @@ populate_lr_context(struct intel_context *ctx,
>    * takes care of the bits that are LRC related: the per-engine backing
>    * objects and the logical ringbuffer.
>    */
> -void intel_lr_context_free(struct intel_context *ctx)
> +void intel_lr_context_free(struct i915_gem_context *ctx)
>   {
>   	int i;
>
> @@ -2489,7 +2489,7 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
>    *
>    * Return: non-zero on error.
>    */
> -static int execlists_context_deferred_alloc(struct intel_context *ctx,
> +static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   					    struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_gem_object *ctx_obj;
> @@ -2539,7 +2539,7 @@ error_deref_obj:
>   }
>
>   void intel_lr_context_reset(struct drm_i915_private *dev_priv,
> -			    struct intel_context *ctx)
> +			    struct i915_gem_context *ctx)
>   {
>   	struct intel_engine_cs *engine;
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 1afba0331dc6..eb2e1d157fed 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -99,16 +99,18 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
>   #define LRC_PPHWSP_PN	(LRC_GUCSHR_PN + 1)
>   #define LRC_STATE_PN	(LRC_PPHWSP_PN + 1)
>
> -void intel_lr_context_free(struct intel_context *ctx);
> +struct i915_gem_context;
> +
> +void intel_lr_context_free(struct i915_gem_context *ctx);
>   uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
> -void intel_lr_context_unpin(struct intel_context *ctx,
> +void intel_lr_context_unpin(struct i915_gem_context *ctx,
>   			    struct intel_engine_cs *engine);
>
>   struct drm_i915_private;
>
>   void intel_lr_context_reset(struct drm_i915_private *dev_priv,
> -			    struct intel_context *ctx);
> -uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> +			    struct i915_gem_context *ctx);
> +uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
>   				     struct intel_engine_cs *engine);
>
>   /* Execlists */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 929e7b4af2a4..b33c876fed20 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -119,7 +119,7 @@ struct intel_ringbuffer {
>   	u32 last_retired_head;
>   };
>
> -struct	intel_context;
> +struct i915_gem_context;
>   struct drm_i915_reg_table;
>
>   /*
> @@ -310,7 +310,7 @@ struct intel_engine_cs {
>
>   	wait_queue_head_t irq_queue;
>
> -	struct intel_context *last_context;
> +	struct i915_gem_context *last_context;
>
>   	struct intel_ring_hangcheck hangcheck;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c
  2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
@ 2016-05-23  9:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> Markup the functions that require the caller to hold struct_mutex with
> lockdep_assert_held(). In the hopefully not-too-distant future we will
> split the struct_mutex up, and in doing so we need to be sure that we
> know what it protects - here the lockdep annotations are invaluable.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h         |  1 +
>   drivers/gpu/drm/i915/i915_gem_context.c | 16 ++++++++++++++--
>   2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8380102bbdd8..48a222ddd8b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3429,6 +3429,7 @@ static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
>
>   static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
>   {
> +	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
>   	kref_put(&ctx->ref, i915_gem_context_free);
>   }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8484da26b5d4..af747f14522e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
>   {
>   	struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
>
> +	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
>   	trace_i915_context_free(ctx);
>
>   	if (i915.enable_execlists)
> @@ -181,6 +182,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
>   	struct drm_i915_gem_object *obj;
>   	int ret;
>
> +	lockdep_assert_held(&dev->struct_mutex);
> +
>   	obj = i915_gem_object_create(dev, size);
>   	if (IS_ERR(obj))
>   		return obj;
> @@ -368,6 +371,8 @@ void i915_gem_context_reset(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>
> +	lockdep_assert_held(&dev->struct_mutex);
> +
>   	if (i915.enable_execlists) {
>   		struct i915_gem_context *ctx;
>
> @@ -433,6 +438,8 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
>   {
>   	struct intel_engine_cs *engine;
>
> +	lockdep_assert_held(&dev_priv->dev->struct_mutex);
> +
>   	for_each_engine(engine, dev_priv) {
>   		if (engine->last_context == NULL)
>   			continue;
> @@ -451,6 +458,8 @@ void i915_gem_context_fini(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct i915_gem_context *dctx = dev_priv->kernel_context;
>
> +	lockdep_assert_held(&dev->struct_mutex);
> +
>   	if (dctx->legacy_hw_ctx.rcs_state)
>   		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
>
> @@ -491,6 +500,8 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
>   {
>   	struct drm_i915_file_private *file_priv = file->driver_priv;
>
> +	lockdep_assert_held(&dev->struct_mutex);
> +
>   	idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
>   	idr_destroy(&file_priv->context_idr);
>   }
> @@ -500,6 +511,8 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
>   {
>   	struct i915_gem_context *ctx;
>
> +	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
> +
>   	ctx = idr_find(&file_priv->context_idr, id);
>   	if (!ctx)
>   		return ERR_PTR(-ENOENT);
> @@ -852,10 +865,9 @@ unpin_out:
>   int i915_switch_context(struct drm_i915_gem_request *req)
>   {
>   	struct intel_engine_cs *engine = req->engine;
> -	struct drm_i915_private *dev_priv = req->i915;
>
>   	WARN_ON(i915.enable_execlists);
> -	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
> +	lockdep_assert_held(&req->i915->dev->struct_mutex);
>
>   	if (engine->id != RCS ||
>   	    req->ctx->legacy_hw_ctx.rcs_state == NULL) {
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get()
  2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
@ 2016-05-23  9:15   ` Tvrtko Ursulin
  0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:15 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> i915_gem_context_get() is a very simple wrapper around idr_find(), so
> simple that it would be smaller to do the lookup inline. Also we use the
> verb 'lookup' to return a pointer from a handle, freeing 'get' to imply
> obtaining a reference to the context.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h            | 17 +++++++++++++++--
>   drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++------------------
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
>   3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 48a222ddd8b3..e4e3614335ec 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3417,11 +3417,24 @@ void i915_gem_context_reset(struct drm_device *dev);
>   int i915_gem_context_open(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 drm_i915_gem_request *req);
> -struct i915_gem_context *
> -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
>   void i915_gem_context_free(struct kref *ctx_ref);
>   struct drm_i915_gem_object *
>   i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
> +
> +static inline struct i915_gem_context *
> +i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
> +{
> +	struct i915_gem_context *ctx;
> +
> +	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
> +
> +	ctx = idr_find(&file_priv->context_idr, id);
> +	if (!ctx)
> +		return ERR_PTR(-ENOENT);
> +
> +	return ctx;
> +}
> +
>   static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
>   {
>   	kref_get(&ctx->ref);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index af747f14522e..aa329175f73a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -506,20 +506,6 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
>   	idr_destroy(&file_priv->context_idr);
>   }
>
> -struct i915_gem_context *
> -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
> -{
> -	struct i915_gem_context *ctx;
> -
> -	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
> -
> -	ctx = idr_find(&file_priv->context_idr, id);
> -	if (!ctx)
> -		return ERR_PTR(-ENOENT);
> -
> -	return ctx;
> -}
> -
>   static inline int
>   mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
>   {
> @@ -951,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> @@ -977,7 +963,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> @@ -1020,7 +1006,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> @@ -1072,7 +1058,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file->driver_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file->driver_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index e61b92d7b0bc..84d990331abd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1075,7 +1075,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
>   	if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
>   		return ERR_PTR(-EINVAL);
>
> -	ctx = i915_gem_context_get(file->driver_priv, ctx_id);
> +	ctx = i915_gem_context_lookup(file->driver_priv, ctx_id);
>   	if (IS_ERR(ctx))
>   		return ctx;
>
>

Makes sense.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
  2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
@ 2016-05-23  9:17   ` Tvrtko Ursulin
  2016-05-23  9:25     ` Chris Wilson
  2016-05-24  8:10     ` Daniel Vetter
  0 siblings, 2 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:17 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> As these are wrappers around kref_get/kref_put() it is preferable to
> follow the naming convention and use the same verb get/put in our
> wrapper names for manipulating a reference to the context.

Not so sure about this one. We got objects, framebuffers and requests 
using (un)reference naming so don't see that making contexts different 
is a good idea.

Regards,

Tvrtko


> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h            |  6 ++++--
>   drivers/gpu/drm/i915/i915_gem.c            |  7 +++----
>   drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++++++++------------
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  6 +++---
>   drivers/gpu/drm/i915/intel_lrc.c           |  4 ++--
>   5 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4e3614335ec..0a3442fcd93e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3435,12 +3435,14 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
>   	return ctx;
>   }
>
> -static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
> +static inline struct i915_gem_context *
> +i915_gem_context_get(struct i915_gem_context *ctx)
>   {
>   	kref_get(&ctx->ref);
> +	return ctx;
>   }
>
> -static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
> +static inline void i915_gem_context_put(struct i915_gem_context *ctx)
>   {
>   	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
>   	kref_put(&ctx->ref, i915_gem_context_free);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 728b66911840..d7ae030e5a0b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1418,7 +1418,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
>   					       request->engine);
>   	}
>
> -	i915_gem_context_unreference(request->ctx);
> +	i915_gem_context_put(request->ctx);
>   	i915_gem_request_unreference(request);
>   }
>
> @@ -2775,8 +2775,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
>   	req->i915 = dev_priv;
>   	req->engine = engine;
>   	req->reset_counter = reset_counter;
> -	req->ctx  = ctx;
> -	i915_gem_context_reference(req->ctx);
> +	req->ctx = i915_gem_context_get(ctx);
>
>   	/*
>   	 * Reserve space in the ring buffer for all the commands required to
> @@ -2798,7 +2797,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
>   	return 0;
>
>   err_ctx:
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   err:
>   	kmem_cache_free(dev_priv->requests, req);
>   	return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index aa329175f73a..a4c6377eea32 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -290,7 +290,7 @@ __create_hw_context(struct drm_device *dev,
>   	return ctx;
>
>   err_out:
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   	return ERR_PTR(ret);
>   }
>
> @@ -351,7 +351,7 @@ err_unpin:
>   		i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
>   err_destroy:
>   	idr_remove(&file_priv->context_idr, ctx->user_handle);
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   	return ERR_PTR(ret);
>   }
>
> @@ -363,7 +363,7 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
>   	} else {
>   		if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
>   			i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> -		i915_gem_context_unreference(ctx);
> +		i915_gem_context_put(ctx);
>   	}
>   }
>
> @@ -463,7 +463,7 @@ void i915_gem_context_fini(struct drm_device *dev)
>   	if (dctx->legacy_hw_ctx.rcs_state)
>   		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
>
> -	i915_gem_context_unreference(dctx);
> +	i915_gem_context_put(dctx);
>   	dev_priv->kernel_context = NULL;
>
>   	ida_destroy(&dev_priv->context_hw_ida);
> @@ -473,7 +473,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
>   {
>   	struct i915_gem_context *ctx = p;
>
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   	return 0;
>   }
>
> @@ -785,10 +785,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>
>   		/* obj is kept alive until the next request by its active ref */
>   		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
> -		i915_gem_context_unreference(from);
> +		i915_gem_context_put(from);
>   	}
> -	i915_gem_context_reference(to);
> -	engine->last_context = to;
> +	engine->last_context = i915_gem_context_get(to);
>
>   	/* GEN8 does *not* require an explicit reload if the PDPs have been
>   	 * setup, and we do not wish to move them.
> @@ -873,10 +872,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
>   		}
>
>   		if (to != engine->last_context) {
> -			i915_gem_context_reference(to);
>   			if (engine->last_context)
> -				i915_gem_context_unreference(engine->last_context);
> -			engine->last_context = to;
> +				i915_gem_context_put(engine->last_context);
> +			engine->last_context = i915_gem_context_get(to);
>   		}
>
>   		return 0;
> @@ -944,7 +942,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>   	}
>
>   	idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   	mutex_unlock(&dev->struct_mutex);
>
>   	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 84d990331abd..14628ce91273 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1492,7 +1492,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>   		goto pre_mutex_err;
>   	}
>
> -	i915_gem_context_reference(ctx);
> +	i915_gem_context_get(ctx);
>
>   	if (ctx->ppgtt)
>   		vm = &ctx->ppgtt->base;
> @@ -1503,7 +1503,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>
>   	eb = eb_create(args);
>   	if (eb == NULL) {
> -		i915_gem_context_unreference(ctx);
> +		i915_gem_context_put(ctx);
>   		mutex_unlock(&dev->struct_mutex);
>   		ret = -ENOMEM;
>   		goto pre_mutex_err;
> @@ -1647,7 +1647,7 @@ err_batch_unpin:
>
>   err:
>   	/* the request owns the ref now */
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   	eb_destroy(eb);
>
>   	mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 426dc90aceed..9a55478e23ac 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -966,7 +966,6 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
>   	if (ret)
>   		goto unpin_map;
>
> -	i915_gem_context_reference(ctx);
>   	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
>   	intel_lr_context_descriptor_update(ctx, engine);
>   	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
> @@ -977,6 +976,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
>   	if (i915.enable_guc_submission)
>   		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
>
> +	i915_gem_context_get(ctx);
>   	return 0;
>
>   unpin_map:
> @@ -1009,7 +1009,7 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
>   	ctx->engine[engine->id].lrc_desc = 0;
>   	ctx->engine[engine->id].lrc_reg_state = NULL;
>
> -	i915_gem_context_unreference(ctx);
> +	i915_gem_context_put(ctx);
>   }
>
>   static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
  2016-05-23  9:17   ` Tvrtko Ursulin
@ 2016-05-23  9:25     ` Chris Wilson
  2016-05-24  8:10     ` Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23  9:25 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 10:17:01AM +0100, Tvrtko Ursulin wrote:
> 
> On 22/05/16 14:02, Chris Wilson wrote:
> >As these are wrappers around kref_get/kref_put() it is preferable to
> >follow the naming convention and use the same verb get/put in our
> >wrapper names for manipulating a reference to the context.
> 
> Not so sure about this one. We got objects, framebuffers and
> requests using (un)reference naming so don't see that making
> contexts different is a good idea.

I've converted objects and requests in the lockless patches. So contexts
were a bit of an eyesore in amongst the *_get()/*_put().
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
  2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
@ 2016-05-23  9:26   ` Tvrtko Ursulin
  2016-05-23 10:17     ` Chris Wilson
  0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:26 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> We want to give a name to the currently anonymous per-engine struct
> inside the context, so that we can assign it to a local variable and
> save clumsy typing. The name we have chosen is intel_context as it
> reflects the HW facing portion of the context state (the logical context
> state, the registers, the ringbuffer etc).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h            |  2 +-
>   drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++---
>   drivers/gpu/drm/i915/intel_lrc.c           | 90 +++++++++++++++---------------
>   3 files changed, 51 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0a3442fcd93e..e6735dc9eeb2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -869,7 +869,7 @@ struct i915_gem_context {
>   	} legacy_hw_ctx;
>
>   	/* Execlists */
> -	struct {
> +	struct intel_context {
>   		struct drm_i915_gem_object *state;
>   		struct intel_ringbuffer *ringbuf;
>   		int pin_count;
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index a292672f36d5..c29c1d19764f 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -363,7 +363,6 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>   	struct i915_gem_context *ctx = client->owner;
>   	struct guc_context_desc desc;
>   	struct sg_table *sg;
> -	enum intel_engine_id id;
>   	u32 gfx_addr;
>
>   	memset(&desc, 0, sizeof(desc));
> @@ -373,10 +372,10 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>   	desc.priority = client->priority;
>   	desc.db_id = client->doorbell_id;
>
> -	for_each_engine_id(engine, dev_priv, id) {
> +	for_each_engine(engine, dev_priv) {
> +		struct intel_context *ce = &ctx->engine[engine->id];
>   		struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
>   		struct drm_i915_gem_object *obj;
> -		uint64_t ctx_desc;
>
>   		/* TODO: We have a design issue to be solved here. Only when we
>   		 * receive the first batch, we know which engine is used by the
> @@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>   		 * for now who owns a GuC client. But for future owner of GuC
>   		 * client, need to make sure lrc is pinned prior to enter here.
>   		 */
> -		obj = ctx->engine[id].state;
> -		if (!obj)
> +		if (!ce->state)
>   			break;	/* XXX: continue? */
>
> -		ctx_desc = intel_lr_context_descriptor(ctx, engine);
> -		lrc->context_desc = (u32)ctx_desc;
> +		lrc->context_desc = lower_32_bits(ce->lrc_desc);

Could have kept use of intel_lr_context_descriptor for better separation.

>
>   		/* The state page is after PPHWSP */
> -		gfx_addr = i915_gem_obj_ggtt_offset(obj);
> +		gfx_addr = i915_gem_obj_ggtt_offset(ce->state);
>   		lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
>   		lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
>   				(engine->guc_id << GUC_ELC_ENGINE_OFFSET);
>
> -		obj = ctx->engine[id].ringbuf->obj;
> +		obj = ce->ringbuf->obj;
>   		gfx_addr = i915_gem_obj_ggtt_offset(obj);
>
>   		lrc->ring_begin = gfx_addr;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 9a55478e23ac..4b7c9680b097 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -300,7 +300,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
>    * 					  descriptor for a pinned context
>    *
>    * @ctx: Context to work on
> - * @ring: Engine the descriptor will be used with
> + * @engine: Engine the descriptor will be used with
>    *
>    * The context descriptor encodes various attributes of a context,
>    * including its GTT address and some flags. Because it's fairly
> @@ -318,16 +318,17 @@ static void
>   intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
>   				   struct intel_engine_cs *engine)
>   {
> +	struct intel_context *ce = &ctx->engine[engine->id];
>   	u64 desc;
>
>   	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
>
>   	desc = engine->ctx_desc_template;			/* bits  0-11 */
> -	desc |= ctx->engine[engine->id].lrc_vma->node.start +	/* bits 12-31 */
> -	       LRC_PPHWSP_PN * PAGE_SIZE;
> +	desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
> +								/* bits 12-31 */
>   	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
>
> -	ctx->engine[engine->id].lrc_desc = desc;
> +	ce->lrc_desc = desc;
>   }
>
>   uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
> @@ -674,6 +675,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
>   int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>   {
>   	struct intel_engine_cs *engine = request->engine;
> +	struct intel_context *ce = &request->ctx->engine[engine->id];
>   	int ret;
>
>   	/* Flush enough space to reduce the likelihood of waiting after
> @@ -682,13 +684,13 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>   	 */
>   	request->reserved_space += EXECLISTS_REQUEST_SIZE;
>
> -	if (request->ctx->engine[engine->id].state == NULL) {
> +	if (!ce->state) {
>   		ret = execlists_context_deferred_alloc(request->ctx, engine);
>   		if (ret)
>   			return ret;
>   	}
>
> -	request->ringbuf = request->ctx->engine[engine->id].ringbuf;
> +	request->ringbuf = ce->ringbuf;
>
>   	if (i915.enable_guc_submission) {
>   		/*
> @@ -711,12 +713,12 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>   	if (ret)
>   		goto err_unpin;
>
> -	if (!request->ctx->engine[engine->id].initialised) {
> +	if (!ce->initialised) {
>   		ret = engine->init_context(request);
>   		if (ret)
>   			goto err_unpin;
>
> -		request->ctx->engine[engine->id].initialised = true;
> +		ce->initialised = true;
>   	}
>
>   	/* Note that after this point, we have committed to using
> @@ -936,24 +938,22 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
>   				struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = ctx->i915;
> -	struct drm_i915_gem_object *ctx_obj;
> -	struct intel_ringbuffer *ringbuf;
> +	struct intel_context *ce = &ctx->engine[engine->id];
>   	void *vaddr;
>   	u32 *lrc_reg_state;
>   	int ret;
>
>   	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
>
> -	if (ctx->engine[engine->id].pin_count++)
> +	if (ce->pin_count++)
>   		return 0;
>
> -	ctx_obj = ctx->engine[engine->id].state;
> -	ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
> -			PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> +	ret = i915_gem_obj_ggtt_pin(ce->state, GEN8_LR_CONTEXT_ALIGN,
> +				    PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
>   	if (ret)
>   		goto err;
>
> -	vaddr = i915_gem_object_pin_map(ctx_obj);
> +	vaddr = i915_gem_object_pin_map(ce->state);
>   	if (IS_ERR(vaddr)) {
>   		ret = PTR_ERR(vaddr);
>   		goto unpin_ctx_obj;
> @@ -961,16 +961,16 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
>
>   	lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
>
> -	ringbuf = ctx->engine[engine->id].ringbuf;
> -	ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ringbuf);
> +	ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ce->ringbuf);
>   	if (ret)
>   		goto unpin_map;
>
> -	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
> +	ce->lrc_vma = i915_gem_obj_to_ggtt(ce->state);
>   	intel_lr_context_descriptor_update(ctx, engine);
> -	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
> -	ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
> -	ctx_obj->dirty = true;
> +
> +	lrc_reg_state[CTX_RING_BUFFER_START+1] = ce->ringbuf->vma->node.start;
> +	ce->lrc_reg_state = lrc_reg_state;
> +	ce->state->dirty = true;
>
>   	/* Invalidate GuC TLB. */
>   	if (i915.enable_guc_submission)
> @@ -980,34 +980,33 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
>   	return 0;
>
>   unpin_map:
> -	i915_gem_object_unpin_map(ctx_obj);
> +	i915_gem_object_unpin_map(ce->state);
>   unpin_ctx_obj:
> -	i915_gem_object_ggtt_unpin(ctx_obj);
> +	i915_gem_object_ggtt_unpin(ce->state);
>   err:
> -	ctx->engine[engine->id].pin_count = 0;
> +	ce->pin_count = 0;
>   	return ret;
>   }
>
>   void intel_lr_context_unpin(struct i915_gem_context *ctx,
>   			    struct intel_engine_cs *engine)
>   {
> -	struct drm_i915_gem_object *ctx_obj;
> +	struct intel_context *ce = &ctx->engine[engine->id];
>
>   	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
> -	GEM_BUG_ON(ctx->engine[engine->id].pin_count == 0);
> +	GEM_BUG_ON(ce->pin_count == 0);
>
> -	if (--ctx->engine[engine->id].pin_count)
> +	if (--ce->pin_count)
>   		return;
>
> -	intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
> +	intel_unpin_ringbuffer_obj(ce->ringbuf);
>
> -	ctx_obj = ctx->engine[engine->id].state;
> -	i915_gem_object_unpin_map(ctx_obj);
> -	i915_gem_object_ggtt_unpin(ctx_obj);
> +	i915_gem_object_unpin_map(ce->state);
> +	i915_gem_object_ggtt_unpin(ce->state);
>
> -	ctx->engine[engine->id].lrc_vma = NULL;
> -	ctx->engine[engine->id].lrc_desc = 0;
> -	ctx->engine[engine->id].lrc_reg_state = NULL;
> +	ce->lrc_vma = NULL;
> +	ce->lrc_desc = 0;
> +	ce->lrc_reg_state = NULL;
>
>   	i915_gem_context_put(ctx);
>   }
> @@ -2493,12 +2492,13 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   					    struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_gem_object *ctx_obj;
> +	struct intel_context *ce = &ctx->engine[engine->id];
>   	uint32_t context_size;
>   	struct intel_ringbuffer *ringbuf;
>   	int ret;
>
>   	WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
> -	WARN_ON(ctx->engine[engine->id].state);
> +	WARN_ON(ce->state);
>
>   	context_size = round_up(intel_lr_context_size(engine), 4096);
>
> @@ -2523,9 +2523,9 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   		goto error_ringbuf;
>   	}
>
> -	ctx->engine[engine->id].ringbuf = ringbuf;
> -	ctx->engine[engine->id].state = ctx_obj;
> -	ctx->engine[engine->id].initialised = engine->init_context == NULL;
> +	ce->ringbuf = ringbuf;
> +	ce->state = ctx_obj;
> +	ce->initialised = engine->init_context == NULL;
>
>   	return 0;
>
> @@ -2533,8 +2533,8 @@ error_ringbuf:
>   	intel_ringbuffer_free(ringbuf);
>   error_deref_obj:
>   	drm_gem_object_unreference(&ctx_obj->base);
> -	ctx->engine[engine->id].ringbuf = NULL;
> -	ctx->engine[engine->id].state = NULL;
> +	ce->ringbuf = NULL;
> +	ce->state = NULL;
>   	return ret;
>   }
>
> @@ -2544,10 +2544,8 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
>   	struct intel_engine_cs *engine;
>
>   	for_each_engine(engine, dev_priv) {
> -		struct drm_i915_gem_object *ctx_obj =
> -				ctx->engine[engine->id].state;
> -		struct intel_ringbuffer *ringbuf =
> -				ctx->engine[engine->id].ringbuf;
> +		struct intel_context *ce = &ctx->engine[engine->id];
> +		struct drm_i915_gem_object *ctx_obj = ce->state;
>   		void *vaddr;
>   		uint32_t *reg_state;
>
> @@ -2566,7 +2564,7 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
>
>   		i915_gem_object_unpin_map(ctx_obj);
>
> -		ringbuf->head = 0;
> -		ringbuf->tail = 0;
> +		ce->ringbuf->head = 0;
> +		ce->ringbuf->tail = 0;
>   	}
>   }
>

Looks OK anyway.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator
  2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-23  9:33   ` Tvrtko Ursulin
  2016-05-23  9:45     ` Chris Wilson
  0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:33 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> Rather than have every context ask "am I owned by the kernel? pin!",
> move that logic into the creator of the kernel context, in order to
> improve code comprehension.

Makes sense.

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_context.c | 52 +++++++++++++++------------------
>   1 file changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a4c6377eea32..51e83a79ab36 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -303,9 +303,7 @@ static struct i915_gem_context *
>   i915_gem_create_context(struct drm_device *dev,
>   			struct drm_i915_file_private *file_priv)
>   {
> -	const bool is_global_default_ctx = file_priv == NULL;
>   	struct i915_gem_context *ctx;
> -	int ret = 0;
>
>   	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
>
> @@ -313,30 +311,14 @@ i915_gem_create_context(struct drm_device *dev,
>   	if (IS_ERR(ctx))
>   		return ctx;
>
> -	if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
> -		/* We may need to do things with the shrinker which
> -		 * require us to immediately switch back to the default
> -		 * context. This can cause a problem as pinning the
> -		 * default context also requires GTT space which may not
> -		 * be available. To avoid this we always pin the default
> -		 * context.
> -		 */
> -		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> -					    get_context_alignment(to_i915(dev)), 0);
> -		if (ret) {
> -			DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
> -			goto err_destroy;
> -		}
> -	}
> -
>   	if (USES_FULL_PPGTT(dev)) {
>   		struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
>
> -		if (IS_ERR_OR_NULL(ppgtt)) {
> +		if (IS_ERR(ppgtt)) {
>   			DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
>   					 PTR_ERR(ppgtt));
> -			ret = PTR_ERR(ppgtt);
> -			goto err_unpin;
> +			i915_gem_context_put(ctx);
> +			return ERR_CAST(ppgtt);
>   		}
>
>   		ctx->ppgtt = ppgtt;
> @@ -345,14 +327,6 @@ i915_gem_create_context(struct drm_device *dev,
>   	trace_i915_context_create(ctx);
>
>   	return ctx;
> -
> -err_unpin:
> -	if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
> -		i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> -err_destroy:
> -	idr_remove(&file_priv->context_idr, ctx->user_handle);

Isn't idr_remove still required in the error path above?

> -	i915_gem_context_put(ctx);
> -	return ERR_PTR(ret);
>   }
>
>   static void i915_gem_context_unpin(struct i915_gem_context *ctx,
> @@ -426,6 +400,26 @@ int i915_gem_context_init(struct drm_device *dev)
>   		return PTR_ERR(ctx);
>   	}
>
> +	if (ctx->legacy_hw_ctx.rcs_state) {
> +		int ret;
> +
> +		/* We may need to do things with the shrinker which
> +		 * require us to immediately switch back to the default
> +		 * context. This can cause a problem as pinning the
> +		 * default context also requires GTT space which may not
> +		 * be available. To avoid this we always pin the default
> +		 * context.
> +		 */
> +		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> +					    get_context_alignment(dev_priv), 0);
> +		if (ret) {
> +			DRM_ERROR("Failed to pinned default global context (error %d)\n",
> +				  ret);
> +			i915_gem_context_put(ctx);
> +			return ret;
> +		}
> +	}
> +
>   	dev_priv->kernel_context = ctx;
>
>   	DRM_DEBUG_DRIVER("%s context support initialized\n",
>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs
  2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-23  9:42   ` Tvrtko Ursulin
  2016-05-23  9:52     ` Chris Wilson
  0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:42 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx



On 22/05/16 14:02, Chris Wilson wrote:
> Print the context's owner (via the pid under file_priv) under debugfs.
>
> Note that since this was originally introducing
> dev_priv->kernel_context, there are a couple of leftover minor chunks.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ae28e6e9d603..945fe4710b37 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   			continue;
>
>   		seq_printf(m, "HW context %u ", ctx->hw_id);
> +		if (IS_ERR(ctx->file_priv)) {
> +			seq_puts(m, "(deleted) ");
> +		} else if (ctx->file_priv) {
> +			struct pid *pid = ctx->file_priv->file->pid;

Hm, can you deref file_priv after the file has been closed / client 
exited? We could still have the context on the list after that point..

> +			struct task_struct *task;
> +
> +			task = get_pid_task(pid, PIDTYPE_PID);
> +			if (task) {
> +				seq_printf(m, "(%s [%d]) ",
> +					   task->comm, task->pid);
> +				put_task_struct(task);
> +			}
> +		} else
> +			seq_puts(m, "(kernel) ");
> +
>   		describe_ctx(m, ctx);
> -		if (ctx == dev_priv->kernel_context)
> -			seq_printf(m, "(kernel context) ");
>
>   		if (i915.enable_execlists) {
>   			seq_putc(m, '\n');
>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends
  2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
@ 2016-05-23  9:45   ` Tvrtko Ursulin
  0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23  9:45 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> Just move the kernel_context memboer of drm_i915_private next to the
> engines it is associated with.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h            | 3 +--
>   drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e6735dc9eeb2..d3d2538d17e8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1760,6 +1760,7 @@ struct drm_i915_private {
>   	wait_queue_head_t gmbus_wait_queue;
>
>   	struct pci_dev *bridge_dev;
> +	struct i915_gem_context *kernel_context;
>   	struct intel_engine_cs engine[I915_NUM_ENGINES];
>   	struct drm_i915_gem_object *semaphore_obj;
>   	uint32_t last_seqno, next_seqno;
> @@ -2013,8 +2014,6 @@ struct drm_i915_private {
>   		void (*stop_engine)(struct intel_engine_cs *engine);
>   	} gt;
>
> -	struct i915_gem_context *kernel_context;
> -
>   	/* perform PHY state sanity checks? */
>   	bool chv_phy_assert[2];

OK bit.

>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index c29c1d19764f..78b70526c197 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -912,11 +912,12 @@ int i915_guc_submission_enable(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
> -	struct i915_gem_context *ctx = dev_priv->kernel_context;
>   	struct i915_guc_client *client;
>
>   	/* client for execbuf submission */
> -	client = guc_client_alloc(dev, GUC_CTX_PRIORITY_KMD_NORMAL, ctx);
> +	client = guc_client_alloc(dev,
> +				  GUC_CTX_PRIORITY_KMD_NORMAL,
> +				  dev_priv->kernel_context);
>   	if (!client) {
>   		DRM_ERROR("Failed to create execbuf guc_client\n");
>   		return -ENOMEM;
>

Churn bit. :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator
  2016-05-23  9:33   ` Tvrtko Ursulin
@ 2016-05-23  9:45     ` Chris Wilson
  0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23  9:45 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 10:33:24AM +0100, Tvrtko Ursulin wrote:
> >@@ -313,30 +311,14 @@ i915_gem_create_context(struct drm_device *dev,
> >  	if (IS_ERR(ctx))
> >  		return ctx;
> >
> >-	if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
> >-		/* We may need to do things with the shrinker which
> >-		 * require us to immediately switch back to the default
> >-		 * context. This can cause a problem as pinning the
> >-		 * default context also requires GTT space which may not
> >-		 * be available. To avoid this we always pin the default
> >-		 * context.
> >-		 */
> >-		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> >-					    get_context_alignment(to_i915(dev)), 0);
> >-		if (ret) {
> >-			DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
> >-			goto err_destroy;
> >-		}
> >-	}
> >-
> >  	if (USES_FULL_PPGTT(dev)) {
> >  		struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
> >
> >-		if (IS_ERR_OR_NULL(ppgtt)) {
> >+		if (IS_ERR(ppgtt)) {
> >  			DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> >  					 PTR_ERR(ppgtt));
> >-			ret = PTR_ERR(ppgtt);
> >-			goto err_unpin;
> >+			i915_gem_context_put(ctx);
> >+			return ERR_CAST(ppgtt);
> >  		}
> >
> >  		ctx->ppgtt = ppgtt;
> >@@ -345,14 +327,6 @@ i915_gem_create_context(struct drm_device *dev,
> >  	trace_i915_context_create(ctx);
> >
> >  	return ctx;
> >-
> >-err_unpin:
> >-	if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
> >-		i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> >-err_destroy:
> >-	idr_remove(&file_priv->context_idr, ctx->user_handle);
> 
> Isn't idr_remove still required in the error path above?

Yes. I can blame a rebase error here since in the kernel it was
extracted upon we call context_close() here instead which does the idr
removal as well.
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs
  2016-05-23  9:42   ` Tvrtko Ursulin
@ 2016-05-23  9:52     ` Chris Wilson
  0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23  9:52 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 10:42:42AM +0100, Tvrtko Ursulin wrote:
> 
> 
> On 22/05/16 14:02, Chris Wilson wrote:
> >Print the context's owner (via the pid under file_priv) under debugfs.
> >
> >Note that since this was originally introducing
> >dev_priv->kernel_context, there are a couple of leftover minor chunks.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index ae28e6e9d603..945fe4710b37 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
> >  			continue;
> >
> >  		seq_printf(m, "HW context %u ", ctx->hw_id);
> >+		if (IS_ERR(ctx->file_priv)) {
> >+			seq_puts(m, "(deleted) ");
> >+		} else if (ctx->file_priv) {
> >+			struct pid *pid = ctx->file_priv->file->pid;
> 
> Hm, can you deref file_priv after the file has been closed / client
> exited? We could still have the context on the list after that
> point..

Hmm, this was extracted from after the context_close patch because I
didn't think we had that bug... We do.

That's easy enough to mark as closed when the file is, which is what we
need later anyway.
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
  2016-05-23  9:26   ` Tvrtko Ursulin
@ 2016-05-23 10:17     ` Chris Wilson
  2016-05-23 10:55       ` Tvrtko Ursulin
  0 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 10:17 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote:
> >@@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> >  		 * for now who owns a GuC client. But for future owner of GuC
> >  		 * client, need to make sure lrc is pinned prior to enter here.
> >  		 */
> >-		obj = ctx->engine[id].state;
> >-		if (!obj)
> >+		if (!ce->state)
> >  			break;	/* XXX: continue? */
> >
> >-		ctx_desc = intel_lr_context_descriptor(ctx, engine);
> >-		lrc->context_desc = (u32)ctx_desc;
> >+		lrc->context_desc = lower_32_bits(ce->lrc_desc);
> 
> Could have kept use of intel_lr_context_descriptor for better separation.

I was leaning the other way, since the code doesn't want
intel_lr_context_descriptor() just happens to want to reuse some of the
bits e.g. engine->ctx_desc_template | lrca

i.e. lrc->context_desc != ce->lrc_desc and I would prefer it to be
clarified as to exactly what the GuC expects.
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 10/12] drm/i915: Merge legacy+execlists context structs
  2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
@ 2016-05-23 10:26   ` Tvrtko Ursulin
  2016-05-23 10:40     ` Chris Wilson
  0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 10:26 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> struct intel_context contains two substructs, one for the legacy RCS and
> one for every execlists engine. Since legacy RCS is a subset of the
> execlists engine support, just combine the two substructs.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c     | 41 +++++-------------
>   drivers/gpu/drm/i915/i915_drv.h         |  7 ---
>   drivers/gpu/drm/i915/i915_gem_context.c | 75 +++++++++++++++++++--------------
>   drivers/gpu/drm/i915/intel_lrc.c        | 26 ------------
>   drivers/gpu/drm/i915/intel_lrc.h        |  1 -
>   5 files changed, 55 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 945fe4710b37..30cb26fe2fa9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -199,13 +199,6 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>   		seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
>   }
>
> -static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
> -{
> -	seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
> -	seq_putc(m, ctx->remap_slice ? 'R' : 'r');
> -	seq_putc(m, ' ');
> -}
> -
>   static int i915_gem_object_list_info(struct seq_file *m, void *data)
>   {
>   	struct drm_info_node *node = m->private;
> @@ -2001,7 +1994,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_engine_cs *engine;
>   	struct i915_gem_context *ctx;
> -	enum intel_engine_id id;
>   	int ret;
>
>   	ret = mutex_lock_interruptible(&dev->struct_mutex);
> @@ -2009,10 +2001,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   		return ret;
>
>   	list_for_each_entry(ctx, &dev_priv->context_list, link) {
> -		if (!i915.enable_execlists &&
> -		    ctx->legacy_hw_ctx.rcs_state == NULL)
> -			continue;
> -
>   		seq_printf(m, "HW context %u ", ctx->hw_id);
>   		if (IS_ERR(ctx->file_priv)) {
>   			seq_puts(m, "(deleted) ");
> @@ -2029,25 +2017,18 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   		} else
>   			seq_puts(m, "(kernel) ");
>
> -		describe_ctx(m, ctx);
> +		seq_putc(m, ctx->remap_slice ? 'R' : 'r');
> +		seq_putc(m, '\n');
>
> -		if (i915.enable_execlists) {
> +		for_each_engine(engine, dev_priv) {
> +			struct intel_context *ce = &ctx->engine[engine->id];
> +			seq_printf(m, "%s: ", engine->name);
> +			seq_putc(m, ce->initialised ? 'I' : 'i');
> +			if (ce->state)
> +				describe_obj(m, ce->state);
> +			if (ce->ringbuf)
> +				describe_ctx_ringbuf(m, ce->ringbuf);
>   			seq_putc(m, '\n');
> -			for_each_engine_id(engine, dev_priv, id) {
> -				struct drm_i915_gem_object *ctx_obj =
> -					ctx->engine[id].state;
> -				struct intel_ringbuffer *ringbuf =
> -					ctx->engine[id].ringbuf;
> -
> -				seq_printf(m, "%s: ", engine->name);
> -				if (ctx_obj)
> -					describe_obj(m, ctx_obj);
> -				if (ringbuf)
> -					describe_ctx_ringbuf(m, ringbuf);
> -				seq_putc(m, '\n');
> -			}
> -		} else {
> -			describe_obj(m, ctx->legacy_hw_ctx.rcs_state);
>   		}
>
>   		seq_putc(m, '\n');
> @@ -2062,10 +2043,10 @@ static void i915_dump_lrc_obj(struct seq_file *m,
>   			      struct i915_gem_context *ctx,
>   			      struct intel_engine_cs *engine)
>   {
> +	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
>   	struct page *page;
>   	uint32_t *reg_state;
>   	int j;
> -	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
>   	unsigned long ggtt_offset = 0;
>
>   	seq_printf(m, "CONTEXT: %s %u\n", engine->name, ctx->hw_id);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d3d2538d17e8..259a0ee7a601 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -862,13 +862,6 @@ struct i915_gem_context {
>   	/* Unique identifier for this context, used by the hw for tracking */
>   	unsigned hw_id;
>
> -	/* Legacy ring buffer submission */
> -	struct {
> -		struct drm_i915_gem_object *rcs_state;
> -		bool initialized;
> -	} legacy_hw_ctx;
> -
> -	/* Execlists */
>   	struct intel_context {
>   		struct drm_i915_gem_object *state;
>   		struct intel_ringbuffer *ringbuf;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 51e83a79ab36..9847235997b9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -152,13 +152,11 @@ static void i915_gem_context_clean(struct i915_gem_context *ctx)
>   void i915_gem_context_free(struct kref *ctx_ref)
>   {
>   	struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
> +	int i;
>
>   	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
>   	trace_i915_context_free(ctx);
>
> -	if (i915.enable_execlists)
> -		intel_lr_context_free(ctx);
> -
>   	/*
>   	 * This context is going away and we need to remove all VMAs still
>   	 * around. This is to handle imported shared objects for which
> @@ -168,8 +166,19 @@ void i915_gem_context_free(struct kref *ctx_ref)
>
>   	i915_ppgtt_put(ctx->ppgtt);
>
> -	if (ctx->legacy_hw_ctx.rcs_state)
> -		drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
> +	for (i = 0; i < I915_NUM_ENGINES; i++) {
> +		struct intel_context *ce = &ctx->engine[i];
> +
> +		if (!ce->state)
> +			continue;
> +
> +		WARN_ON(ce->pin_count);
> +		if (ce->ringbuf)
> +			intel_ringbuffer_free(ce->ringbuf);
> +
> +		drm_gem_object_unreference(&ce->state->base);
> +	}
> +
>   	list_del(&ctx->link);
>
>   	ida_simple_remove(&ctx->i915->context_hw_ida, ctx->hw_id);
> @@ -266,7 +275,7 @@ __create_hw_context(struct drm_device *dev,
>   			ret = PTR_ERR(obj);
>   			goto err_out;
>   		}
> -		ctx->legacy_hw_ctx.rcs_state = obj;
> +		ctx->engine[RCS].state = obj;
>   	}
>
>   	/* Default context will never have a file_priv */
> @@ -335,8 +344,11 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
>   	if (i915.enable_execlists) {
>   		intel_lr_context_unpin(ctx, engine);
>   	} else {
> -		if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
> -			i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> +		struct intel_context *ce = &ctx->engine[engine->id];
> +
> +		if (ce->state)
> +			i915_gem_object_ggtt_unpin(ce->state);
> +
>   		i915_gem_context_put(ctx);
>   	}
>   }
> @@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
>   		return PTR_ERR(ctx);
>   	}
>
> -	if (ctx->legacy_hw_ctx.rcs_state) {
> +	if (ctx->engine[RCS].state) {

Maybe now put a comment saying this is for legacy now, to make the 
asymmetry in ctx pinning paths between the two stick out more.

>   		int ret;
>
>   		/* We may need to do things with the shrinker which
> @@ -410,7 +422,7 @@ int i915_gem_context_init(struct drm_device *dev)
>   		 * be available. To avoid this we always pin the default
>   		 * context.
>   		 */
> -		ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> +		ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
>   					    get_context_alignment(dev_priv), 0);
>   		if (ret) {
>   			DRM_ERROR("Failed to pinned default global context (error %d)\n",
> @@ -435,15 +447,17 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
>   	lockdep_assert_held(&dev_priv->dev->struct_mutex);
>
>   	for_each_engine(engine, dev_priv) {
> -		if (engine->last_context == NULL)
> -			continue;
> +		if (engine->last_context) {
> +			i915_gem_context_unpin(engine->last_context, engine);
> +			engine->last_context = NULL;
> +		}
>
> -		i915_gem_context_unpin(engine->last_context, engine);
> -		engine->last_context = NULL;
> +		/* Force the GPU state to be reinitialised on enabling */
> +		dev_priv->kernel_context->engine[engine->id].initialised =
> +			engine->init_context == NULL;
>   	}
>
>   	/* Force the GPU state to be reinitialised on enabling */
> -	dev_priv->kernel_context->legacy_hw_ctx.initialized = false;
>   	dev_priv->kernel_context->remap_slice = ALL_L3_SLICES(dev_priv);
>   }
>
> @@ -454,8 +468,8 @@ void i915_gem_context_fini(struct drm_device *dev)
>
>   	lockdep_assert_held(&dev->struct_mutex);
>
> -	if (dctx->legacy_hw_ctx.rcs_state)
> -		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
> +	if (!i915.enable_execlists && dctx->engine[RCS].state)
> +		i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
>
>   	i915_gem_context_put(dctx);
>   	dev_priv->kernel_context = NULL;
> @@ -562,7 +576,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
>   	intel_ring_emit(engine, MI_NOOP);
>   	intel_ring_emit(engine, MI_SET_CONTEXT);
>   	intel_ring_emit(engine,
> -			i915_gem_obj_ggtt_offset(req->ctx->legacy_hw_ctx.rcs_state) |
> +			i915_gem_obj_ggtt_offset(req->ctx->engine[RCS].state) |
>   			flags);
>   	/*
>   	 * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
> @@ -639,7 +653,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
>   	if (to->remap_slice)
>   		return false;
>
> -	if (!to->legacy_hw_ctx.initialized)
> +	if (!to->engine[RCS].initialised)
>   		return false;
>
>   	if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
> @@ -704,7 +718,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   		return 0;
>
>   	/* Trying to pin first makes error handling easier. */
> -	ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
> +	ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
>   				    get_context_alignment(engine->i915),
>   				    0);
>   	if (ret)
> @@ -727,7 +741,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   	 *
>   	 * XXX: We need a real interface to do this instead of trickery.
>   	 */
> -	ret = i915_gem_object_set_to_gtt_domain(to->legacy_hw_ctx.rcs_state, false);
> +	ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state, false);
>   	if (ret)
>   		goto unpin_out;
>
> @@ -742,7 +756,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   			goto unpin_out;
>   	}
>
> -	if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
> +	if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
>   		/* NB: If we inhibit the restore, the context is not allowed to
>   		 * die because future work may end up depending on valid address
>   		 * space. This means we must enforce that a page table load
> @@ -766,8 +780,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   	 * MI_SET_CONTEXT instead of when the next seqno has completed.
>   	 */
>   	if (from != NULL) {
> -		from->legacy_hw_ctx.rcs_state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
> -		i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state), req);
> +		from->engine[RCS].state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
> +		i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->engine[RCS].state), req);
>   		/* 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
> @@ -775,10 +789,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   		 * able to defer doing this until we know the object would be
>   		 * swapped, but there is no way to do that yet.
>   		 */
> -		from->legacy_hw_ctx.rcs_state->dirty = 1;
> +		from->engine[RCS].state->dirty = 1;
>
>   		/* obj is kept alive until the next request by its active ref */
> -		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
> +		i915_gem_object_ggtt_unpin(from->engine[RCS].state);
>   		i915_gem_context_put(from);
>   	}
>   	engine->last_context = i915_gem_context_get(to);
> @@ -812,19 +826,19 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   		to->remap_slice &= ~(1<<i);
>   	}
>
> -	if (!to->legacy_hw_ctx.initialized) {
> +	if (!to->engine[RCS].initialised) {
>   		if (engine->init_context) {
>   			ret = engine->init_context(req);
>   			if (ret)
>   				return ret;
>   		}
> -		to->legacy_hw_ctx.initialized = true;
> +		to->engine[RCS].initialised = true;
>   	}
>
>   	return 0;
>
>   unpin_out:
> -	i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
> +	i915_gem_object_ggtt_unpin(to->engine[RCS].state);
>   	return ret;
>   }
>
> @@ -848,8 +862,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
>   	WARN_ON(i915.enable_execlists);
>   	lockdep_assert_held(&req->i915->dev->struct_mutex);
>
> -	if (engine->id != RCS ||
> -	    req->ctx->legacy_hw_ctx.rcs_state == NULL) {
> +	if (!req->ctx->engine[engine->id].state) {
>   		struct i915_gem_context *to = req->ctx;
>   		struct i915_hw_ppgtt *ppgtt =
>   			to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4b7c9680b097..558f069cd6d8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2413,31 +2413,6 @@ populate_lr_context(struct i915_gem_context *ctx,
>   }
>
>   /**
> - * intel_lr_context_free() - free the LRC specific bits of a context
> - * @ctx: the LR context to free.
> - *
> - * The real context freeing is done in i915_gem_context_free: this only
> - * takes care of the bits that are LRC related: the per-engine backing
> - * objects and the logical ringbuffer.
> - */
> -void intel_lr_context_free(struct i915_gem_context *ctx)
> -{
> -	int i;
> -
> -	for (i = I915_NUM_ENGINES; --i >= 0; ) {
> -		struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
> -		struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
> -
> -		if (!ctx_obj)
> -			continue;
> -
> -		WARN_ON(ctx->engine[i].pin_count);
> -		intel_ringbuffer_free(ringbuf);
> -		drm_gem_object_unreference(&ctx_obj->base);
> -	}
> -}
> -
> -/**
>    * intel_lr_context_size() - return the size of the context for an engine
>    * @ring: which engine to find the context size for
>    *
> @@ -2497,7 +2472,6 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   	struct intel_ringbuffer *ringbuf;
>   	int ret;
>
> -	WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
>   	WARN_ON(ce->state);
>
>   	context_size = round_up(intel_lr_context_size(engine), 4096);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index eb2e1d157fed..a8db42a9c50f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -101,7 +101,6 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
>
>   struct i915_gem_context;
>
> -void intel_lr_context_free(struct i915_gem_context *ctx);
>   uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
>   void intel_lr_context_unpin(struct i915_gem_context *ctx,
>   			    struct intel_engine_cs *engine);
>

Looks good to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 11/12] drm/i915: Rearrange i915_gem_context
  2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-23 10:27   ` Tvrtko Ursulin
  0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 10:27 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 22/05/16 14:02, Chris Wilson wrote:
> Pack the integers and related types together inside the struct.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 259a0ee7a601..3b5cf15d85ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -829,7 +829,6 @@ struct i915_ctx_hang_stats {
>   /* This must match up with the value previously used for execbuf2.rsvd1. */
>   #define DEFAULT_CONTEXT_HANDLE 0
>
> -#define CONTEXT_NO_ZEROMAP (1<<0)
>   /**
>    * struct i915_gem_context - as the name implies, represents a context.
>    * @ref: reference count.
> @@ -851,28 +850,31 @@ struct i915_ctx_hang_stats {
>    */
>   struct i915_gem_context {
>   	struct kref ref;
> -	int user_handle;
> -	uint8_t remap_slice;
>   	struct drm_i915_private *i915;
> -	int flags;
>   	struct drm_i915_file_private *file_priv;
> -	struct i915_ctx_hang_stats hang_stats;
>   	struct i915_hw_ppgtt *ppgtt;
>
> +	struct i915_ctx_hang_stats hang_stats;
> +
>   	/* Unique identifier for this context, used by the hw for tracking */
>   	unsigned hw_id;
> +	uint32_t user_handle;
> +	unsigned long flags;
> +#define CONTEXT_NO_ZEROMAP		(1<<0)
>
>   	struct intel_context {
>   		struct drm_i915_gem_object *state;
>   		struct intel_ringbuffer *ringbuf;
> -		int pin_count;
>   		struct i915_vma *lrc_vma;
> -		u64 lrc_desc;
>   		uint32_t *lrc_reg_state;
> +		u64 lrc_desc;
> +		int pin_count;
>   		bool initialised;
>   	} engine[I915_NUM_ENGINES];
>
>   	struct list_head link;
> +
> +	uint8_t remap_slice;
>   };
>
>   enum fb_op_origin {
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 10/12] drm/i915: Merge legacy+execlists context structs
  2016-05-23 10:26   ` Tvrtko Ursulin
@ 2016-05-23 10:40     ` Chris Wilson
  0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 10:40 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 11:26:13AM +0100, Tvrtko Ursulin wrote:
> >@@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
> >  		return PTR_ERR(ctx);
> >  	}
> >
> >-	if (ctx->legacy_hw_ctx.rcs_state) {
> >+	if (ctx->engine[RCS].state) {
> 
> Maybe now put a comment saying this is for legacy now, to make the
> asymmetry in ctx pinning paths between the two stick out more.

Good point. This needs the i915.enable_execlists markup. Ultimately, I
think we should move the pin/unpin into the legacy engine setup for
symmetry with execlists (and killing off one more pair of
i915.enable_execlists).
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
  2016-05-23 10:17     ` Chris Wilson
@ 2016-05-23 10:55       ` Tvrtko Ursulin
  2016-05-23 11:08         ` Chris Wilson
  0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 10:55 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 23/05/16 11:17, Chris Wilson wrote:
> On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote:
>>> @@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>>>   		 * for now who owns a GuC client. But for future owner of GuC
>>>   		 * client, need to make sure lrc is pinned prior to enter here.
>>>   		 */
>>> -		obj = ctx->engine[id].state;
>>> -		if (!obj)
>>> +		if (!ce->state)
>>>   			break;	/* XXX: continue? */
>>>
>>> -		ctx_desc = intel_lr_context_descriptor(ctx, engine);
>>> -		lrc->context_desc = (u32)ctx_desc;
>>> +		lrc->context_desc = lower_32_bits(ce->lrc_desc);
>>
>> Could have kept use of intel_lr_context_descriptor for better separation.
>
> I was leaning the other way, since the code doesn't want
> intel_lr_context_descriptor() just happens to want to reuse some of the
> bits e.g. engine->ctx_desc_template | lrca

Thats true, but it was at least using an exported function with 
documented content, rather than directly fishing out stuff from 
essentially private data elsewhere.

> i.e. lrc->context_desc != ce->lrc_desc and I would prefer it to be
> clarified as to exactly what the GuC expects.

Yes that would be best. Cc-ed Dave for when he is back to comment what 
does GuC really wants in there.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
  2016-05-23 10:55       ` Tvrtko Ursulin
@ 2016-05-23 11:08         ` Chris Wilson
  0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 11:08 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 11:55:16AM +0100, Tvrtko Ursulin wrote:
> 
> On 23/05/16 11:17, Chris Wilson wrote:
> >On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote:
> >>>@@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> >>>  		 * for now who owns a GuC client. But for future owner of GuC
> >>>  		 * client, need to make sure lrc is pinned prior to enter here.
> >>>  		 */
> >>>-		obj = ctx->engine[id].state;
> >>>-		if (!obj)
> >>>+		if (!ce->state)
> >>>  			break;	/* XXX: continue? */
> >>>
> >>>-		ctx_desc = intel_lr_context_descriptor(ctx, engine);
> >>>-		lrc->context_desc = (u32)ctx_desc;
> >>>+		lrc->context_desc = lower_32_bits(ce->lrc_desc);
> >>
> >>Could have kept use of intel_lr_context_descriptor for better separation.
> >
> >I was leaning the other way, since the code doesn't want
> >intel_lr_context_descriptor() just happens to want to reuse some of the
> >bits e.g. engine->ctx_desc_template | lrca
> 
> Thats true, but it was at least using an exported function with
> documented content, rather than directly fishing out stuff from
> essentially private data elsewhere.

It's still fishing out essentially private data though :)
If engine->ctx_desc_template considers GuC for its set of flags, it is
purely by happenstance.
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
  2016-05-23  9:17   ` Tvrtko Ursulin
  2016-05-23  9:25     ` Chris Wilson
@ 2016-05-24  8:10     ` Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2016-05-24  8:10 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, May 23, 2016 at 10:17:01AM +0100, Tvrtko Ursulin wrote:
> 
> On 22/05/16 14:02, Chris Wilson wrote:
> >As these are wrappers around kref_get/kref_put() it is preferable to
> >follow the naming convention and use the same verb get/put in our
> >wrapper names for manipulating a reference to the context.
> 
> Not so sure about this one. We got objects, framebuffers and requests using
> (un)reference naming so don't see that making contexts different is a good
> idea.

And drm_blob too. For core modeset stuff I think converting to _get/put
might be worth it, too, but needs 3-stage approach: Add new _get/put
funcs. 2) cocci-generated patches for each driver/file to convert over. 3)
drop _reference/unreference variants once no longer used.
-Daniel

> 
> Regards,
> 
> Tvrtko
> 
> 
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h            |  6 ++++--
> >  drivers/gpu/drm/i915/i915_gem.c            |  7 +++----
> >  drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++++++++------------
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  6 +++---
> >  drivers/gpu/drm/i915/intel_lrc.c           |  4 ++--
> >  5 files changed, 22 insertions(+), 23 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index e4e3614335ec..0a3442fcd93e 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -3435,12 +3435,14 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
> >  	return ctx;
> >  }
> >
> >-static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
> >+static inline struct i915_gem_context *
> >+i915_gem_context_get(struct i915_gem_context *ctx)
> >  {
> >  	kref_get(&ctx->ref);
> >+	return ctx;
> >  }
> >
> >-static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
> >+static inline void i915_gem_context_put(struct i915_gem_context *ctx)
> >  {
> >  	lockdep_assert_held(&ctx->i915->dev->struct_mutex);
> >  	kref_put(&ctx->ref, i915_gem_context_free);
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 728b66911840..d7ae030e5a0b 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -1418,7 +1418,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
> >  					       request->engine);
> >  	}
> >
> >-	i915_gem_context_unreference(request->ctx);
> >+	i915_gem_context_put(request->ctx);
> >  	i915_gem_request_unreference(request);
> >  }
> >
> >@@ -2775,8 +2775,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
> >  	req->i915 = dev_priv;
> >  	req->engine = engine;
> >  	req->reset_counter = reset_counter;
> >-	req->ctx  = ctx;
> >-	i915_gem_context_reference(req->ctx);
> >+	req->ctx = i915_gem_context_get(ctx);
> >
> >  	/*
> >  	 * Reserve space in the ring buffer for all the commands required to
> >@@ -2798,7 +2797,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
> >  	return 0;
> >
> >  err_ctx:
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  err:
> >  	kmem_cache_free(dev_priv->requests, req);
> >  	return ret;
> >diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> >index aa329175f73a..a4c6377eea32 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_context.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_context.c
> >@@ -290,7 +290,7 @@ __create_hw_context(struct drm_device *dev,
> >  	return ctx;
> >
> >  err_out:
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  	return ERR_PTR(ret);
> >  }
> >
> >@@ -351,7 +351,7 @@ err_unpin:
> >  		i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> >  err_destroy:
> >  	idr_remove(&file_priv->context_idr, ctx->user_handle);
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  	return ERR_PTR(ret);
> >  }
> >
> >@@ -363,7 +363,7 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
> >  	} else {
> >  		if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
> >  			i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> >-		i915_gem_context_unreference(ctx);
> >+		i915_gem_context_put(ctx);
> >  	}
> >  }
> >
> >@@ -463,7 +463,7 @@ void i915_gem_context_fini(struct drm_device *dev)
> >  	if (dctx->legacy_hw_ctx.rcs_state)
> >  		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
> >
> >-	i915_gem_context_unreference(dctx);
> >+	i915_gem_context_put(dctx);
> >  	dev_priv->kernel_context = NULL;
> >
> >  	ida_destroy(&dev_priv->context_hw_ida);
> >@@ -473,7 +473,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
> >  {
> >  	struct i915_gem_context *ctx = p;
> >
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  	return 0;
> >  }
> >
> >@@ -785,10 +785,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> >
> >  		/* obj is kept alive until the next request by its active ref */
> >  		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
> >-		i915_gem_context_unreference(from);
> >+		i915_gem_context_put(from);
> >  	}
> >-	i915_gem_context_reference(to);
> >-	engine->last_context = to;
> >+	engine->last_context = i915_gem_context_get(to);
> >
> >  	/* GEN8 does *not* require an explicit reload if the PDPs have been
> >  	 * setup, and we do not wish to move them.
> >@@ -873,10 +872,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
> >  		}
> >
> >  		if (to != engine->last_context) {
> >-			i915_gem_context_reference(to);
> >  			if (engine->last_context)
> >-				i915_gem_context_unreference(engine->last_context);
> >-			engine->last_context = to;
> >+				i915_gem_context_put(engine->last_context);
> >+			engine->last_context = i915_gem_context_get(to);
> >  		}
> >
> >  		return 0;
> >@@ -944,7 +942,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> >  	}
> >
> >  	idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  	mutex_unlock(&dev->struct_mutex);
> >
> >  	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
> >diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >index 84d990331abd..14628ce91273 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >@@ -1492,7 +1492,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> >  		goto pre_mutex_err;
> >  	}
> >
> >-	i915_gem_context_reference(ctx);
> >+	i915_gem_context_get(ctx);
> >
> >  	if (ctx->ppgtt)
> >  		vm = &ctx->ppgtt->base;
> >@@ -1503,7 +1503,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> >
> >  	eb = eb_create(args);
> >  	if (eb == NULL) {
> >-		i915_gem_context_unreference(ctx);
> >+		i915_gem_context_put(ctx);
> >  		mutex_unlock(&dev->struct_mutex);
> >  		ret = -ENOMEM;
> >  		goto pre_mutex_err;
> >@@ -1647,7 +1647,7 @@ err_batch_unpin:
> >
> >  err:
> >  	/* the request owns the ref now */
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  	eb_destroy(eb);
> >
> >  	mutex_unlock(&dev->struct_mutex);
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 426dc90aceed..9a55478e23ac 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -966,7 +966,6 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
> >  	if (ret)
> >  		goto unpin_map;
> >
> >-	i915_gem_context_reference(ctx);
> >  	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
> >  	intel_lr_context_descriptor_update(ctx, engine);
> >  	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
> >@@ -977,6 +976,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
> >  	if (i915.enable_guc_submission)
> >  		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> >
> >+	i915_gem_context_get(ctx);
> >  	return 0;
> >
> >  unpin_map:
> >@@ -1009,7 +1009,7 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
> >  	ctx->engine[engine->id].lrc_desc = 0;
> >  	ctx->engine[engine->id].lrc_reg_state = NULL;
> >
> >-	i915_gem_context_unreference(ctx);
> >+	i915_gem_context_put(ctx);
> >  }
> >
> >  static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects
  2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
@ 2016-05-24  8:13   ` Daniel Vetter
  2016-05-24  8:21     ` Chris Wilson
  0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2016-05-24  8:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Sun, May 22, 2016 at 02:02:32PM +0100, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 30cb26fe2fa9..3d14eb3215e1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
>  	print_file_stats(m, "[k]batch pool", stats);
>  }
>  
> +static int per_file_ctx_stats(int id, void *ptr, void *data)
> +{
> +	struct i915_gem_context *ctx = ptr;
> +	int n;
> +
> +	for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
> +		if (ctx->engine[n].state)
> +			per_file_stats(0, ctx->engine[n].state, data);
> +		if (ctx->engine[n].ringbuf)
> +			per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> +	}
> +
> +	return 0;
> +}
> +
> +static void print_context_stats(struct seq_file *m,
> +				struct drm_i915_private *dev_priv)
> +{
> +	struct file_stats stats;
> +	struct drm_file *file;
> +
> +	memset(&stats, 0, sizeof(stats));
> +
> +	if (dev_priv->kernel_context)
> +		per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> +
> +	list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {

dev->filelist has grown it's own lock now, dev->filelist_mutex. Should we
have a drm_for_each_file macro which includes a helpful
lockdep_assert_held?

-Daniel

> +		struct drm_i915_file_private *fpriv = file->driver_priv;
> +		idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
> +	}
> +
> +	print_file_stats(m, "[k]contexts", stats);
> +}
> +
>  #define count_vmas(list, member) do { \
>  	list_for_each_entry(vma, list, member) { \
>  		size += i915_gem_obj_total_ggtt_size(vma->obj); \
> @@ -521,6 +555,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>  
>  	seq_putc(m, '\n');
>  	print_batch_pool_stats(m, dev_priv);
> +	print_context_stats(m, dev_priv);
>  
>  	mutex_unlock(&dev->struct_mutex);
>  
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects
  2016-05-24  8:13   ` Daniel Vetter
@ 2016-05-24  8:21     ` Chris Wilson
  0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-24  8:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, May 24, 2016 at 10:13:24AM +0200, Daniel Vetter wrote:
> On Sun, May 22, 2016 at 02:02:32PM +0100, Chris Wilson wrote:
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 30cb26fe2fa9..3d14eb3215e1 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
> >  	print_file_stats(m, "[k]batch pool", stats);
> >  }
> >  
> > +static int per_file_ctx_stats(int id, void *ptr, void *data)
> > +{
> > +	struct i915_gem_context *ctx = ptr;
> > +	int n;
> > +
> > +	for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
> > +		if (ctx->engine[n].state)
> > +			per_file_stats(0, ctx->engine[n].state, data);
> > +		if (ctx->engine[n].ringbuf)
> > +			per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void print_context_stats(struct seq_file *m,
> > +				struct drm_i915_private *dev_priv)
> > +{
> > +	struct file_stats stats;
> > +	struct drm_file *file;
> > +
> > +	memset(&stats, 0, sizeof(stats));
> > +
> > +	if (dev_priv->kernel_context)
> > +		per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> > +
> > +	list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
> 
> dev->filelist has grown it's own lock now, dev->filelist_mutex. Should we
> have a drm_for_each_file macro which includes a helpful
> lockdep_assert_held?

I have a list_for_each_entry_check() in my tree, which allows you to
annotate which lock you are expected to be holding. (Because I wanted to
uplift the drm utilities.)

But iterating over filelist is not something I'd like to encourage.
	drm_debug_for_each_file()
-Chris

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2016-05-24  8:21 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
2016-05-23  9:09   ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
2016-05-23  9:11   ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
2016-05-23  9:15   ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
2016-05-23  9:17   ` Tvrtko Ursulin
2016-05-23  9:25     ` Chris Wilson
2016-05-24  8:10     ` Daniel Vetter
2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
2016-05-23  9:26   ` Tvrtko Ursulin
2016-05-23 10:17     ` Chris Wilson
2016-05-23 10:55       ` Tvrtko Ursulin
2016-05-23 11:08         ` Chris Wilson
2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
2016-05-23  9:33   ` Tvrtko Ursulin
2016-05-23  9:45     ` Chris Wilson
2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
2016-05-23  9:42   ` Tvrtko Ursulin
2016-05-23  9:52     ` Chris Wilson
2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
2016-05-23  9:45   ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
2016-05-23 10:26   ` Tvrtko Ursulin
2016-05-23 10:40     ` Chris Wilson
2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
2016-05-23 10:27   ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
2016-05-24  8:13   ` Daniel Vetter
2016-05-24  8:21     ` Chris Wilson
2016-05-22 13:33 ` ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.