* [PATCH 2/7] drm/i915/ringbuffer: Clear semaphore sync registers on ring init
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
@ 2018-12-01 9:52 ` Chris Wilson
2018-12-01 9:52 ` [PATCH 3/7] drm/i915: Allocate a common scratch page Chris Wilson
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 9:52 UTC (permalink / raw)
To: intel-gfx
Ensure that the sync registers are cleared every time we restart the
ring to avoid stale values from creeping in from random neutrinos.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 992889f9e0ff..81b10d85b738 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -529,6 +529,13 @@ static int init_ring_common(struct intel_engine_cs *engine)
intel_engine_reset_breadcrumbs(engine);
+ if (HAS_LEGACY_SEMAPHORES(engine->i915)) {
+ I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
+ I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
+ if (HAS_VEBOX(dev_priv))
+ I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
+ }
+
/* Enforce ordering by reading HEAD register back */
I915_READ_HEAD(engine);
--
2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/7] drm/i915: Allocate a common scratch page
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
2018-12-01 9:52 ` [PATCH 2/7] drm/i915/ringbuffer: Clear semaphore sync registers on ring init Chris Wilson
@ 2018-12-01 9:52 ` Chris Wilson
2018-12-01 9:52 ` [PATCH 4/7] drm/i915/selftests: Terminate hangcheck sanitycheck forcibly Chris Wilson
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 9:52 UTC (permalink / raw)
To: intel-gfx
Currently we allocate a scratch page for each engine, but since we only
ever write into it for post-sync operations, it is not exposed to
userspace nor do we care for coherency. As we then do not care about its
contents, we can use one page for all, reducing our allocations and
avoid complications by not assuming per-engine isolation.
For later use, it simplifies engine initialisation (by removing the
allocation that required struct_mutex!) and means that we can always rely
on there being a scratch page.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 7 ++++
drivers/gpu/drm/i915/i915_gem.c | 50 ++++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
drivers/gpu/drm/i915/intel_engine_cs.c | 42 ---------------------
drivers/gpu/drm/i915/intel_lrc.c | 17 +++------
drivers/gpu/drm/i915/intel_ringbuffer.c | 33 +++++-----------
drivers/gpu/drm/i915/intel_ringbuffer.h | 5 ---
7 files changed, 71 insertions(+), 85 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d45475287130..0ec65cc48b5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1996,6 +1996,8 @@ struct drm_i915_private {
struct delayed_work idle_work;
ktime_t last_init_time;
+
+ struct i915_vma *scratch;
} gt;
/* perform PHY state sanity checks? */
@@ -3724,4 +3726,9 @@ static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
return I915_HWS_CSB_WRITE_INDEX;
}
+static inline u32 i915_scratch_offset(const struct drm_i915_private *i915)
+{
+ return i915_ggtt_offset(i915->gt.scratch);
+}
+
#endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 834240a9b262..cca4285e3329 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5495,6 +5495,44 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
goto out_ctx;
}
+static int
+i915_gem_init_scratch(struct drm_i915_private *i915, unsigned int size)
+{
+ struct drm_i915_gem_object *obj;
+ struct i915_vma *vma;
+ int ret;
+
+ obj = i915_gem_object_create_stolen(i915, size);
+ if (!obj)
+ obj = i915_gem_object_create_internal(i915, size);
+ if (IS_ERR(obj)) {
+ DRM_ERROR("Failed to allocate scratch page\n");
+ return PTR_ERR(obj);
+ }
+
+ vma = i915_vma_instance(obj, &i915->ggtt.vm, NULL);
+ if (IS_ERR(vma)) {
+ ret = PTR_ERR(vma);
+ goto err_unref;
+ }
+
+ ret = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
+ if (ret)
+ goto err_unref;
+
+ i915->gt.scratch = vma;
+ return 0;
+
+err_unref:
+ i915_gem_object_put(obj);
+ return ret;
+}
+
+static void i915_gem_fini_scratch(struct drm_i915_private *i915)
+{
+ i915_vma_unpin_and_release(&i915->gt.scratch, 0);
+}
+
int i915_gem_init(struct drm_i915_private *dev_priv)
{
int ret;
@@ -5541,12 +5579,19 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
goto err_unlock;
}
- ret = i915_gem_contexts_init(dev_priv);
+ ret = i915_gem_init_scratch(dev_priv,
+ IS_GEN2(dev_priv) ? SZ_256K : PAGE_SIZE);
if (ret) {
GEM_BUG_ON(ret == -EIO);
goto err_ggtt;
}
+ ret = i915_gem_contexts_init(dev_priv);
+ if (ret) {
+ GEM_BUG_ON(ret == -EIO);
+ goto err_scratch;
+ }
+
ret = intel_engines_init(dev_priv);
if (ret) {
GEM_BUG_ON(ret == -EIO);
@@ -5619,6 +5664,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
err_context:
if (ret != -EIO)
i915_gem_contexts_fini(dev_priv);
+err_scratch:
+ i915_gem_fini_scratch(dev_priv);
err_ggtt:
err_unlock:
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
@@ -5670,6 +5717,7 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
intel_uc_fini(dev_priv);
i915_gem_cleanup_engines(dev_priv);
i915_gem_contexts_fini(dev_priv);
+ i915_gem_fini_scratch(dev_priv);
mutex_unlock(&dev_priv->drm.struct_mutex);
intel_cleanup_gt_powersave(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index a6885a59568b..07465123c166 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1571,7 +1571,7 @@ static void gem_record_rings(struct i915_gpu_state *error)
if (HAS_BROKEN_CS_TLB(i915))
ee->wa_batchbuffer =
i915_error_object_create(i915,
- engine->scratch);
+ i915->gt.scratch);
request_record_user_bo(request, ee);
ee->ctx =
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 759c0fd58f8c..2390985384d6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -493,46 +493,6 @@ void intel_engine_setup_common(struct intel_engine_cs *engine)
intel_engine_init_cmd_parser(engine);
}
-int intel_engine_create_scratch(struct intel_engine_cs *engine,
- unsigned int size)
-{
- struct drm_i915_gem_object *obj;
- struct i915_vma *vma;
- int ret;
-
- WARN_ON(engine->scratch);
-
- obj = i915_gem_object_create_stolen(engine->i915, size);
- if (!obj)
- obj = i915_gem_object_create_internal(engine->i915, size);
- if (IS_ERR(obj)) {
- DRM_ERROR("Failed to allocate scratch page\n");
- return PTR_ERR(obj);
- }
-
- vma = i915_vma_instance(obj, &engine->i915->ggtt.vm, NULL);
- if (IS_ERR(vma)) {
- ret = PTR_ERR(vma);
- goto err_unref;
- }
-
- ret = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
- if (ret)
- goto err_unref;
-
- engine->scratch = vma;
- return 0;
-
-err_unref:
- i915_gem_object_put(obj);
- return ret;
-}
-
-void intel_engine_cleanup_scratch(struct intel_engine_cs *engine)
-{
- i915_vma_unpin_and_release(&engine->scratch, 0);
-}
-
static void cleanup_status_page(struct intel_engine_cs *engine)
{
if (HWS_NEEDS_PHYSICAL(engine->i915)) {
@@ -707,8 +667,6 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
{
struct drm_i915_private *i915 = engine->i915;
- intel_engine_cleanup_scratch(engine);
-
cleanup_status_page(engine);
intel_engine_fini_breadcrumbs(engine);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b5511a054f30..de070dca4033 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1286,9 +1286,10 @@ 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++ = i915_ggtt_offset(engine->scratch) + 256;
+ *batch++ = i915_scratch_offset(engine->i915) + 256;
*batch++ = 0;
*batch++ = MI_LOAD_REGISTER_IMM(1);
@@ -1302,7 +1303,7 @@ gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine, u32 *batch)
*batch++ = MI_LOAD_REGISTER_MEM_GEN8 | MI_SRM_LRM_GLOBAL_GTT;
*batch++ = i915_mmio_reg_offset(GEN8_L3SQCREG4);
- *batch++ = i915_ggtt_offset(engine->scratch) + 256;
+ *batch++ = i915_scratch_offset(engine->i915) + 256;
*batch++ = 0;
return batch;
@@ -1339,7 +1340,7 @@ static u32 *gen8_init_indirectctx_bb(struct intel_engine_cs *engine, u32 *batch)
PIPE_CONTROL_GLOBAL_GTT_IVB |
PIPE_CONTROL_CS_STALL |
PIPE_CONTROL_QW_WRITE,
- i915_ggtt_offset(engine->scratch) +
+ i915_scratch_offset(engine->i915) +
2 * CACHELINE_BYTES);
*batch++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
@@ -1969,7 +1970,7 @@ static int gen8_emit_flush_render(struct i915_request *request,
{
struct intel_engine_cs *engine = request->engine;
u32 scratch_addr =
- i915_ggtt_offset(engine->scratch) + 2 * CACHELINE_BYTES;
+ i915_scratch_offset(engine->i915) + 2 * CACHELINE_BYTES;
bool vf_flush_wa = false, dc_flush_wa = false;
u32 *cs, flags = 0;
int len;
@@ -2306,10 +2307,6 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
if (ret)
return ret;
- ret = intel_engine_create_scratch(engine, PAGE_SIZE);
- if (ret)
- goto err_cleanup_common;
-
ret = intel_init_workaround_bb(engine);
if (ret) {
/*
@@ -2322,10 +2319,6 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
}
return 0;
-
-err_cleanup_common:
- intel_engine_cleanup_common(engine);
- return ret;
}
int logical_xcs_ring_init(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 81b10d85b738..a3d3126a3938 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -150,8 +150,7 @@ gen4_render_ring_flush(struct i915_request *rq, u32 mode)
*/
if (mode & EMIT_INVALIDATE) {
*cs++ = GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE;
- *cs++ = i915_ggtt_offset(rq->engine->scratch) |
- PIPE_CONTROL_GLOBAL_GTT;
+ *cs++ = i915_scratch_offset(rq->i915) | PIPE_CONTROL_GLOBAL_GTT;
*cs++ = 0;
*cs++ = 0;
@@ -159,8 +158,7 @@ gen4_render_ring_flush(struct i915_request *rq, u32 mode)
*cs++ = MI_FLUSH;
*cs++ = GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE;
- *cs++ = i915_ggtt_offset(rq->engine->scratch) |
- PIPE_CONTROL_GLOBAL_GTT;
+ *cs++ = i915_scratch_offset(rq->i915) | PIPE_CONTROL_GLOBAL_GTT;
*cs++ = 0;
*cs++ = 0;
}
@@ -212,8 +210,7 @@ gen4_render_ring_flush(struct i915_request *rq, u32 mode)
static int
intel_emit_post_sync_nonzero_flush(struct i915_request *rq)
{
- u32 scratch_addr =
- i915_ggtt_offset(rq->engine->scratch) + 2 * CACHELINE_BYTES;
+ u32 scratch_addr = i915_scratch_offset(rq->i915) + 2 * CACHELINE_BYTES;
u32 *cs;
cs = intel_ring_begin(rq, 6);
@@ -246,8 +243,7 @@ intel_emit_post_sync_nonzero_flush(struct i915_request *rq)
static int
gen6_render_ring_flush(struct i915_request *rq, u32 mode)
{
- u32 scratch_addr =
- i915_ggtt_offset(rq->engine->scratch) + 2 * CACHELINE_BYTES;
+ u32 scratch_addr = i915_scratch_offset(rq->i915) + 2 * CACHELINE_BYTES;
u32 *cs, flags = 0;
int ret;
@@ -316,8 +312,7 @@ gen7_render_ring_cs_stall_wa(struct i915_request *rq)
static int
gen7_render_ring_flush(struct i915_request *rq, u32 mode)
{
- u32 scratch_addr =
- i915_ggtt_offset(rq->engine->scratch) + 2 * CACHELINE_BYTES;
+ u32 scratch_addr = i915_scratch_offset(rq->i915) + 2 * CACHELINE_BYTES;
u32 *cs, flags = 0;
/*
@@ -1002,7 +997,7 @@ i830_emit_bb_start(struct i915_request *rq,
u64 offset, u32 len,
unsigned int dispatch_flags)
{
- u32 *cs, cs_offset = i915_ggtt_offset(rq->engine->scratch);
+ u32 *cs, cs_offset = i915_scratch_offset(rq->i915);
cs = intel_ring_begin(rq, 6);
if (IS_ERR(cs))
@@ -1459,7 +1454,6 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine)
{
struct i915_timeline *timeline;
struct intel_ring *ring;
- unsigned int size;
int err;
intel_engine_setup_common(engine);
@@ -1484,21 +1478,12 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine)
GEM_BUG_ON(engine->buffer);
engine->buffer = ring;
- size = PAGE_SIZE;
- if (HAS_BROKEN_CS_TLB(engine->i915))
- size = I830_WA_SIZE;
- err = intel_engine_create_scratch(engine, size);
- if (err)
- goto err_unpin;
-
err = intel_engine_init_common(engine);
if (err)
- goto err_scratch;
+ goto err_unpin;
return 0;
-err_scratch:
- intel_engine_cleanup_scratch(engine);
err_unpin:
intel_ring_unpin(ring);
err_ring:
@@ -1572,7 +1557,7 @@ static int flush_pd_dir(struct i915_request *rq)
/* Stall until the page table load is complete */
*cs++ = MI_STORE_REGISTER_MEM | MI_SRM_LRM_GLOBAL_GTT;
*cs++ = i915_mmio_reg_offset(RING_PP_DIR_BASE(engine));
- *cs++ = i915_ggtt_offset(engine->scratch);
+ *cs++ = i915_scratch_offset(rq->i915);
*cs++ = MI_NOOP;
intel_ring_advance(rq, cs);
@@ -1681,7 +1666,7 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
/* Insert a delay before the next switch! */
*cs++ = MI_STORE_REGISTER_MEM | MI_SRM_LRM_GLOBAL_GTT;
*cs++ = i915_mmio_reg_offset(last_reg);
- *cs++ = i915_ggtt_offset(engine->scratch);
+ *cs++ = i915_scratch_offset(rq->i915);
*cs++ = MI_NOOP;
}
*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8a2270b209b0..970fb5c05c36 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -451,7 +451,6 @@ struct intel_engine_cs {
struct intel_hw_status_page status_page;
struct i915_ctx_workarounds wa_ctx;
- struct i915_vma *scratch;
u32 irq_keep_mask; /* always keep these interrupts */
u32 irq_enable_mask; /* bitmask to enable ring interrupt */
@@ -908,10 +907,6 @@ void intel_engine_setup_common(struct intel_engine_cs *engine);
int intel_engine_init_common(struct intel_engine_cs *engine);
void intel_engine_cleanup_common(struct intel_engine_cs *engine);
-int intel_engine_create_scratch(struct intel_engine_cs *engine,
- unsigned int size);
-void intel_engine_cleanup_scratch(struct intel_engine_cs *engine);
-
int intel_init_render_ring_buffer(struct intel_engine_cs *engine);
int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine);
int intel_init_blt_ring_buffer(struct intel_engine_cs *engine);
--
2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/7] drm/i915/selftests: Terminate hangcheck sanitycheck forcibly
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
2018-12-01 9:52 ` [PATCH 2/7] drm/i915/ringbuffer: Clear semaphore sync registers on ring init Chris Wilson
2018-12-01 9:52 ` [PATCH 3/7] drm/i915: Allocate a common scratch page Chris Wilson
@ 2018-12-01 9:52 ` Chris Wilson
2018-12-01 9:52 ` [PATCH 5/7] drm/i915/selftests: Reorder request allocation vs vma pinning Chris Wilson
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 9:52 UTC (permalink / raw)
To: intel-gfx
If all else fails and we are stuck eternally waiting for the undying
request, abandon all hope.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index defe671130ab..a48fbe2557ea 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -308,6 +308,7 @@ static int igt_hang_sanitycheck(void *arg)
goto unlock;
for_each_engine(engine, i915, id) {
+ struct igt_wedge_me w;
long timeout;
if (!intel_engine_can_store_dword(engine))
@@ -328,9 +329,14 @@ static int igt_hang_sanitycheck(void *arg)
i915_request_add(rq);
- timeout = i915_request_wait(rq,
- I915_WAIT_LOCKED,
- MAX_SCHEDULE_TIMEOUT);
+ timeout = 0;
+ igt_wedge_on_timeout(&w, i915, HZ / 10 /* 100ms timeout*/)
+ timeout = i915_request_wait(rq,
+ I915_WAIT_LOCKED,
+ MAX_SCHEDULE_TIMEOUT);
+ if (i915_terminally_wedged(&i915->gpu_error))
+ timeout = -EIO;
+
i915_request_put(rq);
if (timeout < 0) {
--
2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/7] drm/i915/selftests: Reorder request allocation vs vma pinning
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (2 preceding siblings ...)
2018-12-01 9:52 ` [PATCH 4/7] drm/i915/selftests: Terminate hangcheck sanitycheck forcibly Chris Wilson
@ 2018-12-01 9:52 ` Chris Wilson
2018-12-01 9:52 ` [PATCH 6/7] drm/i915: Pipeline PDP updates for Braswell Chris Wilson
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 9:52 UTC (permalink / raw)
To: intel-gfx
Impose a restraint that we have all vma pinned for a request prior to
its allocation. This is to simplify request construction, and should
facilitate unravelling the lock interdependencies later.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/selftests/huge_pages.c | 31 +++--
drivers/gpu/drm/i915/selftests/igt_spinner.c | 86 ++++++------
.../gpu/drm/i915/selftests/intel_hangcheck.c | 123 +++++++++---------
3 files changed, 119 insertions(+), 121 deletions(-)
diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
index 26c065c8d2c0..a0c7cbc212ba 100644
--- a/drivers/gpu/drm/i915/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
@@ -972,7 +972,6 @@ static int gpu_write(struct i915_vma *vma,
{
struct i915_request *rq;
struct i915_vma *batch;
- int flags = 0;
int err;
GEM_BUG_ON(!intel_engine_can_store_dword(engine));
@@ -981,14 +980,14 @@ static int gpu_write(struct i915_vma *vma,
if (err)
return err;
- rq = i915_request_alloc(engine, ctx);
- if (IS_ERR(rq))
- return PTR_ERR(rq);
-
batch = gpu_write_dw(vma, dword * sizeof(u32), value);
- if (IS_ERR(batch)) {
- err = PTR_ERR(batch);
- goto err_request;
+ if (IS_ERR(batch))
+ return PTR_ERR(batch);
+
+ rq = i915_request_alloc(engine, ctx);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto err_batch;
}
err = i915_vma_move_to_active(batch, rq, 0);
@@ -996,21 +995,21 @@ static int gpu_write(struct i915_vma *vma,
goto err_request;
i915_gem_object_set_active_reference(batch->obj);
- i915_vma_unpin(batch);
- i915_vma_close(batch);
- err = engine->emit_bb_start(rq,
- batch->node.start, batch->node.size,
- flags);
+ err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
if (err)
goto err_request;
- err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
+ err = engine->emit_bb_start(rq,
+ batch->node.start, batch->node.size,
+ 0);
+err_request:
if (err)
i915_request_skip(rq, err);
-
-err_request:
i915_request_add(rq);
+err_batch:
+ i915_vma_unpin(batch);
+ i915_vma_close(batch);
return err;
}
diff --git a/drivers/gpu/drm/i915/selftests/igt_spinner.c b/drivers/gpu/drm/i915/selftests/igt_spinner.c
index 8cd34f6e6859..0e70df0230b8 100644
--- a/drivers/gpu/drm/i915/selftests/igt_spinner.c
+++ b/drivers/gpu/drm/i915/selftests/igt_spinner.c
@@ -68,48 +68,65 @@ static u64 hws_address(const struct i915_vma *hws,
return hws->node.start + seqno_offset(rq->fence.context);
}
-static int emit_recurse_batch(struct igt_spinner *spin,
- struct i915_request *rq,
- u32 arbitration_command)
+static int move_to_active(struct i915_vma *vma,
+ struct i915_request *rq,
+ unsigned int flags)
{
- struct i915_address_space *vm = &rq->gem_context->ppgtt->vm;
+ int err;
+
+ err = i915_vma_move_to_active(vma, rq, flags);
+ if (err)
+ return err;
+
+ if (!i915_gem_object_has_active_reference(vma->obj)) {
+ i915_gem_object_get(vma->obj);
+ i915_gem_object_set_active_reference(vma->obj);
+ }
+
+ return 0;
+}
+
+struct i915_request *
+igt_spinner_create_request(struct igt_spinner *spin,
+ struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine,
+ u32 arbitration_command)
+{
+ struct i915_address_space *vm = &ctx->ppgtt->vm;
+ struct i915_request *rq = NULL;
struct i915_vma *hws, *vma;
u32 *batch;
int err;
vma = i915_vma_instance(spin->obj, vm, NULL);
if (IS_ERR(vma))
- return PTR_ERR(vma);
+ return ERR_CAST(vma);
hws = i915_vma_instance(spin->hws, vm, NULL);
if (IS_ERR(hws))
- return PTR_ERR(hws);
+ return ERR_CAST(hws);
err = i915_vma_pin(vma, 0, 0, PIN_USER);
if (err)
- return err;
+ return ERR_PTR(err);
err = i915_vma_pin(hws, 0, 0, PIN_USER);
if (err)
goto unpin_vma;
- err = i915_vma_move_to_active(vma, rq, 0);
- if (err)
+ rq = i915_request_alloc(engine, ctx);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
goto unpin_hws;
-
- if (!i915_gem_object_has_active_reference(vma->obj)) {
- i915_gem_object_get(vma->obj);
- i915_gem_object_set_active_reference(vma->obj);
}
- err = i915_vma_move_to_active(hws, rq, 0);
+ err = move_to_active(vma, rq, 0);
if (err)
- goto unpin_hws;
+ goto cancel_rq;
- if (!i915_gem_object_has_active_reference(hws->obj)) {
- i915_gem_object_get(hws->obj);
- i915_gem_object_set_active_reference(hws->obj);
- }
+ err = move_to_active(hws, rq, 0);
+ if (err)
+ goto cancel_rq;
batch = spin->batch;
@@ -127,35 +144,18 @@ static int emit_recurse_batch(struct igt_spinner *spin,
i915_gem_chipset_flush(spin->i915);
- err = rq->engine->emit_bb_start(rq, vma->node.start, PAGE_SIZE, 0);
+ err = engine->emit_bb_start(rq, vma->node.start, PAGE_SIZE, 0);
+cancel_rq:
+ if (err) {
+ i915_request_skip(rq, err);
+ i915_request_add(rq);
+ }
unpin_hws:
i915_vma_unpin(hws);
unpin_vma:
i915_vma_unpin(vma);
- return err;
-}
-
-struct i915_request *
-igt_spinner_create_request(struct igt_spinner *spin,
- struct i915_gem_context *ctx,
- struct intel_engine_cs *engine,
- u32 arbitration_command)
-{
- struct i915_request *rq;
- int err;
-
- rq = i915_request_alloc(engine, ctx);
- if (IS_ERR(rq))
- return rq;
-
- err = emit_recurse_batch(spin, rq, arbitration_command);
- if (err) {
- i915_request_add(rq);
- return ERR_PTR(err);
- }
-
- return rq;
+ return err ? ERR_PTR(err) : rq;
}
static u32
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index a48fbe2557ea..0ff813ad3462 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -102,52 +102,87 @@ static u64 hws_address(const struct i915_vma *hws,
return hws->node.start + offset_in_page(sizeof(u32)*rq->fence.context);
}
-static int emit_recurse_batch(struct hang *h,
- struct i915_request *rq)
+static int move_to_active(struct i915_vma *vma,
+ struct i915_request *rq,
+ unsigned int flags)
+{
+ int err;
+
+ err = i915_vma_move_to_active(vma, rq, flags);
+ if (err)
+ return err;
+
+ if (!i915_gem_object_has_active_reference(vma->obj)) {
+ i915_gem_object_get(vma->obj);
+ i915_gem_object_set_active_reference(vma->obj);
+ }
+
+ return 0;
+}
+
+static struct i915_request *
+hang_create_request(struct hang *h, struct intel_engine_cs *engine)
{
struct drm_i915_private *i915 = h->i915;
struct i915_address_space *vm =
- rq->gem_context->ppgtt ?
- &rq->gem_context->ppgtt->vm :
- &i915->ggtt.vm;
+ h->ctx->ppgtt ? &h->ctx->ppgtt->vm : &i915->ggtt.vm;
+ struct i915_request *rq = NULL;
struct i915_vma *hws, *vma;
unsigned int flags;
u32 *batch;
int err;
+ if (i915_gem_object_is_active(h->obj)) {
+ struct drm_i915_gem_object *obj;
+ void *vaddr;
+
+ obj = i915_gem_object_create_internal(h->i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return ERR_CAST(obj);
+
+ vaddr = i915_gem_object_pin_map(obj,
+ i915_coherent_map_type(h->i915));
+ if (IS_ERR(vaddr)) {
+ i915_gem_object_put(obj);
+ return ERR_CAST(vaddr);
+ }
+
+ i915_gem_object_unpin_map(h->obj);
+ i915_gem_object_put(h->obj);
+
+ h->obj = obj;
+ h->batch = vaddr;
+ }
+
vma = i915_vma_instance(h->obj, vm, NULL);
if (IS_ERR(vma))
- return PTR_ERR(vma);
+ return ERR_CAST(vma);
hws = i915_vma_instance(h->hws, vm, NULL);
if (IS_ERR(hws))
- return PTR_ERR(hws);
+ return ERR_CAST(hws);
err = i915_vma_pin(vma, 0, 0, PIN_USER);
if (err)
- return err;
+ return ERR_PTR(err);
err = i915_vma_pin(hws, 0, 0, PIN_USER);
if (err)
goto unpin_vma;
- err = i915_vma_move_to_active(vma, rq, 0);
- if (err)
+ rq = i915_request_alloc(engine, h->ctx);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
goto unpin_hws;
-
- if (!i915_gem_object_has_active_reference(vma->obj)) {
- i915_gem_object_get(vma->obj);
- i915_gem_object_set_active_reference(vma->obj);
}
- err = i915_vma_move_to_active(hws, rq, 0);
+ err = move_to_active(vma, rq, 0);
if (err)
- goto unpin_hws;
+ goto cancel_rq;
- if (!i915_gem_object_has_active_reference(hws->obj)) {
- i915_gem_object_get(hws->obj);
- i915_gem_object_set_active_reference(hws->obj);
- }
+ err = move_to_active(hws, rq, 0);
+ if (err)
+ goto cancel_rq;
batch = h->batch;
if (INTEL_GEN(i915) >= 8) {
@@ -212,52 +247,16 @@ static int emit_recurse_batch(struct hang *h,
err = rq->engine->emit_bb_start(rq, vma->node.start, PAGE_SIZE, flags);
+cancel_rq:
+ if (err) {
+ i915_request_skip(rq, err);
+ i915_request_add(rq);
+ }
unpin_hws:
i915_vma_unpin(hws);
unpin_vma:
i915_vma_unpin(vma);
- return err;
-}
-
-static struct i915_request *
-hang_create_request(struct hang *h, struct intel_engine_cs *engine)
-{
- struct i915_request *rq;
- int err;
-
- if (i915_gem_object_is_active(h->obj)) {
- struct drm_i915_gem_object *obj;
- void *vaddr;
-
- obj = i915_gem_object_create_internal(h->i915, PAGE_SIZE);
- if (IS_ERR(obj))
- return ERR_CAST(obj);
-
- vaddr = i915_gem_object_pin_map(obj,
- i915_coherent_map_type(h->i915));
- if (IS_ERR(vaddr)) {
- i915_gem_object_put(obj);
- return ERR_CAST(vaddr);
- }
-
- i915_gem_object_unpin_map(h->obj);
- i915_gem_object_put(h->obj);
-
- h->obj = obj;
- h->batch = vaddr;
- }
-
- rq = i915_request_alloc(engine, h->ctx);
- if (IS_ERR(rq))
- return rq;
-
- err = emit_recurse_batch(h, rq);
- if (err) {
- i915_request_add(rq);
- return ERR_PTR(err);
- }
-
- return rq;
+ return err ? ERR_PTR(err) : rq;
}
static u32 hws_seqno(const struct hang *h, const struct i915_request *rq)
--
2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/7] drm/i915: Pipeline PDP updates for Braswell
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (3 preceding siblings ...)
2018-12-01 9:52 ` [PATCH 5/7] drm/i915/selftests: Reorder request allocation vs vma pinning Chris Wilson
@ 2018-12-01 9:52 ` Chris Wilson
2018-12-01 9:52 ` [PATCH 7/7] drm/i915/execlists: Apply a full mb before execution " Chris Wilson
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 9:52 UTC (permalink / raw)
To: intel-gfx
Currently we face a severe problem on Braswell that manifests as invalid
ppGTT accesses. The code tries to maintain the PDP (page directory
pointers) inside the context in two ways, direct write into the context
and a pipelined LRI update. The direct write into the context is
fundamentally racy as it is unserialised with any access (read or write)
the GPU is doing. By asserting that Braswell is not used with vGPU
(currently an unsupported platform) we can eliminate the dangerous
direct write into the context image and solely use the pipelined update.
However, the LRI of the PDP fouls up the GPU, causing it to freeze and
take out the machine with "forcewake ack timeouts". This seems possible
to workaround by preventing the GPU from sleeping (via means of
disabling the power-state management interface, i.e. forcing each ring
to remain awake) around the update.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108656
References: https://bugs.freedesktop.org/show_bug.cgi?id=108714
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 -
drivers/gpu/drm/i915/i915_request.c | 5 -
drivers/gpu/drm/i915/intel_lrc.c | 137 +++++++++++-------------
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +-
4 files changed, 68 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index add1fe7aeb93..62bde517d383 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1423,8 +1423,6 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
gen8_initialize_pd(vm, pd);
gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
GEM_BUG_ON(pdp->used_pdpes > i915_pdpes_per_pdp(vm));
-
- mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
}
ret = gen8_ppgtt_alloc_pd(vm, pd, start, length);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index ca95ab2f4cfa..8ab8e8e6a086 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -719,11 +719,6 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
*/
rq->head = rq->ring->emit;
- /* Unconditionally invalidate GPU caches and TLBs. */
- ret = engine->emit_flush(rq, EMIT_INVALIDATE);
- if (ret)
- goto err_unwind;
-
ret = engine->request_alloc(rq);
if (ret)
goto err_unwind;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index de070dca4033..0a690c557113 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -363,31 +363,12 @@ execlists_context_schedule_out(struct i915_request *rq, unsigned long status)
trace_i915_request_out(rq);
}
-static void
-execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state)
-{
- ASSIGN_CTX_PDP(ppgtt, reg_state, 3);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 2);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 1);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
-}
-
static u64 execlists_update_context(struct i915_request *rq)
{
- struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt;
struct intel_context *ce = rq->hw_context;
- u32 *reg_state = ce->lrc_reg_state;
-
- reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
- /*
- * True 32b PPGTT with dynamic page allocation: update PDP
- * registers and point the unallocated PDPs to scratch page.
- * PML4 is allocated during ppgtt init, so this is not needed
- * in 48-bit mode.
- */
- if (!i915_vm_is_48bit(&ppgtt->vm))
- execlists_update_context_pdps(ppgtt, reg_state);
+ ce->lrc_reg_state[CTX_RING_TAIL + 1] =
+ intel_ring_set_tail(rq->ring, rq->tail);
/*
* Make sure the context image is complete before we submit it to HW.
@@ -1240,29 +1221,80 @@ execlists_context_pin(struct intel_engine_cs *engine,
return __execlists_context_pin(engine, ctx, ce);
}
+static int emit_pdps(struct i915_request *rq)
+{
+ const struct intel_engine_cs * const engine = rq->engine;
+ struct i915_hw_ppgtt * const ppgtt = rq->gem_context->ppgtt;
+ int err, i;
+ u32 *cs;
+
+ err = engine->emit_flush(rq, EMIT_INVALIDATE);
+ if (err)
+ return err;
+
+ cs = intel_ring_begin(rq, 4 * GEN8_3LVL_PDPES + 2);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+
+ /*
+ * Force the GPU (not just the local engine/powerwell!) to remain awake,
+ * or else we may kill the machine with "timed out waiting for
+ * forcewake ack request".
+ */
+
+ *cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES);
+ for (i = GEN8_3LVL_PDPES; i--; ) {
+ const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
+
+ *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(engine, i));
+ *cs++ = upper_32_bits(pd_daddr);
+ *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(engine, i));
+ *cs++ = lower_32_bits(pd_daddr);
+ }
+ *cs++ = MI_NOOP;
+
+ intel_ring_advance(rq, cs);
+
+ err = engine->emit_flush(rq, EMIT_INVALIDATE);
+ if (err)
+ return err;
+
+ return 0;
+}
+
static int execlists_request_alloc(struct i915_request *request)
{
int ret;
GEM_BUG_ON(!request->hw_context->pin_count);
- /* Flush enough space to reduce the likelihood of waiting after
+ /*
+ * Flush enough space to reduce the likelihood of waiting after
* we start building the request - in which case we will just
* have to repeat work.
*/
request->reserved_space += EXECLISTS_REQUEST_SIZE;
- ret = intel_ring_wait_for_space(request->ring, request->reserved_space);
- if (ret)
- return ret;
-
- /* Note that after this point, we have committed to using
+ /*
+ * Note that after this point, we have committed to using
* this request as it is being used to both track the
* state of engine initialisation and liveness of the
* golden renderstate above. Think twice before you try
* to cancel/unwind this request now.
*/
+ /* Unconditionally invalidate GPU caches and TLBs. */
+ if (i915_vm_is_48bit(&request->gem_context->ppgtt->vm)) {
+ ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
+ if (ret)
+ return ret;
+ } else {
+ GEM_BUG_ON(intel_vgpu_active(request->i915));
+ ret = emit_pdps(request);
+ if (ret)
+ return ret;
+ }
+
request->reserved_space -= EXECLISTS_REQUEST_SIZE;
return 0;
}
@@ -1832,56 +1864,11 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
atomic_read(&execlists->tasklet.count));
}
-static int intel_logical_ring_emit_pdps(struct i915_request *rq)
-{
- struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt;
- struct intel_engine_cs *engine = rq->engine;
- const int num_lri_cmds = GEN8_3LVL_PDPES * 2;
- u32 *cs;
- int i;
-
- cs = intel_ring_begin(rq, num_lri_cmds * 2 + 2);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
-
- *cs++ = MI_LOAD_REGISTER_IMM(num_lri_cmds);
- for (i = GEN8_3LVL_PDPES - 1; i >= 0; i--) {
- const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
-
- *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(engine, i));
- *cs++ = upper_32_bits(pd_daddr);
- *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(engine, i));
- *cs++ = lower_32_bits(pd_daddr);
- }
-
- *cs++ = MI_NOOP;
- intel_ring_advance(rq, cs);
-
- return 0;
-}
-
static int gen8_emit_bb_start(struct i915_request *rq,
u64 offset, u32 len,
const unsigned int flags)
{
u32 *cs;
- int ret;
-
- /* Don't rely in hw updating PDPs, specially in lite-restore.
- * Ideally, we should set Force PD Restore in ctx descriptor,
- * but we can't. Force Restore would be a second option, but
- * it is unsafe in case of lite-restore (because the ctx is
- * not idle). PML4 is allocated during ppgtt init so this is
- * not needed in 48-bit.*/
- if ((intel_engine_flag(rq->engine) & rq->gem_context->ppgtt->pd_dirty_rings) &&
- !i915_vm_is_48bit(&rq->gem_context->ppgtt->vm) &&
- !intel_vgpu_active(rq->i915)) {
- ret = intel_logical_ring_emit_pdps(rq);
- if (ret)
- return ret;
-
- rq->gem_context->ppgtt->pd_dirty_rings &= ~intel_engine_flag(rq->engine);
- }
cs = intel_ring_begin(rq, 6);
if (IS_ERR(cs))
@@ -1914,6 +1901,7 @@ static int gen8_emit_bb_start(struct i915_request *rq,
*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
*cs++ = MI_NOOP;
+
intel_ring_advance(rq, cs);
return 0;
@@ -2544,6 +2532,11 @@ static void execlists_init_reg_state(u32 *regs,
* other PDP Descriptors are ignored.
*/
ASSIGN_CTX_PML4(ctx->ppgtt, regs);
+ } else {
+ ASSIGN_CTX_PDP(ctx->ppgtt, regs, 3);
+ ASSIGN_CTX_PDP(ctx->ppgtt, regs, 2);
+ ASSIGN_CTX_PDP(ctx->ppgtt, regs, 1);
+ ASSIGN_CTX_PDP(ctx->ppgtt, regs, 0);
}
if (rcs) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a3d3126a3938..7192a61cb498 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1824,11 +1824,12 @@ static int ring_request_alloc(struct i915_request *request)
*/
request->reserved_space += LEGACY_REQUEST_SIZE;
- ret = intel_ring_wait_for_space(request->ring, request->reserved_space);
+ ret = switch_context(request);
if (ret)
return ret;
- ret = switch_context(request);
+ /* Unconditionally invalidate GPU caches and TLBs. */
+ ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
if (ret)
return ret;
--
2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] drm/i915/execlists: Apply a full mb before execution for Braswell
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (4 preceding siblings ...)
2018-12-01 9:52 ` [PATCH 6/7] drm/i915: Pipeline PDP updates for Braswell Chris Wilson
@ 2018-12-01 9:52 ` Chris Wilson
2018-12-01 18:32 ` Chris Wilson
2018-12-01 10:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging Patchwork
` (3 subsequent siblings)
9 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 9:52 UTC (permalink / raw)
To: intel-gfx
Cc: Chris Wilson, Mika Kuoppala, Tvrtko Ursulin, Joonas Lahtinen,
stable
Braswell is really picky about having our writes posted to memory before
we execute or else the GPU may see stale values. A wmb() is insufficient
as it only ensures the writes are visible to other cores, we need a full
mb() to ensure the writes are in memory and visible to the GPU.
The most frequent failure in flushing before execution is that we see
stale PTE values and execute the wrong pages.
References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/i915/intel_lrc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0a690c557113..cad1228f6503 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -379,8 +379,12 @@ static u64 execlists_update_context(struct i915_request *rq)
* may not be visible to the HW prior to the completion of the UC
* register write and that we may begin execution from the context
* before its image is complete leading to invalid PD chasing.
+ *
+ * Furthermore, Braswell wants a full mb to be sure that the writes
+ * are coherent in memory (visible to the GPU) prior to execution,
+ * and not just visible to other CPUs (as is the result of wmb).
*/
- wmb();
+ mb();
return ce->lrc_desc;
}
--
2.20.0.rc1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 7/7] drm/i915/execlists: Apply a full mb before execution for Braswell
2018-12-01 9:52 ` [PATCH 7/7] drm/i915/execlists: Apply a full mb before execution " Chris Wilson
@ 2018-12-01 18:32 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-12-01 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala, Tvrtko Ursulin, Joonas Lahtinen, stable
Quoting Chris Wilson (2018-12-01 09:52:31)
> Braswell is really picky about having our writes posted to memory before
> we execute or else the GPU may see stale values. A wmb() is insufficient
> as it only ensures the writes are visible to other cores, we need a full
> mb() to ensure the writes are in memory and visible to the GPU.
>
> The most frequent failure in flushing before execution is that we see
> stale PTE values and execute the wrong pages.
>
> References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a690c557113..cad1228f6503 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -379,8 +379,12 @@ static u64 execlists_update_context(struct i915_request *rq)
> * may not be visible to the HW prior to the completion of the UC
> * register write and that we may begin execution from the context
> * before its image is complete leading to invalid PD chasing.
> + *
> + * Furthermore, Braswell wants a full mb to be sure that the writes
> + * are coherent in memory (visible to the GPU) prior to execution,
> + * and not just visible to other CPUs (as is the result of wmb).
> */
> - wmb();
> + mb();
Scratch that. Just took ages to hit this time. It seemed so promising!
-Chris
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (5 preceding siblings ...)
2018-12-01 9:52 ` [PATCH 7/7] drm/i915/execlists: Apply a full mb before execution " Chris Wilson
@ 2018-12-01 10:00 ` Patchwork
2018-12-01 10:03 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-12-01 10:00 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
URL : https://patchwork.freedesktop.org/series/53348/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
a7232ddbd965 drm/i915: Complete the fences as they are cancelled due to wedging
-:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#13:
<1>[ 354.473346] BUG: unable to handle kernel NULL pointer dereference at 0000000000000250
total: 0 errors, 1 warnings, 0 checks, 135 lines checked
1ab5d3906e81 drm/i915/ringbuffer: Clear semaphore sync registers on ring init
14e0cc931853 drm/i915: Allocate a common scratch page
bfd217d7fef0 drm/i915/selftests: Terminate hangcheck sanitycheck forcibly
e050a2a0f245 drm/i915/selftests: Reorder request allocation vs vma pinning
1bd7d7a676a7 drm/i915: Pipeline PDP updates for Braswell
10228149a5d8 drm/i915/execlists: Apply a full mb before execution for Braswell
-:15: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15:
References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
-:15: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")'
#15:
References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
-:36: WARNING:MEMORY_BARRIER: memory barrier without comment
#36: FILE: drivers/gpu/drm/i915/intel_lrc.c:387:
+ mb();
total: 1 errors, 2 warnings, 0 checks, 13 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* ✗ Fi.CI.SPARSE: warning for series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (6 preceding siblings ...)
2018-12-01 10:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging Patchwork
@ 2018-12-01 10:03 ` Patchwork
2018-12-01 10:21 ` ✓ Fi.CI.BAT: success " Patchwork
2018-12-01 20:34 ` ✓ Fi.CI.IGT: " Patchwork
9 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-12-01 10:03 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
URL : https://patchwork.freedesktop.org/series/53348/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Complete the fences as they are cancelled due to wedging
Okay!
Commit: drm/i915/ringbuffer: Clear semaphore sync registers on ring init
Okay!
Commit: drm/i915: Allocate a common scratch page
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3571:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3573:16: warning: expression using sizeof(void)
Commit: drm/i915/selftests: Terminate hangcheck sanitycheck forcibly
Okay!
Commit: drm/i915/selftests: Reorder request allocation vs vma pinning
Okay!
Commit: drm/i915: Pipeline PDP updates for Braswell
Okay!
Commit: drm/i915/execlists: Apply a full mb before execution for Braswell
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (7 preceding siblings ...)
2018-12-01 10:03 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-12-01 10:21 ` Patchwork
2018-12-01 20:34 ` ✓ Fi.CI.IGT: " Patchwork
9 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-12-01 10:21 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
URL : https://patchwork.freedesktop.org/series/53348/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5237 -> Patchwork_10990
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/53348/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_10990 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
* {igt@runner@aborted}:
- {fi-icl-y}: NOTRUN -> FAIL [fdo#108070]
#### Possible fixes ####
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
Participating hosts (49 -> 43)
------------------------------
Additional (1): fi-icl-y
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u
Build changes
-------------
* Linux: CI_DRM_5237 -> Patchwork_10990
CI_DRM_5237: 2f99c4889e4124f9cf50b745d037f432318c4bb4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10990: 10228149a5d8d7315b0f5ee2f60adce5921908d4 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
10228149a5d8 drm/i915/execlists: Apply a full mb before execution for Braswell
1bd7d7a676a7 drm/i915: Pipeline PDP updates for Braswell
e050a2a0f245 drm/i915/selftests: Reorder request allocation vs vma pinning
bfd217d7fef0 drm/i915/selftests: Terminate hangcheck sanitycheck forcibly
14e0cc931853 drm/i915: Allocate a common scratch page
1ab5d3906e81 drm/i915/ringbuffer: Clear semaphore sync registers on ring init
a7232ddbd965 drm/i915: Complete the fences as they are cancelled due to wedging
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10990/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* ✓ Fi.CI.IGT: success for series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
2018-12-01 9:52 [PATCH 1/7] drm/i915: Complete the fences as they are cancelled due to wedging Chris Wilson
` (8 preceding siblings ...)
2018-12-01 10:21 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-12-01 20:34 ` Patchwork
9 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-12-01 20:34 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/7] drm/i915: Complete the fences as they are cancelled due to wedging
URL : https://patchwork.freedesktop.org/series/53348/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5237_full -> Patchwork_10990_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_10990_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_suspend@shrink:
- shard-hsw: NOTRUN -> DMESG-WARN [fdo#108784]
- {shard-iclb}: NOTRUN -> DMESG-WARN [fdo#108784]
* igt@kms_atomic@test_only:
- {shard-iclb}: PASS -> DMESG-WARN [fdo#107724] +29
* igt@kms_busy@extended-modeset-hang-newfb-render-a:
- shard-snb: PASS -> DMESG-WARN [fdo#107956]
* igt@kms_color@pipe-c-legacy-gamma:
- shard-apl: PASS -> FAIL [fdo#104782]
* igt@kms_cursor_crc@cursor-256x256-random:
- shard-apl: PASS -> FAIL [fdo#103232] +1
* igt@kms_cursor_crc@cursor-256x85-random:
- shard-glk: PASS -> FAIL [fdo#103232] +3
* igt@kms_cursor_crc@cursor-64x21-sliding:
- {shard-iclb}: NOTRUN -> FAIL [fdo#103232] +2
* igt@kms_draw_crc@draw-method-rgb565-render-xtiled:
- {shard-iclb}: PASS -> WARN [fdo#108336] +3
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
- shard-apl: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbc-1p-rte:
- shard-glk: PASS -> FAIL [fdo#103167] / [fdo#105682]
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
- shard-glk: PASS -> FAIL [fdo#103167] +2
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt:
- {shard-iclb}: PASS -> DMESG-FAIL [fdo#107724] +4
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt:
- {shard-iclb}: PASS -> FAIL [fdo#103167] +2
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- {shard-iclb}: PASS -> DMESG-FAIL [fdo#103166] / [fdo#107724]
* igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
- shard-kbl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
- {shard-iclb}: PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +11
* igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
- {shard-iclb}: PASS -> FAIL [fdo#103166]
* igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
- shard-glk: PASS -> FAIL [fdo#103166] +1
* igt@kms_setmode@basic:
- shard-apl: PASS -> FAIL [fdo#99912]
- shard-kbl: PASS -> FAIL [fdo#99912]
* igt@perf@blocking:
- shard-hsw: PASS -> FAIL [fdo#102252] +1
#### Possible fixes ####
* igt@gem_ctx_isolation@rcs0-s3:
- shard-snb: DMESG-WARN [fdo#102365] -> PASS
* igt@gem_ppgtt@blt-vs-render-ctxn:
- shard-kbl: INCOMPLETE [fdo#103665] / [fdo#106023] / [fdo#106887] -> PASS
* igt@kms_cursor_crc@cursor-64x64-onscreen:
- shard-apl: FAIL [fdo#103232] -> PASS +2
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
- shard-glk: FAIL [fdo#103167] -> PASS +3
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
- shard-apl: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-wc:
- {shard-iclb}: FAIL [fdo#103167] -> PASS +1
* igt@kms_plane@plane-position-covered-pipe-c-planes:
- shard-glk: FAIL [fdo#103166] -> PASS
* igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
- shard-apl: FAIL [fdo#103166] -> PASS +3
* igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
- {shard-iclb}: FAIL [fdo#103166] -> PASS +1
* igt@kms_vblank@pipe-b-ts-continuation-suspend:
- {shard-iclb}: INCOMPLETE [fdo#107713] -> PASS
* igt@pm_rpm@legacy-planes:
- {shard-iclb}: DMESG-WARN [fdo#108654] -> PASS
#### Warnings ####
* igt@i915_suspend@shrink:
- shard-snb: INCOMPLETE [fdo#105411] / [fdo#106886] -> DMESG-WARN [fdo#108784]
* igt@kms_plane@pixel-format-pipe-a-planes:
- {shard-iclb}: FAIL [fdo#103166] -> DMESG-WARN [fdo#107724] / [fdo#108336] +1
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102252]: https://bugs.freedesktop.org/show_bug.cgi?id=102252
[fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#106023]: https://bugs.freedesktop.org/show_bug.cgi?id=106023
[fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
[fdo#106887]: https://bugs.freedesktop.org/show_bug.cgi?id=106887
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
[fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
[fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (7 -> 6)
------------------------------
Missing (1): shard-skl
Build changes
-------------
* Linux: CI_DRM_5237 -> Patchwork_10990
CI_DRM_5237: 2f99c4889e4124f9cf50b745d037f432318c4bb4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10990: 10228149a5d8d7315b0f5ee2f60adce5921908d4 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10990/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread