[Intel-gfx] [RFC 4/4] drm/i915/guc: Repurpose GuC wq reservation

Michał Winiarski michal.winiarski at intel.com
Tue Mar 28 18:00:29 UTC 2017


Now that we're only driving GuC submissions from the tasklet, we can
simply skip the submission if GuC wq is full. This allows us to
completely remove reservation from the request_alloc path, and only
use it to manage wq between tasklets belonging to different engines.

Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Oscar Mateo <oscar.mateo at intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 54 ++++++++++++------------------
 drivers/gpu/drm/i915/intel_lrc.c           | 25 ++------------
 drivers/gpu/drm/i915/intel_uc.h            |  2 --
 3 files changed, 24 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 9975244..4a7ef70 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -407,11 +407,11 @@ static void guc_stage_desc_fini(struct intel_guc *guc,
 }
 
 /**
- * i915_guc_wq_reserve() - reserve space in the GuC's workqueue
- * @request:	request associated with the commands
+ * guc_wq_reserve() - reserve space in the GuC's workqueue
+ * @client:	GuC client used for submission
  *
  * Return:	0 if space is available
- *		-EAGAIN if space is not currently available
+ *		-ENOSPC if space is not currently available
  *
  * This function must be called (and must return 0) before a request
  * is submitted to the GuC via i915_guc_submit() below. Once a result
@@ -419,18 +419,17 @@ static void guc_stage_desc_fini(struct intel_guc *guc,
  * call to submit().
  *
  * Reservation allows the caller to determine in advance that space
- * will be available for the next submission before committing resources
- * to it, and helps avoid late failures with complicated recovery paths.
+ * will be available for the next submission.
  */
-int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
+static int guc_wq_reserve(struct i915_guc_client *client)
 {
 	const size_t wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
 	struct guc_process_desc *desc = __get_process_desc(client);
 	u32 freespace;
 	int ret;
 
-	spin_lock_irq(&client->wq_lock);
+	spin_lock(&client->wq_lock);
+
 	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
 	freespace -= client->wq_rsvd;
 	if (likely(freespace >= wqi_size)) {
@@ -438,29 +437,12 @@ int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
 		ret = 0;
 	} else {
 		client->no_wq_space++;
-		ret = -EAGAIN;
+		ret = -ENOSPC;
 	}
-	spin_unlock_irq(&client->wq_lock);
-
-	return ret;
-}
 
-static void guc_client_update_wq_rsvd(struct i915_guc_client *client, int size)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&client->wq_lock, flags);
-	client->wq_rsvd += size;
-	spin_unlock_irqrestore(&client->wq_lock, flags);
-}
+	spin_unlock(&client->wq_lock);
 
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request)
-{
-	const int wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
-
-	GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size);
-	guc_client_update_wq_rsvd(client, -wqi_size);
+	return ret;
 }
 
 /* Construct a Work Item and append it to the GuC's Work Queue */
@@ -475,7 +457,9 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	struct guc_wq_item *wqi;
 	u32 freespace, tail, wq_off;
 
-	/* Free space is guaranteed, see i915_guc_wq_reserve() above */
+	lockdep_assert_held(&client->wq_lock);
+
+	/* Free space is guaranteed, see guc_wq_reserve() above */
 	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
 	GEM_BUG_ON(freespace < wqi_size);
 
@@ -526,6 +510,7 @@ static void guc_reset_wq(struct i915_guc_client *client)
 	desc->tail = 0;
 
 	client->wq_tail = 0;
+	client->wq_rsvd = 0;
 }
 
 static int guc_ring_doorbell(struct i915_guc_client *client)
@@ -585,7 +570,7 @@ static int guc_ring_doorbell(struct i915_guc_client *client)
  * __i915_guc_submit() - Submit commands through GuC
  * @rq:		request associated with the commands
  *
- * The caller must have already called i915_guc_wq_reserve() above with
+ * The caller must have already called guc_wq_reserve() above with
  * a result of 0 (success), guaranteeing that there is space in the work
  * queue for the new request, so enqueuing the item cannot fail.
  *
@@ -603,14 +588,13 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	unsigned int engine_id = engine->id;
 	struct intel_guc *guc = &rq->i915->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
-	unsigned long flags;
 	int b_ret;
 
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
 		POSTING_READ_FW(GUC_STATUS);
 
-	spin_lock_irqsave(&client->wq_lock, flags);
+	spin_lock(&client->wq_lock);
 
 	guc_wq_item_append(client, rq);
 	b_ret = guc_ring_doorbell(client);
@@ -623,7 +607,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	guc->submissions[engine_id] += 1;
 	guc->last_seqno[engine_id] = rq->global_seqno;
 
-	spin_unlock_irqrestore(&client->wq_lock, flags);
+	spin_unlock(&client->wq_lock);
 }
 
 static void i915_guc_submit(struct drm_i915_gem_request *rq)
@@ -659,6 +643,7 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 {
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_gem_request *last = port[0].request;
+	struct i915_guc_client *client = engine->i915->guc.execbuf_client;
 	struct rb_node *rb;
 	bool submit = false;
 
@@ -680,6 +665,9 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		struct drm_i915_gem_request *rq =
 			rb_entry(rb, typeof(*rq), priotree.node);
 
+		if (guc_wq_reserve(client) != 0)
+			break;
+
 		if (last && rq->ctx != last->ctx) {
 			if (port != engine->execlist_port)
 				break;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ff34aba..4372a52 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -864,27 +864,14 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 	GEM_BUG_ON(!ce->ring);
 	request->ring = ce->ring;
 
-	if (i915.enable_guc_submission) {
-		/*
-		 * Check that the GuC has space for the request before
-		 * going any further, as the i915_add_request() call
-		 * later on mustn't fail ...
-		 */
-		ret = i915_guc_wq_reserve(request);
-		if (ret)
-			goto err;
-	}
-
 	cs = intel_ring_begin(request, 0);
-	if (IS_ERR(cs)) {
-		ret = PTR_ERR(cs);
-		goto err_unreserve;
-	}
+	if (IS_ERR(cs))
+		return(PTR_ERR(cs));
 
 	if (!ce->initialised) {
 		ret = engine->init_context(request);
 		if (ret)
-			goto err_unreserve;
+			return ret;
 
 		ce->initialised = true;
 	}
@@ -898,12 +885,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 
 	request->reserved_space -= EXECLISTS_REQUEST_SIZE;
 	return 0;
-
-err_unreserve:
-	if (i915.enable_guc_submission)
-		i915_guc_wq_unreserve(request);
-err:
-	return ret;
 }
 
 static void intel_lr_resubmit_requests(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 087192d..092583c 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -215,8 +215,6 @@ u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 /* i915_guc_submission.c */
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
-int i915_guc_wq_reserve(struct drm_i915_gem_request *rq);
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-- 
2.9.3



More information about the Intel-gfx mailing list