* [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, >->reset.flags))
return true;
- if (!gt->scratch) /* Never full initialised, recovery impossible */
+ /* Never fullly initialised, recovery impossible */
+ if (test_bit(I915_WEDGED_ON_INIT, >->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, >->reset.flags);
+}
+
void intel_gt_init_reset(struct intel_gt *gt)
{
init_waitqueue_head(>->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(>->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, >->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(>->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(>->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.