From 59db36cf4d67b74f5b8cb001493847464240d136 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Winiarski?= <michal.winiarski@intel.com>
Date: Thu, 14 Sep 2017 12:51:23 +0200
Subject: [PATCH] drm/i915/guc: Simplify GuC doorbell logic
Mime-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8bit
Git-commit: 59db36cf4d67b74f5b8cb001493847464240d136
Patch-mainline: v4.15-rc1
References: FATE#322643 bsc#1055900

All we're really doing is incrementing a simple counter in a
doorbell_info struct. We can do without extra variables and a separate
counter kept in guc_client. Since it's gone, we're also removing its
debugfs.
The only functional change here, is that we're no longer treating 0 as a
special value. GuC doesn't seem to care, why should we?

V2: Restore desc->tail update.
V3: Drop the retry loop, assert that doorbell cookie doesn't change
behind our back.
V4: WARN rather than BUG, use xchg. (Chris)

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>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170914105125.3031-1-michal.winiarski@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/gpu/drm/i915/i915_debugfs.c        |    4 -
 drivers/gpu/drm/i915/i915_guc_submission.c |   60 ++++++-----------------------
 drivers/gpu/drm/i915/intel_uc.h            |    1 
 3 files changed, 15 insertions(+), 50 deletions(-)

--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2445,8 +2445,8 @@ static void i915_guc_client_info(struct
 
 	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, cookie 0x%x\n",
-		client->doorbell_id, client->doorbell_offset, client->doorbell_cookie);
+	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);
 
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -192,13 +192,12 @@ static int __create_doorbell(struct i915
 
 	doorbell = __get_doorbell(client);
 	doorbell->db_status = GUC_DOORBELL_ENABLED;
-	doorbell->cookie = client->doorbell_cookie;
+	doorbell->cookie = 0;
 
 	err = __guc_allocate_doorbell(client->guc, client->stage_id);
-	if (err) {
+	if (err)
 		doorbell->db_status = GUC_DOORBELL_DISABLED;
-		doorbell->cookie = 0;
-	}
+
 	return err;
 }
 
@@ -466,57 +465,24 @@ static void guc_reset_wq(struct i915_guc
 	client->wq_tail = 0;
 }
 
-static int guc_ring_doorbell(struct i915_guc_client *client)
+static void guc_ring_doorbell(struct i915_guc_client *client)
 {
 	struct guc_process_desc *desc = __get_process_desc(client);
-	union guc_doorbell_qw db_cmp, db_exc, db_ret;
-	union guc_doorbell_qw *db;
-	int attempt = 2, ret = -EAGAIN;
+	struct guc_doorbell_info *db;
+	u32 cookie;
 
 	/* Update the tail so it is visible to GuC */
 	desc->tail = client->wq_tail;
 
-	/* current cookie */
-	db_cmp.db_status = GUC_DOORBELL_ENABLED;
-	db_cmp.cookie = client->doorbell_cookie;
-
-	/* cookie to be updated */
-	db_exc.db_status = GUC_DOORBELL_ENABLED;
-	db_exc.cookie = client->doorbell_cookie + 1;
-	if (db_exc.cookie == 0)
-		db_exc.cookie = 1;
-
 	/* pointer of current doorbell cacheline */
-	db = (union guc_doorbell_qw *)__get_doorbell(client);
+	db = __get_doorbell(client);
 
-	while (attempt--) {
-		/* lets ring the doorbell */
-		db_ret.value_qw = atomic64_cmpxchg((atomic64_t *)db,
-			db_cmp.value_qw, db_exc.value_qw);
-
-		/* if the exchange was successfully executed */
-		if (db_ret.value_qw == db_cmp.value_qw) {
-			/* db was successfully rung */
-			client->doorbell_cookie = db_exc.cookie;
-			ret = 0;
-			break;
-		}
-
-		/* XXX: doorbell was lost and need to acquire it again */
-		if (db_ret.db_status == GUC_DOORBELL_DISABLED)
-			break;
-
-		DRM_WARN("Cookie mismatch. Expected %d, found %d\n",
-			 db_cmp.cookie, db_ret.cookie);
-
-		/* update the cookie to newly read cookie from GuC */
-		db_cmp.cookie = db_ret.cookie;
-		db_exc.cookie = db_ret.cookie + 1;
-		if (db_exc.cookie == 0)
-			db_exc.cookie = 1;
-	}
+	/* we're not expecting the doorbell cookie to change behind our back */
+	cookie = READ_ONCE(db->cookie);
+	WARN_ON_ONCE(xchg(&db->cookie, cookie + 1) != cookie);
 
-	return ret;
+	/* XXX: doorbell was lost and need to acquire it again */
+	GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
 }
 
 /**
@@ -549,7 +515,7 @@ static void i915_guc_submit(struct intel
 			spin_lock(&client->wq_lock);
 
 			guc_wq_item_append(client, rq);
-			WARN_ON(guc_ring_doorbell(client));
+			guc_ring_doorbell(client);
 
 			client->submissions[engine_id] += 1;
 
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -66,7 +66,6 @@ struct i915_guc_client {
 
 	u16 doorbell_id;
 	unsigned long doorbell_offset;
-	u32 doorbell_cookie;
 
 	spinlock_t wq_lock;
 	uint32_t wq_offset;