* [PATCH 1/4] drm/i915: Detect page faults during hangcheck
2015-03-17 16:18 [PATCH 0/4] ppgtt: ring initialization fixes Mika Kuoppala
@ 2015-03-17 16:18 ` Mika Kuoppala
2015-03-17 16:18 ` [PATCH 2/4] drm/i915: Introduce ring->start_ring() Mika Kuoppala
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2015-03-17 16:18 UTC (permalink / raw)
To: intel-gfx; +Cc: miku
From: Chris Wilson <chris@chris-wilson.co.uk>
On Sandybridge+, the GPU provides the ERROR register for detecting page
faults. Hook this up to our hangcheck so that we can dump the error
state soon after such an event occurs. This would be better inside an
interrupt handler, but it serves a purpose here as it detects that our
initial context setup is invalid...
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_irq.c | 5 +++++
drivers/gpu/drm/i915/intel_uncore.c | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 49ad5fb..ea668fc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2929,6 +2929,11 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
if (!i915.enable_hangcheck)
return;
+ if (INTEL_INFO(dev_priv)->gen >= 6 && I915_READ(ERROR_GEN6)) {
+ i915_handle_error(dev, false, "GPU reported a page fault");
+ I915_WRITE(ERROR_GEN6, 0);
+ }
+
for_each_ring(ring, dev_priv, i) {
u64 acthd;
u32 seqno;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index ab5cc94..c58535e 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -359,6 +359,8 @@ static void __intel_uncore_early_sanitize(struct drm_device *dev,
if (IS_GEN6(dev) || IS_GEN7(dev))
__raw_i915_write32(dev_priv, GTFIFODBG,
__raw_i915_read32(dev_priv, GTFIFODBG));
+ if (INTEL_INFO(dev)->gen >= 6)
+ __raw_i915_write32(dev_priv, ERROR_GEN6, 0);
intel_uncore_forcewake_reset(dev, restore_forcewake);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] drm/i915: Introduce ring->start_ring()
2015-03-17 16:18 [PATCH 0/4] ppgtt: ring initialization fixes Mika Kuoppala
2015-03-17 16:18 ` [PATCH 1/4] drm/i915: Detect page faults during hangcheck Mika Kuoppala
@ 2015-03-17 16:18 ` Mika Kuoppala
2015-03-17 16:52 ` Chris Wilson
2015-03-17 16:18 ` [PATCH 3/4] drm/i915: Reorder hw init to avoid executing with invalid context/mm state Mika Kuoppala
2015-03-17 16:18 ` [PATCH 4/4] drm/i915: Stop rings before cleaning up on reset Mika Kuoppala
3 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2015-03-17 16:18 UTC (permalink / raw)
To: intel-gfx; +Cc: miku
This allows us to fill ringbuffer without engine being active.
The next patch will take advantage of this.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 39 ++++++++++++++++++-------
drivers/gpu/drm/i915/intel_lrc.c | 20 +++++++++++++
drivers/gpu/drm/i915/intel_lrc.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 51 ++++++++++++++++++++++++---------
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++
6 files changed, 90 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81f60b4..7a7eb97 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1823,6 +1823,7 @@ struct drm_i915_private {
int (*init_rings)(struct drm_device *dev);
void (*cleanup_ring)(struct intel_engine_cs *ring);
void (*stop_ring)(struct intel_engine_cs *ring);
+ void (*start_ring)(struct intel_engine_cs *ring);
} gt;
uint32_t request_uniq;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e2876bf..8147e2e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2694,6 +2694,8 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
struct drm_i915_gem_request,
list);
+ request->ringbuf->last_retired_head = request->postfix;
+
i915_gem_free_request(request);
}
@@ -2722,6 +2724,28 @@ void i915_gem_restore_fences(struct drm_device *dev)
}
}
+static void
+i915_gem_stop_ringbuffers(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *ring;
+ int i;
+
+ for_each_ring(ring, dev_priv, i)
+ dev_priv->gt.stop_ring(ring);
+}
+
+static void
+i915_gem_start_ringbuffers(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *ring;
+ int i;
+
+ for_each_ring(ring, dev_priv, i)
+ dev_priv->gt.start_ring(ring);
+}
+
void i915_gem_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4648,17 +4672,6 @@ void i915_gem_vma_destroy(struct i915_vma *vma)
kfree(vma);
}
-static void
-i915_gem_stop_ringbuffers(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_engine_cs *ring;
- int i;
-
- for_each_ring(ring, dev_priv, i)
- dev_priv->gt.stop_ring(ring);
-}
-
int
i915_gem_suspend(struct drm_device *dev)
{
@@ -4891,6 +4904,8 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}
+ i915_gem_start_ringbuffers(dev);
+
for (i = 0; i < NUM_L3_SLICES(dev); i++)
i915_gem_l3_remap(&dev_priv->ring[RCS], i);
@@ -4936,11 +4951,13 @@ int i915_gem_init(struct drm_device *dev)
dev_priv->gt.init_rings = i915_gem_init_rings;
dev_priv->gt.cleanup_ring = intel_cleanup_ring_buffer;
dev_priv->gt.stop_ring = intel_stop_ring_buffer;
+ dev_priv->gt.start_ring = intel_start_ring_buffer;
} else {
dev_priv->gt.do_execbuf = intel_execlists_submission;
dev_priv->gt.init_rings = intel_logical_rings_init;
dev_priv->gt.cleanup_ring = intel_logical_ring_cleanup;
dev_priv->gt.stop_ring = intel_logical_ring_stop;
+ dev_priv->gt.start_ring = intel_logical_ring_start;
}
/* This is just a security blanket to placate dragons.
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fcb074b..b2b337b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -756,6 +756,8 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring)
if (!intel_ring_initialized(ring))
return;
+ intel_ring_stop(ring);
+
ret = intel_ring_idle(ring);
if (ret && !i915_reset_in_progress(&to_i915(ring->dev)->gpu_error))
DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
@@ -811,6 +813,22 @@ intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
execlists_context_queue(ring, ctx, ringbuf->tail, request);
}
+void intel_logical_ring_start(struct intel_engine_cs *ring)
+{
+ struct intel_context *ctx = ring->default_context;
+ struct intel_ringbuffer *ringbuf;
+
+ if (WARN_ON(!ctx))
+ return;
+
+ ringbuf = ctx->engine[ring->id].ringbuf;
+ if (WARN_ON(!ringbuf))
+ return;
+
+ intel_ring_start(ring);
+ intel_logical_ring_advance_and_submit(ringbuf, ctx, NULL);
+}
+
static int intel_lr_context_pin(struct intel_engine_cs *ring,
struct intel_context *ctx)
{
@@ -1121,6 +1139,8 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
ring->next_context_status_buffer = 0;
DRM_DEBUG_DRIVER("Execlists enabled for %s\n", ring->name);
+ intel_ring_stop(ring);
+
memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
return 0;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index adb731e4..cb8ad0a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -37,6 +37,7 @@
/* Logical Rings */
void intel_logical_ring_stop(struct intel_engine_cs *ring);
+void intel_logical_ring_start(struct intel_engine_cs *ring);
void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
int intel_logical_rings_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 441e250..90d74ec 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -81,6 +81,22 @@ bool intel_ring_stopped(struct intel_engine_cs *ring)
return dev_priv->gpu_error.stop_rings & intel_ring_flag(ring);
}
+void intel_ring_stop(struct intel_engine_cs *ring)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+ dev_priv->gpu_error.stop_rings |= intel_ring_flag(ring);
+}
+
+void intel_ring_start(struct intel_engine_cs *ring)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+ WARN_ON(!intel_ring_stopped(ring));
+
+ dev_priv->gpu_error.stop_rings &= ~intel_ring_flag(ring);
+}
+
void __intel_ring_advance(struct intel_engine_cs *ring)
{
struct intel_ringbuffer *ringbuf = ring->buffer;
@@ -531,14 +547,15 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *ring)
}
}
-static bool stop_ring(struct intel_engine_cs *ring)
+static bool stop_ring_hw(struct intel_engine_cs *ring)
{
struct drm_i915_private *dev_priv = to_i915(ring->dev);
if (!IS_GEN2(ring->dev)) {
I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING));
if (wait_for((I915_READ_MODE(ring) & MODE_IDLE) != 0, 1000)) {
- DRM_ERROR("%s : timed out trying to stop ring\n", ring->name);
+ if (dev_priv->gpu_error.stop_rings == 0)
+ DRM_ERROR("%s : timed out trying to stop ring\n", ring->name);
/* Sometimes we observe that the idle flag is not
* set even though the ring is empty. So double
* check before giving up.
@@ -570,7 +587,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
- if (!stop_ring(ring)) {
+ if (!stop_ring_hw(ring)) {
/* G45 ring initialization often fails to reset head to zero */
DRM_DEBUG_KMS("%s head not reset to zero "
"ctl %08x head %08x tail %08x start %08x\n",
@@ -580,7 +597,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
I915_READ_TAIL(ring),
I915_READ_START(ring));
- if (!stop_ring(ring)) {
+ if (!stop_ring_hw(ring)) {
DRM_ERROR("failed to set %s head to zero "
"ctl %08x head %08x tail %08x start %08x\n",
ring->name,
@@ -593,6 +610,8 @@ static int init_ring_common(struct intel_engine_cs *ring)
}
}
+ intel_ring_stop(ring);
+
if (I915_NEED_GFX_HWS(dev))
intel_ring_setup_status_page(ring);
else
@@ -611,8 +630,11 @@ static int init_ring_common(struct intel_engine_cs *ring)
if (I915_READ_HEAD(ring))
DRM_DEBUG("%s initialization failed [head=%08x], fudging\n",
ring->name, I915_READ_HEAD(ring));
- I915_WRITE_HEAD(ring, 0);
- (void)I915_READ_HEAD(ring);
+
+ intel_ring_update_space(ringbuf);
+
+ I915_WRITE_HEAD(ring, ringbuf->head);
+ ring->write_tail(ring, ringbuf->head);
I915_WRITE_CTL(ring,
((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES)
@@ -621,7 +643,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
/* If the head is still not zero, the ring is dead */
if (wait_for((I915_READ_CTL(ring) & RING_VALID) != 0 &&
I915_READ_START(ring) == i915_gem_obj_ggtt_offset(obj) &&
- (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) {
+ (I915_READ_HEAD(ring) & HEAD_ADDR) == ringbuf->head, 50)) {
DRM_ERROR("%s initialization failed "
"ctl %08x (valid? %d) head %08x tail %08x start %08x [expected %08lx]\n",
ring->name,
@@ -632,13 +654,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
goto out;
}
- ringbuf->last_retired_head = -1;
- ringbuf->head = I915_READ_HEAD(ring);
- ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR;
- intel_ring_update_space(ringbuf);
-
memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
-
out:
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
@@ -2898,5 +2914,12 @@ intel_stop_ring_buffer(struct intel_engine_cs *ring)
DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
ring->name, ret);
- stop_ring(ring);
+ intel_ring_stop(ring);
+ stop_ring_hw(ring);
+}
+
+void intel_start_ring_buffer(struct intel_engine_cs *ring)
+{
+ intel_ring_start(ring);
+ __intel_ring_advance(ring);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c761fe0..a22f2f3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -387,6 +387,7 @@ void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf);
int intel_alloc_ringbuffer_obj(struct drm_device *dev,
struct intel_ringbuffer *ringbuf);
+void intel_start_ring_buffer(struct intel_engine_cs *ring);
void intel_stop_ring_buffer(struct intel_engine_cs *ring);
void intel_cleanup_ring_buffer(struct intel_engine_cs *ring);
@@ -408,6 +409,8 @@ int __intel_ring_space(int head, int tail, int size);
void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
int intel_ring_space(struct intel_ringbuffer *ringbuf);
bool intel_ring_stopped(struct intel_engine_cs *ring);
+void intel_ring_start(struct intel_engine_cs *ring);
+void intel_ring_stop(struct intel_engine_cs *ring);
void __intel_ring_advance(struct intel_engine_cs *ring);
int __must_check intel_ring_idle(struct intel_engine_cs *ring);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
2015-03-17 16:18 [PATCH 0/4] ppgtt: ring initialization fixes Mika Kuoppala
2015-03-17 16:18 ` [PATCH 1/4] drm/i915: Detect page faults during hangcheck Mika Kuoppala
2015-03-17 16:18 ` [PATCH 2/4] drm/i915: Introduce ring->start_ring() Mika Kuoppala
@ 2015-03-17 16:18 ` Mika Kuoppala
2015-03-17 16:18 ` [PATCH 4/4] drm/i915: Stop rings before cleaning up on reset Mika Kuoppala
3 siblings, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2015-03-17 16:18 UTC (permalink / raw)
To: intel-gfx; +Cc: miku
From: Chris Wilson <chris@chris-wilson.co.uk>
Currently we initialise the rings, add the first context switch to the
ring and execute our golden state then enable (aliasing or full) ppgtt.
However, as we enable ppgtt using direct MMIO but load the PD using
MI_LRI, we end up executing the context switch and golden render state
with an invalid PD generating page faults. To solve this issue, first do
the ppgtt PD setup, then set the default context and write the commands
to run the render state into the ring, before we activate the ring. This
allows us to be sure that the register state is valid before we begin
execution.
This was spotted when writing the seqno/request conversion, but only with
the ERROR capture did I realise that it was a necessity now.
RFC: cleanup the error handling in i915_gem_init_hw.
v2: added intel_ring_reset
v3: adapt to ring->start_ring
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8147e2e..5636351 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4856,11 +4856,25 @@ cleanup_render_ring:
return ret;
}
+static int i915_gem_init_ring_hw(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *ring;
+ int i, ret;
+
+ for_each_ring(ring, dev_priv, i) {
+ ret = ring->init_hw(ring);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int
i915_gem_init_hw(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_engine_cs *ring;
int ret, i;
if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
@@ -4898,13 +4912,7 @@ i915_gem_init_hw(struct drm_device *dev)
*/
init_unused_rings(dev);
- for_each_ring(ring, dev_priv, i) {
- ret = ring->init_hw(ring);
- if (ret)
- goto out;
- }
-
- i915_gem_start_ringbuffers(dev);
+ i915_gem_init_ring_hw(dev);
for (i = 0; i < NUM_L3_SLICES(dev); i++)
i915_gem_l3_remap(&dev_priv->ring[RCS], i);
@@ -4915,6 +4923,9 @@ i915_gem_init_hw(struct drm_device *dev)
i915_gem_cleanup_ringbuffer(dev);
}
+ for (i = 0; i < NUM_L3_SLICES(dev); i++)
+ i915_gem_l3_remap(&dev_priv->ring[RCS], i);
+
ret = i915_gem_context_enable(dev_priv);
if (ret && ret != -EIO) {
DRM_ERROR("Context enable failed %d\n", ret);
@@ -4923,6 +4934,8 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}
+ i915_gem_start_ringbuffers(dev);
+
out:
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
return ret;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread