All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell
@ 2018-10-17  0:17 Daniele Ceraolo Spurio
  2018-10-17  0:17 ` [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17  0:17 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>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 12 ++++++------
 drivers/gpu/drm/i915/selftests/intel_guc.c  |  4 ++--
 2 files changed, 8 insertions(+), 8 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);
diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
index bf27162fb327..c3b6f8d02aa0 100644
--- a/drivers/gpu/drm/i915/selftests/intel_guc.c
+++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
@@ -233,7 +233,7 @@ static int igt_guc_clients(void *args)
 
 	unreserve_doorbell(guc->execbuf_client);
 
-	__create_doorbell(guc->execbuf_client);
+	__init_doorbell(guc->execbuf_client);
 	err = __guc_allocate_doorbell(guc, guc->execbuf_client->stage_id);
 	if (err != -EIO) {
 		pr_err("unexpected (err = %d)", err);
@@ -248,7 +248,7 @@ static int igt_guc_clients(void *args)
 
 out_db:
 	/* clean after test */
-	__destroy_doorbell(guc->execbuf_client);
+	__fini_doorbell(guc->execbuf_client);
 	err = reserve_doorbell(guc->execbuf_client);
 	if (err) {
 		pr_err("failed to reserve back the doorbell back\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] 14+ messages in thread
* [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
@ 2018-10-17  0:17 ` Daniele Ceraolo Spurio
  2018-10-17 15:52   ` Michal Wajdeczko
  2018-10-17  0:17 ` [PATCH 3/4] drm/i915/guc: do not print drbreg on error Daniele Ceraolo Spurio
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17  0:17 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>
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] 14+ messages in thread
* [PATCH 3/4] drm/i915/guc: do not print drbreg on error
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
  2018-10-17  0:17 ` [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
@ 2018-10-17  0:17 ` Daniele Ceraolo Spurio
  2018-10-17 16:37   ` Michal Wajdeczko
  2018-10-17 18:30   ` [PATCH v2] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
  2018-10-17  0:17 ` [PATCH 4/4] HAX enable GuC for CI Daniele Ceraolo Spurio
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17  0:17 UTC (permalink / raw)
  To: intel-gfx

The only content of the register apart from the valid bit is the lower
part of the physical memory address. If the valid bit is 0 the address
is meaningless, while if it is 1 we don't know which descriptor it came
from (since the doorbell is in an unexpected state) so we can't match it
to an expected value. Since we already print the state of the valid bit,
stop priniting the full register contents as they're just confusing.
While at it, move the checking of the valid bit to a common helper.

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

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 8c3b5a9facee..cba84480dad9 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -192,6 +192,12 @@ 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);
+	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 +209,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 +219,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 +871,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);
 
-	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 %d has unexpected state: valid=%s\n",
+			 db_id, yesno(valid));
 
 	return false;
 }
-- 
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] 14+ messages in thread
* [PATCH 4/4] HAX enable GuC for CI
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
  2018-10-17  0:17 ` [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
  2018-10-17  0:17 ` [PATCH 3/4] drm/i915/guc: do not print drbreg on error Daniele Ceraolo Spurio
@ 2018-10-17  0:17 ` Daniele Ceraolo Spurio
  2018-10-17  0:27 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17  0:17 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] 14+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
                   ` (2 preceding siblings ...)
  2018-10-17  0:17 ` [PATCH 4/4] HAX enable GuC for CI Daniele Ceraolo Spurio
@ 2018-10-17  0:27 ` Patchwork
  2018-10-17  0:43 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-10-17  0:27 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell
URL   : https://patchwork.freedesktop.org/series/51090/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
7ece2bdc16d7 drm/i915/guc: rename __create/destroy_doorbell
0b8643c82cf6 drm/i915/guc: reserve the doorbell before selecting the cacheline
0b833f0bbd1e drm/i915/guc: do not print drbreg on error
-:28: WARNING:LINE_SPACING: Missing a blank line after declarations
#28: FILE: drivers/gpu/drm/i915/intel_guc_submission.c:198:
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	return I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;

total: 0 errors, 1 warnings, 0 checks, 50 lines checked
d1732e3157eb 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] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
                   ` (3 preceding siblings ...)
  2018-10-17  0:27 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell Patchwork
@ 2018-10-17  0:43 ` Patchwork
  2018-10-17 15:50 ` [PATCH 1/4] " Michal Wajdeczko
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-10-17  0:43 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell
URL   : https://patchwork.freedesktop.org/series/51090/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4991 -> Patchwork_10483 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10483 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10483, 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/51090/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_gem:
      fi-whl-u:           PASS -> INCOMPLETE
      fi-skl-6600u:       PASS -> INCOMPLETE
      fi-cfl-s3:          PASS -> INCOMPLETE
      fi-skl-iommu:       PASS -> INCOMPLETE
      fi-skl-6700k2:      PASS -> INCOMPLETE
      fi-skl-6700hq:      PASS -> INCOMPLETE
      fi-cfl-8109u:       PASS -> INCOMPLETE
      fi-kbl-7500u:       PASS -> INCOMPLETE
      fi-cfl-8700k:       PASS -> INCOMPLETE
      fi-skl-6770hq:      PASS -> INCOMPLETE
      fi-skl-6260u:       PASS -> INCOMPLETE
      fi-kbl-7567u:       PASS -> INCOMPLETE
      fi-kbl-x1275:       PASS -> INCOMPLETE
      fi-kbl-8809g:       PASS -> INCOMPLETE
      fi-kbl-r:           PASS -> INCOMPLETE

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gem:
      fi-skl-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)
      fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)

    igt@gem_exec_suspend@basic-s3:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

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

    
    ==== Possible fixes ====

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

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

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (40 -> 37) ==

  Additional (1): fi-kbl-guc 
  Missing    (4): fi-byt-squawks fi-ilk-m540 fi-bxt-dsi fi-bsw-cyan 


== Build changes ==

    * Linux: CI_DRM_4991 -> Patchwork_10483

  CI_DRM_4991: c4fc9e99d838c9b9a3f6bdb6e609b0cf820f9aef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4682: 0ac43db33e116b546e5704fe0b4dde21f391e09c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10483: d1732e3157ebe92fefa0541e0392bb8bb8cde8ee @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d1732e3157eb HAX enable GuC for CI
0b833f0bbd1e drm/i915/guc: do not print drbreg on error
0b8643c82cf6 drm/i915/guc: reserve the doorbell before selecting the cacheline
7ece2bdc16d7 drm/i915/guc: rename __create/destroy_doorbell

== Logs ==

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
                   ` (4 preceding siblings ...)
  2018-10-17  0:43 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-10-17 15:50 ` Michal Wajdeczko
  2018-10-17 18:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2) Patchwork
  2018-10-17 19:10 ` ✗ Fi.CI.BAT: failure " Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Michal Wajdeczko @ 2018-10-17 15:50 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Wed, 17 Oct 2018 02:17:15 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> 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>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline
  2018-10-17  0:17 ` [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
@ 2018-10-17 15:52   ` Michal Wajdeczko
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Wajdeczko @ 2018-10-17 15:52 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Wed, 17 Oct 2018 02:17:16 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> Cacheline selection is only needed if we actually manage to reserve a
> doorbell.
>
> 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>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] drm/i915/guc: do not print drbreg on error
  2018-10-17  0:17 ` [PATCH 3/4] drm/i915/guc: do not print drbreg on error Daniele Ceraolo Spurio
@ 2018-10-17 16:37   ` Michal Wajdeczko
  2018-10-17 16:58     ` Daniele Ceraolo Spurio
  2018-10-17 18:30   ` [PATCH v2] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
  1 sibling, 1 reply; 14+ messages in thread
From: Michal Wajdeczko @ 2018-10-17 16:37 UTC (permalink / raw)
  To: intel-gfx, Daniele Ceraolo Spurio

On Wed, 17 Oct 2018 02:17:17 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> The only content of the register apart from the valid bit is the lower
> part of the physical memory address. If the valid bit is 0 the address
> is meaningless, while if it is 1 we don't know which descriptor it came
> from (since the doorbell is in an unexpected state) so we can't match it
> to an expected value. Since we already print the state of the valid bit,
> stop priniting the full register contents as they're just confusing.

typo in priniting

> While at it, move the checking of the valid bit to a common helper.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_submission.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c  
> b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 8c3b5a9facee..cba84480dad9 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -192,6 +192,12 @@ 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);

should we add GEM_BUG_ON(db_id > GUC_NUM_DOORBELLS) here ?

> +	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 +209,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 +219,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 +871,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);

btw, maybe better to check against GUC_NUM_DOORBELLS ?
GUC_DOORBELL_INVALID looks like a fw defined value

and is it ok that we define GUC_NUM_DOORBELLS in intel_guc_fwif.h ?
maybe better place for it is near GEN8_DRBREGL in intel_guc_reg.h

> -	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 %d has unexpected state: valid=%s\n",
> +			 db_id, yesno(valid));

db_id is unsigned so you can use %u (or %hu) for it

> 	return false;
>  }

later in guc_verify_doorbells() we are stopping after detecting first
mismatched doorbell - maybe we should scan all doorbells to get debug
logs for all unexpected states?

with all these nitpicks hopefully accepted,

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

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] drm/i915/guc: do not print drbreg on error
  2018-10-17 16:37   ` Michal Wajdeczko
@ 2018-10-17 16:58     ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17 16:58 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 17/10/18 09:37, Michal Wajdeczko wrote:
> On Wed, 17 Oct 2018 02:17:17 +0200, Daniele Ceraolo Spurio 
> <daniele.ceraolospurio@intel.com> wrote:
> 
>> The only content of the register apart from the valid bit is the lower
>> part of the physical memory address. If the valid bit is 0 the address
>> is meaningless, while if it is 1 we don't know which descriptor it came
>> from (since the doorbell is in an unexpected state) so we can't match it
>> to an expected value. Since we already print the state of the valid bit,
>> stop priniting the full register contents as they're just confusing.
> 
> typo in priniting
> 
>> While at it, move the checking of the valid bit to a common helper.
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_submission.c | 18 ++++++++++--------
>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c 
>> b/drivers/gpu/drm/i915/intel_guc_submission.c
>> index 8c3b5a9facee..cba84480dad9 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
>> @@ -192,6 +192,12 @@ 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);
> 
> should we add GEM_BUG_ON(db_id > GUC_NUM_DOORBELLS) here ?
> 

Can do. I didn't to begin with because we should catch invalid IDs 
before we reach here, but an extra debug check won't hurt

>> +    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 +209,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 +219,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 +871,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);
> 
> btw, maybe better to check against GUC_NUM_DOORBELLS ?
> GUC_DOORBELL_INVALID looks like a fw defined value
> 

It is, but even in FW it is defined as equal to GUC_NUM_DOORBELLS

> and is it ok that we define GUC_NUM_DOORBELLS in intel_guc_fwif.h ?
> maybe better place for it is near GEN8_DRBREGL in intel_guc_reg.h
> 

The problem with that is that we'd have to either move 
GUC_DOORBELL_INVALID as well or include intel_guc_reg.h from 
intel_guc_fwif.h. The latter should be semantically ok I think, since 
some aspects of the interface are dependent on HW

>> -    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 %d has unexpected state: valid=%s\n",
>> +             db_id, yesno(valid));
> 
> db_id is unsigned so you can use %u (or %hu) for it
> 

ack

>>     return false;
>>  }
> 
> later in guc_verify_doorbells() we are stopping after detecting first
> mismatched doorbell - maybe we should scan all doorbells to get debug
> logs for all unexpected states?

ack

Thanks,
Daniele

> 
> with all these nitpicks hopefully accepted,
> 
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 
> Regards,
> Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2] drm/i915/guc: doorbell checking cleanup
  2018-10-17  0:17 ` [PATCH 3/4] drm/i915/guc: do not print drbreg on error Daniele Ceraolo Spurio
  2018-10-17 16:37   ` Michal Wajdeczko
@ 2018-10-17 18:30   ` Daniele Ceraolo Spurio
  1 sibling, 0 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17 18:30 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 1a853cc627e3..9cb41f6a0924 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] 14+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2)
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
                   ` (5 preceding siblings ...)
  2018-10-17 15:50 ` [PATCH 1/4] " Michal Wajdeczko
@ 2018-10-17 18:47 ` Patchwork
  2018-10-17 19:10 ` ✗ Fi.CI.BAT: failure " Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-10-17 18:47 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2)
URL   : https://patchwork.freedesktop.org/series/51090/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f79da532bd81 drm/i915/guc: rename __create/destroy_doorbell
641964fa483b drm/i915/guc: reserve the doorbell before selecting the cacheline
61e396e88d17 drm/i915/guc: doorbell checking cleanup
880bdc1de4f8 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] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2)
  2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
                   ` (6 preceding siblings ...)
  2018-10-17 18:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2) Patchwork
@ 2018-10-17 19:10 ` Patchwork
  2018-10-17 19:52   ` Daniele Ceraolo Spurio
  7 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2018-10-17 19:10 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2)
URL   : https://patchwork.freedesktop.org/series/51090/
State : failure

== Summary ==

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

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10492 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10492, 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/51090/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_execlists:
      fi-skl-iommu:       PASS -> INCOMPLETE

    igt@drv_selftest@live_guc:
      fi-kbl-7567u:       PASS -> INCOMPLETE
      fi-skl-6600u:       PASS -> INCOMPLETE
      fi-skl-6260u:       PASS -> INCOMPLETE
      fi-skl-6700k2:      PASS -> INCOMPLETE
      fi-whl-u:           PASS -> INCOMPLETE
      fi-skl-6770hq:      PASS -> INCOMPLETE
      fi-kbl-7560u:       PASS -> INCOMPLETE
      fi-kbl-8809g:       PASS -> INCOMPLETE
      fi-kbl-r:           PASS -> INCOMPLETE
      fi-kbl-x1275:       PASS -> INCOMPLETE
      fi-skl-6700hq:      PASS -> INCOMPLETE
      fi-cfl-s3:          PASS -> INCOMPLETE
      fi-cfl-8109u:       PASS -> INCOMPLETE
      fi-kbl-7500u:       PASS -> INCOMPLETE
      fi-cfl-8700k:       PASS -> INCOMPLETE

    igt@drv_selftest@live_hangcheck:
      fi-skl-6600u:       PASS -> DMESG-WARN

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_guc:
      fi-skl-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
      fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)

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

    
    ==== Possible fixes ====

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

    
    ==== Warnings ====

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

    
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
  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


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

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u 


== Build changes ==

    * Linux: CI_DRM_5000 -> Patchwork_10492

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


== Linux commits ==

880bdc1de4f8 HAX enable GuC for CI
61e396e88d17 drm/i915/guc: doorbell checking cleanup
641964fa483b drm/i915/guc: reserve the doorbell before selecting the cacheline
f79da532bd81 drm/i915/guc: rename __create/destroy_doorbell

== Logs ==

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2)
  2018-10-17 19:10 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-10-17 19:52   ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-17 19:52 UTC (permalink / raw)
  To: intel-gfx



On 17/10/18 12:10, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2)
> URL   : https://patchwork.freedesktop.org/series/51090/
> State : failure
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_5000 -> Patchwork_10492 =
> 
> == Summary - FAILURE ==
> 
>    Serious unknown changes coming with Patchwork_10492 absolutely need to be
>    verified manually.
>    
>    If you think the reported changes have nothing to do with the changes
>    introduced in Patchwork_10492, 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/51090/revisions/2/mbox/
> 
> == Possible new issues ==
> 
>    Here are the unknown changes that may have been introduced in Patchwork_10492:
> 
>    === IGT changes ===
> 
>      ==== Possible regressions ====
> 
>      igt@drv_selftest@live_execlists:
>        fi-skl-iommu:       PASS -> INCOMPLETE
> 
>      igt@drv_selftest@live_guc:
>        fi-kbl-7567u:       PASS -> INCOMPLETE
>        fi-skl-6600u:       PASS -> INCOMPLETE
>        fi-skl-6260u:       PASS -> INCOMPLETE
>        fi-skl-6700k2:      PASS -> INCOMPLETE
>        fi-whl-u:           PASS -> INCOMPLETE
>        fi-skl-6770hq:      PASS -> INCOMPLETE
>        fi-kbl-7560u:       PASS -> INCOMPLETE
>        fi-kbl-8809g:       PASS -> INCOMPLETE
>        fi-kbl-r:           PASS -> INCOMPLETE
>        fi-kbl-x1275:       PASS -> INCOMPLETE
>        fi-skl-6700hq:      PASS -> INCOMPLETE
>        fi-cfl-s3:          PASS -> INCOMPLETE
>        fi-cfl-8109u:       PASS -> INCOMPLETE
>        fi-kbl-7500u:       PASS -> INCOMPLETE
>        fi-cfl-8700k:       PASS -> INCOMPLETE
> 

This is the invalid doorbell selfest trying to poll a doorbell register 
that doesn't exist, now caught because of the new BUG_ON. Given that it 
is a pure GuC FW interface test I don't think it is worth updating the 
code to detect the invalid case and act on it, so I'm just going to 
remove it as Michal suggested. We do have coverage for the i915 code 
running out of doorbells in the guc_doorbells selftest.

Daniele

>      igt@drv_selftest@live_hangcheck:
>        fi-skl-6600u:       PASS -> DMESG-WARN
> 
>      
> == Known issues ==
> 
>    Here are the changes found in Patchwork_10492 that come from known issues:
> 
>    === IGT changes ===
> 
>      ==== Issues hit ====
> 
>      igt@drv_selftest@live_guc:
>        fi-skl-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)
>        fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
>        fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)
> 
>      igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
>        fi-byt-clapper:     PASS -> FAIL (fdo#107362)
> 
>      
>      ==== Possible fixes ====
> 
>      igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
>        fi-byt-clapper:     FAIL (fdo#107362) -> PASS
> 
>      
>      ==== Warnings ====
> 
>      igt@drv_selftest@live_guc:
>        fi-apl-guc:         DMESG-WARN (fdo#107258) -> INCOMPLETE (fdo#106693)
> 
>      
>    fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
>    fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
>    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
> 
> 
> == Participating hosts (47 -> 41) ==
> 
>    Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u
> 
> 
> == Build changes ==
> 
>      * Linux: CI_DRM_5000 -> Patchwork_10492
> 
>    CI_DRM_5000: b9543c130d4f6edd76ec98090c46044ba6d9493e @ git://anongit.freedesktop.org/gfx-ci/linux
>    IGT_4683: 7766b1e2348b32cc8ed58a972c6fd53b20279549 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>    Patchwork_10492: 880bdc1de4f866988489380c18c9a051f3ab01d7 @ git://anongit.freedesktop.org/gfx-ci/linux
> 
> 
> == Linux commits ==
> 
> 880bdc1de4f8 HAX enable GuC for CI
> 61e396e88d17 drm/i915/guc: doorbell checking cleanup
> 641964fa483b drm/i915/guc: reserve the doorbell before selecting the cacheline
> f79da532bd81 drm/i915/guc: rename __create/destroy_doorbell
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10492/issues.html
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-10-17 19:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17  0:17 [PATCH 1/4] drm/i915/guc: rename __create/destroy_doorbell Daniele Ceraolo Spurio
2018-10-17  0:17 ` [PATCH 2/4] drm/i915/guc: reserve the doorbell before selecting the cacheline Daniele Ceraolo Spurio
2018-10-17 15:52   ` Michal Wajdeczko
2018-10-17  0:17 ` [PATCH 3/4] drm/i915/guc: do not print drbreg on error Daniele Ceraolo Spurio
2018-10-17 16:37   ` Michal Wajdeczko
2018-10-17 16:58     ` Daniele Ceraolo Spurio
2018-10-17 18:30   ` [PATCH v2] drm/i915/guc: doorbell checking cleanup Daniele Ceraolo Spurio
2018-10-17  0:17 ` [PATCH 4/4] HAX enable GuC for CI Daniele Ceraolo Spurio
2018-10-17  0:27 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell Patchwork
2018-10-17  0:43 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-10-17 15:50 ` [PATCH 1/4] " Michal Wajdeczko
2018-10-17 18:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/guc: rename __create/destroy_doorbell (rev2) Patchwork
2018-10-17 19:10 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-10-17 19:52   ` Daniele Ceraolo Spurio

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.