All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] drm/i915: Define explicit wedged on init reset state
@ 2019-09-26 10:06 Michał Winiarski
  2019-09-26 10:06 ` [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch Michał Winiarski
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 10:06 UTC (permalink / raw)
  To: intel-gfx

We're currently using scratch presence as a way of identifying that we
entered wedged state at driver initialization time.
Let's use a separate flag rather than rely on scratch.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_reset.c       | 11 ++++++++++-
 drivers/gpu/drm/i915/gt/intel_reset.h       |  9 +++++++++
 drivers/gpu/drm/i915/gt/intel_reset_types.h |  6 ++++++
 drivers/gpu/drm/i915/i915_gem.c             |  2 +-
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index ae68c3786f63..0f1534ac823f 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -811,7 +811,8 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt)
 	if (!test_bit(I915_WEDGED, &gt->reset.flags))
 		return true;
 
-	if (!gt->scratch) /* Never full initialised, recovery impossible */
+	/* Never fullly initialised, recovery impossible */
+	if (test_bit(I915_WEDGED_ON_INIT, &gt->reset.flags))
 		return false;
 
 	GEM_TRACE("start\n");
@@ -1279,6 +1280,14 @@ int intel_gt_terminally_wedged(struct intel_gt *gt)
 	return intel_gt_is_wedged(gt) ? -EIO : 0;
 }
 
+void intel_gt_set_wedged_on_init(struct intel_gt *gt)
+{
+	BUILD_BUG_ON(I915_RESET_ENGINE + I915_NUM_ENGINES >
+		     I915_WEDGED_ON_INIT);
+	intel_gt_set_wedged(gt);
+	set_bit(I915_WEDGED_ON_INIT, &gt->reset.flags);
+}
+
 void intel_gt_init_reset(struct intel_gt *gt)
 {
 	init_waitqueue_head(&gt->reset.queue);
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
index 52c00199e069..0b6ff1ee7f06 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.h
+++ b/drivers/gpu/drm/i915/gt/intel_reset.h
@@ -45,6 +45,12 @@ void intel_gt_set_wedged(struct intel_gt *gt);
 bool intel_gt_unset_wedged(struct intel_gt *gt);
 int intel_gt_terminally_wedged(struct intel_gt *gt);
 
+/*
+ * There's no unset_wedged_on_init paired with this one.
+ * Once we're wedged on init, there's no going back.
+ */
+void intel_gt_set_wedged_on_init(struct intel_gt *gt);
+
 int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask);
 
 int intel_reset_guc(struct intel_gt *gt);
@@ -68,6 +74,9 @@ void __intel_fini_wedge(struct intel_wedge_me *w);
 
 static inline bool __intel_reset_failed(const struct intel_reset *reset)
 {
+	GEM_BUG_ON(test_bit(I915_WEDGED_ON_INIT, &reset->flags) ?
+		   !test_bit(I915_WEDGED, &reset->flags) : false);
+
 	return unlikely(test_bit(I915_WEDGED, &reset->flags));
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_reset_types.h b/drivers/gpu/drm/i915/gt/intel_reset_types.h
index 31968356e0c0..f43bc3a0fe4f 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_reset_types.h
@@ -29,11 +29,17 @@ struct intel_reset {
 	 * we set the #I915_WEDGED bit. Prior to command submission, e.g.
 	 * i915_request_alloc(), this bit is checked and the sequence
 	 * aborted (with -EIO reported to userspace) if set.
+	 *
+	 * #I915_WEDGED_ON_INIT - If we fail to initialize the GPU we can no
+	 * longer use the GPU - similar to #I915_WEDGED bit. The difference in
+	 * in the way we're handling "forced" unwedged (e.g. through debugfs),
+	 * which is not allowed in case we failed to initialize.
 	 */
 	unsigned long flags;
 #define I915_RESET_BACKOFF	0
 #define I915_RESET_MODESET	1
 #define I915_RESET_ENGINE	2
+#define I915_WEDGED_ON_INIT	(BITS_PER_LONG - 2)
 #define I915_WEDGED		(BITS_PER_LONG - 1)
 
 	struct mutex mutex; /* serialises wedging/unwedging */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2da9544fa9a4..e2897a666225 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1411,7 +1411,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 err_gt:
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	intel_gt_set_wedged(&dev_priv->gt);
+	intel_gt_set_wedged_on_init(&dev_priv->gt);
 	i915_gem_suspend(dev_priv);
 	i915_gem_suspend_late(dev_priv);
 
-- 
2.21.0

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

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

* [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
@ 2019-09-26 10:06 ` Michał Winiarski
  2019-09-26 10:18   ` Chris Wilson
  2019-09-26 10:06 ` [PATCH 3/6] drm/i915: Adjust length of MI_LOAD_REGISTER_REG Michał Winiarski
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 10:06 UTC (permalink / raw)
  To: intel-gfx

Some of our commands (MI_FLUSH_DW / PIPE_CONTROL) require a post-sync write
operation to be performed. Currently we're using dedicated VMA for
PIPE_CONTROL and global HWSP for MI_FLUSH_DW.
On execlists platforms, each of our contexts has an area that can be
used as scratch space. Let's use that instead.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_types.h |  3 --
 drivers/gpu/drm/i915/gt/intel_lrc.c      | 45 +++++++-----------------
 drivers/gpu/drm/i915/gt/intel_lrc.h      |  4 +++
 3 files changed, 17 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 3039cef64b11..e64db4c13df6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -89,9 +89,6 @@ enum intel_gt_scratch_field {
 	/* 8 bytes */
 	INTEL_GT_SCRATCH_FIELD_DEFAULT = 0,
 
-	/* 8 bytes */
-	INTEL_GT_SCRATCH_FIELD_CLEAR_SLM_WA = 128,
-
 	/* 8 bytes */
 	INTEL_GT_SCRATCH_FIELD_RENDER_FLUSH = 128,
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index ab725a6ca0ac..fa385218ce92 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2308,12 +2308,6 @@ gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine, u32 *batch)
 	return batch;
 }
 
-static u32 slm_offset(struct intel_engine_cs *engine)
-{
-	return intel_gt_scratch_offset(engine->gt,
-				       INTEL_GT_SCRATCH_FIELD_CLEAR_SLM_WA);
-}
-
 /*
  * Typically we only have one indirect_ctx and per_ctx batch buffer which are
  * initialized at the beginning and shared across all contexts but this field
@@ -2342,10 +2336,10 @@ static u32 *gen8_init_indirectctx_bb(struct intel_engine_cs *engine, u32 *batch)
 	/* Actual scratch location is at 128 bytes offset */
 	batch = gen8_emit_pipe_control(batch,
 				       PIPE_CONTROL_FLUSH_L3 |
-				       PIPE_CONTROL_GLOBAL_GTT_IVB |
+				       PIPE_CONTROL_STORE_DATA_INDEX |
 				       PIPE_CONTROL_CS_STALL |
 				       PIPE_CONTROL_QW_WRITE,
-				       slm_offset(engine));
+				       LRC_PPHWSP_SCRATCH_ADDR);
 
 	*batch++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
 
@@ -3052,7 +3046,7 @@ static int gen8_emit_flush(struct i915_request *request, u32 mode)
 	}
 
 	*cs++ = cmd;
-	*cs++ = I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT;
+	*cs++ = LRC_PPHWSP_SCRATCH_ADDR;
 	*cs++ = 0; /* upper addr */
 	*cs++ = 0; /* value */
 	intel_ring_advance(request, cs);
@@ -3063,10 +3057,6 @@ static int gen8_emit_flush(struct i915_request *request, u32 mode)
 static int gen8_emit_flush_render(struct i915_request *request,
 				  u32 mode)
 {
-	struct intel_engine_cs *engine = request->engine;
-	u32 scratch_addr =
-		intel_gt_scratch_offset(engine->gt,
-					INTEL_GT_SCRATCH_FIELD_RENDER_FLUSH);
 	bool vf_flush_wa = false, dc_flush_wa = false;
 	u32 *cs, flags = 0;
 	int len;
@@ -3088,7 +3078,7 @@ static int gen8_emit_flush_render(struct i915_request *request,
 		flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_QW_WRITE;
-		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
 
 		/*
 		 * On GEN9: before VF_CACHE_INVALIDATE we need to emit a NULL
@@ -3121,7 +3111,7 @@ static int gen8_emit_flush_render(struct i915_request *request,
 		cs = gen8_emit_pipe_control(cs, PIPE_CONTROL_DC_FLUSH_ENABLE,
 					    0);
 
-	cs = gen8_emit_pipe_control(cs, flags, scratch_addr);
+	cs = gen8_emit_pipe_control(cs, flags, LRC_PPHWSP_SCRATCH_ADDR);
 
 	if (dc_flush_wa)
 		cs = gen8_emit_pipe_control(cs, PIPE_CONTROL_CS_STALL, 0);
@@ -3134,11 +3124,6 @@ static int gen8_emit_flush_render(struct i915_request *request,
 static int gen11_emit_flush_render(struct i915_request *request,
 				   u32 mode)
 {
-	struct intel_engine_cs *engine = request->engine;
-	const u32 scratch_addr =
-		intel_gt_scratch_offset(engine->gt,
-					INTEL_GT_SCRATCH_FIELD_RENDER_FLUSH);
-
 	if (mode & EMIT_FLUSH) {
 		u32 *cs;
 		u32 flags = 0;
@@ -3151,13 +3136,13 @@ static int gen11_emit_flush_render(struct i915_request *request,
 		flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
 		flags |= PIPE_CONTROL_FLUSH_ENABLE;
 		flags |= PIPE_CONTROL_QW_WRITE;
-		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
 
 		cs = intel_ring_begin(request, 6);
 		if (IS_ERR(cs))
 			return PTR_ERR(cs);
 
-		cs = gen8_emit_pipe_control(cs, flags, scratch_addr);
+		cs = gen8_emit_pipe_control(cs, flags, LRC_PPHWSP_SCRATCH_ADDR);
 		intel_ring_advance(request, cs);
 	}
 
@@ -3175,13 +3160,13 @@ static int gen11_emit_flush_render(struct i915_request *request,
 		flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_QW_WRITE;
-		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
 
 		cs = intel_ring_begin(request, 6);
 		if (IS_ERR(cs))
 			return PTR_ERR(cs);
 
-		cs = gen8_emit_pipe_control(cs, flags, scratch_addr);
+		cs = gen8_emit_pipe_control(cs, flags, LRC_PPHWSP_SCRATCH_ADDR);
 		intel_ring_advance(request, cs);
 	}
 
@@ -3196,10 +3181,6 @@ static u32 preparser_disable(bool state)
 static int gen12_emit_flush_render(struct i915_request *request,
 				   u32 mode)
 {
-	const u32 scratch_addr =
-		intel_gt_scratch_offset(request->engine->gt,
-					INTEL_GT_SCRATCH_FIELD_RENDER_FLUSH);
-
 	if (mode & EMIT_FLUSH) {
 		u32 flags = 0;
 		u32 *cs;
@@ -3210,7 +3191,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
 		flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
 		flags |= PIPE_CONTROL_FLUSH_ENABLE;
 
-		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
 		flags |= PIPE_CONTROL_QW_WRITE;
 
 		flags |= PIPE_CONTROL_CS_STALL;
@@ -3219,7 +3200,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
 		if (IS_ERR(cs))
 			return PTR_ERR(cs);
 
-		cs = gen8_emit_pipe_control(cs, flags, scratch_addr);
+		cs = gen8_emit_pipe_control(cs, flags, LRC_PPHWSP_SCRATCH_ADDR);
 		intel_ring_advance(request, cs);
 	}
 
@@ -3235,7 +3216,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
 		flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
 
-		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
 		flags |= PIPE_CONTROL_QW_WRITE;
 
 		flags |= PIPE_CONTROL_CS_STALL;
@@ -3251,7 +3232,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
 		 */
 		*cs++ = preparser_disable(true);
 
-		cs = gen8_emit_pipe_control(cs, flags, scratch_addr);
+		cs = gen8_emit_pipe_control(cs, flags, LRC_PPHWSP_SCRATCH_ADDR);
 
 		*cs++ = preparser_disable(false);
 		intel_ring_advance(request, cs);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h b/drivers/gpu/drm/i915/gt/intel_lrc.h
index dc0252e0589e..66ac616361c1 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
@@ -104,6 +104,10 @@ int intel_execlists_submission_init(struct intel_engine_cs *engine);
  */
 #define LRC_HEADER_PAGES LRC_PPHWSP_PN
 
+/* Space within PPHWSP reserved to be used as scratch */
+#define LRC_PPHWSP_SCRATCH		0x34
+#define LRC_PPHWSP_SCRATCH_ADDR		(LRC_PPHWSP_SCRATCH * sizeof(u32))
+
 void intel_execlists_set_default_submission(struct intel_engine_cs *engine);
 
 void intel_lr_context_reset(struct intel_engine_cs *engine,
-- 
2.21.0

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

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

* [PATCH 3/6] drm/i915: Adjust length of MI_LOAD_REGISTER_REG
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
  2019-09-26 10:06 ` [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch Michał Winiarski
@ 2019-09-26 10:06 ` Michał Winiarski
  2019-09-26 10:15   ` Chris Wilson
  2019-09-26 10:06 ` [PATCH 4/6] drm/i915: Add definitions for MI_MATH command Michał Winiarski
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 10:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

Default length value of MI_LOAD_REGISTER_REG is 1.
Also move it out of cmd-parser-only registers since we're going to use
it in i915.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index f78b13d74e17..9211b1ad401b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -152,6 +152,7 @@
 #define   MI_FLUSH_DW_USE_PPGTT		(0<<2)
 #define MI_LOAD_REGISTER_MEM	   MI_INSTR(0x29, 1)
 #define MI_LOAD_REGISTER_MEM_GEN8  MI_INSTR(0x29, 2)
+#define MI_LOAD_REGISTER_REG    MI_INSTR(0x2A, 1)
 #define MI_BATCH_BUFFER		MI_INSTR(0x30, 1)
 #define   MI_BATCH_NON_SECURE		(1)
 /* for snb/ivb/vlv this also means "batch in ppgtt" when ppgtt is enabled. */
@@ -256,7 +257,6 @@
 #define MI_CLFLUSH              MI_INSTR(0x27, 0)
 #define MI_REPORT_PERF_COUNT    MI_INSTR(0x28, 0)
 #define   MI_REPORT_PERF_COUNT_GGTT (1<<0)
-#define MI_LOAD_REGISTER_REG    MI_INSTR(0x2A, 0)
 #define MI_RS_STORE_DATA_IMM    MI_INSTR(0x2B, 0)
 #define MI_LOAD_URB_MEM         MI_INSTR(0x2C, 0)
 #define MI_STORE_URB_MEM        MI_INSTR(0x2D, 0)
-- 
2.21.0

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

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

* [PATCH 4/6] drm/i915: Add definitions for MI_MATH command
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
  2019-09-26 10:06 ` [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch Michał Winiarski
  2019-09-26 10:06 ` [PATCH 3/6] drm/i915: Adjust length of MI_LOAD_REGISTER_REG Michał Winiarski
@ 2019-09-26 10:06 ` Michał Winiarski
  2019-09-26 13:25   ` Chris Wilson
  2019-09-26 10:06 ` [PATCH 5/6] drm/i915: Don't use scratch in WA batch Michał Winiarski
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 10:06 UTC (permalink / raw)
  To: intel-gfx

We can use it in i915 for updating parts of unmasked registers from
within a batch. We're also adding Gen8+ versions of CS_GPR registers
(aka MI_MATH_REG in the coprocessor).

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 24 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h              |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index 9211b1ad401b..26c286bc9625 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -241,6 +241,30 @@
 #define   PIPE_CONTROL_DEPTH_CACHE_FLUSH		(1<<0)
 #define   PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */
 
+#define MI_MATH(x)			MI_INSTR(0x1A, (x)-1)
+#define MI_MATH_INSTR(opcode, op1, op2) (((opcode) << 20) | \
+					 ((op1) << 10) | (op2))
+/* Opcodes for MI_MATH_INSTR */
+#define   MI_MATH_NOOP			MI_MATH_INSTR(0x0, 0x0, 0x0)
+#define   MI_MATH_LOAD(op1, op2)	MI_MATH_INSTR(0x80, op1, op2)
+#define   MI_MATH_LOADINV(op1, op2)	MI_MATH_INSTR(0x480, op1, op2)
+#define   MI_MATH_LOAD0(op1)		MI_MATH_INSTR(0x081, op1)
+#define   MI_MATH_LOAD1(op1)		MI_MATH_INSTR(0x481, op1)
+#define   MI_MATH_ADD			MI_MATH_INSTR(0x100, 0x0, 0x0)
+#define   MI_MATH_SUB			MI_MATH_INSTR(0x101, 0x0, 0x0)
+#define   MI_MATH_AND			MI_MATH_INSTR(0x102, 0x0, 0x0)
+#define   MI_MATH_OR			MI_MATH_INSTR(0x103, 0x0, 0x0)
+#define   MI_MATH_XOR			MI_MATH_INSTR(0x104, 0x0, 0x0)
+#define   MI_MATH_STORE(op1, op2)	MI_MATH_INSTR(0x180, op1, op2)
+#define   MI_MATH_STOREINV(op1, op2)	MI_MATH_INSTR(0x580, op1, op2)
+/* Registers used as operands in MI_MATH_INSTR */
+#define   MI_MATH_REG(x)		(x)
+#define   MI_MATH_REG_SRCA		0x20
+#define   MI_MATH_REG_SRCB		0x21
+#define   MI_MATH_REG_ACCU		0x31
+#define   MI_MATH_REG_ZF		0x32
+#define   MI_MATH_REG_CF		0x33
+
 /*
  * Commands used only by the command parser
  */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e752de9470bd..fbedf89fc0bb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2483,6 +2483,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   RING_WAIT		(1 << 11) /* gen3+, PRBx_CTL */
 #define   RING_WAIT_SEMAPHORE	(1 << 10) /* gen6+ */
 
+/* There are 16 64-bit CS General Purpose Registers per-engine on Gen8+ */
+#define GEN8_RING_CS_GPR(base, n)	_MMIO(((base) + 0x600) + (n) * 8)
+#define GEN8_RING_CS_GPR_UDW(base, n)	_MMIO(((base) + 0x600) + (n) * 8 + 4)
+
 #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + (i) * 4)
 #define   RING_FORCE_TO_NONPRIV_ACCESS_RW	(0 << 28)    /* CFL+ & Gen11+ */
 #define   RING_FORCE_TO_NONPRIV_ACCESS_RD	(1 << 28)
-- 
2.21.0

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

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

* [PATCH 5/6] drm/i915: Don't use scratch in WA batch.
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (2 preceding siblings ...)
  2019-09-26 10:06 ` [PATCH 4/6] drm/i915: Add definitions for MI_MATH command Michał Winiarski
@ 2019-09-26 10:06 ` Michał Winiarski
  2019-09-26 10:24   ` Chris Wilson
  2019-09-26 13:31   ` Chris Wilson
  2019-09-26 10:06 ` [PATCH 6/6] drm/i915/execlists: Don't allocate scratch Michał Winiarski
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 10:06 UTC (permalink / raw)
  To: intel-gfx

We're currently doing one workaround where we're using scratch as a
temporary storage place, while we're overwriting the value of one
register with some known constant value in order to perform a
workaround.
While we could just do similar thing with CS_GPR register
and MI_LOAD_REGISTER_REG instead of scratch, since we would use CS_GPR
anyways, let's just drop the constant values and do the bitops using
MI_MATH.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine.h   | 53 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_types.h |  3 --
 drivers/gpu/drm/i915/gt/intel_lrc.c      | 27 +++---------
 3 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index d3c6993f4f46..2dfe0b23af1d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -400,6 +400,59 @@ gen8_emit_ggtt_write(u32 *cs, u32 value, u32 gtt_offset, u32 flags)
 	return cs;
 }
 
+/*
+ * We're using CS_GPR registers for the MI_MATH ops.
+ * Note that user contexts are free to use those registers, which means that we
+ * should only use this this function during context initialization, before
+ * context restore (WA batch) or inside i915-owned contexts.
+ */
+static inline u32 *
+gen8_emit_bitop_reg_mask(struct intel_engine_cs *engine,
+			 u32 *cs, u32 op, i915_reg_t reg, u32 mask)
+{
+	const u32 base = engine->mmio_base;
+
+	GEM_BUG_ON(op != MI_MATH_OR && op != MI_MATH_AND);
+
+	*cs++ = MI_LOAD_REGISTER_REG;
+	*cs++ = i915_mmio_reg_offset(reg);
+	*cs++ = i915_mmio_reg_offset(GEN8_RING_CS_GPR(base, 0));
+	*cs++ = MI_NOOP;
+
+	*cs++ = MI_LOAD_REGISTER_IMM(1);
+	*cs++ = i915_mmio_reg_offset(GEN8_RING_CS_GPR(base, 1));
+	*cs++ = mask;
+	*cs++ = MI_NOOP;
+
+	*cs++ = MI_MATH(4);
+	*cs++ = MI_MATH_LOAD(MI_MATH_REG_SRCA, MI_MATH_REG(0));
+	*cs++ = MI_MATH_LOAD(MI_MATH_REG_SRCB, MI_MATH_REG(1));
+	*cs++ = op;
+	*cs++ = MI_MATH_STORE(MI_MATH_REG(0), MI_MATH_REG_ACCU);
+	*cs++ = MI_NOOP;
+
+	*cs++ = MI_LOAD_REGISTER_REG;
+	*cs++ = i915_mmio_reg_offset(GEN8_RING_CS_GPR(base, 0));
+	*cs++ = i915_mmio_reg_offset(reg);
+	*cs++ = MI_NOOP;
+
+	return cs;
+}
+
+static inline u32 *
+gen8_emit_set_register(struct intel_engine_cs *engine,
+		       u32 *cs, i915_reg_t reg, u32 mask)
+{
+	return gen8_emit_bitop_reg_mask(engine, cs, MI_MATH_OR, reg, mask);
+}
+
+static inline u32 *
+gen8_emit_clear_register(struct intel_engine_cs *engine,
+			 u32 *cs, i915_reg_t reg, u32 mask)
+{
+	return gen8_emit_bitop_reg_mask(engine, cs, MI_MATH_AND, reg, ~mask);
+}
+
 static inline void __intel_engine_reset(struct intel_engine_cs *engine,
 					bool stalled)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index e64db4c13df6..d9f25ac520a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -92,9 +92,6 @@ enum intel_gt_scratch_field {
 	/* 8 bytes */
 	INTEL_GT_SCRATCH_FIELD_RENDER_FLUSH = 128,
 
-	/* 8 bytes */
-	INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA = 256,
-
 };
 
 #endif /* __INTEL_GT_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index fa385218ce92..089c17599ca4 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2269,13 +2269,7 @@ static int execlists_request_alloc(struct i915_request *request)
  * PIPE_CONTROL instruction. This is required for the flush to happen correctly
  * but there is a slight complication as this is applied in WA batch where the
  * values are only initialized once so we cannot take register value at the
- * beginning and reuse it further; hence we save its value to memory, upload a
- * constant value with bit21 set and then we restore it back with the saved value.
- * To simplify the WA, a constant value is formed by using the default value
- * of this register. This shouldn't be a problem because we are only modifying
- * it for a short period and this batch in non-premptible. We can ofcourse
- * use additional instructions that read the actual value of the register
- * at that time and set our bit of interest but it makes the WA complicated.
+ * beginning and reuse it further;
  *
  * This WA is also required for Gen9 so extracting as a function avoids
  * code duplication.
@@ -2283,27 +2277,16 @@ static int execlists_request_alloc(struct i915_request *request)
 static u32 *
 gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine, u32 *batch)
 {
-	/* NB no one else is allowed to scribble over scratch + 256! */
-	*batch++ = MI_STORE_REGISTER_MEM_GEN8 | MI_SRM_LRM_GLOBAL_GTT;
-	*batch++ = i915_mmio_reg_offset(GEN8_L3SQCREG4);
-	*batch++ = intel_gt_scratch_offset(engine->gt,
-					   INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA);
-	*batch++ = 0;
-
-	*batch++ = MI_LOAD_REGISTER_IMM(1);
-	*batch++ = i915_mmio_reg_offset(GEN8_L3SQCREG4);
-	*batch++ = 0x40400000 | GEN8_LQSC_FLUSH_COHERENT_LINES;
+	batch = gen8_emit_set_register(engine, batch, GEN8_L3SQCREG4,
+				       GEN8_LQSC_FLUSH_COHERENT_LINES);
 
 	batch = gen8_emit_pipe_control(batch,
 				       PIPE_CONTROL_CS_STALL |
 				       PIPE_CONTROL_DC_FLUSH_ENABLE,
 				       0);
 
-	*batch++ = MI_LOAD_REGISTER_MEM_GEN8 | MI_SRM_LRM_GLOBAL_GTT;
-	*batch++ = i915_mmio_reg_offset(GEN8_L3SQCREG4);
-	*batch++ = intel_gt_scratch_offset(engine->gt,
-					   INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA);
-	*batch++ = 0;
+	batch = gen8_emit_clear_register(engine, batch, GEN8_L3SQCREG4,
+					 GEN8_LQSC_FLUSH_COHERENT_LINES);
 
 	return batch;
 }
-- 
2.21.0

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

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

* [PATCH 6/6] drm/i915/execlists: Don't allocate scratch
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (3 preceding siblings ...)
  2019-09-26 10:06 ` [PATCH 5/6] drm/i915: Don't use scratch in WA batch Michał Winiarski
@ 2019-09-26 10:06 ` Michał Winiarski
  2019-09-26 10:34   ` Chris Wilson
  2019-09-26 12:20   ` [PATCH v2 " Michał Winiarski
  2019-09-26 10:20 ` [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Chris Wilson
                   ` (4 subsequent siblings)
  9 siblings, 2 replies; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 10:06 UTC (permalink / raw)
  To: intel-gfx

We're no longer using it on execlists platforms. There's no point in
allocating it.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 --
 drivers/gpu/drm/i915/gt/intel_gt.c        | 6 ++++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index f451d5076bde..a4e5aceff678 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -669,8 +669,6 @@ static int measure_breadcrumb_dw(struct intel_engine_cs *engine)
 	struct measure_breadcrumb *frame;
 	int dw = -ENOMEM;
 
-	GEM_BUG_ON(!engine->gt->scratch);
-
 	frame = kzalloc(sizeof(*frame), GFP_KERNEL);
 	if (!frame)
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index eef9bdae8ebb..e135a66b7242 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -329,6 +329,9 @@ static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
 	struct i915_vma *vma;
 	int ret;
 
+	if (HAS_EXECLISTS(i915))
+		return 0;
+
 	obj = i915_gem_object_create_stolen(i915, size);
 	if (!obj)
 		obj = i915_gem_object_create_internal(i915, size);
@@ -358,6 +361,9 @@ static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
 
 static void intel_gt_fini_scratch(struct intel_gt *gt)
 {
+	if (HAS_EXECLISTS(gt->i915))
+		return;
+
 	i915_vma_unpin_and_release(&gt->scratch, 0);
 }
 
-- 
2.21.0

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

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

* Re: [PATCH 3/6] drm/i915: Adjust length of MI_LOAD_REGISTER_REG
  2019-09-26 10:06 ` [PATCH 3/6] drm/i915: Adjust length of MI_LOAD_REGISTER_REG Michał Winiarski
@ 2019-09-26 10:15   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 10:15 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx; +Cc: Jani Nikula

Quoting Michał Winiarski (2019-09-26 11:06:32)
> Default length value of MI_LOAD_REGISTER_REG is 1.
> Also move it out of cmd-parser-only registers since we're going to use
> it in i915.

Hmm. So we do find_cmd_in_table() [cmdparser] that ignores the length
field, this should not affect cmdparser. This is checked by
gem_exec_parse/load-register-reg. So long as hsw doesn't prove me wrong,

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

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

* Re: [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch
  2019-09-26 10:06 ` [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch Michał Winiarski
@ 2019-09-26 10:18   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 10:18 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 11:06:31)
> Some of our commands (MI_FLUSH_DW / PIPE_CONTROL) require a post-sync write
> operation to be performed. Currently we're using dedicated VMA for
> PIPE_CONTROL and global HWSP for MI_FLUSH_DW.
> On execlists platforms, each of our contexts has an area that can be
> used as scratch space. Let's use that instead.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@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

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

* Re: [PATCH 1/6] drm/i915: Define explicit wedged on init reset state
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (4 preceding siblings ...)
  2019-09-26 10:06 ` [PATCH 6/6] drm/i915/execlists: Don't allocate scratch Michał Winiarski
@ 2019-09-26 10:20 ` Chris Wilson
  2019-09-26 10:33 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] " Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 10:20 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 11:06:30)
> We're currently using scratch presence as a way of identifying that we
> entered wedged state at driver initialization time.
> Let's use a separate flag rather than rely on scratch.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_reset.c       | 11 ++++++++++-
>  drivers/gpu/drm/i915/gt/intel_reset.h       |  9 +++++++++
>  drivers/gpu/drm/i915/gt/intel_reset_types.h |  6 ++++++
>  drivers/gpu/drm/i915/i915_gem.c             |  2 +-
>  4 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index ae68c3786f63..0f1534ac823f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -811,7 +811,8 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt)
>         if (!test_bit(I915_WEDGED, &gt->reset.flags))
>                 return true;
>  
> -       if (!gt->scratch) /* Never full initialised, recovery impossible */
> +       /* Never fullly initialised, recovery impossible */

You went one 'l' too far in the correction.

Ok, hard to argue against.
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

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

* Re: [PATCH 5/6] drm/i915: Don't use scratch in WA batch.
  2019-09-26 10:06 ` [PATCH 5/6] drm/i915: Don't use scratch in WA batch Michał Winiarski
@ 2019-09-26 10:24   ` Chris Wilson
  2019-09-26 18:24     ` Chris Wilson
  2019-09-26 13:31   ` Chris Wilson
  1 sibling, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 10:24 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 11:06:34)
> We're currently doing one workaround where we're using scratch as a
> temporary storage place, while we're overwriting the value of one
> register with some known constant value in order to perform a
> workaround.
> While we could just do similar thing with CS_GPR register
> and MI_LOAD_REGISTER_REG instead of scratch, since we would use CS_GPR
> anyways, let's just drop the constant values and do the bitops using
> MI_MATH.

I'd like to have your confirmation that the w/a batch is executed before
the CS_GPR are restored from the context image, and I'm going to wait
for gem_ctx_isolation verification :-p
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915: Define explicit wedged on init reset state
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (5 preceding siblings ...)
  2019-09-26 10:20 ` [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Chris Wilson
@ 2019-09-26 10:33 ` Patchwork
  2019-09-26 10:56 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-09-26 10:33 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Define explicit wedged on init reset state
URL   : https://patchwork.freedesktop.org/series/67276/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
8abcbd48e83e drm/i915: Define explicit wedged on init reset state
f0d1bbf3054e drm/i915/execlists: Use per-process HWSP as scratch
15a08f2fa23c drm/i915: Adjust length of MI_LOAD_REGISTER_REG
d0c06fd7dc46 drm/i915: Add definitions for MI_MATH command
-:24: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#24: FILE: drivers/gpu/drm/i915/gt/intel_gpu_commands.h:244:
+#define MI_MATH(x)			MI_INSTR(0x1A, (x)-1)
                   			                  ^

total: 0 errors, 0 warnings, 1 checks, 40 lines checked
6e224dde1fbc drm/i915: Don't use scratch in WA batch.
0dd27d5e1ca2 drm/i915/execlists: Don't allocate scratch

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

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

* Re: [PATCH 6/6] drm/i915/execlists: Don't allocate scratch
  2019-09-26 10:06 ` [PATCH 6/6] drm/i915/execlists: Don't allocate scratch Michał Winiarski
@ 2019-09-26 10:34   ` Chris Wilson
  2019-09-26 12:20   ` [PATCH v2 " Michał Winiarski
  1 sibling, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 10:34 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 11:06:35)
> We're no longer using it on execlists platforms. There's no point in
> allocating it.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 --
>  drivers/gpu/drm/i915/gt/intel_gt.c        | 6 ++++++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index f451d5076bde..a4e5aceff678 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -669,8 +669,6 @@ static int measure_breadcrumb_dw(struct intel_engine_cs *engine)
>         struct measure_breadcrumb *frame;
>         int dw = -ENOMEM;
>  
> -       GEM_BUG_ON(!engine->gt->scratch);
> -
>         frame = kzalloc(sizeof(*frame), GFP_KERNEL);
>         if (!frame)
>                 return -ENOMEM;
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index eef9bdae8ebb..e135a66b7242 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -329,6 +329,9 @@ static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
>         struct i915_vma *vma;
>         int ret;
>  
> +       if (HAS_EXECLISTS(i915))
> +               return 0;

Push the decision to the backends then, and

if (gt->scratch)
	return;

> +
>         obj = i915_gem_object_create_stolen(i915, size);
>         if (!obj)
>                 obj = i915_gem_object_create_internal(i915, size);
> @@ -358,6 +361,9 @@ static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
>  
>  static void intel_gt_fini_scratch(struct intel_gt *gt)
>  {
> +       if (HAS_EXECLISTS(gt->i915))
> +               return;

Not required, as release NULL is a no-op. After pushing the init to the
backends, we keep the central free at the end, agnostic to whether or not
we needed the scratch.

> +
>         i915_vma_unpin_and_release(&gt->scratch, 0);
>  }
>  
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915: Define explicit wedged on init reset state
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (6 preceding siblings ...)
  2019-09-26 10:33 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] " Patchwork
@ 2019-09-26 10:56 ` Patchwork
  2019-09-26 12:28 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2) Patchwork
  2019-09-26 13:14 ` ✗ Fi.CI.BAT: failure " Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-09-26 10:56 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Define explicit wedged on init reset state
URL   : https://patchwork.freedesktop.org/series/67276/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6963 -> Patchwork_14550
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14550/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_switch@legacy-render:
    - fi-bxt-dsi:         [PASS][1] -> [INCOMPLETE][2] ([fdo#103927] / [fdo#111381])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14550/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_gtt:
    - {fi-tgl-u2}:        [INCOMPLETE][3] -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-tgl-u2/igt@i915_selftest@live_gtt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14550/fi-tgl-u2/igt@i915_selftest@live_gtt.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][5] ([fdo#109483]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14550/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381


Participating hosts (50 -> 42)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (9): fi-icl-u4 fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6963 -> Patchwork_14550

  CI-20190529: 20190529
  CI_DRM_6963: 364bf33c246115063174fa2a07e9f5a6bddc9f72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5203: 82326332f7af336d390e00ae87187bc207fd33dd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14550: 0dd27d5e1ca27637642936422f4a065d0280f783 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0dd27d5e1ca2 drm/i915/execlists: Don't allocate scratch
6e224dde1fbc drm/i915: Don't use scratch in WA batch.
d0c06fd7dc46 drm/i915: Add definitions for MI_MATH command
15a08f2fa23c drm/i915: Adjust length of MI_LOAD_REGISTER_REG
f0d1bbf3054e drm/i915/execlists: Use per-process HWSP as scratch
8abcbd48e83e drm/i915: Define explicit wedged on init reset state

== Logs ==

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

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

* [PATCH v2 6/6] drm/i915/execlists: Don't allocate scratch
  2019-09-26 10:06 ` [PATCH 6/6] drm/i915/execlists: Don't allocate scratch Michał Winiarski
  2019-09-26 10:34   ` Chris Wilson
@ 2019-09-26 12:20   ` Michał Winiarski
  2019-09-26 13:26     ` Chris Wilson
  1 sibling, 1 reply; 20+ messages in thread
From: Michał Winiarski @ 2019-09-26 12:20 UTC (permalink / raw)
  To: intel-gfx

We're no longer using it on execlists platforms. There's no point in
allocating it.

v2: Move scratch init to legacy ring submission backend. (Chris)

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c  |  2 --
 drivers/gpu/drm/i915/gt/intel_gt.c         | 17 +++++------------
 drivers/gpu/drm/i915/gt/intel_gt.h         |  2 +-
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c |  5 +++++
 drivers/gpu/drm/i915/i915_gem.c            |  2 --
 5 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index f451d5076bde..a4e5aceff678 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -669,8 +669,6 @@ static int measure_breadcrumb_dw(struct intel_engine_cs *engine)
 	struct measure_breadcrumb *frame;
 	int dw = -ENOMEM;
 
-	GEM_BUG_ON(!engine->gt->scratch);
-
 	frame = kzalloc(sizeof(*frame), GFP_KERNEL);
 	if (!frame)
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index eef9bdae8ebb..e67a6cabc81d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -322,13 +322,17 @@ void intel_gt_driver_register(struct intel_gt *gt)
 		intel_gpu_ips_init(gt->i915);
 }
 
-static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
+int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
 {
 	struct drm_i915_private *i915 = gt->i915;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
 	int ret;
 
+	/* Created lazily - submission backend calls us at its init time */
+	if (gt->scratch)
+		return 0;
+
 	obj = i915_gem_object_create_stolen(i915, size);
 	if (!obj)
 		obj = i915_gem_object_create_internal(i915, size);
@@ -361,17 +365,6 @@ static void intel_gt_fini_scratch(struct intel_gt *gt)
 	i915_vma_unpin_and_release(&gt->scratch, 0);
 }
 
-int intel_gt_init(struct intel_gt *gt)
-{
-	int err;
-
-	err = intel_gt_init_scratch(gt, IS_GEN(gt->i915, 2) ? SZ_256K : SZ_4K);
-	if (err)
-		return err;
-
-	return 0;
-}
-
 void intel_gt_driver_remove(struct intel_gt *gt)
 {
 	GEM_BUG_ON(gt->awake);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index e6ab0bff0efb..b9ed8ea3706e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -30,7 +30,7 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
 void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915);
 void intel_gt_init_hw_early(struct drm_i915_private *i915);
 int __must_check intel_gt_init_hw(struct intel_gt *gt);
-int intel_gt_init(struct intel_gt *gt);
+int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size);
 void intel_gt_driver_register(struct intel_gt *gt);
 
 void intel_gt_driver_unregister(struct intel_gt *gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index 0747b8c9f768..64d22239df8a 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -2315,6 +2315,11 @@ int intel_ring_submission_init(struct intel_engine_cs *engine)
 	struct intel_ring *ring;
 	int err;
 
+	err = intel_gt_init_scratch(engine->gt,
+				    IS_GEN(engine->i915, 2) ? SZ_256K : SZ_4K);
+	if (err)
+		goto err;
+
 	timeline = intel_timeline_create(engine->gt, engine->status_page.vma);
 	if (IS_ERR(timeline)) {
 		err = PTR_ERR(timeline);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e2897a666225..09d387f4d46b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1337,8 +1337,6 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 		goto err_unlock;
 	}
 
-	intel_gt_init(&dev_priv->gt);
-
 	ret = intel_engines_setup(dev_priv);
 	if (ret) {
 		GEM_BUG_ON(ret == -EIO);
-- 
2.21.0

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2)
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (7 preceding siblings ...)
  2019-09-26 10:56 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-26 12:28 ` Patchwork
  2019-09-26 13:14 ` ✗ Fi.CI.BAT: failure " Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-09-26 12:28 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2)
URL   : https://patchwork.freedesktop.org/series/67276/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5883d760ee3b drm/i915: Define explicit wedged on init reset state
cc46370e91dd drm/i915/execlists: Use per-process HWSP as scratch
cf8540776b9a drm/i915: Adjust length of MI_LOAD_REGISTER_REG
381b1a902308 drm/i915: Add definitions for MI_MATH command
-:24: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#24: FILE: drivers/gpu/drm/i915/gt/intel_gpu_commands.h:244:
+#define MI_MATH(x)			MI_INSTR(0x1A, (x)-1)
                   			                  ^

total: 0 errors, 0 warnings, 1 checks, 40 lines checked
1676f481a1d1 drm/i915: Don't use scratch in WA batch.
edec173d749e drm/i915/execlists: Don't allocate scratch

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2)
  2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
                   ` (8 preceding siblings ...)
  2019-09-26 12:28 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2) Patchwork
@ 2019-09-26 13:14 ` Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-09-26 13:14 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2)
URL   : https://patchwork.freedesktop.org/series/67276/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6963 -> Patchwork_14553
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_create@basic:
    - fi-byt-n2820:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-byt-n2820/igt@gem_exec_create@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-byt-n2820/igt@gem_exec_create@basic.html

  * igt@runner@aborted:
    - fi-byt-n2820:       NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-byt-n2820/igt@runner@aborted.html

  
#### Suppressed ####

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

  * igt@gem_exec_suspend@basic-s4-devices:
    - {fi-tgl-u2}:        [PASS][4] -> [INCOMPLETE][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-tgl-u2/igt@gem_exec_suspend@basic-s4-devices.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-tgl-u2/igt@gem_exec_suspend@basic-s4-devices.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][6] -> [INCOMPLETE][7] ([fdo#107718])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-edid-read:
    - {fi-icl-u4}:        [FAIL][8] ([fdo#111045]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][10] ([fdo#109483]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
    - fi-kbl-7500u:       [FAIL][12] ([fdo#111045] / [fdo#111096]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6963/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14553/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096


Participating hosts (50 -> 45)
------------------------------

  Additional (2): fi-kbl-soraka fi-icl-u3 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6963 -> Patchwork_14553

  CI-20190529: 20190529
  CI_DRM_6963: 364bf33c246115063174fa2a07e9f5a6bddc9f72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5203: 82326332f7af336d390e00ae87187bc207fd33dd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14553: edec173d749e20592d7a683e8463cbf82429dc97 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

edec173d749e drm/i915/execlists: Don't allocate scratch
1676f481a1d1 drm/i915: Don't use scratch in WA batch.
381b1a902308 drm/i915: Add definitions for MI_MATH command
cf8540776b9a drm/i915: Adjust length of MI_LOAD_REGISTER_REG
cc46370e91dd drm/i915/execlists: Use per-process HWSP as scratch
5883d760ee3b drm/i915: Define explicit wedged on init reset state

== Logs ==

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

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

* Re: [PATCH 4/6] drm/i915: Add definitions for MI_MATH command
  2019-09-26 10:06 ` [PATCH 4/6] drm/i915: Add definitions for MI_MATH command Michał Winiarski
@ 2019-09-26 13:25   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 13:25 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 11:06:33)
> We can use it in i915 for updating parts of unmasked registers from
> within a batch. We're also adding Gen8+ versions of CS_GPR registers
> (aka MI_MATH_REG in the coprocessor).
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Checked against mesa's xml for convenience,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 24 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h              |  4 ++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
> index 9211b1ad401b..26c286bc9625 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
> @@ -241,6 +241,30 @@
>  #define   PIPE_CONTROL_DEPTH_CACHE_FLUSH               (1<<0)
>  #define   PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */
>  
> +#define MI_MATH(x)                     MI_INSTR(0x1A, (x)-1)
> +#define MI_MATH_INSTR(opcode, op1, op2) (((opcode) << 20) | \
> +                                        ((op1) << 10) | (op2))

Perhaps this would benefit from a touch of REG_FIELD for value checking.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 6/6] drm/i915/execlists: Don't allocate scratch
  2019-09-26 12:20   ` [PATCH v2 " Michał Winiarski
@ 2019-09-26 13:26     ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 13:26 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 13:20:19)
> We're no longer using it on execlists platforms. There's no point in
> allocating it.
> 
> v2: Move scratch init to legacy ring submission backend. (Chris)
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
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

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

* Re: [PATCH 5/6] drm/i915: Don't use scratch in WA batch.
  2019-09-26 10:06 ` [PATCH 5/6] drm/i915: Don't use scratch in WA batch Michał Winiarski
  2019-09-26 10:24   ` Chris Wilson
@ 2019-09-26 13:31   ` Chris Wilson
  1 sibling, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 13:31 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2019-09-26 11:06:34)
> We're currently doing one workaround where we're using scratch as a
> temporary storage place, while we're overwriting the value of one
> register with some known constant value in order to perform a
> workaround.
> While we could just do similar thing with CS_GPR register
> and MI_LOAD_REGISTER_REG instead of scratch, since we would use CS_GPR
> anyways, let's just drop the constant values and do the bitops using
> MI_MATH.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h   | 53 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_types.h |  3 --
>  drivers/gpu/drm/i915/gt/intel_lrc.c      | 27 +++---------
>  3 files changed, 58 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index d3c6993f4f46..2dfe0b23af1d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -400,6 +400,59 @@ gen8_emit_ggtt_write(u32 *cs, u32 value, u32 gtt_offset, u32 flags)
>         return cs;
>  }
>  
> +/*
> + * We're using CS_GPR registers for the MI_MATH ops.
> + * Note that user contexts are free to use those registers, which means that we
> + * should only use this this function during context initialization, before
> + * context restore (WA batch) or inside i915-owned contexts.
> + */
> +static inline u32 *
> +gen8_emit_bitop_reg_mask(struct intel_engine_cs *engine,
> +                        u32 *cs, u32 op, i915_reg_t reg, u32 mask)

Useful, we had discussed using MI_MATH to perform rmw for our
workarounds.

> +{
> +       const u32 base = engine->mmio_base;
> +
> +       GEM_BUG_ON(op != MI_MATH_OR && op != MI_MATH_AND);
> +
> +       *cs++ = MI_LOAD_REGISTER_REG;
> +       *cs++ = i915_mmio_reg_offset(reg);
> +       *cs++ = i915_mmio_reg_offset(GEN8_RING_CS_GPR(base, 0));
> +       *cs++ = MI_NOOP;
> +
> +       *cs++ = MI_LOAD_REGISTER_IMM(1);
> +       *cs++ = i915_mmio_reg_offset(GEN8_RING_CS_GPR(base, 1));
> +       *cs++ = mask;
> +       *cs++ = MI_NOOP;
> +
> +       *cs++ = MI_MATH(4);
> +       *cs++ = MI_MATH_LOAD(MI_MATH_REG_SRCA, MI_MATH_REG(0));
> +       *cs++ = MI_MATH_LOAD(MI_MATH_REG_SRCB, MI_MATH_REG(1));
> +       *cs++ = op;
> +       *cs++ = MI_MATH_STORE(MI_MATH_REG(0), MI_MATH_REG_ACCU);
> +       *cs++ = MI_NOOP;
> +
> +       *cs++ = MI_LOAD_REGISTER_REG;
> +       *cs++ = i915_mmio_reg_offset(GEN8_RING_CS_GPR(base, 0));
> +       *cs++ = i915_mmio_reg_offset(reg);
> +       *cs++ = MI_NOOP;

4 nicely paired redundant MI_NOOP. Can be removed, leaving the packet
still oword aligned.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/6] drm/i915: Don't use scratch in WA batch.
  2019-09-26 10:24   ` Chris Wilson
@ 2019-09-26 18:24     ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-09-26 18:24 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Chris Wilson (2019-09-26 11:24:35)
> Quoting Michał Winiarski (2019-09-26 11:06:34)
> > We're currently doing one workaround where we're using scratch as a
> > temporary storage place, while we're overwriting the value of one
> > register with some known constant value in order to perform a
> > workaround.
> > While we could just do similar thing with CS_GPR register
> > and MI_LOAD_REGISTER_REG instead of scratch, since we would use CS_GPR
> > anyways, let's just drop the constant values and do the bitops using
> > MI_MATH.
> 
> I'd like to have your confirmation that the w/a batch is executed before
> the CS_GPR are restored from the context image, and I'm going to wait
> for gem_ctx_isolation verification :-p

Bad news. It failed a check that CS_GPR was preserved across a context
switch on Broadwell.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2019-09-26 18:25 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 10:06 [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Michał Winiarski
2019-09-26 10:06 ` [PATCH 2/6] drm/i915/execlists: Use per-process HWSP as scratch Michał Winiarski
2019-09-26 10:18   ` Chris Wilson
2019-09-26 10:06 ` [PATCH 3/6] drm/i915: Adjust length of MI_LOAD_REGISTER_REG Michał Winiarski
2019-09-26 10:15   ` Chris Wilson
2019-09-26 10:06 ` [PATCH 4/6] drm/i915: Add definitions for MI_MATH command Michał Winiarski
2019-09-26 13:25   ` Chris Wilson
2019-09-26 10:06 ` [PATCH 5/6] drm/i915: Don't use scratch in WA batch Michał Winiarski
2019-09-26 10:24   ` Chris Wilson
2019-09-26 18:24     ` Chris Wilson
2019-09-26 13:31   ` Chris Wilson
2019-09-26 10:06 ` [PATCH 6/6] drm/i915/execlists: Don't allocate scratch Michał Winiarski
2019-09-26 10:34   ` Chris Wilson
2019-09-26 12:20   ` [PATCH v2 " Michał Winiarski
2019-09-26 13:26     ` Chris Wilson
2019-09-26 10:20 ` [PATCH 1/6] drm/i915: Define explicit wedged on init reset state Chris Wilson
2019-09-26 10:33 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] " Patchwork
2019-09-26 10:56 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-26 12:28 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915: Define explicit wedged on init reset state (rev2) Patchwork
2019-09-26 13:14 ` ✗ Fi.CI.BAT: failure " Patchwork

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