drm/i915/guc: Simplify GuC doorbell logic

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>

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a4c2d07..46ac609 100644 (file)
--- 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_file *m,
 
        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);
 

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a30cff2..2231214 100644 (file)
--- 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_guc_client *client)
 
        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 *client)
        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);
-
-       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;
+       db = __get_doorbell(client);
 
-               DRM_WARN("Cookie mismatch. Expected %d, found %d\n",
-                        db_cmp.cookie, db_ret.cookie);
+       /* 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);
 
-               /* 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;
-       }
-
-       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_engine_cs *engine)
                        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;
 

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index d410516..f505dcc 100644 (file)
--- 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;