intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL
@ 2023-03-20 21:10 Daniele Ceraolo Spurio
  2023-03-20 21:10 ` [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503 Daniele Ceraolo Spurio
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2023-03-20 21:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Chris Wilson

Commit 3db9d590557d ("drm/i915/gt: Reset twice") modified the code to
always hit the GDRST register twice when doing a reset, with the
reported aim to fix invalid post-reset engine state on some platforms
(Jasperlake being the only one actually mentioned).

This is a problem on MTL, due to the fact that we have to apply a time
consuming WA (coming in the next patch) every time we hit the GDRST
register in a way that can include the GSC engine. Even post MTL, the
expectation is that we'll have some work to do before and after hitting
the GDRST if the GSC is involved.

Since the issue requiring the double reset seems to be limited to older
platforms, instead of trying to handle the double-reset on MTL and
future platforms it is just easier to turn it off. The default on MTL is
also for GuC to own engine reset, with i915 only covering full-GT reset.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/intel_reset.c | 35 +++++++++++++++------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 0bb9094fdacd..2c3463f77e5c 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -268,9 +268,27 @@ static int ilk_do_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask,
 static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask)
 {
 	struct intel_uncore *uncore = gt->uncore;
-	int loops = 2;
+	int loops;
 	int err;
 
+	/*
+	 * On some platforms, e.g. Jasperlake, we see that the engine register
+	 * state is not cleared until shortly after GDRST reports completion,
+	 * causing a failure as we try to immediately resume while the internal
+	 * state is still in flux. If we immediately repeat the reset, the
+	 * second reset appears to serialise with the first, and since it is a
+	 * no-op, the registers should retain their reset value. However, there
+	 * is still a concern that upon leaving the second reset, the internal
+	 * engine state is still in flux and not ready for resuming.
+	 *
+	 * Starting on MTL, there are some prep steps that we need to do when
+	 * resetting some engines that need to be applied every time we write to
+	 * GEN6_GDRST. As those are time consuming (tens of ms), we don't want
+	 * to perform that twice, so, since the Jasperlake issue hasn't been
+	 * observed on MTL, we avoid repeating the reset on newer platforms.
+	 */
+	loops = GRAPHICS_VER_FULL(gt->i915) < IP_VER(12, 70) ? 2 : 1;
+
 	/*
 	 * GEN6_GDRST is not in the gt power well, no need to check
 	 * for fifo space for the write or forcewake the chip for
@@ -279,20 +297,7 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask)
 	do {
 		intel_uncore_write_fw(uncore, GEN6_GDRST, hw_domain_mask);
 
-		/*
-		 * Wait for the device to ack the reset requests.
-		 *
-		 * On some platforms, e.g. Jasperlake, we see that the
-		 * engine register state is not cleared until shortly after
-		 * GDRST reports completion, causing a failure as we try
-		 * to immediately resume while the internal state is still
-		 * in flux. If we immediately repeat the reset, the second
-		 * reset appears to serialise with the first, and since
-		 * it is a no-op, the registers should retain their reset
-		 * value. However, there is still a concern that upon
-		 * leaving the second reset, the internal engine state
-		 * is still in flux and not ready for resuming.
-		 */
+		/* Wait for the device to ack the reset requests. */
 		err = __intel_wait_for_register_fw(uncore, GEN6_GDRST,
 						   hw_domain_mask, 0,
 						   2000, 0,
-- 
2.37.3


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

* [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
@ 2023-03-20 21:10 ` Daniele Ceraolo Spurio
  2023-03-22 19:44   ` John Harrison
  2023-03-20 21:41 ` [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Andi Shyti
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Daniele Ceraolo Spurio @ 2023-03-20 21:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matt Roper, dri-devel, Rodrigo Vivi

The WA states that we need to alert the GSC FW before doing a GSC engine
reset and then wait for 200ms. The GuC owns engine reset, so on the i915
side we only need to apply this for full GT reset.

Given that we do full GT resets in the resume paths to cleanup the HW
state and that a long wait in those scenarios would not be acceptable,
a faster path has been introduced where, if the GSC is idle, we try first
to individually reset the GuC and all engines except the GSC and only fall
back to full reset if that fails.

Note: according to the WA specs, if the GSC is idle it should be possible
to only wait for the uC wakeup time (~15ms) instead of the whole 200ms.
However, the GSC FW team have mentioned that the wakeup time can change
based on other things going on in the HW and pcode, so a good security
margin would be required. Given that when the GSC is idle we already
skip the wait & reset entirely and that this reduced wait would still
likely be too long to use in resume paths, it's not worth adding support
for this reduced wait.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_reset.c     | 77 +++++++++++++++++++++--
 drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h |  2 +
 drivers/gpu/drm/i915/i915_reg.h           | 14 ++++-
 3 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 2c3463f77e5c..5f75f59122cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -14,6 +14,8 @@
 
 #include "gt/intel_gt_regs.h"
 
+#include "gt/uc/intel_gsc_fw.h"
+
 #include "i915_drv.h"
 #include "i915_file_private.h"
 #include "i915_gpu_error.h"
@@ -695,6 +697,67 @@ static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
 		return NULL;
 }
 
+static int __reset_guc(struct intel_gt *gt)
+{
+	u32 guc_domain =
+		GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
+
+	return gen6_hw_domain_reset(gt, guc_domain);
+}
+
+static bool needs_wa_14015076503(struct intel_gt *gt, intel_engine_mask_t engine_mask)
+{
+	if (!IS_METEORLAKE(gt->i915) || !HAS_ENGINE(gt, GSC0))
+		return false;
+
+	if (!__HAS_ENGINE(engine_mask, GSC0))
+		return false;
+
+	return intel_gsc_uc_fw_init_done(&gt->uc.gsc);
+}
+
+static intel_engine_mask_t
+wa_14015076503_start(struct intel_gt *gt, intel_engine_mask_t engine_mask, bool first)
+{
+	if (!needs_wa_14015076503(gt, engine_mask))
+		return engine_mask;
+
+	/*
+	 * wa_14015076503: if the GSC FW is loaded, we need to alert it that
+	 * we're going to do a GSC engine reset and then wait for 200ms for the
+	 * FW to get ready for it. However, if this the first ALL_ENGINES reset
+	 * attempt and the GSC is not busy, we can try to instead reset the GuC
+	 * and all the other engines individually to avoid the 200ms wait.
+	 */
+	if (engine_mask == ALL_ENGINES && first && intel_engine_is_idle(gt->engine[GSC0])) {
+		__reset_guc(gt);
+		engine_mask = gt->info.engine_mask & ~BIT(GSC0);
+	} else {
+		intel_uncore_rmw(gt->uncore,
+				 HECI_H_GS1(MTL_GSC_HECI2_BASE),
+				 0, HECI_H_GS1_ER_PREP);
+
+		/* make sure the reset bit is clear when writing the CSR reg */
+		intel_uncore_rmw(gt->uncore,
+				 HECI_H_CSR(MTL_GSC_HECI2_BASE),
+				 HECI_H_CSR_RST, HECI_H_CSR_IG);
+		msleep(200);
+	}
+
+	return engine_mask;
+}
+
+static void
+wa_14015076503_end(struct intel_gt *gt, intel_engine_mask_t engine_mask)
+{
+	if (!needs_wa_14015076503(gt, engine_mask))
+		return;
+
+	intel_uncore_rmw(gt->uncore,
+			 HECI_H_GS1(MTL_GSC_HECI2_BASE),
+			 HECI_H_GS1_ER_PREP, 0);
+}
+
 int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
 {
 	const int retries = engine_mask == ALL_ENGINES ? RESET_MAX_RETRIES : 1;
@@ -712,10 +775,16 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
 	 */
 	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
 	for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
-		GT_TRACE(gt, "engine_mask=%x\n", engine_mask);
+		intel_engine_mask_t reset_mask;
+
+		reset_mask = wa_14015076503_start(gt, engine_mask, !retry);
+
+		GT_TRACE(gt, "engine_mask=%x\n", reset_mask);
 		preempt_disable();
-		ret = reset(gt, engine_mask, retry);
+		ret = reset(gt, reset_mask, retry);
 		preempt_enable();
+
+		wa_14015076503_end(gt, reset_mask);
 	}
 	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
 
@@ -740,14 +809,12 @@ bool intel_has_reset_engine(const struct intel_gt *gt)
 
 int intel_reset_guc(struct intel_gt *gt)
 {
-	u32 guc_domain =
-		GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
 	int ret;
 
 	GEM_BUG_ON(!HAS_GT_UC(gt->i915));
 
 	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
-	ret = gen6_hw_domain_reset(gt, guc_domain);
+	ret = __reset_guc(gt);
 	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
index 4b5dbb44afb4..f4c1106bb2a9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
@@ -9,7 +9,9 @@
 #include <linux/types.h>
 
 struct intel_gsc_uc;
+struct intel_uncore;
 
 int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
 bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d22ffd7a32dc..80e33ede7fac 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -928,8 +928,18 @@
 #define DG1_GSC_HECI2_BASE	0x00259000
 #define DG2_GSC_HECI1_BASE	0x00373000
 #define DG2_GSC_HECI2_BASE	0x00374000
-
-
+#define MTL_GSC_HECI1_BASE	0x00116000
+#define MTL_GSC_HECI2_BASE	0x00117000
+
+#define HECI_H_CSR(base)	_MMIO(base + 0x4)
+#define   HECI_H_CSR_IE		REG_BIT(0)
+#define   HECI_H_CSR_IS		REG_BIT(1)
+#define   HECI_H_CSR_IG		REG_BIT(2)
+#define   HECI_H_CSR_RDY	REG_BIT(3)
+#define   HECI_H_CSR_RST	REG_BIT(4)
+
+#define HECI_H_GS1(base)	_MMIO(base + 0xc4c)
+#define   HECI_H_GS1_ER_PREP	REG_BIT(0)
 
 #define HSW_GTT_CACHE_EN	_MMIO(0x4024)
 #define   GTT_CACHE_EN_ALL	0xF0007FFF
-- 
2.37.3


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

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
  2023-03-20 21:10 ` [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503 Daniele Ceraolo Spurio
@ 2023-03-20 21:41 ` Andi Shyti
  2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Andi Shyti @ 2023-03-20 21:41 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx, dri-devel, Chris Wilson

Hi Daniele,

On Mon, Mar 20, 2023 at 02:10:38PM -0700, Daniele Ceraolo Spurio wrote:
> Commit 3db9d590557d ("drm/i915/gt: Reset twice") modified the code to
> always hit the GDRST register twice when doing a reset, with the
> reported aim to fix invalid post-reset engine state on some platforms
> (Jasperlake being the only one actually mentioned).
> 
> This is a problem on MTL, due to the fact that we have to apply a time
> consuming WA (coming in the next patch) every time we hit the GDRST
> register in a way that can include the GSC engine. Even post MTL, the
> expectation is that we'll have some work to do before and after hitting
> the GDRST if the GSC is involved.
> 
> Since the issue requiring the double reset seems to be limited to older
> platforms, instead of trying to handle the double-reset on MTL and
> future platforms it is just easier to turn it off. The default on MTL is
> also for GuC to own engine reset, with i915 only covering full-GT reset.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Andi

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
  2023-03-20 21:10 ` [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503 Daniele Ceraolo Spurio
  2023-03-20 21:41 ` [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Andi Shyti
@ 2023-03-21 13:33 ` Patchwork
  2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-03-21 13:33 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
URL   : https://patchwork.freedesktop.org/series/115424/
State : warning

== Summary ==

Error: git fetch origin failed



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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
                   ` (2 preceding siblings ...)
  2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork
@ 2023-03-21 13:33 ` Patchwork
  2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-03-21 13:33 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
URL   : https://patchwork.freedesktop.org/series/115424/
State : warning

== Summary ==

Error: git fetch origin failed



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

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
                   ` (3 preceding siblings ...)
  2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-03-21 13:33 ` Patchwork
  2023-03-21 13:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-03-21 13:33 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
URL   : https://patchwork.freedesktop.org/series/115424/
State : warning

== Summary ==

Error: git fetch origin failed



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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
                   ` (4 preceding siblings ...)
  2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
@ 2023-03-21 13:40 ` Patchwork
  2023-03-21 18:29 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2023-03-22 19:44 ` [Intel-gfx] [PATCH 1/2] " John Harrison
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-03-21 13:40 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 10424 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
URL   : https://patchwork.freedesktop.org/series/115424/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12884 -> Patchwork_115424v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/index.html

Participating hosts (35 -> 36)
------------------------------

  Additional (1): bat-rpls-2 

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-rpls-2:         NOTRUN -> [SKIP][1] ([i915#7456])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@debugfs_test@basic-hwmon.html

  * igt@fbdev@read:
    - bat-rpls-2:         NOTRUN -> [SKIP][2] ([i915#2582]) +4 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@fbdev@read.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - bat-rpls-2:         NOTRUN -> [ABORT][3] ([i915#7978])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@gem_exec_suspend@basic-s3@smem.html
    - bat-rpls-1:         NOTRUN -> [ABORT][4] ([i915#6687] / [i915#7978])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-rpls-2:         NOTRUN -> [SKIP][5] ([i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_tiled_pread_basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][6] ([i915#3282])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-rpls-2:         NOTRUN -> [SKIP][7] ([i915#7561])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - bat-rpls-2:         NOTRUN -> [SKIP][8] ([i915#6621])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_lrc:
    - bat-adln-1:         [PASS][9] -> [INCOMPLETE][10] ([i915#4983] / [i915#7609])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-adln-1/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@gt_pm:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][11] ([i915#4258])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@mman:
    - bat-rpls-1:         [PASS][12] -> [TIMEOUT][13] ([i915#6794])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-rpls-1/igt@i915_selftest@live@mman.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-1/igt@i915_selftest@live@mman.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][14] ([i915#6997] / [i915#7913])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@i915_selftest@live@slpc.html

  * igt@kms_busy@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][15] ([i915#1845]) +14 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_busy@basic.html

  * igt@kms_chamelium_edid@hdmi-edid-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][16] ([i915#7828]) +7 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_chamelium_edid@hdmi-edid-read.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-dg1-6:          NOTRUN -> [SKIP][17] ([i915#7828])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-dg1-6/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - bat-rpls-2:         NOTRUN -> [SKIP][18] ([i915#3637]) +3 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-rpls-2:         NOTRUN -> [SKIP][19] ([fdo#109285])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][20] ([i915#1849])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-rpls-2:         NOTRUN -> [SKIP][21] ([i915#1072]) +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rpls-2:         NOTRUN -> [SKIP][22] ([i915#3555])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-rpls-2:         NOTRUN -> [SKIP][23] ([fdo#109295] / [i915#1845] / [i915#3708])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][24] ([fdo#109295] / [i915#3708]) +3 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-rpls-2/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-6:          [ABORT][25] ([i915#7883]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-6/igt@i915_pm_rpm@module-reload.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-dg1-6/igt@i915_pm_rpm@module-reload.html

  
#### Warnings ####

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg1-6:          [SKIP][27] ([i915#3555]) -> [SKIP][28] ([i915#3555] / [i915#4579])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-6/igt@kms_setmode@basic-clone-single-crtc.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-dg1-6/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-rkl-11600:       [SKIP][29] ([i915#3555] / [i915#4098]) -> [SKIP][30] ([i915#3555] / [i915#4098] / [i915#4579])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-dg1-5:          [SKIP][31] ([i915#3555]) -> [SKIP][32] ([i915#3555] / [i915#4579])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-tgl-1115g4:      [SKIP][33] ([i915#3555]) -> [SKIP][34] ([i915#3555] / [i915#4579])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/fi-tgl-1115g4/igt@kms_setmode@basic-clone-single-crtc.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/fi-tgl-1115g4/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-dg1-7:          [SKIP][35] ([i915#3555]) -> [SKIP][36] ([i915#3555] / [i915#4579])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-7/igt@kms_setmode@basic-clone-single-crtc.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/bat-dg1-7/igt@kms_setmode@basic-clone-single-crtc.html

  
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7609]: https://gitlab.freedesktop.org/drm/intel/issues/7609
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7883]: https://gitlab.freedesktop.org/drm/intel/issues/7883
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978


Build changes
-------------

  * Linux: CI_DRM_12884 -> Patchwork_115424v1

  CI-20190529: 20190529
  CI_DRM_12884: 1d4054731cfcb1cb9810d309b70535ae0b90ecf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7208: f327c5d77b6ea6adff1ef6d08f21f232dfe093e3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115424v1: 1d4054731cfcb1cb9810d309b70535ae0b90ecf0 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

bba1cc2069a4 drm/i915/gsc: implement wa 14015076503
1bfe974e9d22 drm/i915: limit double GT reset to pre-MTL

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/index.html

[-- Attachment #2: Type: text/html, Size: 12992 bytes --]

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
                   ` (5 preceding siblings ...)
  2023-03-21 13:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-03-21 18:29 ` Patchwork
  2023-03-22 19:44 ` [Intel-gfx] [PATCH 1/2] " John Harrison
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-03-21 18:29 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 18401 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915: limit double GT reset to pre-MTL
URL   : https://patchwork.freedesktop.org/series/115424/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12884_full -> Patchwork_115424v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (8 -> 8)
------------------------------

  No changes in participating hosts

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [PASS][1] -> [FAIL][2] ([i915#2842])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-apl4/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-apl1/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#79])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-glk3/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-glk1/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html

  
#### Possible fixes ####

  * igt@api_intel_bb@object-reloc-keep-cache:
    - {shard-rkl}:        [SKIP][5] ([i915#3281]) -> [PASS][6] +6 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-4/igt@api_intel_bb@object-reloc-keep-cache.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-5/igt@api_intel_bb@object-reloc-keep-cache.html

  * igt@device_reset@unbind-reset-rebind:
    - {shard-rkl}:        [FAIL][7] ([i915#4778]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-5/igt@device_reset@unbind-reset-rebind.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-4/igt@device_reset@unbind-reset-rebind.html

  * igt@drm_fdinfo@virtual-idle:
    - {shard-rkl}:        [FAIL][9] ([i915#7742]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-4/igt@drm_fdinfo@virtual-idle.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-5/igt@drm_fdinfo@virtual-idle.html

  * {igt@gem_barrier_race@remote-request@rcs0}:
    - {shard-tglu}:       [ABORT][11] ([i915#8211]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@gem_barrier_race@remote-request@rcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-9/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_ctx_persistence@smoketest:
    - {shard-tglu}:       [FAIL][13] ([i915#5099]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-6/igt@gem_ctx_persistence@smoketest.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-1/igt@gem_ctx_persistence@smoketest.html

  * igt@gem_exec_balancer@fairslice:
    - {shard-rkl}:        [SKIP][15] ([i915#6259]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-5/igt@gem_exec_balancer@fairslice.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-4/igt@gem_exec_balancer@fairslice.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - {shard-rkl}:        [FAIL][17] ([i915#2842]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-2/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-4/igt@gem_exec_fair@basic-pace-share@rcs0.html
    - shard-apl:          [FAIL][19] ([i915#2842]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-apl6/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-apl4/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_mmap_gtt@basic-small-bo:
    - {shard-rkl}:        [SKIP][21] ([fdo#109315]) -> [PASS][22] +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-5/igt@gem_mmap_gtt@basic-small-bo.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-4/igt@gem_mmap_gtt@basic-small-bo.html

  * igt@gem_userptr_blits@forbidden-operations:
    - {shard-rkl}:        [SKIP][23] ([i915#3282]) -> [PASS][24] +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-4/igt@gem_userptr_blits@forbidden-operations.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-5/igt@gem_userptr_blits@forbidden-operations.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - {shard-rkl}:        [SKIP][25] ([i915#2527]) -> [PASS][26] +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-4/igt@gen9_exec_parse@batch-invalid-length.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-5/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@i915_pm_dc@dc6-dpms:
    - {shard-tglu}:       [FAIL][27] ([i915#3989] / [i915#454]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-7/igt@i915_pm_dc@dc6-dpms.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-2/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rpm@dpms-lpsp:
    - {shard-tglu}:       [SKIP][29] ([i915#1397]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@i915_pm_rpm@dpms-lpsp.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-7/igt@i915_pm_rpm@dpms-lpsp.html

  * igt@i915_pm_rpm@i2c:
    - {shard-tglu}:       [SKIP][31] ([i915#3547]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@i915_pm_rpm@i2c.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-7/igt@i915_pm_rpm@i2c.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-glk:          [FAIL][33] ([i915#2346]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-glk8/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size:
    - {shard-tglu}:       [SKIP][35] ([i915#1845]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-7/igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - {shard-tglu}:       [SKIP][37] ([i915#1849]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@kms_frontbuffer_tracking@fbc-stridechange.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-7/igt@kms_frontbuffer_tracking@fbc-stridechange.html

  * {igt@kms_plane@invalid-pixel-format-settings}:
    - {shard-tglu}:       [SKIP][39] ([i915#8152]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@kms_plane@invalid-pixel-format-settings.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-7/igt@kms_plane@invalid-pixel-format-settings.html

  * igt@kms_vblank@pipe-b-ts-continuation-modeset:
    - {shard-tglu}:       [SKIP][41] ([i915#1845] / [i915#7651]) -> [PASS][42] +13 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-tglu-9/igt@kms_vblank@pipe-b-ts-continuation-modeset.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-tglu-7/igt@kms_vblank@pipe-b-ts-continuation-modeset.html

  * igt@perf_pmu@idle@rcs0:
    - {shard-rkl}:        [FAIL][43] ([i915#4349]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-4/igt@perf_pmu@idle@rcs0.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-3/igt@perf_pmu@idle@rcs0.html

  * igt@syncobj_timeline@invalid-multi-wait-all-unsubmitted-submitted-signaled:
    - {shard-rkl}:        [SKIP][45] ([i915#2575]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-rkl-5/igt@syncobj_timeline@invalid-multi-wait-all-unsubmitted-submitted-signaled.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-rkl-4/igt@syncobj_timeline@invalid-multi-wait-all-unsubmitted-submitted-signaled.html

  * igt@sysfs_heartbeat_interval@precise@vcs1:
    - {shard-dg1}:        [FAIL][47] ([i915#1755]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/shard-dg1-15/igt@sysfs_heartbeat_interval@precise@vcs1.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/shard-dg1-12/igt@sysfs_heartbeat_interval@precise@vcs1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1755]: https://gitlab.freedesktop.org/drm/intel/issues/1755
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2435]: https://gitlab.freedesktop.org/drm/intel/issues/2435
  [i915#2436]: https://gitlab.freedesktop.org/drm/intel/issues/2436
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4778]: https://gitlab.freedesktop.org/drm/intel/issues/4778
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#5099]: https://gitlab.freedesktop.org/drm/intel/issues/5099
  [i915#5115]: https://gitlab.freedesktop.org/drm/intel/issues/5115
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5608]: https://gitlab.freedesktop.org/drm/intel/issues/5608
  [i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6259]: https://gitlab.freedesktop.org/drm/intel/issues/6259
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6334]: https://gitlab.freedesktop.org/drm/intel/issues/6334
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7052]: https://gitlab.freedesktop.org/drm/intel/issues/7052
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7128]: https://gitlab.freedesktop.org/drm/intel/issues/7128
  [i915#7294]: https://gitlab.freedesktop.org/drm/intel/issues/7294
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7957]: https://gitlab.freedesktop.org/drm/intel/issues/7957
  [i915#8152]: https://gitlab.freedesktop.org/drm/intel/issues/8152
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8282]: https://gitlab.freedesktop.org/drm/intel/issues/8282
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292


Build changes
-------------

  * Linux: CI_DRM_12884 -> Patchwork_115424v1

  CI-20190529: 20190529
  CI_DRM_12884: 1d4054731cfcb1cb9810d309b70535ae0b90ecf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7208: f327c5d77b6ea6adff1ef6d08f21f232dfe093e3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115424v1: 1d4054731cfcb1cb9810d309b70535ae0b90ecf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115424v1/index.html

[-- Attachment #2: Type: text/html, Size: 13137 bytes --]

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

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503
  2023-03-20 21:10 ` [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503 Daniele Ceraolo Spurio
@ 2023-03-22 19:44   ` John Harrison
  2023-03-22 20:59     ` Ceraolo Spurio, Daniele
  0 siblings, 1 reply; 14+ messages in thread
From: John Harrison @ 2023-03-22 19:44 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx; +Cc: Matt Roper, dri-devel, Rodrigo Vivi

On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
> The WA states that we need to alert the GSC FW before doing a GSC engine
> reset and then wait for 200ms. The GuC owns engine reset, so on the i915
> side we only need to apply this for full GT reset.
>
> Given that we do full GT resets in the resume paths to cleanup the HW
> state and that a long wait in those scenarios would not be acceptable,
> a faster path has been introduced where, if the GSC is idle, we try first
> to individually reset the GuC and all engines except the GSC and only fall
> back to full reset if that fails.
I'm confused. If the code path is a resume then surely all engines are 
idle. But, there is presumably a reason why we do a full GT reset on the 
resume - because the hardware state is not guaranteed to be sensible at 
that point. So how is it safe to skip the GSC reset?

>
> Note: according to the WA specs, if the GSC is idle it should be possible
> to only wait for the uC wakeup time (~15ms) instead of the whole 200ms.
> However, the GSC FW team have mentioned that the wakeup time can change
> based on other things going on in the HW and pcode, so a good security
> margin would be required. Given that when the GSC is idle we already
> skip the wait & reset entirely and that this reduced wait would still
> likely be too long to use in resume paths, it's not worth adding support
> for this reduced wait.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_reset.c     | 77 +++++++++++++++++++++--
>   drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h |  2 +
>   drivers/gpu/drm/i915/i915_reg.h           | 14 ++++-
>   3 files changed, 86 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 2c3463f77e5c..5f75f59122cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -14,6 +14,8 @@
>   
>   #include "gt/intel_gt_regs.h"
>   
> +#include "gt/uc/intel_gsc_fw.h"
> +
>   #include "i915_drv.h"
>   #include "i915_file_private.h"
>   #include "i915_gpu_error.h"
> @@ -695,6 +697,67 @@ static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
>   		return NULL;
>   }
>   
> +static int __reset_guc(struct intel_gt *gt)
> +{
> +	u32 guc_domain =
> +		GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
> +
> +	return gen6_hw_domain_reset(gt, guc_domain);
> +}
> +
> +static bool needs_wa_14015076503(struct intel_gt *gt, intel_engine_mask_t engine_mask)
> +{
> +	if (!IS_METEORLAKE(gt->i915) || !HAS_ENGINE(gt, GSC0))
> +		return false;
> +
> +	if (!__HAS_ENGINE(engine_mask, GSC0))
> +		return false;
> +
> +	return intel_gsc_uc_fw_init_done(&gt->uc.gsc);
> +}
> +
> +static intel_engine_mask_t
> +wa_14015076503_start(struct intel_gt *gt, intel_engine_mask_t engine_mask, bool first)
> +{
> +	if (!needs_wa_14015076503(gt, engine_mask))
> +		return engine_mask;
> +
> +	/*
> +	 * wa_14015076503: if the GSC FW is loaded, we need to alert it that
> +	 * we're going to do a GSC engine reset and then wait for 200ms for the
> +	 * FW to get ready for it. However, if this the first ALL_ENGINES reset
if this is

John.

> +	 * attempt and the GSC is not busy, we can try to instead reset the GuC
> +	 * and all the other engines individually to avoid the 200ms wait.
> +	 */
> +	if (engine_mask == ALL_ENGINES && first && intel_engine_is_idle(gt->engine[GSC0])) {
> +		__reset_guc(gt);
> +		engine_mask = gt->info.engine_mask & ~BIT(GSC0);
> +	} else {
> +		intel_uncore_rmw(gt->uncore,
> +				 HECI_H_GS1(MTL_GSC_HECI2_BASE),
> +				 0, HECI_H_GS1_ER_PREP);
> +
> +		/* make sure the reset bit is clear when writing the CSR reg */
> +		intel_uncore_rmw(gt->uncore,
> +				 HECI_H_CSR(MTL_GSC_HECI2_BASE),
> +				 HECI_H_CSR_RST, HECI_H_CSR_IG);
> +		msleep(200);
> +	}
> +
> +	return engine_mask;
> +}
> +
> +static void
> +wa_14015076503_end(struct intel_gt *gt, intel_engine_mask_t engine_mask)
> +{
> +	if (!needs_wa_14015076503(gt, engine_mask))
> +		return;
> +
> +	intel_uncore_rmw(gt->uncore,
> +			 HECI_H_GS1(MTL_GSC_HECI2_BASE),
> +			 HECI_H_GS1_ER_PREP, 0);
> +}
> +
>   int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>   {
>   	const int retries = engine_mask == ALL_ENGINES ? RESET_MAX_RETRIES : 1;
> @@ -712,10 +775,16 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>   	 */
>   	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>   	for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
> -		GT_TRACE(gt, "engine_mask=%x\n", engine_mask);
> +		intel_engine_mask_t reset_mask;
> +
> +		reset_mask = wa_14015076503_start(gt, engine_mask, !retry);
> +
> +		GT_TRACE(gt, "engine_mask=%x\n", reset_mask);
>   		preempt_disable();
> -		ret = reset(gt, engine_mask, retry);
> +		ret = reset(gt, reset_mask, retry);
>   		preempt_enable();
> +
> +		wa_14015076503_end(gt, reset_mask);
>   	}
>   	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>   
> @@ -740,14 +809,12 @@ bool intel_has_reset_engine(const struct intel_gt *gt)
>   
>   int intel_reset_guc(struct intel_gt *gt)
>   {
> -	u32 guc_domain =
> -		GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
>   	int ret;
>   
>   	GEM_BUG_ON(!HAS_GT_UC(gt->i915));
>   
>   	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
> -	ret = gen6_hw_domain_reset(gt, guc_domain);
> +	ret = __reset_guc(gt);
>   	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>   
>   	return ret;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> index 4b5dbb44afb4..f4c1106bb2a9 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> @@ -9,7 +9,9 @@
>   #include <linux/types.h>
>   
>   struct intel_gsc_uc;
> +struct intel_uncore;
>   
>   int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
>   bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
> +
>   #endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d22ffd7a32dc..80e33ede7fac 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -928,8 +928,18 @@
>   #define DG1_GSC_HECI2_BASE	0x00259000
>   #define DG2_GSC_HECI1_BASE	0x00373000
>   #define DG2_GSC_HECI2_BASE	0x00374000
> -
> -
> +#define MTL_GSC_HECI1_BASE	0x00116000
> +#define MTL_GSC_HECI2_BASE	0x00117000
> +
> +#define HECI_H_CSR(base)	_MMIO(base + 0x4)
> +#define   HECI_H_CSR_IE		REG_BIT(0)
> +#define   HECI_H_CSR_IS		REG_BIT(1)
> +#define   HECI_H_CSR_IG		REG_BIT(2)
> +#define   HECI_H_CSR_RDY	REG_BIT(3)
> +#define   HECI_H_CSR_RST	REG_BIT(4)
> +
> +#define HECI_H_GS1(base)	_MMIO(base + 0xc4c)
> +#define   HECI_H_GS1_ER_PREP	REG_BIT(0)
>   
>   #define HSW_GTT_CACHE_EN	_MMIO(0x4024)
>   #define   GTT_CACHE_EN_ALL	0xF0007FFF


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

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
                   ` (6 preceding siblings ...)
  2023-03-21 18:29 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2023-03-22 19:44 ` John Harrison
  2023-03-22 21:03   ` Ceraolo Spurio, Daniele
  7 siblings, 1 reply; 14+ messages in thread
From: John Harrison @ 2023-03-22 19:44 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx; +Cc: Wilson, Chris P, dri-devel

On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
> Commit 3db9d590557d ("drm/i915/gt: Reset twice") modified the code to
> always hit the GDRST register twice when doing a reset, with the
> reported aim to fix invalid post-reset engine state on some platforms
> (Jasperlake being the only one actually mentioned).
It still concerns me that there are no actual details about this issue 
from a hardware perspective as to what/why it goes wrong, the comment is 
fully of non-definitive language - 'appears to', 'should', 'is still a 
concern that'. And there is no w/a number associated with it. It all 
feels extremely suspect and warrants a great big FIXME tag as a minimum.

John.


>
> This is a problem on MTL, due to the fact that we have to apply a time
> consuming WA (coming in the next patch) every time we hit the GDRST
> register in a way that can include the GSC engine. Even post MTL, the
> expectation is that we'll have some work to do before and after hitting
> the GDRST if the GSC is involved.
>
> Since the issue requiring the double reset seems to be limited to older
> platforms, instead of trying to handle the double-reset on MTL and
> future platforms it is just easier to turn it off. The default on MTL is
> also for GuC to own engine reset, with i915 only covering full-GT reset.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_reset.c | 35 +++++++++++++++------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 0bb9094fdacd..2c3463f77e5c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -268,9 +268,27 @@ static int ilk_do_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask,
>   static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask)
>   {
>   	struct intel_uncore *uncore = gt->uncore;
> -	int loops = 2;
> +	int loops;
>   	int err;
>   
> +	/*
> +	 * On some platforms, e.g. Jasperlake, we see that the engine register
> +	 * state is not cleared until shortly after GDRST reports completion,
> +	 * causing a failure as we try to immediately resume while the internal
> +	 * state is still in flux. If we immediately repeat the reset, the
> +	 * second reset appears to serialise with the first, and since it is a
> +	 * no-op, the registers should retain their reset value. However, there
> +	 * is still a concern that upon leaving the second reset, the internal
> +	 * engine state is still in flux and not ready for resuming.
> +	 *
> +	 * Starting on MTL, there are some prep steps that we need to do when
> +	 * resetting some engines that need to be applied every time we write to
> +	 * GEN6_GDRST. As those are time consuming (tens of ms), we don't want
> +	 * to perform that twice, so, since the Jasperlake issue hasn't been
> +	 * observed on MTL, we avoid repeating the reset on newer platforms.
> +	 */
> +	loops = GRAPHICS_VER_FULL(gt->i915) < IP_VER(12, 70) ? 2 : 1;
> +
>   	/*
>   	 * GEN6_GDRST is not in the gt power well, no need to check
>   	 * for fifo space for the write or forcewake the chip for
> @@ -279,20 +297,7 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask)
>   	do {
>   		intel_uncore_write_fw(uncore, GEN6_GDRST, hw_domain_mask);
>   
> -		/*
> -		 * Wait for the device to ack the reset requests.
> -		 *
> -		 * On some platforms, e.g. Jasperlake, we see that the
> -		 * engine register state is not cleared until shortly after
> -		 * GDRST reports completion, causing a failure as we try
> -		 * to immediately resume while the internal state is still
> -		 * in flux. If we immediately repeat the reset, the second
> -		 * reset appears to serialise with the first, and since
> -		 * it is a no-op, the registers should retain their reset
> -		 * value. However, there is still a concern that upon
> -		 * leaving the second reset, the internal engine state
> -		 * is still in flux and not ready for resuming.
> -		 */
> +		/* Wait for the device to ack the reset requests. */
>   		err = __intel_wait_for_register_fw(uncore, GEN6_GDRST,
>   						   hw_domain_mask, 0,
>   						   2000, 0,


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

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503
  2023-03-22 19:44   ` John Harrison
@ 2023-03-22 20:59     ` Ceraolo Spurio, Daniele
  2023-03-22 21:09       ` John Harrison
  0 siblings, 1 reply; 14+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-03-22 20:59 UTC (permalink / raw)
  To: John Harrison, intel-gfx; +Cc: Matt Roper, dri-devel, Rodrigo Vivi



On 3/22/2023 12:44 PM, John Harrison wrote:
> On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
>> The WA states that we need to alert the GSC FW before doing a GSC engine
>> reset and then wait for 200ms. The GuC owns engine reset, so on the i915
>> side we only need to apply this for full GT reset.
>>
>> Given that we do full GT resets in the resume paths to cleanup the HW
>> state and that a long wait in those scenarios would not be acceptable,
>> a faster path has been introduced where, if the GSC is idle, we try 
>> first
>> to individually reset the GuC and all engines except the GSC and only 
>> fall
>> back to full reset if that fails.
> I'm confused. If the code path is a resume then surely all engines are 
> idle. But, there is presumably a reason why we do a full GT reset on 
> the resume - because the hardware state is not guaranteed to be 
> sensible at that point. So how is it safe to skip the GSC reset?

Differently from other engines, the GSCCS only role is to forward the 
commands to the GSC FW, so it doesn't have any HW outside of the CS 
itself and therefore it has no state that we don't explicitly re-init on 
resume or on context switch (LRC or power context). The HW for the GSC 
uC is managed by the GSC FW so we don't need to care about that. I can 
add this to the commit message if it makes things clearer

Daniele

>
>>
>> Note: according to the WA specs, if the GSC is idle it should be 
>> possible
>> to only wait for the uC wakeup time (~15ms) instead of the whole 200ms.
>> However, the GSC FW team have mentioned that the wakeup time can change
>> based on other things going on in the HW and pcode, so a good security
>> margin would be required. Given that when the GSC is idle we already
>> skip the wait & reset entirely and that this reduced wait would still
>> likely be too long to use in resume paths, it's not worth adding support
>> for this reduced wait.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: John Harrison <john.c.harrison@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_reset.c     | 77 +++++++++++++++++++++--
>>   drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h |  2 +
>>   drivers/gpu/drm/i915/i915_reg.h           | 14 ++++-
>>   3 files changed, 86 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
>> b/drivers/gpu/drm/i915/gt/intel_reset.c
>> index 2c3463f77e5c..5f75f59122cf 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>> @@ -14,6 +14,8 @@
>>     #include "gt/intel_gt_regs.h"
>>   +#include "gt/uc/intel_gsc_fw.h"
>> +
>>   #include "i915_drv.h"
>>   #include "i915_file_private.h"
>>   #include "i915_gpu_error.h"
>> @@ -695,6 +697,67 @@ static reset_func intel_get_gpu_reset(const 
>> struct intel_gt *gt)
>>           return NULL;
>>   }
>>   +static int __reset_guc(struct intel_gt *gt)
>> +{
>> +    u32 guc_domain =
>> +        GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : 
>> GEN9_GRDOM_GUC;
>> +
>> +    return gen6_hw_domain_reset(gt, guc_domain);
>> +}
>> +
>> +static bool needs_wa_14015076503(struct intel_gt *gt, 
>> intel_engine_mask_t engine_mask)
>> +{
>> +    if (!IS_METEORLAKE(gt->i915) || !HAS_ENGINE(gt, GSC0))
>> +        return false;
>> +
>> +    if (!__HAS_ENGINE(engine_mask, GSC0))
>> +        return false;
>> +
>> +    return intel_gsc_uc_fw_init_done(&gt->uc.gsc);
>> +}
>> +
>> +static intel_engine_mask_t
>> +wa_14015076503_start(struct intel_gt *gt, intel_engine_mask_t 
>> engine_mask, bool first)
>> +{
>> +    if (!needs_wa_14015076503(gt, engine_mask))
>> +        return engine_mask;
>> +
>> +    /*
>> +     * wa_14015076503: if the GSC FW is loaded, we need to alert it 
>> that
>> +     * we're going to do a GSC engine reset and then wait for 200ms 
>> for the
>> +     * FW to get ready for it. However, if this the first 
>> ALL_ENGINES reset
> if this is
>
> John.
>
>> +     * attempt and the GSC is not busy, we can try to instead reset 
>> the GuC
>> +     * and all the other engines individually to avoid the 200ms wait.
>> +     */
>> +    if (engine_mask == ALL_ENGINES && first && 
>> intel_engine_is_idle(gt->engine[GSC0])) {
>> +        __reset_guc(gt);
>> +        engine_mask = gt->info.engine_mask & ~BIT(GSC0);
>> +    } else {
>> +        intel_uncore_rmw(gt->uncore,
>> +                 HECI_H_GS1(MTL_GSC_HECI2_BASE),
>> +                 0, HECI_H_GS1_ER_PREP);
>> +
>> +        /* make sure the reset bit is clear when writing the CSR reg */
>> +        intel_uncore_rmw(gt->uncore,
>> +                 HECI_H_CSR(MTL_GSC_HECI2_BASE),
>> +                 HECI_H_CSR_RST, HECI_H_CSR_IG);
>> +        msleep(200);
>> +    }
>> +
>> +    return engine_mask;
>> +}
>> +
>> +static void
>> +wa_14015076503_end(struct intel_gt *gt, intel_engine_mask_t 
>> engine_mask)
>> +{
>> +    if (!needs_wa_14015076503(gt, engine_mask))
>> +        return;
>> +
>> +    intel_uncore_rmw(gt->uncore,
>> +             HECI_H_GS1(MTL_GSC_HECI2_BASE),
>> +             HECI_H_GS1_ER_PREP, 0);
>> +}
>> +
>>   int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t 
>> engine_mask)
>>   {
>>       const int retries = engine_mask == ALL_ENGINES ? 
>> RESET_MAX_RETRIES : 1;
>> @@ -712,10 +775,16 @@ int __intel_gt_reset(struct intel_gt *gt, 
>> intel_engine_mask_t engine_mask)
>>        */
>>       intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>>       for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
>> -        GT_TRACE(gt, "engine_mask=%x\n", engine_mask);
>> +        intel_engine_mask_t reset_mask;
>> +
>> +        reset_mask = wa_14015076503_start(gt, engine_mask, !retry);
>> +
>> +        GT_TRACE(gt, "engine_mask=%x\n", reset_mask);
>>           preempt_disable();
>> -        ret = reset(gt, engine_mask, retry);
>> +        ret = reset(gt, reset_mask, retry);
>>           preempt_enable();
>> +
>> +        wa_14015076503_end(gt, reset_mask);
>>       }
>>       intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>>   @@ -740,14 +809,12 @@ bool intel_has_reset_engine(const struct 
>> intel_gt *gt)
>>     int intel_reset_guc(struct intel_gt *gt)
>>   {
>> -    u32 guc_domain =
>> -        GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : 
>> GEN9_GRDOM_GUC;
>>       int ret;
>>         GEM_BUG_ON(!HAS_GT_UC(gt->i915));
>>         intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>> -    ret = gen6_hw_domain_reset(gt, guc_domain);
>> +    ret = __reset_guc(gt);
>>       intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>>         return ret;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
>> index 4b5dbb44afb4..f4c1106bb2a9 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
>> @@ -9,7 +9,9 @@
>>   #include <linux/types.h>
>>     struct intel_gsc_uc;
>> +struct intel_uncore;
>>     int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
>>   bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index d22ffd7a32dc..80e33ede7fac 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -928,8 +928,18 @@
>>   #define DG1_GSC_HECI2_BASE    0x00259000
>>   #define DG2_GSC_HECI1_BASE    0x00373000
>>   #define DG2_GSC_HECI2_BASE    0x00374000
>> -
>> -
>> +#define MTL_GSC_HECI1_BASE    0x00116000
>> +#define MTL_GSC_HECI2_BASE    0x00117000
>> +
>> +#define HECI_H_CSR(base)    _MMIO(base + 0x4)
>> +#define   HECI_H_CSR_IE        REG_BIT(0)
>> +#define   HECI_H_CSR_IS        REG_BIT(1)
>> +#define   HECI_H_CSR_IG        REG_BIT(2)
>> +#define   HECI_H_CSR_RDY    REG_BIT(3)
>> +#define   HECI_H_CSR_RST    REG_BIT(4)
>> +
>> +#define HECI_H_GS1(base)    _MMIO(base + 0xc4c)
>> +#define   HECI_H_GS1_ER_PREP    REG_BIT(0)
>>     #define HSW_GTT_CACHE_EN    _MMIO(0x4024)
>>   #define   GTT_CACHE_EN_ALL    0xF0007FFF
>


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

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-22 19:44 ` [Intel-gfx] [PATCH 1/2] " John Harrison
@ 2023-03-22 21:03   ` Ceraolo Spurio, Daniele
  2023-03-23  0:16     ` Andi Shyti
  0 siblings, 1 reply; 14+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-03-22 21:03 UTC (permalink / raw)
  To: John Harrison, intel-gfx; +Cc: Wilson, Chris P, dri-devel



On 3/22/2023 12:44 PM, John Harrison wrote:
> On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
>> Commit 3db9d590557d ("drm/i915/gt: Reset twice") modified the code to
>> always hit the GDRST register twice when doing a reset, with the
>> reported aim to fix invalid post-reset engine state on some platforms
>> (Jasperlake being the only one actually mentioned).
> It still concerns me that there are no actual details about this issue 
> from a hardware perspective as to what/why it goes wrong, the comment 
> is fully of non-definitive language - 'appears to', 'should', 'is 
> still a concern that'. And there is no w/a number associated with it. 
> It all feels extremely suspect and warrants a great big FIXME tag as a 
> minimum.

I agree that the whole thing is unclear and we could add a FIXME, but 
IMO that is outside the scope of this patch as I'm not adding the code 
in question. This should be discussed with the original author/reviewers.

Daniele

>
> John.
>
>
>>
>> This is a problem on MTL, due to the fact that we have to apply a time
>> consuming WA (coming in the next patch) every time we hit the GDRST
>> register in a way that can include the GSC engine. Even post MTL, the
>> expectation is that we'll have some work to do before and after hitting
>> the GDRST if the GSC is involved.
>>
>> Since the issue requiring the double reset seems to be limited to older
>> platforms, instead of trying to handle the double-reset on MTL and
>> future platforms it is just easier to turn it off. The default on MTL is
>> also for GuC to own engine reset, with i915 only covering full-GT reset.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_reset.c | 35 +++++++++++++++------------
>>   1 file changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
>> b/drivers/gpu/drm/i915/gt/intel_reset.c
>> index 0bb9094fdacd..2c3463f77e5c 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>> @@ -268,9 +268,27 @@ static int ilk_do_reset(struct intel_gt *gt, 
>> intel_engine_mask_t engine_mask,
>>   static int gen6_hw_domain_reset(struct intel_gt *gt, u32 
>> hw_domain_mask)
>>   {
>>       struct intel_uncore *uncore = gt->uncore;
>> -    int loops = 2;
>> +    int loops;
>>       int err;
>>   +    /*
>> +     * On some platforms, e.g. Jasperlake, we see that the engine 
>> register
>> +     * state is not cleared until shortly after GDRST reports 
>> completion,
>> +     * causing a failure as we try to immediately resume while the 
>> internal
>> +     * state is still in flux. If we immediately repeat the reset, the
>> +     * second reset appears to serialise with the first, and since 
>> it is a
>> +     * no-op, the registers should retain their reset value. 
>> However, there
>> +     * is still a concern that upon leaving the second reset, the 
>> internal
>> +     * engine state is still in flux and not ready for resuming.
>> +     *
>> +     * Starting on MTL, there are some prep steps that we need to do 
>> when
>> +     * resetting some engines that need to be applied every time we 
>> write to
>> +     * GEN6_GDRST. As those are time consuming (tens of ms), we 
>> don't want
>> +     * to perform that twice, so, since the Jasperlake issue hasn't 
>> been
>> +     * observed on MTL, we avoid repeating the reset on newer 
>> platforms.
>> +     */
>> +    loops = GRAPHICS_VER_FULL(gt->i915) < IP_VER(12, 70) ? 2 : 1;
>> +
>>       /*
>>        * GEN6_GDRST is not in the gt power well, no need to check
>>        * for fifo space for the write or forcewake the chip for
>> @@ -279,20 +297,7 @@ static int gen6_hw_domain_reset(struct intel_gt 
>> *gt, u32 hw_domain_mask)
>>       do {
>>           intel_uncore_write_fw(uncore, GEN6_GDRST, hw_domain_mask);
>>   -        /*
>> -         * Wait for the device to ack the reset requests.
>> -         *
>> -         * On some platforms, e.g. Jasperlake, we see that the
>> -         * engine register state is not cleared until shortly after
>> -         * GDRST reports completion, causing a failure as we try
>> -         * to immediately resume while the internal state is still
>> -         * in flux. If we immediately repeat the reset, the second
>> -         * reset appears to serialise with the first, and since
>> -         * it is a no-op, the registers should retain their reset
>> -         * value. However, there is still a concern that upon
>> -         * leaving the second reset, the internal engine state
>> -         * is still in flux and not ready for resuming.
>> -         */
>> +        /* Wait for the device to ack the reset requests. */
>>           err = __intel_wait_for_register_fw(uncore, GEN6_GDRST,
>>                              hw_domain_mask, 0,
>>                              2000, 0,
>


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

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503
  2023-03-22 20:59     ` Ceraolo Spurio, Daniele
@ 2023-03-22 21:09       ` John Harrison
  0 siblings, 0 replies; 14+ messages in thread
From: John Harrison @ 2023-03-22 21:09 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: Matt Roper, dri-devel, Rodrigo Vivi

On 3/22/2023 13:59, Ceraolo Spurio, Daniele wrote:
> On 3/22/2023 12:44 PM, John Harrison wrote:
>> On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
>>> The WA states that we need to alert the GSC FW before doing a GSC 
>>> engine
>>> reset and then wait for 200ms. The GuC owns engine reset, so on the 
>>> i915
>>> side we only need to apply this for full GT reset.
>>>
>>> Given that we do full GT resets in the resume paths to cleanup the HW
>>> state and that a long wait in those scenarios would not be acceptable,
>>> a faster path has been introduced where, if the GSC is idle, we try 
>>> first
>>> to individually reset the GuC and all engines except the GSC and 
>>> only fall
>>> back to full reset if that fails.
>> I'm confused. If the code path is a resume then surely all engines 
>> are idle. But, there is presumably a reason why we do a full GT reset 
>> on the resume - because the hardware state is not guaranteed to be 
>> sensible at that point. So how is it safe to skip the GSC reset?
>
> Differently from other engines, the GSCCS only role is to forward the 
> commands to the GSC FW, so it doesn't have any HW outside of the CS 
> itself and therefore it has no state that we don't explicitly re-init 
> on resume or on context switch (LRC or power context). The HW for the 
> GSC uC is managed by the GSC FW so we don't need to care about that. I 
> can add this to the commit message if it makes things clearer
Okay. With a suitable extra comment to explain the above and the comment 
fix below:
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

>
> Daniele
>
>>
>>>
>>> Note: according to the WA specs, if the GSC is idle it should be 
>>> possible
>>> to only wait for the uC wakeup time (~15ms) instead of the whole 200ms.
>>> However, the GSC FW team have mentioned that the wakeup time can change
>>> based on other things going on in the HW and pcode, so a good security
>>> margin would be required. Given that when the GSC is idle we already
>>> skip the wait & reset entirely and that this reduced wait would still
>>> likely be too long to use in resume paths, it's not worth adding 
>>> support
>>> for this reduced wait.
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Cc: John Harrison <john.c.harrison@intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/gt/intel_reset.c     | 77 
>>> +++++++++++++++++++++--
>>>   drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h |  2 +
>>>   drivers/gpu/drm/i915/i915_reg.h           | 14 ++++-
>>>   3 files changed, 86 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
>>> b/drivers/gpu/drm/i915/gt/intel_reset.c
>>> index 2c3463f77e5c..5f75f59122cf 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>>> @@ -14,6 +14,8 @@
>>>     #include "gt/intel_gt_regs.h"
>>>   +#include "gt/uc/intel_gsc_fw.h"
>>> +
>>>   #include "i915_drv.h"
>>>   #include "i915_file_private.h"
>>>   #include "i915_gpu_error.h"
>>> @@ -695,6 +697,67 @@ static reset_func intel_get_gpu_reset(const 
>>> struct intel_gt *gt)
>>>           return NULL;
>>>   }
>>>   +static int __reset_guc(struct intel_gt *gt)
>>> +{
>>> +    u32 guc_domain =
>>> +        GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : 
>>> GEN9_GRDOM_GUC;
>>> +
>>> +    return gen6_hw_domain_reset(gt, guc_domain);
>>> +}
>>> +
>>> +static bool needs_wa_14015076503(struct intel_gt *gt, 
>>> intel_engine_mask_t engine_mask)
>>> +{
>>> +    if (!IS_METEORLAKE(gt->i915) || !HAS_ENGINE(gt, GSC0))
>>> +        return false;
>>> +
>>> +    if (!__HAS_ENGINE(engine_mask, GSC0))
>>> +        return false;
>>> +
>>> +    return intel_gsc_uc_fw_init_done(&gt->uc.gsc);
>>> +}
>>> +
>>> +static intel_engine_mask_t
>>> +wa_14015076503_start(struct intel_gt *gt, intel_engine_mask_t 
>>> engine_mask, bool first)
>>> +{
>>> +    if (!needs_wa_14015076503(gt, engine_mask))
>>> +        return engine_mask;
>>> +
>>> +    /*
>>> +     * wa_14015076503: if the GSC FW is loaded, we need to alert it 
>>> that
>>> +     * we're going to do a GSC engine reset and then wait for 200ms 
>>> for the
>>> +     * FW to get ready for it. However, if this the first 
>>> ALL_ENGINES reset
>> if this is
>>
>> John.
>>
>>> +     * attempt and the GSC is not busy, we can try to instead reset 
>>> the GuC
>>> +     * and all the other engines individually to avoid the 200ms wait.
>>> +     */
>>> +    if (engine_mask == ALL_ENGINES && first && 
>>> intel_engine_is_idle(gt->engine[GSC0])) {
>>> +        __reset_guc(gt);
>>> +        engine_mask = gt->info.engine_mask & ~BIT(GSC0);
>>> +    } else {
>>> +        intel_uncore_rmw(gt->uncore,
>>> +                 HECI_H_GS1(MTL_GSC_HECI2_BASE),
>>> +                 0, HECI_H_GS1_ER_PREP);
>>> +
>>> +        /* make sure the reset bit is clear when writing the CSR 
>>> reg */
>>> +        intel_uncore_rmw(gt->uncore,
>>> +                 HECI_H_CSR(MTL_GSC_HECI2_BASE),
>>> +                 HECI_H_CSR_RST, HECI_H_CSR_IG);
>>> +        msleep(200);
>>> +    }
>>> +
>>> +    return engine_mask;
>>> +}
>>> +
>>> +static void
>>> +wa_14015076503_end(struct intel_gt *gt, intel_engine_mask_t 
>>> engine_mask)
>>> +{
>>> +    if (!needs_wa_14015076503(gt, engine_mask))
>>> +        return;
>>> +
>>> +    intel_uncore_rmw(gt->uncore,
>>> +             HECI_H_GS1(MTL_GSC_HECI2_BASE),
>>> +             HECI_H_GS1_ER_PREP, 0);
>>> +}
>>> +
>>>   int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t 
>>> engine_mask)
>>>   {
>>>       const int retries = engine_mask == ALL_ENGINES ? 
>>> RESET_MAX_RETRIES : 1;
>>> @@ -712,10 +775,16 @@ int __intel_gt_reset(struct intel_gt *gt, 
>>> intel_engine_mask_t engine_mask)
>>>        */
>>>       intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>>>       for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
>>> -        GT_TRACE(gt, "engine_mask=%x\n", engine_mask);
>>> +        intel_engine_mask_t reset_mask;
>>> +
>>> +        reset_mask = wa_14015076503_start(gt, engine_mask, !retry);
>>> +
>>> +        GT_TRACE(gt, "engine_mask=%x\n", reset_mask);
>>>           preempt_disable();
>>> -        ret = reset(gt, engine_mask, retry);
>>> +        ret = reset(gt, reset_mask, retry);
>>>           preempt_enable();
>>> +
>>> +        wa_14015076503_end(gt, reset_mask);
>>>       }
>>>       intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>>>   @@ -740,14 +809,12 @@ bool intel_has_reset_engine(const struct 
>>> intel_gt *gt)
>>>     int intel_reset_guc(struct intel_gt *gt)
>>>   {
>>> -    u32 guc_domain =
>>> -        GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : 
>>> GEN9_GRDOM_GUC;
>>>       int ret;
>>>         GEM_BUG_ON(!HAS_GT_UC(gt->i915));
>>>         intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>>> -    ret = gen6_hw_domain_reset(gt, guc_domain);
>>> +    ret = __reset_guc(gt);
>>>       intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>>>         return ret;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
>>> index 4b5dbb44afb4..f4c1106bb2a9 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
>>> @@ -9,7 +9,9 @@
>>>   #include <linux/types.h>
>>>     struct intel_gsc_uc;
>>> +struct intel_uncore;
>>>     int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
>>>   bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
>>> +
>>>   #endif
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index d22ffd7a32dc..80e33ede7fac 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -928,8 +928,18 @@
>>>   #define DG1_GSC_HECI2_BASE    0x00259000
>>>   #define DG2_GSC_HECI1_BASE    0x00373000
>>>   #define DG2_GSC_HECI2_BASE    0x00374000
>>> -
>>> -
>>> +#define MTL_GSC_HECI1_BASE    0x00116000
>>> +#define MTL_GSC_HECI2_BASE    0x00117000
>>> +
>>> +#define HECI_H_CSR(base)    _MMIO(base + 0x4)
>>> +#define   HECI_H_CSR_IE        REG_BIT(0)
>>> +#define   HECI_H_CSR_IS        REG_BIT(1)
>>> +#define   HECI_H_CSR_IG        REG_BIT(2)
>>> +#define   HECI_H_CSR_RDY    REG_BIT(3)
>>> +#define   HECI_H_CSR_RST    REG_BIT(4)
>>> +
>>> +#define HECI_H_GS1(base)    _MMIO(base + 0xc4c)
>>> +#define   HECI_H_GS1_ER_PREP    REG_BIT(0)
>>>     #define HSW_GTT_CACHE_EN    _MMIO(0x4024)
>>>   #define   GTT_CACHE_EN_ALL    0xF0007FFF
>>
>


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

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL
  2023-03-22 21:03   ` Ceraolo Spurio, Daniele
@ 2023-03-23  0:16     ` Andi Shyti
  0 siblings, 0 replies; 14+ messages in thread
From: Andi Shyti @ 2023-03-23  0:16 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele; +Cc: intel-gfx, dri-devel, Wilson, Chris P

Hi,

> On 3/22/2023 12:44 PM, John Harrison wrote:
> > On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
> > > Commit 3db9d590557d ("drm/i915/gt: Reset twice") modified the code to
> > > always hit the GDRST register twice when doing a reset, with the
> > > reported aim to fix invalid post-reset engine state on some platforms
> > > (Jasperlake being the only one actually mentioned).
> >
> > It still concerns me that there are no actual details about this issue
> > from a hardware perspective as to what/why it goes wrong, the comment is
> > fully of non-definitive language - 'appears to', 'should', 'is still a
> > concern that'. And there is no w/a number associated with it. It all
> > feels extremely suspect and warrants a great big FIXME tag as a minimum.
> 
> I agree that the whole thing is unclear and we could add a FIXME, but IMO
> that is outside the scope of this patch as I'm not adding the code in
> question. This should be discussed with the original author/reviewers.

Sorry for chiming in a bit late. I'm with Daniele on this one;
the patch just takes things back to how they were before Chris's
patch, so we should look into the reasoning behind Chris's patch
itself.

As mentioned in the commit log, in Jasperlake (and only
Jasperlake), a second reset attempt is needed to clear the
register state. If I remember right, Chris discovered this
through experimentation, and I don't think any hardware folks
have documented it.

Chris can probably give more details on this.

In general, I'm on board with Daniele's patch since it's doing
what the driver has always done, which is why I gave it a quick
review already in V1.

On the other hand, the price to pay having something like this:

  loops = GRAPHICS_VER_FULL(gt->i915) < IP_VER(12, 70) ? 2 : 1;

Is the following perhaps a bit more self-explanatory?

  /*
   * The big comment with explanation
   */
  if (IS_PLATFORM(i915, INTEL_JASPERLAKE))
  	/* try again */ ;

Andi

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

end of thread, other threads:[~2023-03-23  0:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-20 21:10 [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Daniele Ceraolo Spurio
2023-03-20 21:10 ` [Intel-gfx] [PATCH 2/2] drm/i915/gsc: implement wa 14015076503 Daniele Ceraolo Spurio
2023-03-22 19:44   ` John Harrison
2023-03-22 20:59     ` Ceraolo Spurio, Daniele
2023-03-22 21:09       ` John Harrison
2023-03-20 21:41 ` [Intel-gfx] [PATCH 1/2] drm/i915: limit double GT reset to pre-MTL Andi Shyti
2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork
2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-03-21 13:33 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2023-03-21 13:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-03-21 18:29 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-03-22 19:44 ` [Intel-gfx] [PATCH 1/2] " John Harrison
2023-03-22 21:03   ` Ceraolo Spurio, Daniele
2023-03-23  0:16     ` Andi Shyti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).