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_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a30cff2..2231214 100644
--- 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);
+ 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);
+ /* 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);
- /* 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;
- }
-
- 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;