drm/i915/guc: kill doorbell code and selftests

Instead of relying on the workqueue, the upcoming reworked GuC
submission flow will offer the host driver indipendent control over
the execution status of each context submitted to GuC. As part of this,
the doorbell usage model has been reworked, with each doorbell being
paired to a single lrc and a doorbell ring representing new work
available for that specific context. This mechanism, however, limits
the number of contexts that can be registered with GuC to the number of
doorbells, which is an undesired limitation. To avoid this limitation,
we requested the GuC team to also provide a H2G that will allow the host
to notify the GuC of work available for a specified lrc, so we can use
that mechanism instead of relying on the doorbells. We can therefore drop
the doorbell code we currently have, also given the fact that in the
unlikely case we'd want to switch back to using doorbells we'd have to
heavily rework it.
The workqueue will still have a use in the new interface to pass special
commands, so that code has been retained for now.

With the doorbells gone and the GuC client becoming even simpler, the
existing GuC selftests don't give us any meaningful coverage so we can
remove them as well. Some selftests might come with the new code, but
they will look different from what we have now so if doesn't seem worth
it to keep the file around in the meantime.

v2: fix comments and commit message (John)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191205220243.27403-3-daniele.ceraolospurio@intel.com

drivers/gpu/drm/i915/gt/uc/intel_guc.h

@@ -20,8 +20,8 @@ struct __guc_ads_blob;
 
 /*
  * Top level structure of GuC. It handles firmware loading and manages client
- * pool and doorbells. intel_guc owns a intel_guc_client to replace the legacy
- * ExecList submission.
+ * pool. intel_guc owns a intel_guc_client to replace the legacy ExecList
+ * submission.
  */
 struct intel_guc {
 	struct intel_uc_fw fw;
@@ -50,10 +50,6 @@ struct intel_guc {
 
 	struct intel_guc_client *execbuf_client;
 
-	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
-	/* Cyclic counter mod pagesize	*/
-	u32 db_cacheline;
-
 	/* Control params for fw initialization */
 	u32 params[GUC_CTL_MAX_DWORDS];
 

drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h

@@ -31,7 +31,7 @@
 
 #define GUC_DOORBELL_INVALID		256
 
-#define GUC_DB_SIZE			(PAGE_SIZE)
+#define GUC_PD_SIZE			(PAGE_SIZE)
 #define GUC_WQ_SIZE			(PAGE_SIZE * 2)
 
 /* Work queue item header definitions */

drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h

@@ -44,21 +44,14 @@ struct intel_guc_client {
 	/* bitmap of (host) engine ids */
 	u32 priority;
 	u32 stage_id;
-	u32 proc_desc_offset;
-
-	u16 doorbell_id;
-	unsigned long doorbell_offset;
 
 	/* Protects GuC client's WQ access */
 	spinlock_t wq_lock;
-
-	/* For testing purposes, use nop WQ items instead of real ones */
-	I915_SELFTEST_DECLARE(bool use_nop_wqi);
 };
 
 void intel_guc_submission_init_early(struct intel_guc *guc);
 int intel_guc_submission_init(struct intel_guc *guc);
-int intel_guc_submission_enable(struct intel_guc *guc);
+void intel_guc_submission_enable(struct intel_guc *guc);
 void intel_guc_submission_disable(struct intel_guc *guc);
 void intel_guc_submission_fini(struct intel_guc *guc);
 int intel_guc_preempt_work_create(struct intel_guc *guc);

drivers/gpu/drm/i915/gt/uc/intel_uc.c

@@ -486,11 +486,8 @@ int intel_uc_init_hw(struct intel_uc *uc)
 	if (ret)
 		goto err_communication;
 
-	if (intel_uc_supports_guc_submission(uc)) {
-		ret = intel_guc_submission_enable(guc);
-		if (ret)
-			goto err_communication;
-	}
+	if (intel_uc_supports_guc_submission(uc))
+		intel_guc_submission_enable(guc);
 
 	dev_info(i915->drm.dev, "%s firmware %s version %u.%u %s:%s\n",
 		 intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_GUC), guc->fw.path,

drivers/gpu/drm/i915/gt/uc/selftest_guc.c

-// SPDX-License-Identifier: MIT
-/*
- * Copyright © 2017 Intel Corporation
- */
-
-#include "i915_selftest.h"
-#include "gem/i915_gem_pm.h"
-
-/* max doorbell number + negative test for each client type */
-#define ATTEMPTS (GUC_NUM_DOORBELLS + GUC_CLIENT_PRIORITY_NUM)
-
-static struct intel_guc_client *clients[ATTEMPTS];
-
-static bool available_dbs(struct intel_guc *guc, u32 priority)
-{
-	unsigned long offset;
-	unsigned long end;
-	u16 id;
-
-	/* first half is used for normal priority, second half for high */
-	offset = 0;
-	end = GUC_NUM_DOORBELLS / 2;
-	if (priority <= GUC_CLIENT_PRIORITY_HIGH) {
-		offset = end;
-		end += offset;
-	}
-
-	id = find_next_zero_bit(guc->doorbell_bitmap, end, offset);
-	if (id < end)
-		return true;
-
-	return false;
-}
-
-static int check_all_doorbells(struct intel_guc *guc)
-{
-	u16 db_id;
-
-	pr_info_once("Max number of doorbells: %d", GUC_NUM_DOORBELLS);
-	for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id) {
-		if (!doorbell_ok(guc, db_id)) {
-			pr_err("doorbell %d, not ok\n", db_id);
-			return -EIO;
-		}
-	}
-
-	return 0;
-}
-
-static int ring_doorbell_nop(struct intel_guc_client *client)
-{
-	struct guc_process_desc *desc = __get_process_desc(client);
-	int err;
-
-	client->use_nop_wqi = true;
-
-	spin_lock_irq(&client->wq_lock);
-
-	guc_wq_item_append(client, 0, 0, 0, 0);
-	guc_ring_doorbell(client);
-
-	spin_unlock_irq(&client->wq_lock);
-
-	client->use_nop_wqi = false;
-
-	/* if there are no issues GuC will update the WQ head and keep the
-	 * WQ in active status
-	 */
-	err = wait_for(READ_ONCE(desc->head) == READ_ONCE(desc->tail), 10);
-	if (err) {
-		pr_err("doorbell %u ring failed!\n", client->doorbell_id);
-		return -EIO;
-	}
-
-	if (desc->wq_status != WQ_STATUS_ACTIVE) {
-		pr_err("doorbell %u ring put WQ in bad state (%u)!\n",
-		       client->doorbell_id, desc->wq_status);
-		return -EIO;
-	}
-
-	return 0;
-}
-
-/*
- * Basic client sanity check, handy to validate create_clients.
- */
-static int validate_client(struct intel_guc_client *client, int client_priority)
-{
-	if (client->priority != client_priority ||
-	    client->doorbell_id == GUC_DOORBELL_INVALID)
-		return -EINVAL;
-	else
-		return 0;
-}
-
-static bool client_doorbell_in_sync(struct intel_guc_client *client)
-{
-	return !client || doorbell_ok(client->guc, client->doorbell_id);
-}
-
-/*
- * Check that we're able to synchronize guc_clients with their doorbells
- *
- * We're creating clients and reserving doorbells once, at module load. During
- * module lifetime, GuC, doorbell HW, and i915 state may go out of sync due to
- * GuC being reset. In other words - GuC clients are still around, but the
- * status of their doorbells may be incorrect. This is the reason behind
- * validating that the doorbells status expected by the driver matches what the
- * GuC/HW have.
- */
-static int igt_guc_clients(void *arg)
-{
-	struct intel_gt *gt = arg;
-	struct intel_guc *guc = &gt->uc.guc;
-	intel_wakeref_t wakeref;
-	int err = 0;
-
-	GEM_BUG_ON(!HAS_GT_UC(gt->i915));
-	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-
-	err = check_all_doorbells(guc);
-	if (err)
-		goto unlock;
-
-	/*
-	 * Get rid of clients created during driver load because the test will
-	 * recreate them.
-	 */
-	guc_clients_disable(guc);
-	guc_clients_destroy(guc);
-	if (guc->execbuf_client) {
-		pr_err("guc_clients_destroy lied!\n");
-		err = -EINVAL;
-		goto unlock;
-	}
-
-	err = guc_clients_create(guc);
-	if (err) {
-		pr_err("Failed to create clients\n");
-		goto unlock;
-	}
-	GEM_BUG_ON(!guc->execbuf_client);
-
-	err = validate_client(guc->execbuf_client,
-			      GUC_CLIENT_PRIORITY_KMD_NORMAL);
-	if (err) {
-		pr_err("execbug client validation failed\n");
-		goto out;
-	}
-
-	/* the client should now have reserved a doorbell */
-	if (!has_doorbell(guc->execbuf_client)) {
-		pr_err("guc_clients_create didn't reserve doorbells\n");
-		err = -EINVAL;
-		goto out;
-	}
-
-	/* Now enable the clients */
-	guc_clients_enable(guc);
-
-	/* each client should now have received a doorbell */
-	if (!client_doorbell_in_sync(guc->execbuf_client)) {
-		pr_err("failed to initialize the doorbells\n");
-		err = -EINVAL;
-		goto out;
-	}
-
-	/*
-	 * Basic test - an attempt to reallocate a valid doorbell to the
-	 * client it is currently assigned should not cause a failure.
-	 */
-	err = create_doorbell(guc->execbuf_client);
-
-out:
-	/*
-	 * Leave clean state for other test, plus the driver always destroy the
-	 * clients during unload.
-	 */
-	guc_clients_disable(guc);
-	guc_clients_destroy(guc);
-	guc_clients_create(guc);
-	guc_clients_enable(guc);
-unlock:
-	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
-	return err;
-}
-
-/*
- * Create as many clients as number of doorbells. Note that there's already
- * client(s)/doorbell(s) created during driver load, but this test creates
- * its own and do not interact with the existing ones.
- */
-static int igt_guc_doorbells(void *arg)
-{
-	struct intel_gt *gt = arg;
-	struct intel_guc *guc = &gt->uc.guc;
-	intel_wakeref_t wakeref;
-	int i, err = 0;
-	u16 db_id;
-
-	GEM_BUG_ON(!HAS_GT_UC(gt->i915));
-	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-
-	err = check_all_doorbells(guc);
-	if (err)
-		goto unlock;
-
-	for (i = 0; i < ATTEMPTS; i++) {
-		clients[i] = guc_client_alloc(guc, i % GUC_CLIENT_PRIORITY_NUM);
-
-		if (!clients[i]) {
-			pr_err("[%d] No guc client\n", i);
-			err = -EINVAL;
-			goto out;
-		}
-
-		if (IS_ERR(clients[i])) {
-			if (PTR_ERR(clients[i]) != -ENOSPC) {
-				pr_err("[%d] unexpected error\n", i);
-				err = PTR_ERR(clients[i]);
-				goto out;
-			}
-
-			if (available_dbs(guc, i % GUC_CLIENT_PRIORITY_NUM)) {
-				pr_err("[%d] non-db related alloc fail\n", i);
-				err = -EINVAL;
-				goto out;
-			}
-
-			/* expected, ran out of dbs for this client type */
-			continue;
-		}
-
-		/*
-		 * The check below is only valid because we keep a doorbell
-		 * assigned during the whole life of the client.
-		 */
-		if (clients[i]->stage_id >= GUC_NUM_DOORBELLS) {
-			pr_err("[%d] more clients than doorbells (%d >= %d)\n",
-			       i, clients[i]->stage_id, GUC_NUM_DOORBELLS);
-			err = -EINVAL;
-			goto out;
-		}
-
-		err = validate_client(clients[i], i % GUC_CLIENT_PRIORITY_NUM);
-		if (err) {
-			pr_err("[%d] client_alloc sanity check failed!\n", i);
-			err = -EINVAL;
-			goto out;
-		}
-
-		db_id = clients[i]->doorbell_id;
-
-		err = __guc_client_enable(clients[i]);
-		if (err) {
-			pr_err("[%d] Failed to create a doorbell\n", i);
-			goto out;
-		}
-
-		/* doorbell id shouldn't change, we are holding the mutex */
-		if (db_id != clients[i]->doorbell_id) {
-			pr_err("[%d] doorbell id changed (%d != %d)\n",
-			       i, db_id, clients[i]->doorbell_id);
-			err = -EINVAL;
-			goto out;
-		}
-
-		err = check_all_doorbells(guc);
-		if (err)
-			goto out;
-
-		err = ring_doorbell_nop(clients[i]);
-		if (err)
-			goto out;
-	}
-
-out:
-	for (i = 0; i < ATTEMPTS; i++)
-		if (!IS_ERR_OR_NULL(clients[i])) {
-			__guc_client_disable(clients[i]);
-			guc_client_free(clients[i]);
-		}
-unlock:
-	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
-	return err;
-}
-
-int intel_guc_live_selftest(struct drm_i915_private *i915)
-{
-	static const struct i915_subtest tests[] = {
-		SUBTEST(igt_guc_clients),
-		SUBTEST(igt_guc_doorbells),
-	};
-
-	if (!USES_GUC_SUBMISSION(i915))
-		return 0;
-
-	return intel_gt_live_subtests(tests, &i915->gt);
-}

drivers/gpu/drm/i915/i915_debugfs.c

@@ -1805,17 +1805,10 @@ static int i915_guc_info(struct seq_file *m, void *data)
 
 	GEM_BUG_ON(!guc->execbuf_client);
 
-	seq_printf(m, "\nDoorbell map:\n");
-	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", client);
-	seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
+	seq_printf(m, "\tPriority %d, GuC stage index: %u\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);
+		   client->stage_id);
 	/* Add more as required ... */
 
 	return 0;

drivers/gpu/drm/i915/selftests/i915_live_selftests.h

@@ -37,7 +37,6 @@ selftest(reset, intel_reset_live_selftests)
 selftest(memory_region, intel_memory_region_live_selftests)
 selftest(hangcheck, intel_hangcheck_live_selftests)
 selftest(execlists, intel_execlists_live_selftests)
-selftest(guc, intel_guc_live_selftest)
 selftest(perf, i915_perf_live_selftests)
 /* Here be dragons: keep last to run last! */
 selftest(late_gt_pm, intel_gt_pm_late_selftests)