From a529a1c9db931a5d4fd6683199d73a8363bc7e83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Winiarski?= <michal.winiarski@intel.com>
Date: Mon, 18 Sep 2017 11:25:35 +0200
Subject: [PATCH] drm/i915/guc: Cleanup adding GuC work items
Mime-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8bit
Git-commit: a529a1c9db931a5d4fd6683199d73a8363bc7e83
Patch-mainline: v4.15-rc1
References: FATE#322643 bsc#1055900

We can just operate on the wq_tail directly (in the process descriptor).
This allows us to remove the duplicated tail from the client. While I'm
here let's also remove the constants kept in the client and document our
locking requirements. This causes a small change in one of GuC debugfs
files. We're no longer reporting constant values (which I don't think
is a problem), but we're also no longer reporting the tail (does anyone
care?).

V2: Update tail after wqi contents. (Chris)
V3: Really update tail after wqi contents.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20170918092536.12287-1-michal.winiarski@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/gpu/drm/i915/i915_debugfs.c        |    2 -
 drivers/gpu/drm/i915/i915_guc_submission.c |   37 ++++++++++++-----------------
 drivers/gpu/drm/i915/intel_uc.h            |    4 ---
 3 files changed, 16 insertions(+), 27 deletions(-)

--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2447,8 +2447,6 @@ static void i915_guc_client_info(struct
 		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);
-	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
-		client->wq_size, client->wq_offset, client->wq_tail);
 
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -305,7 +305,7 @@ static void guc_proc_desc_init(struct in
 	desc->db_base_addr = 0;
 
 	desc->stage_id = client->stage_id;
-	desc->wq_size_bytes = client->wq_size;
+	desc->wq_size_bytes = GUC_WQ_SIZE;
 	desc->wq_status = WQ_STATUS_ACTIVE;
 	desc->priority = client->priority;
 }
@@ -390,8 +390,8 @@ static void guc_stage_desc_init(struct i
 	desc->db_trigger_cpu = (uintptr_t)__get_doorbell(client);
 	desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
 	desc->process_desc = gfx_addr + client->proc_desc_offset;
-	desc->wq_addr = gfx_addr + client->wq_offset;
-	desc->wq_size = client->wq_size;
+	desc->wq_addr = gfx_addr + GUC_DB_SIZE;
+	desc->wq_size = GUC_WQ_SIZE;
 
 	desc->desc_private = (uintptr_t)client;
 }
@@ -416,14 +416,12 @@ static void guc_wq_item_append(struct i9
 	struct i915_gem_context *ctx = rq->ctx;
 	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_wq_item *wqi;
-	u32 freespace, tail, wq_off;
+	u32 ring_tail, wq_off;
 
-	/* Free space is guaranteed */
-	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
-	GEM_BUG_ON(freespace < wqi_size);
+	lockdep_assert_held(&client->wq_lock);
 
-	tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
-	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
+	ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
+	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
 
 	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
 	 * should not have the case where structure wqi is across page, neither
@@ -434,11 +432,11 @@ static void guc_wq_item_append(struct i9
 	 */
 	BUILD_BUG_ON(wqi_size != 16);
 
-	/* postincrement WQ tail for next time */
-	wq_off = client->wq_tail;
+	/* Free space is guaranteed. */
+	wq_off = READ_ONCE(desc->tail);
+	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
+			      GUC_WQ_SIZE) < wqi_size);
 	GEM_BUG_ON(wq_off & (wqi_size - 1));
-	client->wq_tail += wqi_size;
-	client->wq_tail &= client->wq_size - 1;
 
 	/* WQ starts from the page after doorbell / process_desc */
 	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
@@ -451,8 +449,11 @@ static void guc_wq_item_append(struct i9
 
 	wqi->context_desc = lower_32_bits(intel_lr_context_descriptor(ctx, engine));
 
-	wqi->submit_element_info = tail << WQ_RING_TAIL_SHIFT;
+	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
 	wqi->fence_id = rq->global_seqno;
+
+	/* Postincrement WQ tail for next time. */
+	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
 }
 
 static void guc_reset_wq(struct i915_guc_client *client)
@@ -461,18 +462,14 @@ static void guc_reset_wq(struct i915_guc
 
 	desc->head = 0;
 	desc->tail = 0;
-
-	client->wq_tail = 0;
 }
 
 static void guc_ring_doorbell(struct i915_guc_client *client)
 {
-	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_doorbell_info *db;
 	u32 cookie;
 
-	/* Update the tail so it is visible to GuC */
-	desc->tail = client->wq_tail;
+	lockdep_assert_held(&client->wq_lock);
 
 	/* pointer of current doorbell cacheline */
 	db = __get_doorbell(client);
@@ -812,8 +809,6 @@ guc_client_alloc(struct drm_i915_private
 	client->engines = engines;
 	client->priority = priority;
 	client->doorbell_id = GUC_DOORBELL_INVALID;
-	client->wq_offset = GUC_DB_SIZE;
-	client->wq_size = GUC_WQ_SIZE;
 	spin_lock_init(&client->wq_lock);
 
 	ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -68,10 +68,6 @@ struct i915_guc_client {
 	unsigned long doorbell_offset;
 
 	spinlock_t wq_lock;
-	uint32_t wq_offset;
-	uint32_t wq_size;
-	uint32_t wq_tail;
-
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
 };