[Intel-gfx] [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine

[Dave Gordon]

When using a single GuC client for multiple engines, the i915 driver has
to merge all work items into a single work queue, which the GuC firmware
then demultiplexes into separate submission queues per engine. In
theory, this could lead to the single queue becoming a bottleneck in
which an excess of outstanding work for one or more engines might
prevent work for an idle engine reaching the hardware.

To reduce this risk, we can create one GuC client per engine. Each will
have its own workqueue, to be used only for work targeting a single
engine, so there will be no cross-engine contention for workqueue slots.

Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 25 ++++++++++++++++-----
 drivers/gpu/drm/i915/i915_guc_submission.c | 35 +++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_guc.h           |  2 +-
 3 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 90aef45..5cbb8ef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2570,20 +2570,26 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_guc guc;
-	struct i915_guc_client client = {};
+	struct i915_guc_client *clients;
 	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
 	u64 total = 0;
 
 	if (!HAS_GUC_SCHED(dev_priv))
 		return 0;
 
+	clients = kcalloc(I915_NUM_ENGINES, sizeof(*clients), GFP_KERNEL);
+	if (clients == NULL)
+		return -ENOMEM;
+
 	if (mutex_lock_interruptible(&dev->struct_mutex))
-		return 0;
+		goto done;
 
 	/* Take a local copy of the GuC data, so we can dump it at leisure */
 	guc = dev_priv->guc;
-	if (guc.execbuf_client)
-		client = *guc.execbuf_client;
+	for_each_engine_id(engine, dev_priv, id)
+		if (guc.exec_clients[id])
+			clients[id] = *guc.exec_clients[id];
 
 	mutex_unlock(&dev->struct_mutex);
 
@@ -2606,11 +2612,18 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	}
 	seq_printf(m, "\t%s: %llu\n", "Total", total);
 
-	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
-	i915_guc_client_info(m, dev_priv, &client);
+	for_each_engine_id(engine, dev_priv, id) {
+		seq_printf(m, "\nGuC exec_client[%d] @ %p:\n",
+				id, guc.exec_clients[id]);
+		if (guc.exec_clients[id])
+			i915_guc_client_info(m, dev_priv, &clients[id]);
+	}
 
 	/* Add more as required ... */
 
+done:
+	kfree(clients);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index dc5f485..b0f9945 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -434,7 +434,9 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
 int i915_guc_wq_check_space(struct drm_i915_gem_request *request)
 {
 	const size_t wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *gc = request->i915->guc.execbuf_client;
+	enum intel_engine_id engine_id = request->engine->id;
+	struct intel_guc *guc = &request->i915->guc;
+	struct i915_guc_client *gc = guc->exec_clients[engine_id];
 	struct guc_process_desc *desc;
 	u32 freespace;
 
@@ -589,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
 {
 	unsigned int engine_id = rq->engine->id;
 	struct intel_guc *guc = &rq->i915->guc;
-	struct i915_guc_client *client = guc->execbuf_client;
+	struct i915_guc_client *client = guc->exec_clients[engine_id];
 	int b_ret;
 
 	guc_add_workqueue_item(client, rq);
@@ -723,7 +725,7 @@ static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
  */
 static void guc_init_doorbell_hw(struct intel_guc *guc)
 {
-	struct i915_guc_client *client = guc->execbuf_client;
+	struct i915_guc_client *client = guc->exec_clients[RCS];
 	uint16_t db_id;
 	int i, err;
 
@@ -1004,17 +1006,21 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 	struct i915_guc_client *client;
+	struct intel_engine_cs *engine;
 
-	/* client for execbuf submission */
-	client = guc_client_alloc(dev_priv,
-				  GUC_CTX_PRIORITY_KMD_NORMAL,
-				  dev_priv->kernel_context);
-	if (!client) {
-		DRM_ERROR("Failed to create execbuf guc_client\n");
-		return -ENOMEM;
+	for_each_engine(engine, dev_priv) {
+		/* client for execbuf submission */
+		client = guc_client_alloc(dev_priv,
+					  GUC_CTX_PRIORITY_KMD_NORMAL,
+					  dev_priv->kernel_context);
+		if (!client) {
+			DRM_ERROR("Failed to create GuC client(s)\n");
+			return -ENOMEM;
+		}
+
+		guc->exec_clients[engine->id] = client;
 	}
 
-	guc->execbuf_client = client;
 	host2guc_sample_forcewake(guc, client);
 	guc_init_doorbell_hw(guc);
 
@@ -1024,9 +1030,12 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_engine_cs *engine;
 
-	guc_client_free(dev_priv, guc->execbuf_client);
-	guc->execbuf_client = NULL;
+	for_each_engine(engine, dev_priv) {
+		guc_client_free(dev_priv, guc->exec_clients[engine->id]);
+		guc->exec_clients[engine->id] = NULL;
+	}
 }
 
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 3e3e743..7b4cc4d 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,7 +132,7 @@ struct intel_guc {
 	struct drm_i915_gem_object *ctx_pool_obj;
 	struct ida ctx_ids;
 
-	struct i915_guc_client *execbuf_client;
+	struct i915_guc_client *exec_clients[I915_NUM_ENGINES];
 
 	DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS);
 	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
-- 
1.9.1