drm/i915/guc: Remove client->submissions
diff mbox series

Message ID 20190814002145.29056-1-daniele.ceraolospurio@intel.com
State New
Headers show
Series
  • drm/i915/guc: Remove client->submissions
Related show

Commit Message

Daniele Ceraolo Spurio Aug. 14, 2019, 12:21 a.m. UTC
The engine->guc_id is GuC FW defined and it is not guaranteed to be
below I915_NUM_ENGINES, so we shouldn't use it with the i915-defined
client->submissions, as we might overflow.
Instead of fixing it, just get rid of client->submissions, because the
information we get from it is not interesting anymore now that we only
have 1 client.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 --
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |  2 --
 drivers/gpu/drm/i915/i915_debugfs.c           | 30 ++++---------------
 3 files changed, 6 insertions(+), 28 deletions(-)

Comments

Chris Wilson Aug. 14, 2019, 8 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2019-08-14 01:21:45)
> The engine->guc_id is GuC FW defined and it is not guaranteed to be
> below I915_NUM_ENGINES, so we shouldn't use it with the i915-defined
> client->submissions, as we might overflow.
> Instead of fixing it, just get rid of client->submissions, because the
> information we get from it is not interesting anymore now that we only
> have 1 client.

It was ever useful? Umpteen billion submissions here, umpteen billion
submissions there. Or the other extreme, 1. No more, no less :)
 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Chris Wilson Aug. 14, 2019, 8:10 a.m. UTC | #2
Quoting Chris Wilson (2019-08-14 09:00:18)
> Quoting Daniele Ceraolo Spurio (2019-08-14 01:21:45)
> > The engine->guc_id is GuC FW defined and it is not guaranteed to be
> > below I915_NUM_ENGINES, so we shouldn't use it with the i915-defined
> > client->submissions, as we might overflow.
> > Instead of fixing it, just get rid of client->submissions, because the
> > information we get from it is not interesting anymore now that we only
> > have 1 client.
> 
> It was ever useful? Umpteen billion submissions here, umpteen billion
> submissions there. Or the other extreme, 1. No more, no less :)
>  
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

And pushed.
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 54c3ae2c49ff..f325d3dd564f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -470,8 +470,6 @@  static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
 	guc_wq_item_append(client, engine->guc_id, ctx_desc,
 			   ring_tail, rq->fence.seqno);
 	guc_ring_doorbell(client);
-
-	client->submissions[engine->guc_id] += 1;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 963da3029af9..54d716828352 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -51,8 +51,6 @@  struct intel_guc_client {
 
 	/* Protects GuC client's WQ access */
 	spinlock_t wq_lock;
-	/* Per-engine counts of GuC submissions */
-	u64 submissions[I915_NUM_ENGINES];
 
 	/* For testing purposes, use nop WQ items instead of real ones */
 	I915_SELFTEST_DECLARE(bool use_nop_wqi);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2c640987c24d..9e2282d95ec3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1882,31 +1882,11 @@  static void i915_guc_log_info(struct seq_file *m,
 	}
 }
 
-static void i915_guc_client_info(struct seq_file *m,
-				 struct drm_i915_private *dev_priv,
-				 struct intel_guc_client *client)
-{
-	struct intel_engine_cs *engine;
-	u64 tot = 0;
-
-	seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
-		client->priority, client->stage_id, client->proc_desc_offset);
-	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
-		client->doorbell_id, client->doorbell_offset);
-
-	for_each_uabi_engine(engine, dev_priv) {
-		u64 submissions = client->submissions[engine->guc_id];
-		tot += submissions;
-		seq_printf(m, "\tSubmissions: %llu %s\n",
-				submissions, engine->name);
-	}
-	seq_printf(m, "\tTotal: %llu\n", tot);
-}
-
 static int i915_guc_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	const struct intel_guc *guc = &dev_priv->gt.uc.guc;
+	struct intel_guc_client *client = guc->execbuf_client;
 
 	if (!USES_GUC(dev_priv))
 		return -ENODEV;
@@ -1922,9 +1902,11 @@  static int i915_guc_info(struct seq_file *m, void *data)
 	seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
 	seq_printf(m, "Doorbell next cacheline: 0x%x\n", guc->db_cacheline);
 
-	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
-	i915_guc_client_info(m, dev_priv, guc->execbuf_client);
-
+	seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
+	seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
+		client->priority, client->stage_id, client->proc_desc_offset);
+	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
+		client->doorbell_id, client->doorbell_offset);
 	/* Add more as required ... */
 
 	return 0;