[PATCH 0/4] drm/i915: Untangle execlist tracking

[PATCH 0/4] drm/i915: Untangle execlist tracking

[Nick Hoath]

This patchset merges execlist queue items in to gem requests. It does this by 
using the reference count added by the "Replace seqno values with
request structures" patchset to ensure that the gem request is available for
the whole execlist submission lifespan.

v2: merge intel_ctx_submit_request and drm_i915_gem_request, rebase changes &
   add cover letter

v3: Rebase over upstreamed "Replace seqno values with request structures" and
add overzealous freeing fix.

Issue: VIZ-4274

Nick Hoath (4):
  drm/i915: execlist request keeps ptr/ref to gem_request
  drm/i915: Removed duplicate members from submit_request
  drm/i915: Remove FIXME_lrc_ctx backpointer
  drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

 drivers/gpu/drm/i915/i915_debugfs.c     |   4 +-
 drivers/gpu/drm/i915/i915_drv.h         |  24 +++++++
 drivers/gpu/drm/i915/i915_gem.c         |  11 ++-
 drivers/gpu/drm/i915/intel_lrc.c        | 123 +++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_lrc.h        |  18 +++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  15 ++--
 6 files changed, 122 insertions(+), 73 deletions(-)

-- 
2.1.1

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

[PATCH 1/4] drm/i915: execlist request keeps ptr/ref to gem_request

[Nick Hoath]

Add a reference and pointer from the execlist queue item to the associated
gem request. For execlist requests that don't have a request, create one
as a placeholder.

Issue: VIZ-4274
v1: Rebase after upstream of "Replace seqno values with request structures" patchset.
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 31 +++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_lrc.h |  6 +++++-
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 89b5577..1bc7c2f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -535,7 +535,8 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 
 static int execlists_context_queue(struct intel_engine_cs *ring,
 				   struct intel_context *to,
-				   u32 tail)
+				   u32 tail,
+				   struct drm_i915_gem_request *request)
 {
 	struct intel_ctx_submit_request *req = NULL, *cursor;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
@@ -554,6 +555,21 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	req->ring = ring;
 	req->tail = tail;
 
+	if (!request) {
+		/*
+		 * If there isn't a request associated with this submission,
+		 * create one as a temporary holder.
+		 */
+		WARN(1, "execlist context submission without request");
+		request = kzalloc(sizeof(*request), GFP_KERNEL);
+		if (request == NULL)
+			return -ENOMEM;
+		request->ctx = to;
+		request->ring = ring;
+	}
+	req->request = request;
+	i915_gem_request_reference(request);
+
 	intel_runtime_pm_get(dev_priv);
 
 	spin_lock_irqsave(&ring->execlist_lock, flags);
@@ -767,6 +783,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 			intel_lr_context_unpin(ring, ctx);
 		intel_runtime_pm_put(dev_priv);
 		i915_gem_context_unreference(req->ctx);
+		i915_gem_request_unreference(req->request);
 		list_del(&req->execlist_link);
 		kfree(req);
 	}
@@ -819,7 +836,8 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
  * on a queue waiting for the ELSP to be ready to accept a new context submission. At that
  * point, the tail *inside* the context is updated and the ELSP written to.
  */
-void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
+void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
+					   struct drm_i915_gem_request *request)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	struct intel_context *ctx = ringbuf->FIXME_lrc_ctx;
@@ -829,7 +847,7 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
 	if (intel_ring_stopped(ring))
 		return;
 
-	execlists_context_queue(ring, ctx, ringbuf->tail);
+	execlists_context_queue(ring, ctx, ringbuf->tail, request);
 }
 
 static int intel_lr_context_pin(struct intel_engine_cs *ring,
@@ -973,7 +991,7 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
 		return ret;
 
 	/* Force the context submission in case we have been skipping it */
-	intel_logical_ring_advance_and_submit(ringbuf);
+	intel_logical_ring_advance_and_submit(ringbuf, NULL);
 
 	/* With GEM the hangcheck timer should kick us out of the loop,
 	 * leaving it early runs the risk of corrupting GEM state (due
@@ -1311,7 +1329,8 @@ static void gen8_set_seqno(struct intel_engine_cs *ring, u32 seqno)
 	intel_write_status_page(ring, I915_GEM_HWS_INDEX, seqno);
 }
 
-static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
+static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
+			     struct drm_i915_gem_request *request)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	u32 cmd;
@@ -1333,7 +1352,7 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
 		i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
 	intel_logical_ring_emit(ringbuf, MI_NOOP);
-	intel_logical_ring_advance_and_submit(ringbuf);
+	intel_logical_ring_advance_and_submit(ringbuf, request);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 960fcbd..7a82bc9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -39,7 +39,9 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
 int intel_logical_rings_init(struct drm_device *dev);
 
 int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf);
-void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf);
+void intel_logical_ring_advance_and_submit(
+				struct intel_ringbuffer *ringbuf,
+				struct drm_i915_gem_request *request);
 /**
  * intel_logical_ring_advance() - advance the ringbuffer tail
  * @ringbuf: Ringbuffer to advance.
@@ -110,6 +112,8 @@ struct intel_ctx_submit_request {
 	struct list_head execlist_link;
 
 	int elsp_submitted;
+
+	struct drm_i915_gem_request *request;
 };
 
 void intel_lrc_irq_handler(struct intel_engine_cs *ring);
-- 
2.1.1

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

[PATCH 2/4] drm/i915: Removed duplicate members from submit_request

[Nick Hoath]

Where there were duplicate variables for the tail, context and ring (engine)
in the gem request and the execlist queue item, use the one from the request
and remove the duplicate from the execlist queue item.

Issue: VIZ-4274

v1: Rebase
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c     |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c    | 20 +++++++++-----------
 drivers/gpu/drm/i915/intel_lrc.h    |  4 ----
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 479e0c1..3fc8508 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1915,11 +1915,11 @@ static int i915_execlists(struct seq_file *m, void *data)
 		if (head_req) {
 			struct drm_i915_gem_object *ctx_obj;
 
-			ctx_obj = head_req->ctx->engine[ring_id].state;
+			ctx_obj = head_req->request->ctx->engine[ring_id].state;
 			seq_printf(m, "\tHead request id: %u\n",
 				   intel_execlists_ctx_id(ctx_obj));
 			seq_printf(m, "\tHead request tail: %u\n",
-				   head_req->tail);
+				   head_req->request->tail);
 		}
 
 		seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index de241eb..ca98b1b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2666,7 +2666,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 				execlist_link);
 		list_del(&submit_req->execlist_link);
 		intel_runtime_pm_put(dev_priv);
-		i915_gem_context_unreference(submit_req->ctx);
+		i915_gem_context_unreference(submit_req->request->ctx);
 		kfree(submit_req);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1bc7c2f..cbbd327 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -418,7 +418,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 				 execlist_link) {
 		if (!req0) {
 			req0 = cursor;
-		} else if (req0->ctx == cursor->ctx) {
+		} else if (req0->request->ctx == cursor->request->ctx) {
 			/* Same ctx: ignore first request, as second request
 			 * will update tail past first request's workload */
 			cursor->elsp_submitted = req0->elsp_submitted;
@@ -434,9 +434,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 
 	WARN_ON(req1 && req1->elsp_submitted);
 
-	execlists_submit_contexts(ring, req0->ctx, req0->tail,
-				  req1 ? req1->ctx : NULL,
-				  req1 ? req1->tail : 0);
+	execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
+				  req1 ? req1->request->ctx : NULL,
+				  req1 ? req1->request->tail : 0);
 
 	req0->elsp_submitted++;
 	if (req1)
@@ -456,7 +456,7 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 
 	if (head_req != NULL) {
 		struct drm_i915_gem_object *ctx_obj =
-				head_req->ctx->engine[ring->id].state;
+				head_req->request->ctx->engine[ring->id].state;
 		if (intel_execlists_ctx_id(ctx_obj) == request_id) {
 			WARN(head_req->elsp_submitted == 0,
 			     "Never submitted head request\n");
@@ -552,9 +552,6 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	if (to != ring->default_context)
 		intel_lr_context_pin(ring, to);
 
-	req->ring = ring;
-	req->tail = tail;
-
 	if (!request) {
 		/*
 		 * If there isn't a request associated with this submission,
@@ -569,6 +566,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	}
 	req->request = request;
 	i915_gem_request_reference(request);
+	i915_gem_context_reference(req->request->ctx);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -585,7 +583,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 					   struct intel_ctx_submit_request,
 					   execlist_link);
 
-		if (to == tail_req->ctx) {
+		if (to == tail_req->request->ctx) {
 			WARN(tail_req->elsp_submitted != 0,
 				"More than 2 already-submitted reqs queued\n");
 			list_del(&tail_req->execlist_link);
@@ -775,14 +773,14 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 	spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 	list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
-		struct intel_context *ctx = req->ctx;
+		struct intel_context *ctx = req->request->ctx;
 		struct drm_i915_gem_object *ctx_obj =
 				ctx->engine[ring->id].state;
 
 		if (ctx_obj && (ctx != ring->default_context))
 			intel_lr_context_unpin(ring, ctx);
 		intel_runtime_pm_put(dev_priv);
-		i915_gem_context_unreference(req->ctx);
+		i915_gem_context_unreference(ctx);
 		i915_gem_request_unreference(req->request);
 		list_del(&req->execlist_link);
 		kfree(req);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 7a82bc9..376c307 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -105,10 +105,6 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
  * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
  */
 struct intel_ctx_submit_request {
-	struct intel_context *ctx;
-	struct intel_engine_cs *ring;
-	u32 tail;
-
 	struct list_head execlist_link;
 
 	int elsp_submitted;
-- 
2.1.1

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

[PATCH 3/4] drm/i915: Remove FIXME_lrc_ctx backpointer

[Nick Hoath]

The first pass implementation of execlists required a backpointer to the context to be held
in the intel_ringbuffer. However the context pointer is available higher in the call stack.
Remove the backpointer from the ring buffer structure and instead pass it down through the
call stack.

v2: Integrate this changeset with the removal of duplicate request/execlist queue item members.
v3: Rebase
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  7 ++--
 drivers/gpu/drm/i915/intel_lrc.c        | 68 ++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_lrc.h        |  8 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++----
 4 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca98b1b..50034c4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2432,8 +2432,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 		return -ENOMEM;
 
 	if (i915.enable_execlists) {
-		struct intel_context *ctx = request->ctx;
-		ringbuf = ctx->engine[ring->id].ringbuf;
+		ringbuf = request->ctx->engine[ring->id].ringbuf;
 	} else
 		ringbuf = ring->buffer;
 
@@ -2446,7 +2445,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	 * what.
 	 */
 	if (i915.enable_execlists) {
-		ret = logical_ring_flush_all_caches(ringbuf);
+		ret = logical_ring_flush_all_caches(ringbuf, request->ctx);
 		if (ret)
 			return ret;
 	} else {
@@ -2463,7 +2462,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	request_ring_position = intel_ring_get_tail(ringbuf);
 
 	if (i915.enable_execlists) {
-		ret = ring->emit_request(ringbuf);
+		ret = ring->emit_request(ringbuf, request->ctx, request);
 		if (ret)
 			return ret;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index cbbd327..99fcb56 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -563,6 +563,8 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 			return -ENOMEM;
 		request->ctx = to;
 		request->ring = ring;
+	} else {
+		WARN_ON(to != request->ctx);
 	}
 	req->request = request;
 	i915_gem_request_reference(request);
@@ -601,7 +603,8 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	return 0;
 }
 
-static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
+static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
+					      struct intel_context *ctx)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	uint32_t flush_domains;
@@ -611,7 +614,8 @@ static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
 	if (ring->gpu_caches_dirty)
 		flush_domains = I915_GEM_GPU_DOMAINS;
 
-	ret = ring->emit_flush(ringbuf, I915_GEM_GPU_DOMAINS, flush_domains);
+	ret = ring->emit_flush(ringbuf, ctx,
+			       I915_GEM_GPU_DOMAINS, flush_domains);
 	if (ret)
 		return ret;
 
@@ -620,6 +624,7 @@ static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
 }
 
 static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
+				 struct intel_context *ctx,
 				 struct list_head *vmas)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
@@ -647,7 +652,7 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
 	/* Unconditionally invalidate gpu caches and ensure that we do flush
 	 * any residual writes from the previous batch.
 	 */
-	return logical_ring_invalidate_all_caches(ringbuf);
+	return logical_ring_invalidate_all_caches(ringbuf, ctx);
 }
 
 /**
@@ -727,13 +732,13 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 		return -EINVAL;
 	}
 
-	ret = execlists_move_to_gpu(ringbuf, vmas);
+	ret = execlists_move_to_gpu(ringbuf, ctx, vmas);
 	if (ret)
 		return ret;
 
 	if (ring == &dev_priv->ring[RCS] &&
 	    instp_mode != dev_priv->relative_constants_mode) {
-		ret = intel_logical_ring_begin(ringbuf, 4);
+		ret = intel_logical_ring_begin(ringbuf, ctx, 4);
 		if (ret)
 			return ret;
 
@@ -746,7 +751,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 		dev_priv->relative_constants_mode = instp_mode;
 	}
 
-	ret = ring->emit_bb_start(ringbuf, exec_start, flags);
+	ret = ring->emit_bb_start(ringbuf, ctx, exec_start, flags);
 	if (ret)
 		return ret;
 
@@ -809,7 +814,8 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring)
 	I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING));
 }
 
-int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
+int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf,
+				  struct intel_context *ctx)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	int ret;
@@ -817,7 +823,7 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
 	if (!ring->gpu_caches_dirty)
 		return 0;
 
-	ret = ring->emit_flush(ringbuf, 0, I915_GEM_GPU_DOMAINS);
+	ret = ring->emit_flush(ringbuf, ctx, 0, I915_GEM_GPU_DOMAINS);
 	if (ret)
 		return ret;
 
@@ -835,10 +841,10 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
  * point, the tail *inside* the context is updated and the ELSP written to.
  */
 void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
+					   struct intel_context *ctx,
 					   struct drm_i915_gem_request *request)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
-	struct intel_context *ctx = ringbuf->FIXME_lrc_ctx;
 
 	intel_logical_ring_advance(ringbuf);
 
@@ -976,6 +982,7 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
 }
 
 static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
+				       struct intel_context *ctx,
 				       int bytes)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
@@ -989,7 +996,7 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
 		return ret;
 
 	/* Force the context submission in case we have been skipping it */
-	intel_logical_ring_advance_and_submit(ringbuf, NULL);
+	intel_logical_ring_advance_and_submit(ringbuf, ctx, NULL);
 
 	/* With GEM the hangcheck timer should kick us out of the loop,
 	 * leaving it early runs the risk of corrupting GEM state (due
@@ -1024,13 +1031,14 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
 	return ret;
 }
 
-static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf)
+static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf,
+				    struct intel_context *ctx)
 {
 	uint32_t __iomem *virt;
 	int rem = ringbuf->size - ringbuf->tail;
 
 	if (ringbuf->space < rem) {
-		int ret = logical_ring_wait_for_space(ringbuf, rem);
+		int ret = logical_ring_wait_for_space(ringbuf, ctx, rem);
 
 		if (ret)
 			return ret;
@@ -1047,18 +1055,19 @@ static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf)
 	return 0;
 }
 
-static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, int bytes)
+static int logical_ring_prepare(struct intel_ringbuffer *ringbuf,
+				struct intel_context *ctx, int bytes)
 {
 	int ret;
 
 	if (unlikely(ringbuf->tail + bytes > ringbuf->effective_size)) {
-		ret = logical_ring_wrap_buffer(ringbuf);
+		ret = logical_ring_wrap_buffer(ringbuf, ctx);
 		if (unlikely(ret))
 			return ret;
 	}
 
 	if (unlikely(ringbuf->space < bytes)) {
-		ret = logical_ring_wait_for_space(ringbuf, bytes);
+		ret = logical_ring_wait_for_space(ringbuf, ctx, bytes);
 		if (unlikely(ret))
 			return ret;
 	}
@@ -1079,7 +1088,8 @@ static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, int bytes)
  *
  * Return: non-zero if the ringbuffer is not ready to be written to.
  */
-int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords)
+int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
+			     struct intel_context *ctx, int num_dwords)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	struct drm_device *dev = ring->dev;
@@ -1091,12 +1101,12 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords)
 	if (ret)
 		return ret;
 
-	ret = logical_ring_prepare(ringbuf, num_dwords * sizeof(uint32_t));
+	ret = logical_ring_prepare(ringbuf, ctx, num_dwords * sizeof(uint32_t));
 	if (ret)
 		return ret;
 
 	/* Preallocate the olr before touching the ring */
-	ret = logical_ring_alloc_request(ring, ringbuf->FIXME_lrc_ctx);
+	ret = logical_ring_alloc_request(ring, ctx);
 	if (ret)
 		return ret;
 
@@ -1117,11 +1127,11 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
 		return 0;
 
 	ring->gpu_caches_dirty = true;
-	ret = logical_ring_flush_all_caches(ringbuf);
+	ret = logical_ring_flush_all_caches(ringbuf, ctx);
 	if (ret)
 		return ret;
 
-	ret = intel_logical_ring_begin(ringbuf, w->count * 2 + 2);
+	ret = intel_logical_ring_begin(ringbuf, ctx, w->count * 2 + 2);
 	if (ret)
 		return ret;
 
@@ -1135,7 +1145,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
 	intel_logical_ring_advance(ringbuf);
 
 	ring->gpu_caches_dirty = true;
-	ret = logical_ring_flush_all_caches(ringbuf);
+	ret = logical_ring_flush_all_caches(ringbuf, ctx);
 	if (ret)
 		return ret;
 
@@ -1185,12 +1195,13 @@ static int gen8_init_render_ring(struct intel_engine_cs *ring)
 }
 
 static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf,
+			      struct intel_context *ctx,
 			      u64 offset, unsigned flags)
 {
 	bool ppgtt = !(flags & I915_DISPATCH_SECURE);
 	int ret;
 
-	ret = intel_logical_ring_begin(ringbuf, 4);
+	ret = intel_logical_ring_begin(ringbuf, ctx, 4);
 	if (ret)
 		return ret;
 
@@ -1238,6 +1249,7 @@ static void gen8_logical_ring_put_irq(struct intel_engine_cs *ring)
 }
 
 static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
+			   struct intel_context *ctx,
 			   u32 invalidate_domains,
 			   u32 unused)
 {
@@ -1247,7 +1259,7 @@ static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
 	uint32_t cmd;
 	int ret;
 
-	ret = intel_logical_ring_begin(ringbuf, 4);
+	ret = intel_logical_ring_begin(ringbuf, ctx, 4);
 	if (ret)
 		return ret;
 
@@ -1276,6 +1288,7 @@ static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
 }
 
 static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
+				  struct intel_context *ctx,
 				  u32 invalidate_domains,
 				  u32 flush_domains)
 {
@@ -1302,7 +1315,7 @@ static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
 		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
 	}
 
-	ret = intel_logical_ring_begin(ringbuf, 6);
+	ret = intel_logical_ring_begin(ringbuf, ctx, 6);
 	if (ret)
 		return ret;
 
@@ -1328,13 +1341,14 @@ static void gen8_set_seqno(struct intel_engine_cs *ring, u32 seqno)
 }
 
 static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
+			     struct intel_context *ctx,
 			     struct drm_i915_gem_request *request)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	u32 cmd;
 	int ret;
 
-	ret = intel_logical_ring_begin(ringbuf, 6);
+	ret = intel_logical_ring_begin(ringbuf, ctx, 6);
 	if (ret)
 		return ret;
 
@@ -1350,7 +1364,7 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
 		i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
 	intel_logical_ring_emit(ringbuf, MI_NOOP);
-	intel_logical_ring_advance_and_submit(ringbuf, request);
+	intel_logical_ring_advance_and_submit(ringbuf, ctx, request);
 
 	return 0;
 }
@@ -1638,6 +1652,7 @@ int intel_lr_context_render_state_init(struct intel_engine_cs *ring,
 		return 0;
 
 	ret = ring->emit_bb_start(ringbuf,
+			ctx,
 			so.ggtt_offset,
 			I915_DISPATCH_SECURE);
 	if (ret)
@@ -1894,7 +1909,6 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 	}
 
 	ringbuf->ring = ring;
-	ringbuf->FIXME_lrc_ctx = ctx;
 
 	ringbuf->size = 32 * PAGE_SIZE;
 	ringbuf->effective_size = ringbuf->size;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 376c307..80a80ea 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -38,9 +38,11 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring);
 void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
 int intel_logical_rings_init(struct drm_device *dev);
 
-int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf);
+int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf,
+				  struct intel_context *ctx);
 void intel_logical_ring_advance_and_submit(
 				struct intel_ringbuffer *ringbuf,
+				struct intel_context *ctx,
 				struct drm_i915_gem_request *request);
 /**
  * intel_logical_ring_advance() - advance the ringbuffer tail
@@ -63,7 +65,9 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
 	ringbuf->tail += 4;
 }
-int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords);
+int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
+			     struct intel_context *ctx,
+			     int num_dwords);
 
 /* Logical Ring Contexts */
 int intel_lr_context_render_state_init(struct intel_engine_cs *ring,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 6dbb6f4..cabf48f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -99,13 +99,6 @@ struct intel_ringbuffer {
 
 	struct intel_engine_cs *ring;
 
-	/*
-	 * FIXME: This backpointer is an artifact of the history of how the
-	 * execlist patches came into being. It will get removed once the basic
-	 * code has landed.
-	 */
-	struct intel_context *FIXME_lrc_ctx;
-
 	u32 head;
 	u32 tail;
 	int space;
@@ -123,6 +116,8 @@ struct intel_ringbuffer {
 	u32 last_retired_head;
 };
 
+struct	intel_context;
+
 struct  intel_engine_cs {
 	const char	*name;
 	enum intel_ring_id {
@@ -239,11 +234,15 @@ struct  intel_engine_cs {
 	struct list_head execlist_retired_req_list;
 	u8 next_context_status_buffer;
 	u32             irq_keep_mask; /* bitmask for interrupts that should not be masked */
-	int		(*emit_request)(struct intel_ringbuffer *ringbuf);
+	int		(*emit_request)(struct intel_ringbuffer *ringbuf,
+					struct intel_context *ctx,
+					struct drm_i915_gem_request *request);
 	int		(*emit_flush)(struct intel_ringbuffer *ringbuf,
+				      struct intel_context *ctx,
 				      u32 invalidate_domains,
 				      u32 flush_domains);
 	int		(*emit_bb_start)(struct intel_ringbuffer *ringbuf,
+					 struct intel_context *ctx,
 					 u64 offset, unsigned flags);
 
 	/**
-- 
2.1.1

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

[PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Nick Hoath]

Move all remaining elements that were unique to execlists queue items
in to the associated request.

Issue: VIZ-4274

v2: Rebase. Fixed issue of overzealous freeing of request.
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  8 +++----
 drivers/gpu/drm/i915/i915_drv.h     | 24 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c     |  6 ++---
 drivers/gpu/drm/i915/intel_lrc.c    | 44 +++++++++++++++----------------------
 4 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3fc8508..4470fbe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1875,7 +1875,7 @@ static int i915_execlists(struct seq_file *m, void *data)
 	intel_runtime_pm_get(dev_priv);
 
 	for_each_ring(ring, dev_priv, ring_id) {
-		struct intel_ctx_submit_request *head_req = NULL;
+		struct drm_i915_gem_request *head_req = NULL;
 		int count = 0;
 		unsigned long flags;
 
@@ -1908,18 +1908,18 @@ static int i915_execlists(struct seq_file *m, void *data)
 		list_for_each(cursor, &ring->execlist_queue)
 			count++;
 		head_req = list_first_entry_or_null(&ring->execlist_queue,
-				struct intel_ctx_submit_request, execlist_link);
+				struct drm_i915_gem_request, execlist_link);
 		spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 		seq_printf(m, "\t%d requests in queue\n", count);
 		if (head_req) {
 			struct drm_i915_gem_object *ctx_obj;
 
-			ctx_obj = head_req->request->ctx->engine[ring_id].state;
+			ctx_obj = head_req->ctx->engine[ring_id].state;
 			seq_printf(m, "\tHead request id: %u\n",
 				   intel_execlists_ctx_id(ctx_obj));
 			seq_printf(m, "\tHead request tail: %u\n",
-				   head_req->request->tail);
+				   head_req->tail);
 		}
 
 		seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d68c75f..114adc3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2036,6 +2036,30 @@ struct drm_i915_gem_request {
 	struct list_head client_list;
 
 	uint32_t uniq;
+
+	/**
+	 * The ELSP only accepts two elements at a time, so we queue
+	 * context/tail pairs on a given queue (ring->execlist_queue) until the
+	 * hardware is available. The queue serves a double purpose: we also use
+	 * it to keep track of the up to 2 contexts currently in the hardware
+	 * (usually one in execution and the other queued up by the GPU): We
+	 * only remove elements from the head
+	 * of the queue when the hardware informs us that an element has been
+	 * completed.
+	 *
+	 * All accesses to the queue are mediated by a spinlock
+	 * (ring->execlist_lock).
+	 */
+
+	/** Execlist link in the submission queue.*/
+	struct list_head execlist_link;
+
+	/** Execlists workqueue for processing this request in a bottom half */
+	struct work_struct work;
+
+	/** Execlists no. of times this request has been sent to the ELSP */
+	int elsp_submitted;
+
 };
 
 void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 50034c4..aa737fb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2658,14 +2658,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	 * pinned in place.
 	 */
 	while (!list_empty(&ring->execlist_queue)) {
-		struct intel_ctx_submit_request *submit_req;
+		struct drm_i915_gem_request *submit_req;
 
 		submit_req = list_first_entry(&ring->execlist_queue,
-				struct intel_ctx_submit_request,
+				struct drm_i915_gem_request,
 				execlist_link);
 		list_del(&submit_req->execlist_link);
 		intel_runtime_pm_put(dev_priv);
-		i915_gem_context_unreference(submit_req->request->ctx);
+		i915_gem_context_unreference(submit_req->ctx);
 		kfree(submit_req);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 99fcb56..c312a83 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -405,8 +405,8 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
 
 static void execlists_context_unqueue(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
-	struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
+	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
+	struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
 
 	assert_spin_locked(&ring->execlist_lock);
 
@@ -418,7 +418,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 				 execlist_link) {
 		if (!req0) {
 			req0 = cursor;
-		} else if (req0->request->ctx == cursor->request->ctx) {
+		} else if (req0->ctx == cursor->ctx) {
 			/* Same ctx: ignore first request, as second request
 			 * will update tail past first request's workload */
 			cursor->elsp_submitted = req0->elsp_submitted;
@@ -434,9 +434,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 
 	WARN_ON(req1 && req1->elsp_submitted);
 
-	execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
-				  req1 ? req1->request->ctx : NULL,
-				  req1 ? req1->request->tail : 0);
+	execlists_submit_contexts(ring, req0->ctx, req0->tail,
+				  req1 ? req1->ctx : NULL,
+				  req1 ? req1->tail : 0);
 
 	req0->elsp_submitted++;
 	if (req1)
@@ -446,17 +446,17 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 					   u32 request_id)
 {
-	struct intel_ctx_submit_request *head_req;
+	struct drm_i915_gem_request *head_req;
 
 	assert_spin_locked(&ring->execlist_lock);
 
 	head_req = list_first_entry_or_null(&ring->execlist_queue,
-					    struct intel_ctx_submit_request,
+					    struct drm_i915_gem_request,
 					    execlist_link);
 
 	if (head_req != NULL) {
 		struct drm_i915_gem_object *ctx_obj =
-				head_req->request->ctx->engine[ring->id].state;
+				head_req->ctx->engine[ring->id].state;
 		if (intel_execlists_ctx_id(ctx_obj) == request_id) {
 			WARN(head_req->elsp_submitted == 0,
 			     "Never submitted head request\n");
@@ -538,17 +538,11 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 				   u32 tail,
 				   struct drm_i915_gem_request *request)
 {
-	struct intel_ctx_submit_request *req = NULL, *cursor;
+	struct drm_i915_gem_request *cursor;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	int num_elements = 0;
 
-	req = kzalloc(sizeof(*req), GFP_KERNEL);
-	if (req == NULL)
-		return -ENOMEM;
-	req->ctx = to;
-	i915_gem_context_reference(req->ctx);
-
 	if (to != ring->default_context)
 		intel_lr_context_pin(ring, to);
 
@@ -566,9 +560,8 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	} else {
 		WARN_ON(to != request->ctx);
 	}
-	req->request = request;
 	i915_gem_request_reference(request);
-	i915_gem_context_reference(req->request->ctx);
+	i915_gem_context_reference(request->ctx);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -579,13 +572,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 			break;
 
 	if (num_elements > 2) {
-		struct intel_ctx_submit_request *tail_req;
+		struct drm_i915_gem_request *tail_req;
 
 		tail_req = list_last_entry(&ring->execlist_queue,
-					   struct intel_ctx_submit_request,
+					   struct drm_i915_gem_request,
 					   execlist_link);
 
-		if (to == tail_req->request->ctx) {
+		if (to == tail_req->ctx) {
 			WARN(tail_req->elsp_submitted != 0,
 				"More than 2 already-submitted reqs queued\n");
 			list_del(&tail_req->execlist_link);
@@ -594,7 +587,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 		}
 	}
 
-	list_add_tail(&req->execlist_link, &ring->execlist_queue);
+	list_add_tail(&request->execlist_link, &ring->execlist_queue);
 	if (num_elements == 0)
 		execlists_context_unqueue(ring);
 
@@ -763,7 +756,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 
 void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req, *tmp;
+	struct drm_i915_gem_request *req, *tmp;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	struct list_head retired_list;
@@ -778,7 +771,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 	spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 	list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
-		struct intel_context *ctx = req->request->ctx;
+		struct intel_context *ctx = req->ctx;
 		struct drm_i915_gem_object *ctx_obj =
 				ctx->engine[ring->id].state;
 
@@ -786,9 +779,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 			intel_lr_context_unpin(ring, ctx);
 		intel_runtime_pm_put(dev_priv);
 		i915_gem_context_unreference(ctx);
-		i915_gem_request_unreference(req->request);
+		i915_gem_request_unreference(req);
 		list_del(&req->execlist_link);
-		kfree(req);
 	}
 }
 
-- 
2.1.1

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

Re: [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +3-3              360/366              360/366
SNB                                  448/450              448/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_drv_suspend_fence-restore-untiled      DMESG_WARN(1, M26)PASS(10, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(10, M37M26)      PASS(1, M26)
*ILK  igt_kms_flip_flip-vs-dpms-interruptible      PASS(1, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_plain-flip-fb-recreate-interruptible      PASS(1, M26)      DMESG_WARN(1, M26)
 ILK  igt_kms_flip_rcs-flip-vs-dpms      DMESG_WARN(1, M26)PASS(9, M37M26)      PASS(1, M26)
*ILK  igt_kms_flip_vblank-vs-hang      PASS(1, M26)      DMESG_WARN(1, M26)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Daniel Vetter]

On Tue, Dec 16, 2014 at 12:32:30PM +0000, Nick Hoath wrote:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d68c75f..114adc3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2036,6 +2036,30 @@ struct drm_i915_gem_request {
>  	struct list_head client_list;
>  
>  	uint32_t uniq;
> +
> +	/**
> +	 * The ELSP only accepts two elements at a time, so we queue
> +	 * context/tail pairs on a given queue (ring->execlist_queue) until the
> +	 * hardware is available. The queue serves a double purpose: we also use
> +	 * it to keep track of the up to 2 contexts currently in the hardware
> +	 * (usually one in execution and the other queued up by the GPU): We
> +	 * only remove elements from the head
> +	 * of the queue when the hardware informs us that an element has been
> +	 * completed.
> +	 *
> +	 * All accesses to the queue are mediated by a spinlock
> +	 * (ring->execlist_lock).
> +	 */
> +
> +	/** Execlist link in the submission queue.*/
> +	struct list_head execlist_link;
> +
> +	/** Execlists workqueue for processing this request in a bottom half */
> +	struct work_struct work;

Hm I've thought this would drop out with the other rework we've had
floating around. It's also not used anywhere, so I guess it disappeared
for real and is just a leftover from rebasing?

Otherwise I guess this is ready for final review I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Daniel Vetter]

On Wed, Dec 17, 2014 at 09:39:46PM +0100, Daniel Vetter wrote:
> On Tue, Dec 16, 2014 at 12:32:30PM +0000, Nick Hoath wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index d68c75f..114adc3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2036,6 +2036,30 @@ struct drm_i915_gem_request {
> >  	struct list_head client_list;
> >  
> >  	uint32_t uniq;
> > +
> > +	/**
> > +	 * The ELSP only accepts two elements at a time, so we queue
> > +	 * context/tail pairs on a given queue (ring->execlist_queue) until the
> > +	 * hardware is available. The queue serves a double purpose: we also use
> > +	 * it to keep track of the up to 2 contexts currently in the hardware
> > +	 * (usually one in execution and the other queued up by the GPU): We
> > +	 * only remove elements from the head
> > +	 * of the queue when the hardware informs us that an element has been
> > +	 * completed.
> > +	 *
> > +	 * All accesses to the queue are mediated by a spinlock
> > +	 * (ring->execlist_lock).
> > +	 */
> > +
> > +	/** Execlist link in the submission queue.*/
> > +	struct list_head execlist_link;
> > +
> > +	/** Execlists workqueue for processing this request in a bottom half */
> > +	struct work_struct work;
> 
> Hm I've thought this would drop out with the other rework we've had
> floating around. It's also not used anywhere, so I guess it disappeared
> for real and is just a leftover from rebasing?

And a plea from your maintainer: I've asked for exactly this in my reply,
but there was no reply from you nor any mention about anything in the
in-patch changelog. So I had to digg through all the patches and the old
thread to figure out again what it is my subconscious told me is missing.

This isn't efficient, so please always acknowledge review feedbacka and
questions either with a reply or in the commit message changelogs (there
it needs the name of the reviewer in case there was lots of feedback.)

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Nick Hoath]

Move all remaining elements that were unique to execlists queue items
in to the associated request.

Issue: VIZ-4274

v2: Rebase. Fixed issue of overzealous freeing of request.
v3: Removed re-addition of cleanup work queue (found by Daniel Vetter)
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  8 +++----
 drivers/gpu/drm/i915/i915_drv.h     | 21 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c     |  6 ++---
 drivers/gpu/drm/i915/intel_lrc.c    | 44 +++++++++++++++----------------------
 4 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d4cc482..b2dfac4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1928,7 +1928,7 @@ static int i915_execlists(struct seq_file *m, void *data)
 	intel_runtime_pm_get(dev_priv);
 
 	for_each_ring(ring, dev_priv, ring_id) {
-		struct intel_ctx_submit_request *head_req = NULL;
+		struct drm_i915_gem_request *head_req = NULL;
 		int count = 0;
 		unsigned long flags;
 
@@ -1961,18 +1961,18 @@ static int i915_execlists(struct seq_file *m, void *data)
 		list_for_each(cursor, &ring->execlist_queue)
 			count++;
 		head_req = list_first_entry_or_null(&ring->execlist_queue,
-				struct intel_ctx_submit_request, execlist_link);
+				struct drm_i915_gem_request, execlist_link);
 		spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 		seq_printf(m, "\t%d requests in queue\n", count);
 		if (head_req) {
 			struct drm_i915_gem_object *ctx_obj;
 
-			ctx_obj = head_req->request->ctx->engine[ring_id].state;
+			ctx_obj = head_req->ctx->engine[ring_id].state;
 			seq_printf(m, "\tHead request id: %u\n",
 				   intel_execlists_ctx_id(ctx_obj));
 			seq_printf(m, "\tHead request tail: %u\n",
-				   head_req->request->tail);
+				   head_req->tail);
 		}
 
 		seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 921e4c5..3702073 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2078,6 +2078,27 @@ struct drm_i915_gem_request {
 	struct list_head client_list;
 
 	uint32_t uniq;
+
+	/**
+	 * The ELSP only accepts two elements at a time, so we queue
+	 * context/tail pairs on a given queue (ring->execlist_queue) until the
+	 * hardware is available. The queue serves a double purpose: we also use
+	 * it to keep track of the up to 2 contexts currently in the hardware
+	 * (usually one in execution and the other queued up by the GPU): We
+	 * only remove elements from the head
+	 * of the queue when the hardware informs us that an element has been
+	 * completed.
+	 *
+	 * All accesses to the queue are mediated by a spinlock
+	 * (ring->execlist_lock).
+	 */
+
+	/** Execlist link in the submission queue.*/
+	struct list_head execlist_link;
+
+	/** Execlists no. of times this request has been sent to the ELSP */
+	int elsp_submitted;
+
 };
 
 void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73c1a22..f7f235c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2661,14 +2661,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	 * pinned in place.
 	 */
 	while (!list_empty(&ring->execlist_queue)) {
-		struct intel_ctx_submit_request *submit_req;
+		struct drm_i915_gem_request *submit_req;
 
 		submit_req = list_first_entry(&ring->execlist_queue,
-				struct intel_ctx_submit_request,
+				struct drm_i915_gem_request,
 				execlist_link);
 		list_del(&submit_req->execlist_link);
 		intel_runtime_pm_put(dev_priv);
-		i915_gem_context_unreference(submit_req->request->ctx);
+		i915_gem_context_unreference(submit_req->ctx);
 		kfree(submit_req);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 427d561..966777c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -404,8 +404,8 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
 
 static void execlists_context_unqueue(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
-	struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
+	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
+	struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
 
 	assert_spin_locked(&ring->execlist_lock);
 
@@ -417,7 +417,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 				 execlist_link) {
 		if (!req0) {
 			req0 = cursor;
-		} else if (req0->request->ctx == cursor->request->ctx) {
+		} else if (req0->ctx == cursor->ctx) {
 			/* Same ctx: ignore first request, as second request
 			 * will update tail past first request's workload */
 			cursor->elsp_submitted = req0->elsp_submitted;
@@ -433,9 +433,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 
 	WARN_ON(req1 && req1->elsp_submitted);
 
-	execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
-				  req1 ? req1->request->ctx : NULL,
-				  req1 ? req1->request->tail : 0);
+	execlists_submit_contexts(ring, req0->ctx, req0->tail,
+				  req1 ? req1->ctx : NULL,
+				  req1 ? req1->tail : 0);
 
 	req0->elsp_submitted++;
 	if (req1)
@@ -445,17 +445,17 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 					   u32 request_id)
 {
-	struct intel_ctx_submit_request *head_req;
+	struct drm_i915_gem_request *head_req;
 
 	assert_spin_locked(&ring->execlist_lock);
 
 	head_req = list_first_entry_or_null(&ring->execlist_queue,
-					    struct intel_ctx_submit_request,
+					    struct drm_i915_gem_request,
 					    execlist_link);
 
 	if (head_req != NULL) {
 		struct drm_i915_gem_object *ctx_obj =
-				head_req->request->ctx->engine[ring->id].state;
+				head_req->ctx->engine[ring->id].state;
 		if (intel_execlists_ctx_id(ctx_obj) == request_id) {
 			WARN(head_req->elsp_submitted == 0,
 			     "Never submitted head request\n");
@@ -537,17 +537,11 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 				   u32 tail,
 				   struct drm_i915_gem_request *request)
 {
-	struct intel_ctx_submit_request *req = NULL, *cursor;
+	struct drm_i915_gem_request *cursor;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	int num_elements = 0;
 
-	req = kzalloc(sizeof(*req), GFP_KERNEL);
-	if (req == NULL)
-		return -ENOMEM;
-	req->ctx = to;
-	i915_gem_context_reference(req->ctx);
-
 	if (to != ring->default_context)
 		intel_lr_context_pin(ring, to);
 
@@ -565,9 +559,8 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	} else {
 		WARN_ON(to != request->ctx);
 	}
-	req->request = request;
 	i915_gem_request_reference(request);
-	i915_gem_context_reference(req->request->ctx);
+	i915_gem_context_reference(request->ctx);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -578,13 +571,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 			break;
 
 	if (num_elements > 2) {
-		struct intel_ctx_submit_request *tail_req;
+		struct drm_i915_gem_request *tail_req;
 
 		tail_req = list_last_entry(&ring->execlist_queue,
-					   struct intel_ctx_submit_request,
+					   struct drm_i915_gem_request,
 					   execlist_link);
 
-		if (to == tail_req->request->ctx) {
+		if (to == tail_req->ctx) {
 			WARN(tail_req->elsp_submitted != 0,
 				"More than 2 already-submitted reqs queued\n");
 			list_del(&tail_req->execlist_link);
@@ -593,7 +586,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 		}
 	}
 
-	list_add_tail(&req->execlist_link, &ring->execlist_queue);
+	list_add_tail(&request->execlist_link, &ring->execlist_queue);
 	if (num_elements == 0)
 		execlists_context_unqueue(ring);
 
@@ -762,7 +755,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 
 void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req, *tmp;
+	struct drm_i915_gem_request *req, *tmp;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	struct list_head retired_list;
@@ -777,7 +770,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 	spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 	list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
-		struct intel_context *ctx = req->request->ctx;
+		struct intel_context *ctx = req->ctx;
 		struct drm_i915_gem_object *ctx_obj =
 				ctx->engine[ring->id].state;
 
@@ -785,9 +778,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 			intel_lr_context_unpin(ring, ctx);
 		intel_runtime_pm_put(dev_priv);
 		i915_gem_context_unreference(ctx);
-		i915_gem_request_unreference(req->request);
+		i915_gem_request_unreference(req);
 		list_del(&req->execlist_link);
-		kfree(req);
 	}
 }
 
-- 
2.1.1

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

[PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Nick Hoath]

Move all remaining elements that were unique to execlists queue items
in to the associated request.

Issue: VIZ-4274

v2: Rebase. Fixed issue of overzealous freeing of request.
v3: Removed re-addition of cleanup work queue (found by Daniel Vetter)
v4: Rebase.
v5: Actual removal of intel_ctx_submit_request. Update both tail and postfix
pointer in __i915_add_request (found by Thomas Daniel)

Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++----
 drivers/gpu/drm/i915/i915_drv.h      | 21 +++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c      | 12 +++++-----
 drivers/gpu/drm/i915/i915_reg.h      | 32 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  6 ++++-
 drivers/gpu/drm/i915/intel_lrc.c     | 44 ++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_lrc.h     | 27 ----------------------
 7 files changed, 87 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d4cc482..b2dfac4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1928,7 +1928,7 @@ static int i915_execlists(struct seq_file *m, void *data)
 	intel_runtime_pm_get(dev_priv);
 
 	for_each_ring(ring, dev_priv, ring_id) {
-		struct intel_ctx_submit_request *head_req = NULL;
+		struct drm_i915_gem_request *head_req = NULL;
 		int count = 0;
 		unsigned long flags;
 
@@ -1961,18 +1961,18 @@ static int i915_execlists(struct seq_file *m, void *data)
 		list_for_each(cursor, &ring->execlist_queue)
 			count++;
 		head_req = list_first_entry_or_null(&ring->execlist_queue,
-				struct intel_ctx_submit_request, execlist_link);
+				struct drm_i915_gem_request, execlist_link);
 		spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 		seq_printf(m, "\t%d requests in queue\n", count);
 		if (head_req) {
 			struct drm_i915_gem_object *ctx_obj;
 
-			ctx_obj = head_req->request->ctx->engine[ring_id].state;
+			ctx_obj = head_req->ctx->engine[ring_id].state;
 			seq_printf(m, "\tHead request id: %u\n",
 				   intel_execlists_ctx_id(ctx_obj));
 			seq_printf(m, "\tHead request tail: %u\n",
-				   head_req->request->tail);
+				   head_req->tail);
 		}
 
 		seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index af024f2..8c6a207 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2089,6 +2089,27 @@ struct drm_i915_gem_request {
 	struct list_head client_list;
 
 	uint32_t uniq;
+
+	/**
+	 * The ELSP only accepts two elements at a time, so we queue
+	 * context/tail pairs on a given queue (ring->execlist_queue) until the
+	 * hardware is available. The queue serves a double purpose: we also use
+	 * it to keep track of the up to 2 contexts currently in the hardware
+	 * (usually one in execution and the other queued up by the GPU): We
+	 * only remove elements from the head
+	 * of the queue when the hardware informs us that an element has been
+	 * completed.
+	 *
+	 * All accesses to the queue are mediated by a spinlock
+	 * (ring->execlist_lock).
+	 */
+
+	/** Execlist link in the submission queue.*/
+	struct list_head execlist_link;
+
+	/** Execlists no. of times this request has been sent to the ELSP */
+	int elsp_submitted;
+
 };
 
 void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0649559..0195e3f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2414,7 +2414,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct drm_i915_gem_request *request;
 	struct intel_ringbuffer *ringbuf;
-	u32 request_ring_position, request_start;
+	u32 request_start;
 	int ret;
 
 	request = ring->outstanding_lazy_request;
@@ -2449,7 +2449,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	 * GPU processing the request, we never over-estimate the
 	 * position of the head.
 	 */
-	request_ring_position = intel_ring_get_tail(ringbuf);
+	request->postfix = intel_ring_get_tail(ringbuf);
 
 	if (i915.enable_execlists) {
 		ret = ring->emit_request(ringbuf, request);
@@ -2462,7 +2462,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	}
 
 	request->head = request_start;
-	request->postfix = request_ring_position;
+	request->tail = intel_ring_get_tail(ringbuf);
 
 	/* Whilst this request exists, batch_obj will be on the
 	 * active_list, and so will hold the active reference. Only when this
@@ -2649,14 +2649,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	 * pinned in place.
 	 */
 	while (!list_empty(&ring->execlist_queue)) {
-		struct intel_ctx_submit_request *submit_req;
+		struct drm_i915_gem_request *submit_req;
 
 		submit_req = list_first_entry(&ring->execlist_queue,
-				struct intel_ctx_submit_request,
+				struct drm_i915_gem_request,
 				execlist_link);
 		list_del(&submit_req->execlist_link);
 		intel_runtime_pm_put(dev_priv);
-		i915_gem_context_unreference(submit_req->request->ctx);
+		i915_gem_context_unreference(submit_req->ctx);
 		kfree(submit_req);
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0f32fd1a..9ae81db 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5223,6 +5223,10 @@ enum punit_power_well {
 #define  HDC_DONOT_FETCH_MEM_WHEN_MASKED	(1<<11)
 #define  HDC_FENCE_DEST_SLM_DISABLE		(1<<14)
 
+/* GEN8 chicken */
+#define GEN9_SLICE_COMMON_ECO_CHICKEN0		0x07308
+#define GEN9_DISABLE_PIXEL_MASK_CAMMING		(1<<14)
+
 /* WaCatErrorRejectionIssue */
 #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		0x9030
 #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
@@ -6169,6 +6173,7 @@ enum punit_power_well {
 #define HALF_SLICE_CHICKEN3		0xe184
 #define   HSW_SAMPLE_C_PERFORMANCE	(1<<9)
 #define   GEN8_CENTROID_PIXEL_OPT_DIS	(1<<8)
+#define   GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC	(1<<5)
 #define   GEN8_SAMPLER_POWER_BYPASS_DIS	(1<<1)
 
 /* Audio */
@@ -7145,4 +7150,31 @@ enum punit_power_well {
 #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
 #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
 
+/*
+ * Chicken Registers for LLC/eLLC Hot Spotting Avoidance Mode for
+ * 3D/Media Compressed Resources
+ */
+#define GEN9_COMMON_SLICE_CHICKEN2_REG			0x7014
+#define GEN9_COMMON_SLICE_CHICKEN2_NEW_HASH_ENABLE	(1<<29 | 1<<13)
+#define GEN9_HALF_SLICE_CHICKEN7_REG			0xE194
+#define GEN9_HALF_SLICE_CHICKEN7_NEW_HASH_ENABLE	(1<<24 | 1<<8)
+#define GEN9_MMCD_MISC_CTRL_REG				0x4DDC
+#define GEN9_MMCD_MISC_CTRL_NEW_HASH_ENABLE		(1<<31 | 1<<27)
+#define GEN9_CHICKEN_MISC1_REG				0x42080
+#define GEN9_CHICKEN_MISC1_NEW_HASH_ENABLE		(1<<15)
+
+/* Unit Level Clock Gating Control */
+#define GEN9_UCGCTL6					0x9430
+#define GEN9_GAPS_UNIT_CLOCK_GATE_DISABLE		(1<<24)
+#define GEN9_HDC_UNIT_CLOCK_GATE_DISABLE		(1<<28)
+
+/* Mirror of EU disable fuses */
+#define MIRROR_EU_DISABLE0				0x9134
+#define MIRROR_EU_DISABLE1				0x9138
+#define MIRROR_EU_DISABLE_SS0				(0x7<<0)
+#define MIRROR_EU_DISABLE_SS1				(0x7<<8)
+#define MIRROR_EU_DISABLE_SS2				(0x7<<16)
+#define MIRROR_EU_DISABLE_SS3				(0x7<<24)
+
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d1a4de8..2942b11 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9514,9 +9514,13 @@ static int intel_gen9_queue_flip(struct drm_device *dev,
 
 	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
 	intel_ring_emit(ring, DERRMR);
+	/* WaClearRenderResponseMasks:bxt */
 	intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE |
 				DERRMR_PIPEB_PRI_FLIP_DONE |
-				DERRMR_PIPEC_PRI_FLIP_DONE));
+				DERRMR_PIPEC_PRI_FLIP_DONE |
+				DERRMR_PIPEA_SPR_FLIP_DONE |
+				DERRMR_PIPEB_SPR_FLIP_DONE |
+				DERRMR_PIPEC_SPR_FLIP_DONE));
 	intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) |
 			      MI_SRM_LRM_GLOBAL_GTT);
 	intel_ring_emit(ring, DERRMR);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8275b7a..369ae55 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -404,8 +404,8 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
 
 static void execlists_context_unqueue(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
-	struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
+	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
+	struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
 
 	assert_spin_locked(&ring->execlist_lock);
 
@@ -417,7 +417,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 				 execlist_link) {
 		if (!req0) {
 			req0 = cursor;
-		} else if (req0->request->ctx == cursor->request->ctx) {
+		} else if (req0->ctx == cursor->ctx) {
 			/* Same ctx: ignore first request, as second request
 			 * will update tail past first request's workload */
 			cursor->elsp_submitted = req0->elsp_submitted;
@@ -433,9 +433,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 
 	WARN_ON(req1 && req1->elsp_submitted);
 
-	execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
-				  req1 ? req1->request->ctx : NULL,
-				  req1 ? req1->request->tail : 0);
+	execlists_submit_contexts(ring, req0->ctx, req0->tail,
+				  req1 ? req1->ctx : NULL,
+				  req1 ? req1->tail : 0);
 
 	req0->elsp_submitted++;
 	if (req1)
@@ -445,17 +445,17 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 					   u32 request_id)
 {
-	struct intel_ctx_submit_request *head_req;
+	struct drm_i915_gem_request *head_req;
 
 	assert_spin_locked(&ring->execlist_lock);
 
 	head_req = list_first_entry_or_null(&ring->execlist_queue,
-					    struct intel_ctx_submit_request,
+					    struct drm_i915_gem_request,
 					    execlist_link);
 
 	if (head_req != NULL) {
 		struct drm_i915_gem_object *ctx_obj =
-				head_req->request->ctx->engine[ring->id].state;
+				head_req->ctx->engine[ring->id].state;
 		if (intel_execlists_ctx_id(ctx_obj) == request_id) {
 			WARN(head_req->elsp_submitted == 0,
 			     "Never submitted head request\n");
@@ -537,15 +537,11 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 				   u32 tail,
 				   struct drm_i915_gem_request *request)
 {
-	struct intel_ctx_submit_request *req = NULL, *cursor;
+	struct drm_i915_gem_request *cursor;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	int num_elements = 0;
 
-	req = kzalloc(sizeof(*req), GFP_KERNEL);
-	if (req == NULL)
-		return -ENOMEM;
-
 	if (to != ring->default_context)
 		intel_lr_context_pin(ring, to);
 
@@ -559,14 +555,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 		if (request == NULL)
 			return -ENOMEM;
 		request->ring = ring;
+		request->ctx = to;
 	} else {
 		WARN_ON(to != request->ctx);
 	}
-	request->ctx = to;
 	request->tail = tail;
-	req->request = request;
 	i915_gem_request_reference(request);
-	i915_gem_context_reference(req->request->ctx);
+	i915_gem_context_reference(request->ctx);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -577,13 +572,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 			break;
 
 	if (num_elements > 2) {
-		struct intel_ctx_submit_request *tail_req;
+		struct drm_i915_gem_request *tail_req;
 
 		tail_req = list_last_entry(&ring->execlist_queue,
-					   struct intel_ctx_submit_request,
+					   struct drm_i915_gem_request,
 					   execlist_link);
 
-		if (to == tail_req->request->ctx) {
+		if (to == tail_req->ctx) {
 			WARN(tail_req->elsp_submitted != 0,
 				"More than 2 already-submitted reqs queued\n");
 			list_del(&tail_req->execlist_link);
@@ -592,7 +587,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 		}
 	}
 
-	list_add_tail(&req->execlist_link, &ring->execlist_queue);
+	list_add_tail(&request->execlist_link, &ring->execlist_queue);
 	if (num_elements == 0)
 		execlists_context_unqueue(ring);
 
@@ -761,7 +756,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 
 void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req, *tmp;
+	struct drm_i915_gem_request *req, *tmp;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	struct list_head retired_list;
@@ -776,7 +771,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 	spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 	list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
-		struct intel_context *ctx = req->request->ctx;
+		struct intel_context *ctx = req->ctx;
 		struct drm_i915_gem_object *ctx_obj =
 				ctx->engine[ring->id].state;
 
@@ -784,9 +779,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 			intel_lr_context_unpin(ring, ctx);
 		intel_runtime_pm_put(dev_priv);
 		i915_gem_context_unreference(ctx);
-		i915_gem_request_unreference(req->request);
+		i915_gem_request_unreference(req);
 		list_del(&req->execlist_link);
-		kfree(req);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 80a80ea..6f2d7da 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -89,33 +89,6 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 			       u64 exec_start, u32 flags);
 u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
 
-/**
- * struct intel_ctx_submit_request - queued context submission request
- * @ctx: Context to submit to the ELSP.
- * @ring: Engine to submit it to.
- * @tail: how far in the context's ringbuffer this request goes to.
- * @execlist_link: link in the submission queue.
- * @work: workqueue for processing this request in a bottom half.
- * @elsp_submitted: no. of times this request has been sent to the ELSP.
- *
- * The ELSP only accepts two elements at a time, so we queue context/tail
- * pairs on a given queue (ring->execlist_queue) until the hardware is
- * available. The queue serves a double purpose: we also use it to keep track
- * of the up to 2 contexts currently in the hardware (usually one in execution
- * and the other queued up by the GPU): We only remove elements from the head
- * of the queue when the hardware informs us that an element has been
- * completed.
- *
- * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
- */
-struct intel_ctx_submit_request {
-	struct list_head execlist_link;
-
-	int elsp_submitted;
-
-	struct drm_i915_gem_request *request;
-};
-
 void intel_lrc_irq_handler(struct intel_engine_cs *ring);
 void intel_execlists_retire_requests(struct intel_engine_cs *ring);
 
-- 
2.1.1

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

Re: [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              354/354              352/354
ILK                 -1              354/354              353/354
SNB              +1-1              401/424              401/424
IVB                                  488/488              488/488
BYT                                  278/278              278/278
HSW                                  529/529              529/529
BDW                 -1              405/405              404/405
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_mixed_blits      PASS(2, M7M23)      NRUN(1, M23)
*PNV  igt_gen3_render_linear_blits      PASS(2, M7M23)      CRASH(1, M23)
*ILK  igt_gem_unfence_active_buffers      PASS(2, M37M26)      DMESG_WARN(1, M26)
 SNB  igt_kms_flip_flip-vs-dpms-off-vs-modeset-interruptible      NSPT(1, M35)PASS(5, M35M22)      PASS(1, M22)
*SNB  igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible      PASS(6, M35M22)      DMESG_WARN(1, M22)
*BDW  igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible      PASS(5, M30M28)      DMESG_WARN(1, M28)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Nick Hoath]

Move all remaining elements that were unique to execlists queue items
in to the associated request.

Issue: VIZ-4274

v2: Rebase. Fixed issue of overzealous freeing of request.
v3: Removed re-addition of cleanup work queue (found by Daniel Vetter)
v4: Rebase.
v5: Actual removal of intel_ctx_submit_request. Update both tail and postfix
pointer in __i915_add_request (found by Thomas Daniel)
v6: Removed unrelated changes

Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++----
 drivers/gpu/drm/i915/i915_drv.h      | 21 +++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c      | 12 +++++-----
 drivers/gpu/drm/i915/i915_reg.h      | 32 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  6 ++++-
 drivers/gpu/drm/i915/intel_lrc.c     | 44 ++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_lrc.h     | 27 ----------------------
 7 files changed, 87 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d4cc482..b2dfac4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1928,7 +1928,7 @@ static int i915_execlists(struct seq_file *m, void *data)
 	intel_runtime_pm_get(dev_priv);
 
 	for_each_ring(ring, dev_priv, ring_id) {
-		struct intel_ctx_submit_request *head_req = NULL;
+		struct drm_i915_gem_request *head_req = NULL;
 		int count = 0;
 		unsigned long flags;
 
@@ -1961,18 +1961,18 @@ static int i915_execlists(struct seq_file *m, void *data)
 		list_for_each(cursor, &ring->execlist_queue)
 			count++;
 		head_req = list_first_entry_or_null(&ring->execlist_queue,
-				struct intel_ctx_submit_request, execlist_link);
+				struct drm_i915_gem_request, execlist_link);
 		spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 		seq_printf(m, "\t%d requests in queue\n", count);
 		if (head_req) {
 			struct drm_i915_gem_object *ctx_obj;
 
-			ctx_obj = head_req->request->ctx->engine[ring_id].state;
+			ctx_obj = head_req->ctx->engine[ring_id].state;
 			seq_printf(m, "\tHead request id: %u\n",
 				   intel_execlists_ctx_id(ctx_obj));
 			seq_printf(m, "\tHead request tail: %u\n",
-				   head_req->request->tail);
+				   head_req->tail);
 		}
 
 		seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index af024f2..8c6a207 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2089,6 +2089,27 @@ struct drm_i915_gem_request {
 	struct list_head client_list;
 
 	uint32_t uniq;
+
+	/**
+	 * The ELSP only accepts two elements at a time, so we queue
+	 * context/tail pairs on a given queue (ring->execlist_queue) until the
+	 * hardware is available. The queue serves a double purpose: we also use
+	 * it to keep track of the up to 2 contexts currently in the hardware
+	 * (usually one in execution and the other queued up by the GPU): We
+	 * only remove elements from the head
+	 * of the queue when the hardware informs us that an element has been
+	 * completed.
+	 *
+	 * All accesses to the queue are mediated by a spinlock
+	 * (ring->execlist_lock).
+	 */
+
+	/** Execlist link in the submission queue.*/
+	struct list_head execlist_link;
+
+	/** Execlists no. of times this request has been sent to the ELSP */
+	int elsp_submitted;
+
 };
 
 void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0649559..0195e3f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2414,7 +2414,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct drm_i915_gem_request *request;
 	struct intel_ringbuffer *ringbuf;
-	u32 request_ring_position, request_start;
+	u32 request_start;
 	int ret;
 
 	request = ring->outstanding_lazy_request;
@@ -2449,7 +2449,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	 * GPU processing the request, we never over-estimate the
 	 * position of the head.
 	 */
-	request_ring_position = intel_ring_get_tail(ringbuf);
+	request->postfix = intel_ring_get_tail(ringbuf);
 
 	if (i915.enable_execlists) {
 		ret = ring->emit_request(ringbuf, request);
@@ -2462,7 +2462,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	}
 
 	request->head = request_start;
-	request->postfix = request_ring_position;
+	request->tail = intel_ring_get_tail(ringbuf);
 
 	/* Whilst this request exists, batch_obj will be on the
 	 * active_list, and so will hold the active reference. Only when this
@@ -2649,14 +2649,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	 * pinned in place.
 	 */
 	while (!list_empty(&ring->execlist_queue)) {
-		struct intel_ctx_submit_request *submit_req;
+		struct drm_i915_gem_request *submit_req;
 
 		submit_req = list_first_entry(&ring->execlist_queue,
-				struct intel_ctx_submit_request,
+				struct drm_i915_gem_request,
 				execlist_link);
 		list_del(&submit_req->execlist_link);
 		intel_runtime_pm_put(dev_priv);
-		i915_gem_context_unreference(submit_req->request->ctx);
+		i915_gem_context_unreference(submit_req->ctx);
 		kfree(submit_req);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8275b7a..369ae55 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -404,8 +404,8 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
 
 static void execlists_context_unqueue(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
-	struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
+	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
+	struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
 
 	assert_spin_locked(&ring->execlist_lock);
 
@@ -417,7 +417,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 				 execlist_link) {
 		if (!req0) {
 			req0 = cursor;
-		} else if (req0->request->ctx == cursor->request->ctx) {
+		} else if (req0->ctx == cursor->ctx) {
 			/* Same ctx: ignore first request, as second request
 			 * will update tail past first request's workload */
 			cursor->elsp_submitted = req0->elsp_submitted;
@@ -433,9 +433,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 
 	WARN_ON(req1 && req1->elsp_submitted);
 
-	execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
-				  req1 ? req1->request->ctx : NULL,
-				  req1 ? req1->request->tail : 0);
+	execlists_submit_contexts(ring, req0->ctx, req0->tail,
+				  req1 ? req1->ctx : NULL,
+				  req1 ? req1->tail : 0);
 
 	req0->elsp_submitted++;
 	if (req1)
@@ -445,17 +445,17 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 					   u32 request_id)
 {
-	struct intel_ctx_submit_request *head_req;
+	struct drm_i915_gem_request *head_req;
 
 	assert_spin_locked(&ring->execlist_lock);
 
 	head_req = list_first_entry_or_null(&ring->execlist_queue,
-					    struct intel_ctx_submit_request,
+					    struct drm_i915_gem_request,
 					    execlist_link);
 
 	if (head_req != NULL) {
 		struct drm_i915_gem_object *ctx_obj =
-				head_req->request->ctx->engine[ring->id].state;
+				head_req->ctx->engine[ring->id].state;
 		if (intel_execlists_ctx_id(ctx_obj) == request_id) {
 			WARN(head_req->elsp_submitted == 0,
 			     "Never submitted head request\n");
@@ -537,15 +537,11 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 				   u32 tail,
 				   struct drm_i915_gem_request *request)
 {
-	struct intel_ctx_submit_request *req = NULL, *cursor;
+	struct drm_i915_gem_request *cursor;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	int num_elements = 0;
 
-	req = kzalloc(sizeof(*req), GFP_KERNEL);
-	if (req == NULL)
-		return -ENOMEM;
-
 	if (to != ring->default_context)
 		intel_lr_context_pin(ring, to);
 
@@ -559,14 +555,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 		if (request == NULL)
 			return -ENOMEM;
 		request->ring = ring;
+		request->ctx = to;
 	} else {
 		WARN_ON(to != request->ctx);
 	}
-	request->ctx = to;
 	request->tail = tail;
-	req->request = request;
 	i915_gem_request_reference(request);
-	i915_gem_context_reference(req->request->ctx);
+	i915_gem_context_reference(request->ctx);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -577,13 +572,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 			break;
 
 	if (num_elements > 2) {
-		struct intel_ctx_submit_request *tail_req;
+		struct drm_i915_gem_request *tail_req;
 
 		tail_req = list_last_entry(&ring->execlist_queue,
-					   struct intel_ctx_submit_request,
+					   struct drm_i915_gem_request,
 					   execlist_link);
 
-		if (to == tail_req->request->ctx) {
+		if (to == tail_req->ctx) {
 			WARN(tail_req->elsp_submitted != 0,
 				"More than 2 already-submitted reqs queued\n");
 			list_del(&tail_req->execlist_link);
@@ -592,7 +587,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 		}
 	}
 
-	list_add_tail(&req->execlist_link, &ring->execlist_queue);
+	list_add_tail(&request->execlist_link, &ring->execlist_queue);
 	if (num_elements == 0)
 		execlists_context_unqueue(ring);
 
@@ -761,7 +756,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 
 void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 {
-	struct intel_ctx_submit_request *req, *tmp;
+	struct drm_i915_gem_request *req, *tmp;
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	unsigned long flags;
 	struct list_head retired_list;
@@ -776,7 +771,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 	spin_unlock_irqrestore(&ring->execlist_lock, flags);
 
 	list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
-		struct intel_context *ctx = req->request->ctx;
+		struct intel_context *ctx = req->ctx;
 		struct drm_i915_gem_object *ctx_obj =
 				ctx->engine[ring->id].state;
 
@@ -784,9 +779,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
 			intel_lr_context_unpin(ring, ctx);
 		intel_runtime_pm_put(dev_priv);
 		i915_gem_context_unreference(ctx);
-		i915_gem_request_unreference(req->request);
+		i915_gem_request_unreference(req);
 		list_del(&req->execlist_link);
-		kfree(req);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 80a80ea..6f2d7da 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -89,33 +89,6 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
 			       u64 exec_start, u32 flags);
 u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
 
-/**
- * struct intel_ctx_submit_request - queued context submission request
- * @ctx: Context to submit to the ELSP.
- * @ring: Engine to submit it to.
- * @tail: how far in the context's ringbuffer this request goes to.
- * @execlist_link: link in the submission queue.
- * @work: workqueue for processing this request in a bottom half.
- * @elsp_submitted: no. of times this request has been sent to the ELSP.
- *
- * The ELSP only accepts two elements at a time, so we queue context/tail
- * pairs on a given queue (ring->execlist_queue) until the hardware is
- * available. The queue serves a double purpose: we also use it to keep track
- * of the up to 2 contexts currently in the hardware (usually one in execution
- * and the other queued up by the GPU): We only remove elements from the head
- * of the queue when the hardware informs us that an element has been
- * completed.
- *
- * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
- */
-struct intel_ctx_submit_request {
-	struct list_head execlist_link;
-
-	int elsp_submitted;
-
-	struct drm_i915_gem_request *request;
-};
-
 void intel_lrc_irq_handler(struct intel_engine_cs *ring);
 void intel_execlists_retire_requests(struct intel_engine_cs *ring);
 
-- 
2.1.1

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

Re: [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5587
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  353/353              353/353
ILK                                  200/200              200/200
SNB                                  400/422              400/422
IVB                                  487/487              487/487
BYT                                  296/296              296/296
HSW                 -1              487/508              486/508
BDW                                  401/402              401/402
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 HSW  igt_kms_flip_event_leak      NSPT(2, M19)PASS(1, M19)      NSPT(1, M19)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request

[Tvrtko Ursulin]

On 01/15/2015 01:10 PM, Nick Hoath wrote:
>   void intel_execlists_retire_requests(struct intel_engine_cs *ring)
>   {
> -	struct intel_ctx_submit_request *req, *tmp;
> +	struct drm_i915_gem_request *req, *tmp;
>   	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>   	unsigned long flags;
>   	struct list_head retired_list;
> @@ -776,7 +771,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
>   	spin_unlock_irqrestore(&ring->execlist_lock, flags);
>
>   	list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
> -		struct intel_context *ctx = req->request->ctx;
> +		struct intel_context *ctx = req->ctx;
>   		struct drm_i915_gem_object *ctx_obj =
>   				ctx->engine[ring->id].state;
>
> @@ -784,9 +779,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
>   			intel_lr_context_unpin(ring, ctx);
>   		intel_runtime_pm_put(dev_priv);
>   		i915_gem_context_unreference(ctx);
> -		i915_gem_request_unreference(req->request);
> +		i915_gem_request_unreference(req);
>   		list_del(&req->execlist_link);

It looks like the this req unreference can be the last one in which case 
list_del explodes. I don't know if it was intended that it cannot be the 
last unreference, but I have a log which proves it can be. :)

Regards,

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