All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest
@ 2018-10-18  0:46 Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 2/6] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18  0:46 UTC (permalink / raw)
  To: intel-gfx

The test requires driver tweaks to avoid causing error messages
on intentionally-triggered errors and to stop accessing non
existing register. However, this is a pure GuC FW interface test
and should be covered by FW validation, so it isn't really worth
tweaking the driver for it and we're better off dropping it instead.

Testing the driver running out of doorbells is already covered by
igt_guc_doorbells

Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/selftests/intel_guc.c | 42 ----------------------
 1 file changed, 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
index bf27162fb327..464f7d5defad 100644
--- a/drivers/gpu/drm/i915/selftests/intel_guc.c
+++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
@@ -217,48 +217,6 @@ static int igt_guc_clients(void *args)
 	if (err)
 		goto out;
 
-	/*
-	 * Negative test - a client with no doorbell (invalid db id).
-	 * After destroying the doorbell, the db id is changed to
-	 * GUC_DOORBELL_INVALID and the firmware will reject any attempt to
-	 * allocate a doorbell with an invalid id (db has to be reserved before
-	 * allocation).
-	 */
-	destroy_doorbell(guc->execbuf_client);
-	if (client_doorbell_in_sync(guc->execbuf_client)) {
-		pr_err("destroy db did not work\n");
-		err = -EINVAL;
-		goto out;
-	}
-
-	unreserve_doorbell(guc->execbuf_client);
-
-	__create_doorbell(guc->execbuf_client);
-	err = __guc_allocate_doorbell(guc, guc->execbuf_client->stage_id);
-	if (err != -EIO) {
-		pr_err("unexpected (err = %d)", err);
-		goto out_db;
-	}
-
-	if (!available_dbs(guc, guc->execbuf_client->priority)) {
-		pr_err("doorbell not available when it should\n");
-		err = -EIO;
-		goto out_db;
-	}
-
-out_db:
-	/* clean after test */
-	__destroy_doorbell(guc->execbuf_client);
-	err = reserve_doorbell(guc->execbuf_client);
-	if (err) {
-		pr_err("failed to reserve back the doorbell back\n");
-	}
-	err = create_doorbell(guc->execbuf_client);
-	if (err) {
-		pr_err("recreate doorbell failed\n");
-		goto out;
-	}
-
 out:
 	/*
 	 * Leave clean state for other test, plus the driver always destroy the
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] drm/i915/guc: rename __create/destroy_doorbell
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
@ 2018-10-18  0:46 ` Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 3/6] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18  0:46 UTC (permalink / raw)
  To: intel-gfx

The 2 functions don't create or destroy anything, they just update the
doorbell state in memory. Use init and fini instead for clarity.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index eae668442ebe..b089e5283307 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -192,7 +192,7 @@ static struct guc_doorbell_info *__get_doorbell(struct intel_guc_client *client)
 	return client->vaddr + client->doorbell_offset;
 }
 
-static void __create_doorbell(struct intel_guc_client *client)
+static void __init_doorbell(struct intel_guc_client *client)
 {
 	struct guc_doorbell_info *doorbell;
 
@@ -201,7 +201,7 @@ static void __create_doorbell(struct intel_guc_client *client)
 	doorbell->cookie = 0;
 }
 
-static void __destroy_doorbell(struct intel_guc_client *client)
+static void __fini_doorbell(struct intel_guc_client *client)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
 	struct guc_doorbell_info *doorbell;
@@ -226,11 +226,11 @@ static int create_doorbell(struct intel_guc_client *client)
 		return -ENODEV; /* internal setup error, should never happen */
 
 	__update_doorbell_desc(client, client->doorbell_id);
-	__create_doorbell(client);
+	__init_doorbell(client);
 
 	ret = __guc_allocate_doorbell(client->guc, client->stage_id);
 	if (ret) {
-		__destroy_doorbell(client);
+		__fini_doorbell(client);
 		__update_doorbell_desc(client, GUC_DOORBELL_INVALID);
 		DRM_DEBUG_DRIVER("Couldn't create client %u doorbell: %d\n",
 				 client->stage_id, ret);
@@ -246,7 +246,7 @@ static int destroy_doorbell(struct intel_guc_client *client)
 
 	GEM_BUG_ON(!has_doorbell(client));
 
-	__destroy_doorbell(client);
+	__fini_doorbell(client);
 	ret = __guc_deallocate_doorbell(client->guc, client->stage_id);
 	if (ret)
 		DRM_ERROR("Couldn't destroy client %u doorbell: %d\n",
@@ -1087,7 +1087,7 @@ static void __guc_client_disable(struct intel_guc_client *client)
 	if (intel_guc_is_alive(client->guc))
 		destroy_doorbell(client);
 	else
-		__destroy_doorbell(client);
+		__fini_doorbell(client);
 
 	guc_stage_desc_fini(client);
 	guc_proc_desc_fini(client);
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] drm/i915/guc: reserve the doorbell before selecting the cacheline
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 2/6] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
@ 2018-10-18  0:46 ` Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18  0:46 UTC (permalink / raw)
  To: intel-gfx

Cacheline selection is only needed if we actually manage to reserve a
doorbell.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index b089e5283307..8c3b5a9facee 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -955,6 +955,10 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	}
 	client->vaddr = vaddr;
 
+	ret = reserve_doorbell(client);
+	if (ret)
+		goto err_vaddr;
+
 	client->doorbell_offset = __select_cacheline(guc);
 
 	/*
@@ -967,10 +971,6 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	else
 		client->proc_desc_offset = (GUC_DB_SIZE / 2);
 
-	ret = reserve_doorbell(client);
-	if (ret)
-		goto err_vaddr;
-
 	DRM_DEBUG_DRIVER("new priority %u client %p for engine(s) 0x%x: stage_id %u\n",
 			 priority, client, client->engines, client->stage_id);
 	DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%lx\n",
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 2/6] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 3/6] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
@ 2018-10-18  0:46 ` Daniele Ceraolo Spurio
  2018-10-18 12:50   ` Michal Wajdeczko
  2018-10-18 23:10   ` [PATCH v3] " Daniele Ceraolo Spurio
  2018-10-18  0:46 ` [PATCH v2 5/6] drm/i915/guc: fix comment about fallback to execlists Daniele Ceraolo Spurio
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18  0:46 UTC (permalink / raw)
  To: intel-gfx

A collection of very small cleanups/improvements around doorbell checking
that do not deserve their own patch:

- Move doorbell-related HW defs to intel_guc_reg.h

- use GUC_NUM_DOORBELLS instead of GUC_DOORBELL_INVALID where
  appropriate

- do not stop on error in guc_verify_doorbells

- do not print drbreg on error: the only content of the register
  apart from the valid bit is the lower part of the physical memory
  address, which we can't use even if valid because we don't know
  which descriptor it came from (since the doorbell is in an unexpected
  state)

- Move the checking of doorbell valid bit to a common helper.

v2: add more cleanups (move defs, use GUC_NUM_DOORBELLS, don't stop in
    guc_verify_doorbells) (Michal)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h       |  6 ++---
 drivers/gpu/drm/i915/intel_guc_reg.h        |  4 +++
 drivers/gpu/drm/i915/intel_guc_submission.c | 27 ++++++++++++---------
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index d1bbaba6e012..427c621f3e72 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -23,6 +23,8 @@
 #ifndef _INTEL_GUC_FWIF_H
 #define _INTEL_GUC_FWIF_H
 
+#include "intel_guc_reg.h"
+
 #define GUC_CLIENT_PRIORITY_KMD_HIGH	0
 #define GUC_CLIENT_PRIORITY_HIGH	1
 #define GUC_CLIENT_PRIORITY_KMD_NORMAL	2
@@ -59,9 +61,6 @@
 #define WQ_RING_TAIL_MAX		0x7FF	/* 2^11 QWords */
 #define WQ_RING_TAIL_MASK		(WQ_RING_TAIL_MAX << WQ_RING_TAIL_SHIFT)
 
-#define GUC_DOORBELL_ENABLED		1
-#define GUC_DOORBELL_DISABLED		0
-
 #define GUC_STAGE_DESC_ATTR_ACTIVE	BIT(0)
 #define GUC_STAGE_DESC_ATTR_PENDING_DB	BIT(1)
 #define GUC_STAGE_DESC_ATTR_KERNEL	BIT(2)
@@ -233,7 +232,6 @@ union guc_doorbell_qw {
 	u64 value_qw;
 } __packed;
 
-#define GUC_NUM_DOORBELLS	256
 #define GUC_DOORBELL_INVALID	(GUC_NUM_DOORBELLS)
 
 #define GUC_DB_SIZE			(PAGE_SIZE)
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index d86084742a4a..f438ee81dd1e 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -104,6 +104,10 @@
 #define GUC_SEND_INTERRUPT		_MMIO(0xc4c8)
 #define   GUC_SEND_TRIGGER		  (1<<0)
 
+#define GUC_NUM_DOORBELLS		256
+#define GUC_DOORBELL_ENABLED		1
+#define GUC_DOORBELL_DISABLED		0
+
 #define GEN8_DRBREGL(x)			_MMIO(0x1000 + (x) * 8)
 #define   GEN8_DRB_VALID		  (1<<0)
 #define GEN8_DRBREGU(x)			_MMIO(0x1000 + (x) * 8 + 4)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 8c3b5a9facee..b11d5eefcc88 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -192,6 +192,14 @@ static struct guc_doorbell_info *__get_doorbell(struct intel_guc_client *client)
 	return client->vaddr + client->doorbell_offset;
 }
 
+static bool __doorbell_valid(struct intel_guc *guc, u16 db_id)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
+	return I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;
+}
+
 static void __init_doorbell(struct intel_guc_client *client)
 {
 	struct guc_doorbell_info *doorbell;
@@ -203,7 +211,6 @@ static void __init_doorbell(struct intel_guc_client *client)
 
 static void __fini_doorbell(struct intel_guc_client *client)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
 	struct guc_doorbell_info *doorbell;
 	u16 db_id = client->doorbell_id;
 
@@ -214,7 +221,7 @@ static void __fini_doorbell(struct intel_guc_client *client)
 	 * to go to zero after updating db_status before we call the GuC to
 	 * release the doorbell
 	 */
-	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID), 10))
+	if (wait_for_us(!__doorbell_valid(client->guc, db_id), 10))
 		WARN_ONCE(true, "Doorbell never became invalid after disable\n");
 }
 
@@ -866,20 +873,17 @@ guc_reset_prepare(struct intel_engine_cs *engine)
 /* Check that a doorbell register is in the expected state */
 static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	u32 drbregl;
 	bool valid;
 
-	GEM_BUG_ON(db_id >= GUC_DOORBELL_INVALID);
+	GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
 
-	drbregl = I915_READ(GEN8_DRBREGL(db_id));
-	valid = drbregl & GEN8_DRB_VALID;
+	valid = __doorbell_valid(guc, db_id);
 
 	if (test_bit(db_id, guc->doorbell_bitmap) == valid)
 		return true;
 
-	DRM_DEBUG_DRIVER("Doorbell %d has unexpected state (0x%x): valid=%s\n",
-			 db_id, drbregl, yesno(valid));
+	DRM_DEBUG_DRIVER("Doorbell %u has unexpected state: valid=%s\n",
+			 db_id, yesno(valid));
 
 	return false;
 }
@@ -887,12 +891,13 @@ static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
 static bool guc_verify_doorbells(struct intel_guc *guc)
 {
 	u16 db_id;
+	bool doorbells_ok = true;
 
 	for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id)
 		if (!doorbell_ok(guc, db_id))
-			return false;
+			doorbells_ok = false;
 
-	return true;
+	return doorbells_ok;
 }
 
 /**
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] drm/i915/guc: fix comment about fallback to execlists
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (2 preceding siblings ...)
  2018-10-18  0:46 ` [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
@ 2018-10-18  0:46 ` Daniele Ceraolo Spurio
  2018-10-18 12:51   ` Michal Wajdeczko
  2018-10-18  0:46 ` [PATCH v2 6/6] HAX enable GuC for CI Daniele Ceraolo Spurio
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18  0:46 UTC (permalink / raw)
  To: intel-gfx

We stopped supporting fallback to execlists in commit 121981fafe69
(drm/i915/guc: Combine enable_guc_loading|submission modparams). We
do instead reset and retry in some cases, depending on the workarounds
required by the platform.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index a9e6fcce467c..e722bbc1fa1d 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -217,8 +217,8 @@ static int guc_wait_ucode(struct intel_guc *guc)
 	 * NB: Docs recommend not using the interrupt for completion.
 	 * Measurements indicate this should take no more than 20ms, so a
 	 * timeout here indicates that the GuC has failed and is unusable.
-	 * (Higher levels of the driver will attempt to fall back to
-	 * execlist mode if this happens.)
+	 * (Higher levels of the driver may decide to reset the GuC and
+	 * attempt the ucode load again if this happens.)
 	 */
 	ret = wait_for(guc_ready(guc, &status), 100);
 	DRM_DEBUG_DRIVER("GuC status %#x\n", status);
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] HAX enable GuC for CI
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (3 preceding siblings ...)
  2018-10-18  0:46 ` [PATCH v2 5/6] drm/i915/guc: fix comment about fallback to execlists Daniele Ceraolo Spurio
@ 2018-10-18  0:46 ` Daniele Ceraolo Spurio
  2018-10-18  1:07 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest Patchwork
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18  0:46 UTC (permalink / raw)
  To: intel-gfx

From: Michal Wajdeczko <michal.wajdeczko@intel.com>

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 7e56c516c815..c681537bcb92 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -45,7 +45,7 @@ struct drm_printer;
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc, 0) \
+	param(int, enable_guc, -1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (4 preceding siblings ...)
  2018-10-18  0:46 ` [PATCH v2 6/6] HAX enable GuC for CI Daniele Ceraolo Spurio
@ 2018-10-18  1:07 ` Patchwork
  2018-10-18  1:32 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-10-18  1:07 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest
URL   : https://patchwork.freedesktop.org/series/51153/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
fa13f8331474 drm/i915/guc: drop negative doorbell alloc selftest
39bc0cc1f17e drm/i915/guc: rename __create/destroy_doorbell
7f1415dbb6bb drm/i915/guc: reserve the doorbell before selecting the cacheline
40055de181e2 drm/i915/guc: doorbell checking cleanup
5c042442b76f drm/i915/guc: fix comment about fallback to execlists
-:6: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 121981fafe69 ("drm/i915/guc: Combine enable_guc_loading|submission modparams")'
#6: 
We stopped supporting fallback to execlists in commit 121981fafe69

total: 1 errors, 0 warnings, 0 checks, 10 lines checked
c16382c64357 HAX enable GuC for CI
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 8 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (5 preceding siblings ...)
  2018-10-18  1:07 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest Patchwork
@ 2018-10-18  1:32 ` Patchwork
  2018-10-18 12:21 ` [PATCH v2 1/6] " Michal Wajdeczko
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-10-18  1:32 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest
URL   : https://patchwork.freedesktop.org/series/51153/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_5000 -> Patchwork_10499 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10499 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10499, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51153/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10499:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_execlists:
      fi-whl-u:           PASS -> DMESG-WARN

    
== Known issues ==

  Here are the changes found in Patchwork_10499 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_execlists:
      fi-apl-guc:         PASS -> INCOMPLETE (fdo#106693)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-WARN (fdo#102614)
      fi-icl-u2:          PASS -> FAIL (fdo#103167)

    igt@pm_rpm@module-reload:
      fi-glk-j4005:       PASS -> FAIL (fdo#108338)

    igt@prime_vgem@basic-fence-flip:
      fi-cfl-8700k:       PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_guc:
      fi-apl-guc:         DMESG-WARN (fdo#107258) -> SKIP

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#108338 https://bugs.freedesktop.org/show_bug.cgi?id=108338


== Participating hosts (47 -> 38) ==

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-icl-u fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-pnv-d510 fi-kbl-7560u 


== Build changes ==

    * Linux: CI_DRM_5000 -> Patchwork_10499

  CI_DRM_5000: b9543c130d4f6edd76ec98090c46044ba6d9493e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4683: 7766b1e2348b32cc8ed58a972c6fd53b20279549 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10499: c16382c643579a54b64aeb78ded9afd75c3a16a9 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c16382c64357 HAX enable GuC for CI
5c042442b76f drm/i915/guc: fix comment about fallback to execlists
40055de181e2 drm/i915/guc: doorbell checking cleanup
7f1415dbb6bb drm/i915/guc: reserve the doorbell before selecting the cacheline
39bc0cc1f17e drm/i915/guc: rename __create/destroy_doorbell
fa13f8331474 drm/i915/guc: drop negative doorbell alloc selftest

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10499/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (6 preceding siblings ...)
  2018-10-18  1:32 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-10-18 12:21 ` Michal Wajdeczko
  2018-10-18 12:43   ` Chris Wilson
  2018-10-18 23:22 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2) Patchwork
  2018-10-18 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork
  9 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2018-10-18 12:21 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Thu, 18 Oct 2018 02:46:05 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> The test requires driver tweaks to avoid causing error messages
> on intentionally-triggered errors and to stop accessing non
> existing register. However, this is a pure GuC FW interface test
> and should be covered by FW validation, so it isn't really worth
> tweaking the driver for it and we're better off dropping it instead.
>
> Testing the driver running out of doorbells is already covered by
> igt_guc_doorbells
>
> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest
  2018-10-18 12:21 ` [PATCH v2 1/6] " Michal Wajdeczko
@ 2018-10-18 12:43   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2018-10-18 12:43 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, Michal Wajdeczko, intel-gfx

Quoting Michal Wajdeczko (2018-10-18 13:21:33)
> On Thu, 18 Oct 2018 02:46:05 +0200, Daniele Ceraolo Spurio  
> <daniele.ceraolospurio@intel.com> wrote:
> 
> > The test requires driver tweaks to avoid causing error messages
> > on intentionally-triggered errors and to stop accessing non
> > existing register. However, this is a pure GuC FW interface test
> > and should be covered by FW validation, so it isn't really worth
> > tweaking the driver for it and we're better off dropping it instead.
> >
> > Testing the driver running out of doorbells is already covered by
> > igt_guc_doorbells
> >
> > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

And dropped from testing, thanks.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup
  2018-10-18  0:46 ` [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
@ 2018-10-18 12:50   ` Michal Wajdeczko
  2018-10-18 23:10   ` [PATCH v3] " Daniele Ceraolo Spurio
  1 sibling, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2018-10-18 12:50 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Thu, 18 Oct 2018 02:46:08 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> A collection of very small cleanups/improvements around doorbell checking
> that do not deserve their own patch:
>
> - Move doorbell-related HW defs to intel_guc_reg.h
>
> - use GUC_NUM_DOORBELLS instead of GUC_DOORBELL_INVALID where
>   appropriate
>
> - do not stop on error in guc_verify_doorbells
>
> - do not print drbreg on error: the only content of the register
>   apart from the valid bit is the lower part of the physical memory
>   address, which we can't use even if valid because we don't know
>   which descriptor it came from (since the doorbell is in an unexpected
>   state)
>
> - Move the checking of doorbell valid bit to a common helper.
>
> v2: add more cleanups (move defs, use GUC_NUM_DOORBELLS, don't stop in
>     guc_verify_doorbells) (Michal)
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h       |  6 ++---
>  drivers/gpu/drm/i915/intel_guc_reg.h        |  4 +++
>  drivers/gpu/drm/i915/intel_guc_submission.c | 27 ++++++++++++---------
>  3 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h  
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index d1bbaba6e012..427c621f3e72 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -23,6 +23,8 @@
>  #ifndef _INTEL_GUC_FWIF_H
>  #define _INTEL_GUC_FWIF_H
> +#include "intel_guc_reg.h"

hmm, I would prefer to keep fwif definitions independent of
our other headers - see below

> +
>  #define GUC_CLIENT_PRIORITY_KMD_HIGH	0
>  #define GUC_CLIENT_PRIORITY_HIGH	1
>  #define GUC_CLIENT_PRIORITY_KMD_NORMAL	2
> @@ -59,9 +61,6 @@
>  #define WQ_RING_TAIL_MAX		0x7FF	/* 2^11 QWords */
>  #define WQ_RING_TAIL_MASK		(WQ_RING_TAIL_MAX << WQ_RING_TAIL_SHIFT)
> -#define GUC_DOORBELL_ENABLED		1
> -#define GUC_DOORBELL_DISABLED		0
> -
>  #define GUC_STAGE_DESC_ATTR_ACTIVE	BIT(0)
>  #define GUC_STAGE_DESC_ATTR_PENDING_DB	BIT(1)
>  #define GUC_STAGE_DESC_ATTR_KERNEL	BIT(2)
> @@ -233,7 +232,6 @@ union guc_doorbell_qw {
>  	u64 value_qw;
>  } __packed;
> -#define GUC_NUM_DOORBELLS	256
>  #define GUC_DOORBELL_INVALID	(GUC_NUM_DOORBELLS)

we don't have to use GUC_NUM_DOORBELLS here as it is pure
fw decision how to identify invalid index - and this could
change in future fw versions ie. to ~0, so for now:

#define GUC_DOORBELL_INVALID	256

> #define GUC_DB_SIZE			(PAGE_SIZE)
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h  
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index d86084742a4a..f438ee81dd1e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -104,6 +104,10 @@
>  #define GUC_SEND_INTERRUPT		_MMIO(0xc4c8)
>  #define   GUC_SEND_TRIGGER		  (1<<0)
> +#define GUC_NUM_DOORBELLS		256
> +#define GUC_DOORBELL_ENABLED		1
> +#define GUC_DOORBELL_DISABLED		0

ENABLED/DISABLED are related to guc_doorbell_info.db_status
so maybe we want to move that struct definition here too ?

> +
>  #define GEN8_DRBREGL(x)			_MMIO(0x1000 + (x) * 8)
>  #define   GEN8_DRB_VALID		  (1<<0)
>  #define GEN8_DRBREGU(x)			_MMIO(0x1000 + (x) * 8 + 4)
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c  
> b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 8c3b5a9facee..b11d5eefcc88 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -192,6 +192,14 @@ static struct guc_doorbell_info  
> *__get_doorbell(struct intel_guc_client *client)
>  	return client->vaddr + client->doorbell_offset;
>  }
> +static bool __doorbell_valid(struct intel_guc *guc, u16 db_id)
> +{
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
> +	GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
> +	return I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;
> +}
> +
>  static void __init_doorbell(struct intel_guc_client *client)
>  {
>  	struct guc_doorbell_info *doorbell;
> @@ -203,7 +211,6 @@ static void __init_doorbell(struct intel_guc_client  
> *client)
> static void __fini_doorbell(struct intel_guc_client *client)
>  {
> -	struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
>  	struct guc_doorbell_info *doorbell;
>  	u16 db_id = client->doorbell_id;
> @@ -214,7 +221,7 @@ static void __fini_doorbell(struct intel_guc_client  
> *client)
>  	 * to go to zero after updating db_status before we call the GuC to
>  	 * release the doorbell
>  	 */
> -	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),  
> 10))
> +	if (wait_for_us(!__doorbell_valid(client->guc, db_id), 10))
>  		WARN_ONCE(true, "Doorbell never became invalid after disable\n");
>  }
> @@ -866,20 +873,17 @@ guc_reset_prepare(struct intel_engine_cs *engine)
>  /* Check that a doorbell register is in the expected state */
>  static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
>  {
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -	u32 drbregl;
>  	bool valid;
> -	GEM_BUG_ON(db_id >= GUC_DOORBELL_INVALID);
> +	GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
> -	drbregl = I915_READ(GEN8_DRBREGL(db_id));
> -	valid = drbregl & GEN8_DRB_VALID;
> +	valid = __doorbell_valid(guc, db_id);
> 	if (test_bit(db_id, guc->doorbell_bitmap) == valid)
>  		return true;
> -	DRM_DEBUG_DRIVER("Doorbell %d has unexpected state (0x%x): valid=%s\n",
> -			 db_id, drbregl, yesno(valid));
> +	DRM_DEBUG_DRIVER("Doorbell %u has unexpected state: valid=%s\n",
> +			 db_id, yesno(valid));
> 	return false;
>  }
> @@ -887,12 +891,13 @@ static bool doorbell_ok(struct intel_guc *guc, u16  
> db_id)
>  static bool guc_verify_doorbells(struct intel_guc *guc)
>  {
>  	u16 db_id;
> +	bool doorbells_ok = true;
> 	for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id)
>  		if (!doorbell_ok(guc, db_id))
> -			return false;
> +			doorbells_ok = false;
> -	return true;
> +	return doorbells_ok;
>  }
> /**

with redefined GUC_DOORBELL_INVALID and guc_doorbell_info moved,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/6] drm/i915/guc: fix comment about fallback to execlists
  2018-10-18  0:46 ` [PATCH v2 5/6] drm/i915/guc: fix comment about fallback to execlists Daniele Ceraolo Spurio
@ 2018-10-18 12:51   ` Michal Wajdeczko
  0 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2018-10-18 12:51 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Thu, 18 Oct 2018 02:46:09 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> We stopped supporting fallback to execlists in commit 121981fafe69
> (drm/i915/guc: Combine enable_guc_loading|submission modparams). We
> do instead reset and retry in some cases, depending on the workarounds
> required by the platform.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3] drm/i915/guc: doorbell checking cleanup
  2018-10-18  0:46 ` [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
  2018-10-18 12:50   ` Michal Wajdeczko
@ 2018-10-18 23:10   ` Daniele Ceraolo Spurio
  2018-10-19 10:22     ` Michal Wajdeczko
  1 sibling, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-18 23:10 UTC (permalink / raw)
  To: intel-gfx

A collection of very small cleanups/improvements around doorbell checking
that do not deserve their own patch:

- Move doorbell-related HW defs to intel_guc_reg.h

- use GUC_NUM_DOORBELLS instead of GUC_DOORBELL_INVALID where
  appropriate

- do not stop on error in guc_verify_doorbells

- do not print drbreg on error: the only content of the register
  apart from the valid bit is the lower part of the physical memory
  address, which we can't use even if valid because we don't know
  which descriptor it came from (since the doorbell is in an unexpected
  state)

- Move the checking of doorbell valid bit to a common helper.

v2: add more cleanups (move defs, use GUC_NUM_DOORBELLS, don't stop in
    guc_verify_doorbells) (Michal)

v3: move more things to intel_guc_reg, redefine
    GUC_DOORBELL_INVALID (Michal), drop guc_doorbell_qw since it just
    duplicates guc_doorbell_info

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h       | 28 ++++-----------------
 drivers/gpu/drm/i915/intel_guc_reg.h        | 12 +++++++++
 drivers/gpu/drm/i915/intel_guc_submission.c | 27 ++++++++++++--------
 3 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index d1bbaba6e012..5e44bc0e5a9b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -39,6 +39,11 @@
 #define GUC_VIDEO_ENGINE2		4
 #define GUC_MAX_ENGINES_NUM		(GUC_VIDEO_ENGINE2 + 1)
 
+#define GUC_DOORBELL_INVALID		256
+
+#define GUC_DB_SIZE			(PAGE_SIZE)
+#define GUC_WQ_SIZE			(PAGE_SIZE * 2)
+
 /* Work queue item header definitions */
 #define WQ_STATUS_ACTIVE		1
 #define WQ_STATUS_SUSPENDED		2
@@ -59,9 +64,6 @@
 #define WQ_RING_TAIL_MAX		0x7FF	/* 2^11 QWords */
 #define WQ_RING_TAIL_MASK		(WQ_RING_TAIL_MAX << WQ_RING_TAIL_SHIFT)
 
-#define GUC_DOORBELL_ENABLED		1
-#define GUC_DOORBELL_DISABLED		0
-
 #define GUC_STAGE_DESC_ATTR_ACTIVE	BIT(0)
 #define GUC_STAGE_DESC_ATTR_PENDING_DB	BIT(1)
 #define GUC_STAGE_DESC_ATTR_KERNEL	BIT(2)
@@ -219,26 +221,6 @@ struct uc_css_header {
 	u32 header_info;
 } __packed;
 
-struct guc_doorbell_info {
-	u32 db_status;
-	u32 cookie;
-	u32 reserved[14];
-} __packed;
-
-union guc_doorbell_qw {
-	struct {
-		u32 db_status;
-		u32 cookie;
-	};
-	u64 value_qw;
-} __packed;
-
-#define GUC_NUM_DOORBELLS	256
-#define GUC_DOORBELL_INVALID	(GUC_NUM_DOORBELLS)
-
-#define GUC_DB_SIZE			(PAGE_SIZE)
-#define GUC_WQ_SIZE			(PAGE_SIZE * 2)
-
 /* Work item for submitting workloads into work queue of GuC. */
 struct guc_wq_item {
 	u32 header;
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index d86084742a4a..57e7ad522c2f 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -104,6 +104,18 @@
 #define GUC_SEND_INTERRUPT		_MMIO(0xc4c8)
 #define   GUC_SEND_TRIGGER		  (1<<0)
 
+#define GUC_NUM_DOORBELLS		256
+
+/* format of the HW-monitored doorbell cacheline */
+struct guc_doorbell_info {
+	u32 db_status;
+#define GUC_DOORBELL_DISABLED		0
+#define GUC_DOORBELL_ENABLED		1
+
+	u32 cookie;
+	u32 reserved[14];
+} __packed;
+
 #define GEN8_DRBREGL(x)			_MMIO(0x1000 + (x) * 8)
 #define   GEN8_DRB_VALID		  (1<<0)
 #define GEN8_DRBREGU(x)			_MMIO(0x1000 + (x) * 8 + 4)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 8c3b5a9facee..b11d5eefcc88 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -192,6 +192,14 @@ static struct guc_doorbell_info *__get_doorbell(struct intel_guc_client *client)
 	return client->vaddr + client->doorbell_offset;
 }
 
+static bool __doorbell_valid(struct intel_guc *guc, u16 db_id)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
+	return I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;
+}
+
 static void __init_doorbell(struct intel_guc_client *client)
 {
 	struct guc_doorbell_info *doorbell;
@@ -203,7 +211,6 @@ static void __init_doorbell(struct intel_guc_client *client)
 
 static void __fini_doorbell(struct intel_guc_client *client)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
 	struct guc_doorbell_info *doorbell;
 	u16 db_id = client->doorbell_id;
 
@@ -214,7 +221,7 @@ static void __fini_doorbell(struct intel_guc_client *client)
 	 * to go to zero after updating db_status before we call the GuC to
 	 * release the doorbell
 	 */
-	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID), 10))
+	if (wait_for_us(!__doorbell_valid(client->guc, db_id), 10))
 		WARN_ONCE(true, "Doorbell never became invalid after disable\n");
 }
 
@@ -866,20 +873,17 @@ guc_reset_prepare(struct intel_engine_cs *engine)
 /* Check that a doorbell register is in the expected state */
 static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	u32 drbregl;
 	bool valid;
 
-	GEM_BUG_ON(db_id >= GUC_DOORBELL_INVALID);
+	GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
 
-	drbregl = I915_READ(GEN8_DRBREGL(db_id));
-	valid = drbregl & GEN8_DRB_VALID;
+	valid = __doorbell_valid(guc, db_id);
 
 	if (test_bit(db_id, guc->doorbell_bitmap) == valid)
 		return true;
 
-	DRM_DEBUG_DRIVER("Doorbell %d has unexpected state (0x%x): valid=%s\n",
-			 db_id, drbregl, yesno(valid));
+	DRM_DEBUG_DRIVER("Doorbell %u has unexpected state: valid=%s\n",
+			 db_id, yesno(valid));
 
 	return false;
 }
@@ -887,12 +891,13 @@ static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
 static bool guc_verify_doorbells(struct intel_guc *guc)
 {
 	u16 db_id;
+	bool doorbells_ok = true;
 
 	for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id)
 		if (!doorbell_ok(guc, db_id))
-			return false;
+			doorbells_ok = false;
 
-	return true;
+	return doorbells_ok;
 }
 
 /**
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2)
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (7 preceding siblings ...)
  2018-10-18 12:21 ` [PATCH v2 1/6] " Michal Wajdeczko
@ 2018-10-18 23:22 ` Patchwork
  2018-10-18 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork
  9 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-10-18 23:22 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2)
URL   : https://patchwork.freedesktop.org/series/51153/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
fbae90ba884e drm/i915/guc: rename __create/destroy_doorbell
f4bc25c400b4 drm/i915/guc: reserve the doorbell before selecting the cacheline
747285afe5cd drm/i915/guc: doorbell checking cleanup
e85772877439 drm/i915/guc: fix comment about fallback to execlists
-:6: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 121981fafe69 ("drm/i915/guc: Combine enable_guc_loading|submission modparams")'
#6: 
We stopped supporting fallback to execlists in commit 121981fafe69

total: 1 errors, 0 warnings, 0 checks, 10 lines checked
94b98d416ac2 HAX enable GuC for CI
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 8 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2)
  2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
                   ` (8 preceding siblings ...)
  2018-10-18 23:22 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2) Patchwork
@ 2018-10-18 23:44 ` Patchwork
  9 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-10-18 23:44 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2)
URL   : https://patchwork.freedesktop.org/series/51153/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_5011 -> Patchwork_10510 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10510 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10510, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51153/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10510:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_execlists:
      fi-whl-u:           PASS -> INCOMPLETE
      fi-kbl-r:           PASS -> INCOMPLETE
      fi-skl-6260u:       PASS -> INCOMPLETE

    igt@drv_selftest@live_hangcheck:
      fi-cfl-s3:          PASS -> INCOMPLETE
      fi-whl-u:           PASS -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_10510 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362) +1

    igt@pm_rpm@module-reload:
      fi-skl-6600u:       PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@drv_getparams_basic@basic-subslice-total:
      fi-snb-2520m:       DMESG-WARN (fdo#103713) -> PASS +10

    igt@kms_frontbuffer_tracking@basic:
      fi-skl-6700hq:      INCOMPLETE -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS +1

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-cfl-8109u:       INCOMPLETE (fdo#106070, fdo#108126) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126


== Participating hosts (42 -> 39) ==

  Missing    (3): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


== Build changes ==

    * Linux: CI_DRM_5011 -> Patchwork_10510

  CI_DRM_5011: f66154472620fdd1f364c577ade311ce2b9d445a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4684: 6f27fddc6dd79c0486181b64201c6773c5c42a24 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10510: 94b98d416ac2d4f1c055e934daf2a41c6c4ebebd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

94b98d416ac2 HAX enable GuC for CI
e85772877439 drm/i915/guc: fix comment about fallback to execlists
747285afe5cd drm/i915/guc: doorbell checking cleanup
f4bc25c400b4 drm/i915/guc: reserve the doorbell before selecting the cacheline
fbae90ba884e drm/i915/guc: rename __create/destroy_doorbell

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10510/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] drm/i915/guc: doorbell checking cleanup
  2018-10-18 23:10   ` [PATCH v3] " Daniele Ceraolo Spurio
@ 2018-10-19 10:22     ` Michal Wajdeczko
  0 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2018-10-19 10:22 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Fri, 19 Oct 2018 01:10:10 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> A collection of very small cleanups/improvements around doorbell checking
> that do not deserve their own patch:
>
> - Move doorbell-related HW defs to intel_guc_reg.h
>
> - use GUC_NUM_DOORBELLS instead of GUC_DOORBELL_INVALID where
>   appropriate
>
> - do not stop on error in guc_verify_doorbells
>
> - do not print drbreg on error: the only content of the register
>   apart from the valid bit is the lower part of the physical memory
>   address, which we can't use even if valid because we don't know
>   which descriptor it came from (since the doorbell is in an unexpected
>   state)
>
> - Move the checking of doorbell valid bit to a common helper.
>
> v2: add more cleanups (move defs, use GUC_NUM_DOORBELLS, don't stop in
>     guc_verify_doorbells) (Michal)
>
> v3: move more things to intel_guc_reg, redefine
>     GUC_DOORBELL_INVALID (Michal), drop guc_doorbell_qw since it just
>     duplicates guc_doorbell_info
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-10-19 10:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-18  0:46 [PATCH v2 1/6] drm/i915/guc: drop negative doorbell alloc selftest Daniele Ceraolo Spurio
2018-10-18  0:46 ` [PATCH v2 2/6] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
2018-10-18  0:46 ` [PATCH v2 3/6] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
2018-10-18  0:46 ` [PATCH v2 4/6] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
2018-10-18 12:50   ` Michal Wajdeczko
2018-10-18 23:10   ` [PATCH v3] " Daniele Ceraolo Spurio
2018-10-19 10:22     ` Michal Wajdeczko
2018-10-18  0:46 ` [PATCH v2 5/6] drm/i915/guc: fix comment about fallback to execlists Daniele Ceraolo Spurio
2018-10-18 12:51   ` Michal Wajdeczko
2018-10-18  0:46 ` [PATCH v2 6/6] HAX enable GuC for CI Daniele Ceraolo Spurio
2018-10-18  1:07 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest Patchwork
2018-10-18  1:32 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-10-18 12:21 ` [PATCH v2 1/6] " Michal Wajdeczko
2018-10-18 12:43   ` Chris Wilson
2018-10-18 23:22 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/guc: drop negative doorbell alloc selftest (rev2) Patchwork
2018-10-18 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.