[PATCH 1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[PATCH 1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[Chris Wilson]

Despite what I think the prm recommends, commit f2253bd9859b
("drm/i915/ringbuffer: EMIT_INVALIDATE after switch context") turned out
to be a huge mistake when enabling Ironlake contexts as the GPU would
hang on either a MI_FLUSH or PIPE_CONTROL immediately following the
MI_SET_CONTEXT of an active mesa context (more vanilla contexts, e.g.
simple rendercopies with igt, do not suffer).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3844581f622c..8feb2d9b7b60 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1882,12 +1882,12 @@ static int ring_request_alloc(struct i915_request *request)
 	 */
 	request->reserved_space += LEGACY_REQUEST_SIZE;
 
-	ret = switch_context(request);
+	/* Unconditionally invalidate GPU caches and TLBs. */
+	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
 	if (ret)
 		return ret;
 
-	/* Unconditionally invalidate GPU caches and TLBs. */
-	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
+	ret = switch_context(request);
 	if (ret)
 		return ret;
 
-- 
2.20.1

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

[PATCH 2/3] drm/i915: Enable render context support for Ironlake (gen5)

[Chris Wilson]

Ironlake does support being able to saving and reloading context specific
registers between contexts, providing isolation of the basic GPU state
(as programmable by userspace). This allows userspace to assume that the
GPU retains their state from one batch to the next, minimising the
amount of state it needs to reload, or manually save and restore.

v2: Fix off-by-one in reading CXT_SIZE, and add a comment that the
CXT_SIZE and context-layout do not match in bspec, but the difference is
irrelevant as we overallocate the full page anyway (Ville).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 13 +++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index eea9bec04f1b..fc8be2fcb4e6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -211,6 +211,22 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
 			return round_up(GEN6_CXT_TOTAL_SIZE(cxt_size) * 64,
 					PAGE_SIZE);
 		case 5:
+			/*
+			 * There is a discrepancy here between the size reported
+			 * by the register and the size of the context layout
+			 * in the docs. Both are described as authorative!
+			 *
+			 * The discrepancy is on the order of a few cachelines,
+			 * but the total is under one page (4k), which is our
+			 * minimum allocation anyway so it should all come
+			 * out in the wash.
+			 */
+			cxt_size = I915_READ(CXT_SIZE) + 1;
+			DRM_DEBUG_DRIVER("gen%d CXT_SIZE = %d bytes [0x%08x]\n",
+					 INTEL_GEN(dev_priv),
+					 cxt_size * 64,
+					 cxt_size - 1);
+			return round_up(cxt_size * 64, PAGE_SIZE);
 		case 4:
 		case 3:
 		case 2:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8feb2d9b7b60..2d2e33cd3fae 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1640,11 +1640,14 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
 		/* These flags are for resource streamer on HSW+ */
 		flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN;
 	else
+		/* We need to save the extended state for powersaving modes */
 		flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN;
 
 	len = 4;
 	if (IS_GEN(i915, 7))
 		len += 2 + (num_engines ? 4 * num_engines + 6 : 0);
+	else if (IS_GEN(i915, 5))
+		len += 2;
 	if (flags & MI_FORCE_RESTORE) {
 		GEM_BUG_ON(flags & MI_RESTORE_INHIBIT);
 		flags &= ~MI_FORCE_RESTORE;
@@ -1673,6 +1676,14 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
 						GEN6_PSMI_SLEEP_MSG_DISABLE);
 			}
 		}
+	} else if (IS_GEN(i915, 5)) {
+		/*
+		 * This w/a is only listed for pre-production ilk a/b steppings,
+		 * but is also mentioned for programming the powerctx. To be
+		 * safe, just apply the workaround; we do not use SyncFlush so
+		 * this should never take effect and so be a no-op!
+		 */
+		*cs++ = MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN;
 	}
 
 	if (force_restore) {
@@ -1726,6 +1737,8 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
 			*cs++ = MI_NOOP;
 		}
 		*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
+	} else if (IS_GEN(i915, 5)) {
+		*cs++ = MI_SUSPEND_FLUSH;
 	}
 
 	intel_ring_advance(rq, cs);
-- 
2.20.1

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

[PATCH 3/3] drm/i915: Enable render context support for gen4 (Broadwater to Cantiga)

[Chris Wilson]

Broadwater and the rest of gen4  do support being able to saving and
reloading context specific registers between contexts, providing isolation
of the basic GPU state (as programmable by userspace). This allows
userspace to assume that the GPU retains their state from one batch to the
next, minimising the amount of state it needs to reload and manually save
across batches.

v2: CONSTANT_BUFFER woes

Running through piglit turned up an interesting issue, a GPU hang inside
the context load. The context image includes the CONSTANT_BUFFER command
that loads an address into a on-gpu buffer, and the context load was
executing that immediately. However, since it was reading from the GTT
there is no guarantee that the GTT retains the same configuration as
when the context was saved, resulting in stray reads and a GPU hang.

Having tried issuing a CONSTANT_BUFFER (to disable the command) from the
ring before saving the context to no avail, we resort to patching out
the instruction inside the context image before loading.

This does impose that gen4 always reissues CONSTANT_BUFFER commands on
each batch, but due to the use of a shared GTT that was and will remain
a requirement.

v3: ECOSPKD to the rescue

Ville found the magic bit in the ECOSPKD to disable saving and restoring
the CONSTANT_BUFFER from the context image, thereby completely avoiding
the GPU hangs from chasing invalid pointers. This appears to be the
default behaviour for gen5, and so we just need to tweak gen4 to match.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/i915_reg.h         |  3 +++
 drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b74824f0b5b1..5815703ac35f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2665,6 +2665,9 @@ enum i915_power_well_id {
 # define MODE_IDLE					(1 << 9)
 # define STOP_RING					(1 << 8)
 
+#define ECOSPKD		_MMIO(0x21d0)
+# define CONSTANT_BUFFER_SR_DISABLE BIT(4)
+
 #define GEN6_GT_MODE	_MMIO(0x20d0)
 #define GEN7_GT_MODE	_MMIO(0x7008)
 #define   GEN6_WIZ_HASHING(hi, lo)			(((hi) << 9) | ((lo) << 7))
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index fc8be2fcb4e6..f9db2e0bca12 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -211,6 +211,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
 			return round_up(GEN6_CXT_TOTAL_SIZE(cxt_size) * 64,
 					PAGE_SIZE);
 		case 5:
+		case 4:
 			/*
 			 * There is a discrepancy here between the size reported
 			 * by the register and the size of the context layout
@@ -227,7 +228,6 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
 					 cxt_size * 64,
 					 cxt_size - 1);
 			return round_up(cxt_size * 64, PAGE_SIZE);
-		case 4:
 		case 3:
 		case 2:
 		/* For the special day when i810 gets merged. */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2d2e33cd3fae..26b276ed00b3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -832,6 +832,20 @@ static int init_render_ring(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	/*
+	 * Disable CONSTANT_BUFFER before it is loaded from the context
+	 * image. For as it is loaded, it is executed and the stored
+	 * address may no longer be valid, leading to a GPU hang.
+	 *
+	 * This imposes the requirement that userspace reload their
+	 * CONSTANT_BUFFER on every batch, fortunately a requirement
+	 * they are already accustomed to from before contexts were
+	 * enabled.
+	 */
+	if (IS_GEN(dev_priv, 4))
+		I915_WRITE(ECOSPKD,
+			   _MASKED_BIT_ENABLE(CONSTANT_BUFFER_SR_DISABLE));
+
 	/* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
 	if (IS_GEN_RANGE(dev_priv, 4, 6))
 		I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
-- 
2.20.1

_______________________________________________
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/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context
URL   : https://patchwork.freedesktop.org/series/59752/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5956 -> Patchwork_12842
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +57

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic:
    - fi-elk-e7500:       SKIP [fdo#109271] -> PASS +5

  * igt@gem_ctx_exec@basic:
    - fi-bwr-2160:        SKIP [fdo#109271] -> PASS +5

  * igt@gem_ctx_param@basic-default:
    - fi-ilk-650:         SKIP [fdo#109271] -> PASS +5

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      FAIL [fdo#108511] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720


Participating hosts (46 -> 36)
------------------------------

  Additional (1): fi-bsw-n3050 
  Missing    (11): fi-ilk-m540 fi-hsw-peppy fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-icl-u3 fi-byt-n2820 fi-bdw-samus fi-snb-2600 


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

  * Linux: CI_DRM_5956 -> Patchwork_12842

  CI_DRM_5956: 84826560c8c7420ccf497f5ecce91862f357fac1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12842: 5123afc5f51d36b59e8132f25709b8e6a1678672 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5123afc5f51d drm/i915: Enable render context support for gen4 (Broadwater to Cantiga)
ea8c760fb66f drm/i915: Enable render context support for Ironlake (gen5)
6469fe48fe5a drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12842/
_______________________________________________
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/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context
URL   : https://patchwork.freedesktop.org/series/59752/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5956_full -> Patchwork_12842_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@audio@hdmi-integrity}:
    - shard-glk:          INCOMPLETE [fdo#103359] / [k.org#198133] -> TIMEOUT

  * {igt@audio@hdmi-integrity-after-suspend}:
    - shard-apl:          NOTRUN -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_capture@capture-bsd2:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +42

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +5

  * igt@i915_suspend@forcewake:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#108566]

  * igt@kms_busy@basic-flip-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +8

  * igt@kms_busy@extended-pageflip-hang-newfb-render-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +4

  * igt@kms_cursor_crc@cursor-64x64-random:
    - shard-snb:          PASS -> SKIP [fdo#109271]

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-xtiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#103060]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#109507]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-cur-indfb-move:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +100

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-render:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +18

  * igt@kms_lease@cursor_implicit_plane:
    - shard-apl:          NOTRUN -> FAIL [fdo#110278]

  * igt@kms_lease@page_flip_implicit_plane:
    - shard-apl:          NOTRUN -> FAIL [fdo#110281]

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_psr@psr2_sprite_mmap_cpu:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         PASS -> FAIL [fdo#100047]

  * igt@kms_universal_plane@universal-plane-gen9-features-pipe-d:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@tools_test@tools_test:
    - shard-apl:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         FAIL [fdo#108686] -> PASS

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +9

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#108145] / [fdo#110403] -> PASS

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         SKIP [fdo#109441] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +4

  
#### Warnings ####

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         DMESG-FAIL [fdo#110271] -> FAIL [fdo#104097]

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

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110271]: https://bugs.freedesktop.org/show_bug.cgi?id=110271
  [fdo#110278]: https://bugs.freedesktop.org/show_bug.cgi?id=110278
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [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 (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


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

  * Linux: CI_DRM_5956 -> Patchwork_12842

  CI_DRM_5956: 84826560c8c7420ccf497f5ecce91862f357fac1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12842: 5123afc5f51d36b59e8132f25709b8e6a1678672 @ 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_12842/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Enable render context support for gen4 (Broadwater to Cantiga)

[Chris Wilson]

Broadwater and the rest of gen4  do support being able to saving and
reloading context specific registers between contexts, providing isolation
of the basic GPU state (as programmable by userspace). This allows
userspace to assume that the GPU retains their state from one batch to the
next, minimising the amount of state it needs to reload and manually save
across batches.

v2: CONSTANT_BUFFER woes

Running through piglit turned up an interesting issue, a GPU hang inside
the context load. The context image includes the CONSTANT_BUFFER command
that loads an address into a on-gpu buffer, and the context load was
executing that immediately. However, since it was reading from the GTT
there is no guarantee that the GTT retains the same configuration as
when the context was saved, resulting in stray reads and a GPU hang.

Having tried issuing a CONSTANT_BUFFER (to disable the command) from the
ring before saving the context to no avail, we resort to patching out
the instruction inside the context image before loading.

This does impose that gen4 always reissues CONSTANT_BUFFER commands on
each batch, but due to the use of a shared GTT that was and will remain
a requirement.

v3: ECOSKPD to the rescue

Ville found the magic bit in the ECOSKPD to disable saving and restoring
the CONSTANT_BUFFER from the context image, thereby completely avoiding
the GPU hangs from chasing invalid pointers. This appears to be the
default behaviour for gen5, and so we just need to tweak gen4 to match.

v4: Fix spelling of ECOSKPD and discover it already exists

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/i915_reg.h         |  1 +
 drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b74824f0b5b1..6f0a0866c802 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2870,6 +2870,7 @@ enum i915_power_well_id {
 #define GFX_FLSH_CNTL_GEN6	_MMIO(0x101008)
 #define   GFX_FLSH_CNTL_EN	(1 << 0)
 #define ECOSKPD		_MMIO(0x21d0)
+#define   ECO_CONSTANT_BUFFER_SR_DISABLE REG_BIT(4)
 #define   ECO_GATING_CX_ONLY	(1 << 3)
 #define   ECO_FLIP_DONE		(1 << 0)
 
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index fc8be2fcb4e6..f9db2e0bca12 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -211,6 +211,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
 			return round_up(GEN6_CXT_TOTAL_SIZE(cxt_size) * 64,
 					PAGE_SIZE);
 		case 5:
+		case 4:
 			/*
 			 * There is a discrepancy here between the size reported
 			 * by the register and the size of the context layout
@@ -227,7 +228,6 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
 					 cxt_size * 64,
 					 cxt_size - 1);
 			return round_up(cxt_size * 64, PAGE_SIZE);
-		case 4:
 		case 3:
 		case 2:
 		/* For the special day when i810 gets merged. */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2d2e33cd3fae..a19c50147f6c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -832,6 +832,20 @@ static int init_render_ring(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	/*
+	 * Disable CONSTANT_BUFFER before it is loaded from the context
+	 * image. For as it is loaded, it is executed and the stored
+	 * address may no longer be valid, leading to a GPU hang.
+	 *
+	 * This imposes the requirement that userspace reload their
+	 * CONSTANT_BUFFER on every batch, fortunately a requirement
+	 * they are already accustomed to from before contexts were
+	 * enabled.
+	 */
+	if (IS_GEN(dev_priv, 4))
+		I915_WRITE(ECOSKPD,
+			   _MASKED_BIT_ENABLE(ECO_CONSTANT_BUFFER_SR_DISABLE));
+
 	/* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
 	if (IS_GEN_RANGE(dev_priv, 4, 6))
 		I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
-- 
2.20.1

_______________________________________________
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/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context (rev2)
URL   : https://patchwork.freedesktop.org/series/59752/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5958 -> Patchwork_12845
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-snb-2520m:       NOTRUN -> SKIP [fdo#109271] +52

  * igt@gem_exec_store@basic-bsd2:
    - fi-hsw-4770:        NOTRUN -> SKIP [fdo#109271] +41

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       PASS -> DMESG-WARN [fdo#105128] / [fdo#107139]

  * igt@gem_mmap_gtt@basic-write-cpu-read-gtt:
    - fi-apl-guc:         NOTRUN -> SKIP [fdo#109271] +48

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL [fdo#108511]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         NOTRUN -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@basic-flip-c:
    - fi-snb-2520m:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-bxt-dsi:         NOTRUN -> SKIP [fdo#109271] +47

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +57
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] +47

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]
    - fi-skl-iommu:       NOTRUN -> FAIL [fdo#104108] / [fdo#108602]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic:
    - fi-elk-e7500:       SKIP [fdo#109271] -> PASS +5

  * igt@gem_ctx_exec@basic:
    - fi-ilk-650:         SKIP [fdo#109271] -> PASS +5

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-glk-dsi:         FAIL [fdo#103191] -> PASS

  
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (36 -> 32)
------------------------------

  Additional (6): fi-bxt-dsi fi-bsw-n3050 fi-byt-j1900 fi-apl-guc fi-snb-2520m fi-hsw-4770 
  Missing    (10): fi-ilk-m540 fi-hsw-peppy fi-byt-squawks fi-skl-6260u fi-ctg-p8600 fi-whl-u fi-bxt-j4205 fi-gdg-551 fi-icl-u3 fi-bdw-samus 


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

  * Linux: CI_DRM_5958 -> Patchwork_12845

  CI_DRM_5958: dabbe59fd2bb5dca3d2607c49369161ec7160df3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12845: 6878f8a9284efe2167acd0b11aea0b565c63ef65 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6878f8a9284e drm/i915: Enable render context support for gen4 (Broadwater to Cantiga)
6b9436fbada4 drm/i915: Enable render context support for Ironlake (gen5)
260bff3d0251 drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12845/
_______________________________________________
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/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context (rev2)
URL   : https://patchwork.freedesktop.org/series/59752/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5958_full -> Patchwork_12845_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@audio@hdmi-integrity}:
    - shard-skl:          NOTRUN -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-self-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +3

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +4

  * igt@i915_suspend@forcewake:
    - shard-snb:          PASS -> DMESG-WARN [fdo#102365]

  * igt@kms_ccs@pipe-b-crc-primary-basic:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713]

  * igt@kms_chamelium@hdmi-crc-fast:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +2

  * igt@kms_cursor_legacy@cursorb-vs-flipa-atomic-transitions:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_draw_crc@draw-method-rgb565-render-xtiled:
    - shard-snb:          PASS -> SKIP [fdo#109271] +3

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +5

  * igt@kms_frontbuffer_tracking@fbcpsr-tilingchange:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +8

  * igt@kms_pipe_b_c_ivb@enable-pipe-c-while-b-has-3-lanes:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@kms_pipe_crc_basic@read-crc-pipe-e:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +2

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +13

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +4

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441] +1

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_vrr@flip-suspend:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +178

  * igt@prime_vgem@fence-write-hang:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109295]

  * igt@tools_test@sysfs_l3_parity:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109307]

  
#### Possible fixes ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         DMESG-WARN [fdo#108686] -> PASS

  * igt@i915_pm_rpm@gem-idle:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +3

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-xtiled:
    - shard-skl:          FAIL [fdo#103184] -> PASS

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          INCOMPLETE [fdo#109507] -> PASS

  * igt@kms_flip@plain-flip-ts-check-interruptible:
    - shard-skl:          FAIL [fdo#100368] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +3

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS +1

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@psr2_sprite_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         FAIL [fdo#100047] -> PASS

  * igt@prime_busy@hang-default:
    - shard-snb:          FAIL [fdo#108807] -> PASS

  
#### Warnings ####

  * igt@i915_pm_rpm@modeset-pc8-residency-stress:
    - shard-skl:          SKIP [fdo#109271] -> INCOMPLETE [fdo#107807]

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

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108807]: https://bugs.freedesktop.org/show_bug.cgi?id=108807
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


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

  * Linux: CI_DRM_5958 -> Patchwork_12845

  CI_DRM_5958: dabbe59fd2bb5dca3d2607c49369161ec7160df3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12845: 6878f8a9284efe2167acd0b11aea0b565c63ef65 @ 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_12845/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[Ville Syrjälä]

On Fri, Apr 19, 2019 at 12:17:47PM +0100, Chris Wilson wrote:
> Despite what I think the prm recommends, commit f2253bd9859b
> ("drm/i915/ringbuffer: EMIT_INVALIDATE after switch context") turned out
> to be a huge mistake when enabling Ironlake contexts as the GPU would
> hang on either a MI_FLUSH or PIPE_CONTROL immediately following the
> MI_SET_CONTEXT of an active mesa context (more vanilla contexts, e.g.
> simple rendercopies with igt, do not suffer).

Where is the recommendation you mention? I couldn't immediately find it
in the docs. I did find the following statemtement:

"[DevCTG+]: For the invalidate operation of the pipe control, the
 following pointers are affected. The
 invalidate operation affects the restore of these packets. If the pipe
 control invalidate operation is completed
 before the context save, the indirect pointers will not be restored from
 memory.
 1. Pipeline State Pointer
 2. Media State Pointer
 3. Constant Buffer Packet"

Which maybe has something to do with this ordering?

But it's all black magic anyways. If it works it works.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3844581f622c..8feb2d9b7b60 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1882,12 +1882,12 @@ static int ring_request_alloc(struct i915_request *request)
>  	 */
>  	request->reserved_space += LEGACY_REQUEST_SIZE;
>  
> -	ret = switch_context(request);
> +	/* Unconditionally invalidate GPU caches and TLBs. */
> +	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
>  	if (ret)
>  		return ret;
>  
> -	/* Unconditionally invalidate GPU caches and TLBs. */
> -	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
> +	ret = switch_context(request);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[Chris Wilson]

Quoting Ville Syrjälä (2019-04-23 16:36:47)
> On Fri, Apr 19, 2019 at 12:17:47PM +0100, Chris Wilson wrote:
> > Despite what I think the prm recommends, commit f2253bd9859b
> > ("drm/i915/ringbuffer: EMIT_INVALIDATE after switch context") turned out
> > to be a huge mistake when enabling Ironlake contexts as the GPU would
> > hang on either a MI_FLUSH or PIPE_CONTROL immediately following the
> > MI_SET_CONTEXT of an active mesa context (more vanilla contexts, e.g.
> > simple rendercopies with igt, do not suffer).
> 
> Where is the recommendation you mention? I couldn't immediately find it
> in the docs. I did find the following statemtement:

Sadly, it probably Chinese whispers from our code comments. I have
recollection of us having a comment about expected ordering and not
obeying because we "always invalidate before the batch". I have some
recollection of seeing related instructions in the ye old bspec, but I
cannot lay my hands on anything concreter right now.
 
> "[DevCTG+]: For the invalidate operation of the pipe control, the
>  following pointers are affected. The
>  invalidate operation affects the restore of these packets. If the pipe
>  control invalidate operation is completed
>  before the context save, the indirect pointers will not be restored from
>  memory.
>  1. Pipeline State Pointer
>  2. Media State Pointer
>  3. Constant Buffer Packet"
> 
> Which maybe has something to do with this ordering?

That's very interesting. If the invalidate is preventing the loading of
dangling pointer, then yes, it fits with the observation in this patch.

There's also a comment about ensure we flush media before switching to a
3D context (which we do by virtue of the flush-everything after a
request -- or we should!).

> But it's all black magic anyways. If it works it works.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Proof is in the eating. Ta,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx