[PATCH 1/2] drm/i915: Cancel reset preparations on failed resets

[PATCH 1/2] drm/i915: Cancel reset preparations on failed resets

[Mika Kuoppala]

Our reset handling has a retry layer further up in the
chain. As we have told the engine to prepare for reset,
and failed it, make sure to remove that preparation so
that the next attempted reset has a clean slate by triggering
another full prepare cycle for the engines.

v2: ret as int, simplified cleanup (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b36a3b5736a0..bb03f6d8b3d1 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2093,21 +2093,25 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv,
 {
 	struct intel_engine_cs *engine;
 	unsigned int tmp;
+	int ret;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
-		if (gen8_reset_engine_start(engine))
+	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+		if (gen8_reset_engine_start(engine)) {
+			ret = -EIO;
 			goto not_ready;
+		}
+	}
 
 	if (INTEL_GEN(dev_priv) >= 11)
-		return gen11_reset_engines(dev_priv, engine_mask);
+		ret = gen11_reset_engines(dev_priv, engine_mask);
 	else
-		return gen6_reset_engines(dev_priv, engine_mask);
+		ret = gen6_reset_engines(dev_priv, engine_mask);
 
 not_ready:
 	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
 		gen8_reset_engine_cancel(engine);
 
-	return -EIO;
+	return ret;
 }
 
 typedef int (*reset_func)(struct drm_i915_private *, unsigned engine_mask);
-- 
2.17.0

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

[PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Mika Kuoppala]

There is a problem with kbl up to rev E0 where a heavy
memory/fabric traffic from adjacent engine(s) can cause an engine
reset to fail. This traffic can be from normal memory accesses
or it can be from heavy polling on a semaphore wait.

For engine hogging causing a fail, we already fallback to
full reset. Which effectively stops all engines and thus
we only add a workaround documentation.

For the semaphore wait loop poll case, we add one microsecond
poll interval to semaphore wait to guarantee bandwidth for
the reset preration. The side effect is that we make semaphore
completion latencies also 1us longer.

v2: Let full reset handle the adjacent engine idling (Chris)

References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
References: VTHSD#2227190, HSDES#1604216706, BSID#0917
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h          | 1 +
 drivers/gpu/drm/i915/intel_uncore.c      | 2 ++
 drivers/gpu/drm/i915/intel_workarounds.c | 9 +++++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f0317bde3aab..0e8c7896cd74 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2242,6 +2242,7 @@ enum i915_power_well_id {
 #define RING_RESET_CTL(base)	_MMIO((base)+0xd0)
 #define   RESET_CTL_REQUEST_RESET  (1 << 0)
 #define   RESET_CTL_READY_TO_RESET (1 << 1)
+#define RING_SEMA_WAIT_POLL(base) _MMIO((base)+0x24c)
 
 #define HSW_GTT_CACHE_EN	_MMIO(0x4024)
 #define   GTT_CACHE_EN_ALL	0xF0007FFF
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bb03f6d8b3d1..b892ca8396e8 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2174,6 +2174,8 @@ int intel_gpu_reset(struct drm_i915_private *dev_priv, unsigned engine_mask)
 		 * Thus assume it is best to stop engines on all gens
 		 * where we have a gpu reset.
 		 *
+		 * WaKBLVECSSemaphoreWaitPoll:kbl (on ALL_ENGINES)
+		 *
 		 * WaMediaResetMainRingCleanup:ctg,elk (presumably)
 		 *
 		 * FIXME: Wa for more modern gens needs to be validated
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index b1ab56a1ec31..5655d39c65cb 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -666,6 +666,15 @@ static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
 		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
 		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+
+	/* WaKBLVECSSemaphoreWaitPoll:kbl */
+	if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
+		struct intel_engine_cs *engine;
+		unsigned int tmp;
+
+		for_each_engine(engine, dev_priv, tmp)
+			I915_WRITE(RING_SEMA_WAIT_POLL(engine->mmio_base), 1);
+	}
 }
 
 static void glk_gt_workarounds_apply(struct drm_i915_private *dev_priv)
-- 
2.17.0

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

Re: [PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Chris Wilson]

Quoting Mika Kuoppala (2018-06-05 17:03:57)
> There is a problem with kbl up to rev E0 where a heavy
> memory/fabric traffic from adjacent engine(s) can cause an engine
> reset to fail. This traffic can be from normal memory accesses
> or it can be from heavy polling on a semaphore wait.
> 
> For engine hogging causing a fail, we already fallback to
> full reset. Which effectively stops all engines and thus
> we only add a workaround documentation.
> 
> For the semaphore wait loop poll case, we add one microsecond
> poll interval to semaphore wait to guarantee bandwidth for
> the reset preration. The side effect is that we make semaphore
> completion latencies also 1us longer.
> 
> v2: Let full reset handle the adjacent engine idling (Chris)
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
> References: VTHSD#2227190, HSDES#1604216706, BSID#0917
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index b1ab56a1ec31..5655d39c65cb 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -666,6 +666,15 @@ static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>         I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>                    I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>                    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> +
> +       /* WaKBLVECSSemaphoreWaitPoll:kbl */
> +       if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {

Hmm, what revision was production? Just checking we need to ship this
w/a...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Cancel reset preparations on failed resets

[Chris Wilson]

Quoting Mika Kuoppala (2018-06-05 17:03:56)
> Our reset handling has a retry layer further up in the
> chain. As we have told the engine to prepare for reset,
> and failed it, make sure to remove that preparation so
> that the next attempted reset has a clean slate by triggering
> another full prepare cycle for the engines.
> 
> v2: ret as int, simplified cleanup (Chris)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Cancel reset preparations on failed resets

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Cancel reset preparations on failed resets
URL   : https://patchwork.freedesktop.org/series/44288/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5ca5cfc067d0 drm/i915: Cancel reset preparations on failed resets
e892d64e5be1 drm/i915: Add WaKBLVECSSemaphoreWaitPoll
-:34: CHECK:SPACING: spaces preferred around that '+' (ctx:VxV)
#34: FILE: drivers/gpu/drm/i915/i915_reg.h:2245:
+#define RING_SEMA_WAIT_POLL(base) _MMIO((base)+0x24c)
                                               ^

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

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Cancel reset preparations on failed resets

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Cancel reset preparations on failed resets
URL   : https://patchwork.freedesktop.org/series/44288/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4281 -> Patchwork_9207 =

== Summary - WARNING ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       PASS -> DMESG-FAIL (fdo#106103, fdo#102614)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-cnl-y3:          NOTRUN -> INCOMPLETE (fdo#105086)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


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

  Additional (1): fi-cnl-y3 
  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4281 -> Patchwork_9207

  CI_DRM_4281: b2c949c853cfc65073492bba5b96ef54c9886b17 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9207: e892d64e5be1b3a5d608abb5725dba3f0b252168 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e892d64e5be1 drm/i915: Add WaKBLVECSSemaphoreWaitPoll
5ca5cfc067d0 drm/i915: Cancel reset preparations on failed resets

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Cancel reset preparations on failed resets

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Cancel reset preparations on failed resets
URL   : https://patchwork.freedesktop.org/series/44288/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4281_full -> Patchwork_9207_full =

== Summary - WARNING ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@gem_ctx_switch@basic-all-light:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

    igt@gem_eio@hibernate:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@gem_exec_basic@gtt-vebox:
      shard-snb:          SKIP -> INCOMPLETE (fdo#105411)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-apl:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103822)

    igt@kms_setmode@basic:
      shard-kbl:          NOTRUN -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#106023, fdo#103665) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
      shard-glk:          FAIL (fdo#105703) -> PASS

    igt@kms_flip@flip-vs-absolute-wf_vblank:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-hsw:          FAIL (fdo#105189) -> PASS

    
    ==== Warnings ====

    igt@gem_eio@suspend:
      shard-snb:          INCOMPLETE (fdo#105411) -> DMESG-FAIL (fdo#106808)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106808 https://bugs.freedesktop.org/show_bug.cgi?id=106808
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4281 -> Patchwork_9207

  CI_DRM_4281: b2c949c853cfc65073492bba5b96ef54c9886b17 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9207: e892d64e5be1b3a5d608abb5725dba3f0b252168 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

Re: [PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2018-06-05 17:03:57)
>> There is a problem with kbl up to rev E0 where a heavy
>> memory/fabric traffic from adjacent engine(s) can cause an engine
>> reset to fail. This traffic can be from normal memory accesses
>> or it can be from heavy polling on a semaphore wait.
>> 
>> For engine hogging causing a fail, we already fallback to
>> full reset. Which effectively stops all engines and thus
>> we only add a workaround documentation.
>> 
>> For the semaphore wait loop poll case, we add one microsecond
>> poll interval to semaphore wait to guarantee bandwidth for
>> the reset preration. The side effect is that we make semaphore
>> completion latencies also 1us longer.
>> 
>> v2: Let full reset handle the adjacent engine idling (Chris)
>> 
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
>> References: VTHSD#2227190, HSDES#1604216706, BSID#0917
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>> index b1ab56a1ec31..5655d39c65cb 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> @@ -666,6 +666,15 @@ static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>>         I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>>                    I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>>                    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>> +
>> +       /* WaKBLVECSSemaphoreWaitPoll:kbl */
>> +       if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
>
> Hmm, what revision was production? Just checking we need to ship this
> w/a...

The bspec list of revs seems outdated so can't trust that blindly
but already 0x1 is not preprod on that list. Also found nuc in lab
which is 0x02.

-Mika


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

Re: [PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Chris Wilson]

Quoting Mika Kuoppala (2018-06-06 09:40:11)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2018-06-05 17:03:57)
> >> There is a problem with kbl up to rev E0 where a heavy
> >> memory/fabric traffic from adjacent engine(s) can cause an engine
> >> reset to fail. This traffic can be from normal memory accesses
> >> or it can be from heavy polling on a semaphore wait.
> >> 
> >> For engine hogging causing a fail, we already fallback to
> >> full reset. Which effectively stops all engines and thus
> >> we only add a workaround documentation.
> >> 
> >> For the semaphore wait loop poll case, we add one microsecond
> >> poll interval to semaphore wait to guarantee bandwidth for
> >> the reset preration. The side effect is that we make semaphore
> >> completion latencies also 1us longer.
> >> 
> >> v2: Let full reset handle the adjacent engine idling (Chris)
> >> 
> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
> >> References: VTHSD#2227190, HSDES#1604216706, BSID#0917
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> ---
> >> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> >> index b1ab56a1ec31..5655d39c65cb 100644
> >> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> >> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> >> @@ -666,6 +666,15 @@ static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> >>         I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
> >>                    I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> >>                    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> >> +
> >> +       /* WaKBLVECSSemaphoreWaitPoll:kbl */
> >> +       if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
> >
> > Hmm, what revision was production? Just checking we need to ship this
> > w/a...
> 
> The bspec list of revs seems outdated so can't trust that blindly
> but already 0x1 is not preprod on that list. Also found nuc in lab
> which is 0x02.

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

Care to update intel_detect_preproduction_hw() ?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Joonas Lahtinen]

Quoting Mika Kuoppala (2018-06-05 19:03:57)
> There is a problem with kbl up to rev E0 where a heavy
> memory/fabric traffic from adjacent engine(s) can cause an engine
> reset to fail. This traffic can be from normal memory accesses
> or it can be from heavy polling on a semaphore wait.
> 
> For engine hogging causing a fail, we already fallback to
> full reset. Which effectively stops all engines and thus
> we only add a workaround documentation.
> 
> For the semaphore wait loop poll case, we add one microsecond
> poll interval to semaphore wait to guarantee bandwidth for
> the reset preration. The side effect is that we make semaphore
> completion latencies also 1us longer.
> 
> v2: Let full reset handle the adjacent engine idling (Chris)
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
> References: VTHSD#2227190, HSDES#1604216706, BSID#0917
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Skip the RCS engine and this is;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

[PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Mika Kuoppala]

There is a problem with kbl up to rev E0 where a heavy
memory/fabric traffic from adjacent engine(s) can cause an engine
reset to fail. This traffic can be from normal memory accesses
or it can be from heavy polling on a semaphore wait.

For engine hogging causing a fail, we already fallback to
full reset. Which effectively stops all engines and thus
we only add a workaround documentation.

For the semaphore wait loop poll case, we add one microsecond
poll interval to semaphore wait to guarantee bandwidth for
the reset preration. The side effect is that we make semaphore
completion latencies also 1us longer.

v2: Let full reset handle the adjacent engine idling (Chris)
v3: Skip render engine (Joonas), please checkpatch on define (Mika)

References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
References: VTHSD#2227190, HSDES#1604216706, BSID#0917
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h          |  1 +
 drivers/gpu/drm/i915/intel_uncore.c      |  2 ++
 drivers/gpu/drm/i915/intel_workarounds.c | 13 +++++++++++++
 3 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f0317bde3aab..987def26ce82 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2242,6 +2242,7 @@ enum i915_power_well_id {
 #define RING_RESET_CTL(base)	_MMIO((base)+0xd0)
 #define   RESET_CTL_REQUEST_RESET  (1 << 0)
 #define   RESET_CTL_READY_TO_RESET (1 << 1)
+#define RING_SEMA_WAIT_POLL(base) _MMIO((base) + 0x24c)
 
 #define HSW_GTT_CACHE_EN	_MMIO(0x4024)
 #define   GTT_CACHE_EN_ALL	0xF0007FFF
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bb03f6d8b3d1..b892ca8396e8 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2174,6 +2174,8 @@ int intel_gpu_reset(struct drm_i915_private *dev_priv, unsigned engine_mask)
 		 * Thus assume it is best to stop engines on all gens
 		 * where we have a gpu reset.
 		 *
+		 * WaKBLVECSSemaphoreWaitPoll:kbl (on ALL_ENGINES)
+		 *
 		 * WaMediaResetMainRingCleanup:ctg,elk (presumably)
 		 *
 		 * FIXME: Wa for more modern gens needs to be validated
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index b1ab56a1ec31..24b929ce3341 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -666,6 +666,19 @@ static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
 		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
 		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+
+	/* WaKBLVECSSemaphoreWaitPoll:kbl */
+	if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
+		struct intel_engine_cs *engine;
+		unsigned int tmp;
+
+		for_each_engine(engine, dev_priv, tmp) {
+			if (engine->id == RCS)
+				continue;
+
+			I915_WRITE(RING_SEMA_WAIT_POLL(engine->mmio_base), 1);
+		}
+	}
 }
 
 static void glk_gt_workarounds_apply(struct drm_i915_private *dev_priv)
-- 
2.17.0

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Cancel reset preparations on failed resets (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Cancel reset preparations on failed resets (rev2)
URL   : https://patchwork.freedesktop.org/series/44288/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4290 -> Patchwork_9233 =

== Summary - WARNING ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      fi-glk-j4005:       PASS -> FAIL (fdo#103481)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-c:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-cnl-psr:         DMESG-WARN (fdo#104951) -> PASS

    
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097


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

  Additional (1): fi-bdw-5557u 
  Missing    (3): fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4290 -> Patchwork_9233

  CI_DRM_4290: fb7bd7a9480b64dd46964ef5a00aeef33410376d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4510: d1a93aa7e1507de76c6c71be15931cc4b90111bb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9233: ed692445602e01cd5e11ac97572f2e98c354db65 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ed692445602e drm/i915: Add WaKBLVECSSemaphoreWaitPoll
41fcfbcc5c48 drm/i915: Cancel reset preparations on failed resets

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Cancel reset preparations on failed resets (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Cancel reset preparations on failed resets (rev2)
URL   : https://patchwork.freedesktop.org/series/44288/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4290_full -> Patchwork_9233_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_big:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
      shard-hsw:          PASS -> FAIL (fdo#104873)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#102887, fdo#105363)

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822, fdo#104724) +1

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-glk:          FAIL (fdo#105347) -> PASS
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS
      shard-kbl:          FAIL (fdo#99912) -> PASS

    
    ==== Warnings ====

    igt@kms_sysfs_edid_timing:
      shard-hsw:          FAIL (fdo#100047) -> WARN (fdo#100047)

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4290 -> Patchwork_9233

  CI_DRM_4290: fb7bd7a9480b64dd46964ef5a00aeef33410376d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4510: d1a93aa7e1507de76c6c71be15931cc4b90111bb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9233: ed692445602e01cd5e11ac97572f2e98c354db65 @ 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_9233/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add WaKBLVECSSemaphoreWaitPoll

[Mika Kuoppala]

Joonas Lahtinen <joonas.lahtinen@linux.intel.com> writes:

> Quoting Mika Kuoppala (2018-06-05 19:03:57)
>> There is a problem with kbl up to rev E0 where a heavy
>> memory/fabric traffic from adjacent engine(s) can cause an engine
>> reset to fail. This traffic can be from normal memory accesses
>> or it can be from heavy polling on a semaphore wait.
>> 
>> For engine hogging causing a fail, we already fallback to
>> full reset. Which effectively stops all engines and thus
>> we only add a workaround documentation.
>> 
>> For the semaphore wait loop poll case, we add one microsecond
>> poll interval to semaphore wait to guarantee bandwidth for
>> the reset preration. The side effect is that we make semaphore
>> completion latencies also 1us longer.
>> 
>> v2: Let full reset handle the adjacent engine idling (Chris)
>> 
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
>> References: VTHSD#2227190, HSDES#1604216706, BSID#0917
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>
> Skip the RCS engine and this is;
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

RCS engine skipped on v2, and both patches pushed.
Thank you both for review.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Cancel reset preparations on failed resets

[Chris Wilson]

Quoting Mika Kuoppala (2018-05-30 16:02:05)
> Our reset handling has a retry layer further up in the
> chain. As we have told the engine to prepare for reset,
> and failed it, make sure to remove that preparation so
> that the next attempted reset has a clean slate by triggering
> another full prepare cycle for the engines. Note that with
> successful reset, there is nothing to cleanup.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index b36a3b5736a0..68fe4c16acfb 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -2092,22 +2092,26 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv,
>                               unsigned engine_mask)
>  {
>         struct intel_engine_cs *engine;
> -       unsigned int tmp;
> +       unsigned int tmp, ret;

int ret; Might as well keep the types clean.

> -       for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> -               if (gen8_reset_engine_start(engine))
> +       for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
> +               if (gen8_reset_engine_start(engine)) {
> +                       ret = -EIO;
>                         goto not_ready;
> +               }
> +       }
>  
>         if (INTEL_GEN(dev_priv) >= 11)
> -               return gen11_reset_engines(dev_priv, engine_mask);
> +               ret = gen11_reset_engines(dev_priv, engine_mask);
>         else
> -               return gen6_reset_engines(dev_priv, engine_mask);
> +               ret = gen6_reset_engines(dev_priv, engine_mask);
>  
>  not_ready:
> -       for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> -               gen8_reset_engine_cancel(engine);
> +       if (ret)

Or we just always clear the bit here. On the success path, it will be
just a nop. On the not_ready path we're already clearing untouched
engines, so might as well go whole hog and overkill everything?

Might as well throw in a
References: https://bugs.freedesktop.org/show_bug.cgi?id=106684
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Cancel reset preparations on failed resets

[Mika Kuoppala]

Our reset handling has a retry layer further up in the
chain. As we have told the engine to prepare for reset,
and failed it, make sure to remove that preparation so
that the next attempted reset has a clean slate by triggering
another full prepare cycle for the engines. Note that with
successful reset, there is nothing to cleanup.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b36a3b5736a0..68fe4c16acfb 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2092,22 +2092,26 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv,
 			      unsigned engine_mask)
 {
 	struct intel_engine_cs *engine;
-	unsigned int tmp;
+	unsigned int tmp, ret;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
-		if (gen8_reset_engine_start(engine))
+	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+		if (gen8_reset_engine_start(engine)) {
+			ret = -EIO;
 			goto not_ready;
+		}
+	}
 
 	if (INTEL_GEN(dev_priv) >= 11)
-		return gen11_reset_engines(dev_priv, engine_mask);
+		ret = gen11_reset_engines(dev_priv, engine_mask);
 	else
-		return gen6_reset_engines(dev_priv, engine_mask);
+		ret = gen6_reset_engines(dev_priv, engine_mask);
 
 not_ready:
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
-		gen8_reset_engine_cancel(engine);
+	if (ret)
+		for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
+			gen8_reset_engine_cancel(engine);
 
-	return -EIO;
+	return ret;
 }
 
 typedef int (*reset_func)(struct drm_i915_private *, unsigned engine_mask);
-- 
2.17.0

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