* [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone
@ 2017-10-09 9:32 Chris Wilson
2017-10-09 9:32 ` [PATCH 2/4] drm/i915/selftests: Pretty print engine state when requests fail to start Chris Wilson
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 9:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
We can use drm_printer to hide the differences between printk and
seq_printf, and so make the i915_engine_info pretty printer able to be
called from different contexts and not just debugfs. For instance, I
want to use the pretty printer to debug kselftests.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 148 +----------------------------
drivers/gpu/drm/i915/intel_engine_cs.c | 160 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +
3 files changed, 168 insertions(+), 144 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f7817c667958..9ec2bcd9a695 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3292,9 +3292,9 @@ static int i915_display_info(struct seq_file *m, void *unused)
static int i915_engine_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
- struct i915_gpu_error *error = &dev_priv->gpu_error;
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ struct drm_printer p;
intel_runtime_pm_get(dev_priv);
@@ -3303,149 +3303,9 @@ static int i915_engine_info(struct seq_file *m, void *unused)
seq_printf(m, "Global active requests: %d\n",
dev_priv->gt.active_requests);
- for_each_engine(engine, dev_priv, id) {
- struct intel_breadcrumbs *b = &engine->breadcrumbs;
- struct drm_i915_gem_request *rq;
- struct rb_node *rb;
- u64 addr;
-
- seq_printf(m, "%s\n", engine->name);
- seq_printf(m, "\tcurrent seqno %x, last %x, hangcheck %x [%d ms], inflight %d\n",
- intel_engine_get_seqno(engine),
- intel_engine_last_submit(engine),
- engine->hangcheck.seqno,
- jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp),
- engine->timeline->inflight_seqnos);
- seq_printf(m, "\tReset count: %d\n",
- i915_reset_engine_count(error, engine));
-
- rcu_read_lock();
-
- seq_printf(m, "\tRequests:\n");
-
- rq = list_first_entry(&engine->timeline->requests,
- struct drm_i915_gem_request, link);
- if (&rq->link != &engine->timeline->requests)
- print_request(m, rq, "\t\tfirst ");
-
- rq = list_last_entry(&engine->timeline->requests,
- struct drm_i915_gem_request, link);
- if (&rq->link != &engine->timeline->requests)
- print_request(m, rq, "\t\tlast ");
-
- rq = i915_gem_find_active_request(engine);
- if (rq) {
- print_request(m, rq, "\t\tactive ");
- seq_printf(m,
- "\t\t[head %04x, postfix %04x, tail %04x, batch 0x%08x_%08x]\n",
- rq->head, rq->postfix, rq->tail,
- rq->batch ? upper_32_bits(rq->batch->node.start) : ~0u,
- rq->batch ? lower_32_bits(rq->batch->node.start) : ~0u);
- }
-
- seq_printf(m, "\tRING_START: 0x%08x [0x%08x]\n",
- I915_READ(RING_START(engine->mmio_base)),
- rq ? i915_ggtt_offset(rq->ring->vma) : 0);
- seq_printf(m, "\tRING_HEAD: 0x%08x [0x%08x]\n",
- I915_READ(RING_HEAD(engine->mmio_base)) & HEAD_ADDR,
- rq ? rq->ring->head : 0);
- seq_printf(m, "\tRING_TAIL: 0x%08x [0x%08x]\n",
- I915_READ(RING_TAIL(engine->mmio_base)) & TAIL_ADDR,
- rq ? rq->ring->tail : 0);
- seq_printf(m, "\tRING_CTL: 0x%08x [%s]\n",
- I915_READ(RING_CTL(engine->mmio_base)),
- I915_READ(RING_CTL(engine->mmio_base)) & (RING_WAIT | RING_WAIT_SEMAPHORE) ? "waiting" : "");
-
- rcu_read_unlock();
-
- addr = intel_engine_get_active_head(engine);
- seq_printf(m, "\tACTHD: 0x%08x_%08x\n",
- upper_32_bits(addr), lower_32_bits(addr));
- addr = intel_engine_get_last_batch_head(engine);
- seq_printf(m, "\tBBADDR: 0x%08x_%08x\n",
- upper_32_bits(addr), lower_32_bits(addr));
-
- if (i915_modparams.enable_execlists) {
- const u32 *hws = &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
- struct intel_engine_execlists * const execlists = &engine->execlists;
- u32 ptr, read, write;
- unsigned int idx;
-
- seq_printf(m, "\tExeclist status: 0x%08x %08x\n",
- I915_READ(RING_EXECLIST_STATUS_LO(engine)),
- I915_READ(RING_EXECLIST_STATUS_HI(engine)));
-
- ptr = I915_READ(RING_CONTEXT_STATUS_PTR(engine));
- read = GEN8_CSB_READ_PTR(ptr);
- write = GEN8_CSB_WRITE_PTR(ptr);
- seq_printf(m, "\tExeclist CSB read %d [%d cached], write %d [%d from hws], interrupt posted? %s\n",
- read, execlists->csb_head,
- write,
- intel_read_status_page(engine, intel_hws_csb_write_index(engine->i915)),
- yesno(test_bit(ENGINE_IRQ_EXECLIST,
- &engine->irq_posted)));
- if (read >= GEN8_CSB_ENTRIES)
- read = 0;
- if (write >= GEN8_CSB_ENTRIES)
- write = 0;
- if (read > write)
- write += GEN8_CSB_ENTRIES;
- while (read < write) {
- idx = ++read % GEN8_CSB_ENTRIES;
- seq_printf(m, "\tExeclist CSB[%d]: 0x%08x [0x%08x in hwsp], context: %d [%d in hwsp]\n",
- idx,
- I915_READ(RING_CONTEXT_STATUS_BUF_LO(engine, idx)),
- hws[idx * 2],
- I915_READ(RING_CONTEXT_STATUS_BUF_HI(engine, idx)),
- hws[idx * 2 + 1]);
- }
-
- rcu_read_lock();
- for (idx = 0; idx < execlists_num_ports(execlists); idx++) {
- unsigned int count;
-
- rq = port_unpack(&execlists->port[idx], &count);
- if (rq) {
- seq_printf(m, "\t\tELSP[%d] count=%d, ",
- idx, count);
- print_request(m, rq, "rq: ");
- } else {
- seq_printf(m, "\t\tELSP[%d] idle\n",
- idx);
- }
- }
- rcu_read_unlock();
-
- spin_lock_irq(&engine->timeline->lock);
- for (rb = execlists->first; rb; rb = rb_next(rb)) {
- struct i915_priolist *p =
- rb_entry(rb, typeof(*p), node);
-
- list_for_each_entry(rq, &p->requests,
- priotree.link)
- print_request(m, rq, "\t\tQ ");
- }
- spin_unlock_irq(&engine->timeline->lock);
- } else if (INTEL_GEN(dev_priv) > 6) {
- seq_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
- I915_READ(RING_PP_DIR_BASE(engine)));
- seq_printf(m, "\tPP_DIR_BASE_READ: 0x%08x\n",
- I915_READ(RING_PP_DIR_BASE_READ(engine)));
- seq_printf(m, "\tPP_DIR_DCLV: 0x%08x\n",
- I915_READ(RING_PP_DIR_DCLV(engine)));
- }
-
- spin_lock_irq(&b->rb_lock);
- for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
- struct intel_wait *w = rb_entry(rb, typeof(*w), node);
-
- seq_printf(m, "\t%s [%d] waiting for %x\n",
- w->tsk->comm, w->tsk->pid, w->seqno);
- }
- spin_unlock_irq(&b->rb_lock);
-
- seq_puts(m, "\n");
- }
+ p = drm_seq_file_printer(m);
+ for_each_engine(engine, dev_priv, id)
+ intel_engine_dump(engine, &p);
intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 807a7aafc089..a59b2a30ff5a 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -22,6 +22,8 @@
*
*/
+#include <drm/drm_print.h>
+
#include "i915_drv.h"
#include "intel_ringbuffer.h"
#include "intel_lrc.h"
@@ -1616,6 +1618,164 @@ bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
}
}
+static void print_request(struct drm_printer *m,
+ struct drm_i915_gem_request *rq,
+ const char *prefix)
+{
+ drm_printf(m, "%s%x [%x:%x] prio=%d @ %dms: %s\n", prefix,
+ rq->global_seqno, rq->ctx->hw_id, rq->fence.seqno,
+ rq->priotree.priority,
+ jiffies_to_msecs(jiffies - rq->emitted_jiffies),
+ rq->timeline->common->name);
+}
+
+void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m)
+{
+ struct intel_breadcrumbs *b = &engine->breadcrumbs;
+ struct i915_gpu_error *error = &engine->i915->gpu_error;
+ struct drm_i915_private *dev_priv = engine->i915;
+ struct drm_i915_gem_request *rq;
+ struct rb_node *rb;
+ u64 addr;
+
+ drm_printf(m, "%s\n", engine->name);
+ drm_printf(m, "\tcurrent seqno %x, last %x, hangcheck %x [%d ms], inflight %d\n",
+ intel_engine_get_seqno(engine),
+ intel_engine_last_submit(engine),
+ engine->hangcheck.seqno,
+ jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp),
+ engine->timeline->inflight_seqnos);
+ drm_printf(m, "\tReset count: %d\n",
+ i915_reset_engine_count(error, engine));
+
+ rcu_read_lock();
+
+ drm_printf(m, "\tRequests:\n");
+
+ rq = list_first_entry(&engine->timeline->requests,
+ struct drm_i915_gem_request, link);
+ if (&rq->link != &engine->timeline->requests)
+ print_request(m, rq, "\t\tfirst ");
+
+ rq = list_last_entry(&engine->timeline->requests,
+ struct drm_i915_gem_request, link);
+ if (&rq->link != &engine->timeline->requests)
+ print_request(m, rq, "\t\tlast ");
+
+ rq = i915_gem_find_active_request(engine);
+ if (rq) {
+ print_request(m, rq, "\t\tactive ");
+ drm_printf(m,
+ "\t\t[head %04x, postfix %04x, tail %04x, batch 0x%08x_%08x]\n",
+ rq->head, rq->postfix, rq->tail,
+ rq->batch ? upper_32_bits(rq->batch->node.start) : ~0u,
+ rq->batch ? lower_32_bits(rq->batch->node.start) : ~0u);
+ }
+
+ drm_printf(m, "\tRING_START: 0x%08x [0x%08x]\n",
+ I915_READ(RING_START(engine->mmio_base)),
+ rq ? i915_ggtt_offset(rq->ring->vma) : 0);
+ drm_printf(m, "\tRING_HEAD: 0x%08x [0x%08x]\n",
+ I915_READ(RING_HEAD(engine->mmio_base)) & HEAD_ADDR,
+ rq ? rq->ring->head : 0);
+ drm_printf(m, "\tRING_TAIL: 0x%08x [0x%08x]\n",
+ I915_READ(RING_TAIL(engine->mmio_base)) & TAIL_ADDR,
+ rq ? rq->ring->tail : 0);
+ drm_printf(m, "\tRING_CTL: 0x%08x [%s]\n",
+ I915_READ(RING_CTL(engine->mmio_base)),
+ I915_READ(RING_CTL(engine->mmio_base)) & (RING_WAIT | RING_WAIT_SEMAPHORE) ? "waiting" : "");
+
+ rcu_read_unlock();
+
+ addr = intel_engine_get_active_head(engine);
+ drm_printf(m, "\tACTHD: 0x%08x_%08x\n",
+ upper_32_bits(addr), lower_32_bits(addr));
+ addr = intel_engine_get_last_batch_head(engine);
+ drm_printf(m, "\tBBADDR: 0x%08x_%08x\n",
+ upper_32_bits(addr), lower_32_bits(addr));
+
+ if (i915_modparams.enable_execlists) {
+ const u32 *hws = &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
+ struct intel_engine_execlists * const execlists = &engine->execlists;
+ u32 ptr, read, write;
+ unsigned int idx;
+
+ drm_printf(m, "\tExeclist status: 0x%08x %08x\n",
+ I915_READ(RING_EXECLIST_STATUS_LO(engine)),
+ I915_READ(RING_EXECLIST_STATUS_HI(engine)));
+
+ ptr = I915_READ(RING_CONTEXT_STATUS_PTR(engine));
+ read = GEN8_CSB_READ_PTR(ptr);
+ write = GEN8_CSB_WRITE_PTR(ptr);
+ drm_printf(m, "\tExeclist CSB read %d [%d cached], write %d [%d from hws], interrupt posted? %s\n",
+ read, execlists->csb_head,
+ write,
+ intel_read_status_page(engine, intel_hws_csb_write_index(engine->i915)),
+ yesno(test_bit(ENGINE_IRQ_EXECLIST,
+ &engine->irq_posted)));
+ if (read >= GEN8_CSB_ENTRIES)
+ read = 0;
+ if (write >= GEN8_CSB_ENTRIES)
+ write = 0;
+ if (read > write)
+ write += GEN8_CSB_ENTRIES;
+ while (read < write) {
+ idx = ++read % GEN8_CSB_ENTRIES;
+ drm_printf(m, "\tExeclist CSB[%d]: 0x%08x [0x%08x in hwsp], context: %d [%d in hwsp]\n",
+ idx,
+ I915_READ(RING_CONTEXT_STATUS_BUF_LO(engine, idx)),
+ hws[idx * 2],
+ I915_READ(RING_CONTEXT_STATUS_BUF_HI(engine, idx)),
+ hws[idx * 2 + 1]);
+ }
+
+ rcu_read_lock();
+ for (idx = 0; idx < execlists_num_ports(execlists); idx++) {
+ unsigned int count;
+
+ rq = port_unpack(&execlists->port[idx], &count);
+ if (rq) {
+ drm_printf(m, "\t\tELSP[%d] count=%d, ",
+ idx, count);
+ print_request(m, rq, "rq: ");
+ } else {
+ drm_printf(m, "\t\tELSP[%d] idle\n",
+ idx);
+ }
+ }
+ rcu_read_unlock();
+
+ spin_lock_irq(&engine->timeline->lock);
+ for (rb = execlists->first; rb; rb = rb_next(rb)) {
+ struct i915_priolist *p =
+ rb_entry(rb, typeof(*p), node);
+
+ list_for_each_entry(rq, &p->requests,
+ priotree.link)
+ print_request(m, rq, "\t\tQ ");
+ }
+ spin_unlock_irq(&engine->timeline->lock);
+ } else if (INTEL_GEN(dev_priv) > 6) {
+ drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
+ I915_READ(RING_PP_DIR_BASE(engine)));
+ drm_printf(m, "\tPP_DIR_BASE_READ: 0x%08x\n",
+ I915_READ(RING_PP_DIR_BASE_READ(engine)));
+ drm_printf(m, "\tPP_DIR_DCLV: 0x%08x\n",
+ I915_READ(RING_PP_DIR_DCLV(engine)));
+ }
+
+ spin_lock_irq(&b->rb_lock);
+ for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
+ struct intel_wait *w = rb_entry(rb, typeof(*w), node);
+
+ drm_printf(m, "\t%s [%d] waiting for %x\n",
+ w->tsk->comm, w->tsk->pid, w->seqno);
+ }
+ spin_unlock_irq(&b->rb_lock);
+
+ drm_printf(m, "\n");
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftests/mock_engine.c"
#endif
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0fedda17488c..17186f067408 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -7,6 +7,8 @@
#include "i915_gem_timeline.h"
#include "i915_selftest.h"
+struct drm_printer;
+
#define I915_CMD_HASH_ORDER 9
/* Early gen2 devices have a cacheline of just 32 bytes, using 64 is overkill,
@@ -839,4 +841,6 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915);
bool intel_engine_can_store_dword(struct intel_engine_cs *engine);
+void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *p);
+
#endif /* _INTEL_RINGBUFFER_H_ */
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] drm/i915/selftests: Pretty print engine state when requests fail to start
2017-10-09 9:32 [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone Chris Wilson
@ 2017-10-09 9:32 ` Chris Wilson
2017-10-09 9:32 ` [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests Chris Wilson
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 9:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
During hangcheck testing, we try to execute requests following the GPU
reset, and in particular want to try and debug when those fail.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index 08159b268893..7e1bdd88eda3 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -624,8 +624,11 @@ static int igt_wait_reset(void *arg)
__i915_add_request(rq, true);
if (!wait_for_hang(&h, rq)) {
+ struct drm_printer p = drm_info_printer(i915->drm.dev);
+
pr_err("Failed to start request %x, at %x\n",
rq->fence.seqno, hws_seqno(&h, rq));
+ intel_engine_dump(rq->engine, &p);
i915_reset(i915, 0);
i915_gem_set_wedged(i915);
@@ -716,8 +719,12 @@ static int igt_reset_queue(void *arg)
__i915_add_request(rq, true);
if (!wait_for_hang(&h, prev)) {
+ struct drm_printer p = drm_info_printer(i915->drm.dev);
+
pr_err("Failed to start request %x, at %x\n",
prev->fence.seqno, hws_seqno(&h, prev));
+ intel_engine_dump(rq->engine, &p);
+
i915_gem_request_put(rq);
i915_gem_request_put(prev);
@@ -818,8 +825,11 @@ static int igt_handle_error(void *arg)
__i915_add_request(rq, true);
if (!wait_for_hang(&h, rq)) {
+ struct drm_printer p = drm_info_printer(i915->drm.dev);
+
pr_err("Failed to start request %x, at %x\n",
rq->fence.seqno, hws_seqno(&h, rq));
+ intel_engine_dump(rq->engine, &p);
i915_reset(i915, 0);
i915_gem_set_wedged(i915);
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests
2017-10-09 9:32 [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone Chris Wilson
2017-10-09 9:32 ` [PATCH 2/4] drm/i915/selftests: Pretty print engine state when requests fail to start Chris Wilson
@ 2017-10-09 9:32 ` Chris Wilson
2017-10-09 10:14 ` Mika Kuoppala
2017-10-09 9:32 ` [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held Chris Wilson
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 9:32 UTC (permalink / raw)
To: intel-gfx
Resetting the engine requires us to hold the forcewake wakeref to
prevent RC6 trying to happen in the middle of the reset sequence.
Normally, this is taken by i915_handle_error(), but as we are calling
the lowlevel functions ourselves, we need to hold it. Wrap the entire
live_hangcheck set of subtests in a single forcewake section for
simplicity.
This greatly improves the reliability of drv_selftest/live_hangcheck on
Haswell, where it would exhibit an inability to restart a request
because it lost its PD registers (PD_DIR_BASE reported as 0).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index 7e1bdd88eda3..a9e0decccc1f 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -878,9 +878,18 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_reset_queue),
SUBTEST(igt_handle_error),
};
+ int err;
if (!intel_has_gpu_reset(i915))
return 0;
- return i915_subtests(tests, i915);
+ intel_runtime_pm_get(i915);
+ intel_uncore_forcewake_get(i915, FORCEWAKE_ALL);
+
+ err = i915_subtests(tests, i915);
+
+ intel_uncore_forcewake_put(i915, FORCEWAKE_ALL);
+ intel_runtime_pm_put(i915);
+
+ return err;
}
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held
2017-10-09 9:32 [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone Chris Wilson
2017-10-09 9:32 ` [PATCH 2/4] drm/i915/selftests: Pretty print engine state when requests fail to start Chris Wilson
2017-10-09 9:32 ` [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests Chris Wilson
@ 2017-10-09 9:32 ` Chris Wilson
2017-10-09 10:05 ` Mika Kuoppala
2017-10-09 10:24 ` [PATCH v2] " Chris Wilson
2017-10-09 10:22 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone Patchwork
2017-10-09 12:01 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone (rev2) Patchwork
4 siblings, 2 replies; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 9:32 UTC (permalink / raw)
To: intel-gfx
Add assert_forcewakes_active() (the complementary function to
assert_forcewakes_inactive) that documents the requirement of a
function for its callers to be holding the forcewake ref (i.e. the
function is part of a sequence over which RC6 must be prevented).
One such example is during ringbuffer reset, where RC6 must be held
across the whole reinitialisation sequence.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++++++++-
drivers/gpu/drm/i915/intel_uncore.c | 12 ++++++++++++
drivers/gpu/drm/i915/intel_uncore.h | 2 ++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05c08b0bc172..4285f09ff8b8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -579,7 +579,16 @@ static int init_ring_common(struct intel_engine_cs *engine)
static void reset_ring_common(struct intel_engine_cs *engine,
struct drm_i915_gem_request *request)
{
- /* Try to restore the logical GPU state to match the continuation
+ /*
+ * RC6 must be prevented until the reset is complete and the engine
+ * reinitialised. If it occurs in the middle of this sequence, the
+ * state written to/loaded from the power context is ill-defined (e.g.
+ * the PP_BASE_DIR may be lost).
+ */
+ assert_forcewakes_active(engine->i915, FORCEWAKE_ALL);
+
+ /*
+ * Try to restore the logical GPU state to match the continuation
* of the request queue. If we skip the context/PD restore, then
* the next request may try to execute assuming that its context
* is valid and loaded on the GPU and so may try to access invalid
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b3c3f94fc7e4..3d41667919dc 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -629,6 +629,18 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
WARN_ON(dev_priv->uncore.fw_domains_active);
}
+void assert_forcewakes_active(struct drm_i915_private *dev_priv,
+ enum forcewake_domains fw_domains)
+{
+ if (!dev_priv->uncore.funcs.force_wake_get)
+ return;
+
+ assert_rpm_wakelock_held(dev_priv);
+
+ fw_domains &= dev_priv->uncore.fw_domains;
+ WARN_ON(fw_domains & ~dev_priv->uncore.fw_domains_active);
+}
+
/* We give fast paths for the really cool registers */
#define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000)
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 66eae2ce2f29..582771251b57 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -137,6 +137,8 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv);
u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
+void assert_forcewakes_active(struct drm_i915_private *dev_priv,
+ enum forcewake_domains fw_domains);
const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
enum forcewake_domains
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held
2017-10-09 9:32 ` [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held Chris Wilson
@ 2017-10-09 10:05 ` Mika Kuoppala
2017-10-09 10:14 ` Chris Wilson
2017-10-09 10:24 ` [PATCH v2] " Chris Wilson
1 sibling, 1 reply; 11+ messages in thread
From: Mika Kuoppala @ 2017-10-09 10:05 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Add assert_forcewakes_active() (the complementary function to
> assert_forcewakes_inactive) that documents the requirement of a
> function for its callers to be holding the forcewake ref (i.e. the
> function is part of a sequence over which RC6 must be prevented).
>
> One such example is during ringbuffer reset, where RC6 must be held
> across the whole reinitialisation sequence.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++++++++-
> drivers/gpu/drm/i915/intel_uncore.c | 12 ++++++++++++
> drivers/gpu/drm/i915/intel_uncore.h | 2 ++
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 05c08b0bc172..4285f09ff8b8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -579,7 +579,16 @@ static int init_ring_common(struct intel_engine_cs *engine)
> static void reset_ring_common(struct intel_engine_cs *engine,
> struct drm_i915_gem_request *request)
> {
> - /* Try to restore the logical GPU state to match the continuation
> + /*
> + * RC6 must be prevented until the reset is complete and the engine
> + * reinitialised. If it occurs in the middle of this sequence, the
> + * state written to/loaded from the power context is ill-defined (e.g.
> + * the PP_BASE_DIR may be lost).
> + */
> + assert_forcewakes_active(engine->i915, FORCEWAKE_ALL);
> +
> + /*
> + * Try to restore the logical GPU state to match the continuation
> * of the request queue. If we skip the context/PD restore, then
> * the next request may try to execute assuming that its context
> * is valid and loaded on the GPU and so may try to access invalid
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index b3c3f94fc7e4..3d41667919dc 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -629,6 +629,18 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
> WARN_ON(dev_priv->uncore.fw_domains_active);
> }
>
> +void assert_forcewakes_active(struct drm_i915_private *dev_priv,
> + enum forcewake_domains fw_domains)
> +{
> + if (!dev_priv->uncore.funcs.force_wake_get)
> + return;
> +
> + assert_rpm_wakelock_held(dev_priv);
> +
> + fw_domains &= dev_priv->uncore.fw_domains;
> + WARN_ON(fw_domains & ~dev_priv->uncore.fw_domains_active);
> +}
> +
Adding a debug telling about current domains seems not
to add any value...atleast yet.
I remember yearning after this in the past.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> /* We give fast paths for the really cool registers */
> #define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 66eae2ce2f29..582771251b57 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -137,6 +137,8 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv);
>
> u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
> void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> +void assert_forcewakes_active(struct drm_i915_private *dev_priv,
> + enum forcewake_domains fw_domains);
> const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
>
> enum forcewake_domains
> --
> 2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held
2017-10-09 10:05 ` Mika Kuoppala
@ 2017-10-09 10:14 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 10:14 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2017-10-09 11:05:40)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Add assert_forcewakes_active() (the complementary function to
> > assert_forcewakes_inactive) that documents the requirement of a
> > function for its callers to be holding the forcewake ref (i.e. the
> > function is part of a sequence over which RC6 must be prevented).
> >
> > One such example is during ringbuffer reset, where RC6 must be held
> > across the whole reinitialisation sequence.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++++++++-
> > drivers/gpu/drm/i915/intel_uncore.c | 12 ++++++++++++
> > drivers/gpu/drm/i915/intel_uncore.h | 2 ++
> > 3 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 05c08b0bc172..4285f09ff8b8 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -579,7 +579,16 @@ static int init_ring_common(struct intel_engine_cs *engine)
> > static void reset_ring_common(struct intel_engine_cs *engine,
> > struct drm_i915_gem_request *request)
> > {
> > - /* Try to restore the logical GPU state to match the continuation
> > + /*
> > + * RC6 must be prevented until the reset is complete and the engine
> > + * reinitialised. If it occurs in the middle of this sequence, the
> > + * state written to/loaded from the power context is ill-defined (e.g.
> > + * the PP_BASE_DIR may be lost).
> > + */
> > + assert_forcewakes_active(engine->i915, FORCEWAKE_ALL);
> > +
> > + /*
> > + * Try to restore the logical GPU state to match the continuation
> > * of the request queue. If we skip the context/PD restore, then
> > * the next request may try to execute assuming that its context
> > * is valid and loaded on the GPU and so may try to access invalid
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index b3c3f94fc7e4..3d41667919dc 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -629,6 +629,18 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
> > WARN_ON(dev_priv->uncore.fw_domains_active);
> > }
> >
> > +void assert_forcewakes_active(struct drm_i915_private *dev_priv,
> > + enum forcewake_domains fw_domains)
> > +{
> > + if (!dev_priv->uncore.funcs.force_wake_get)
> > + return;
> > +
> > + assert_rpm_wakelock_held(dev_priv);
> > +
> > + fw_domains &= dev_priv->uncore.fw_domains;
> > + WARN_ON(fw_domains & ~dev_priv->uncore.fw_domains_active);
> > +}
> > +
>
> Adding a debug telling about current domains seems not
> to add any value...atleast yet.
But will if it ever fires, so yes I had better add it.
s/WARN_ON/WARN(".../
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests
2017-10-09 9:32 ` [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests Chris Wilson
@ 2017-10-09 10:14 ` Mika Kuoppala
2017-10-09 10:28 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Mika Kuoppala @ 2017-10-09 10:14 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Resetting the engine requires us to hold the forcewake wakeref to
> prevent RC6 trying to happen in the middle of the reset sequence.
> Normally, this is taken by i915_handle_error(), but as we are calling
There is no explicit forcewake grab on i915_handle_error().
What I am missing in here?
-Mika
> the lowlevel functions ourselves, we need to hold it. Wrap the entire
> live_hangcheck set of subtests in a single forcewake section for
> simplicity.
>
> This greatly improves the reliability of drv_selftest/live_hangcheck on
> Haswell, where it would exhibit an inability to restart a request
> because it lost its PD registers (PD_DIR_BASE reported as 0).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> index 7e1bdd88eda3..a9e0decccc1f 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> @@ -878,9 +878,18 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
> SUBTEST(igt_reset_queue),
> SUBTEST(igt_handle_error),
> };
> + int err;
>
> if (!intel_has_gpu_reset(i915))
> return 0;
>
> - return i915_subtests(tests, i915);
> + intel_runtime_pm_get(i915);
> + intel_uncore_forcewake_get(i915, FORCEWAKE_ALL);
> +
> + err = i915_subtests(tests, i915);
> +
> + intel_uncore_forcewake_put(i915, FORCEWAKE_ALL);
> + intel_runtime_pm_put(i915);
> +
> + return err;
> }
> --
> 2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone
2017-10-09 9:32 [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone Chris Wilson
` (2 preceding siblings ...)
2017-10-09 9:32 ` [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held Chris Wilson
@ 2017-10-09 10:22 ` Patchwork
2017-10-09 12:01 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone (rev2) Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-10-09 10:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone
URL : https://patchwork.freedesktop.org/series/31581/
State : success
== Summary ==
Series 31581v1 series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone
https://patchwork.freedesktop.org/api/1.0/series/31581/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS (fi-cfl-s) fdo#103026
Test kms_frontbuffer_tracking:
Subgroup basic:
dmesg-warn -> PASS (fi-bdw-5557u) fdo#102473
fdo#103026 https://bugs.freedesktop.org/show_bug.cgi?id=103026
fdo#102473 https://bugs.freedesktop.org/show_bug.cgi?id=102473
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:458s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:475s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:394s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:561s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:284s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:527s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:522s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:535s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:521s
fi-cfl-s total:289 pass:257 dwarn:0 dfail:0 fail:0 skip:32 time:575s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:619s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:434s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:593s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:434s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:417s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:516s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:474s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:506s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:579s
fi-kbl-7567u total:289 pass:265 dwarn:4 dfail:0 fail:0 skip:20 time:491s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:591s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:654s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:482s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:655s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:531s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:522s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:475s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:590s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:429s
1d09197a9f9140f4795ed109018602c9e807d957 drm-tip: 2017y-10m-09d-07h-50m-48s UTC integration manifest
d74035e172f0 drm/i915: Provide an assert for when we expect forcewake to be held
24f01eb50454 drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests
3a5353da4bd1 drm/i915/selftests: Pretty print engine state when requests fail to start
c7f4aff8aa19 drm/i915: Make i915_engine_info pretty printer to standalone
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5946/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] drm/i915: Provide an assert for when we expect forcewake to be held
2017-10-09 9:32 ` [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held Chris Wilson
2017-10-09 10:05 ` Mika Kuoppala
@ 2017-10-09 10:24 ` Chris Wilson
1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 10:24 UTC (permalink / raw)
To: intel-gfx
Add assert_forcewakes_active() (the complementary function to
assert_forcewakes_inactive) that documents the requirement of a
function for its callers to be holding the forcewake ref (i.e. the
function is part of a sequence over which RC6 must be prevented).
One such example is during ringbuffer reset, where RC6 must be held
across the whole reinitialisation sequence.
v2: Include debug information in the WARN so we know which fw domain is
missing.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> #v1
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++++++++-
drivers/gpu/drm/i915/intel_uncore.c | 18 +++++++++++++++++-
drivers/gpu/drm/i915/intel_uncore.h | 2 ++
3 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05c08b0bc172..4285f09ff8b8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -579,7 +579,16 @@ static int init_ring_common(struct intel_engine_cs *engine)
static void reset_ring_common(struct intel_engine_cs *engine,
struct drm_i915_gem_request *request)
{
- /* Try to restore the logical GPU state to match the continuation
+ /*
+ * RC6 must be prevented until the reset is complete and the engine
+ * reinitialised. If it occurs in the middle of this sequence, the
+ * state written to/loaded from the power context is ill-defined (e.g.
+ * the PP_BASE_DIR may be lost).
+ */
+ assert_forcewakes_active(engine->i915, FORCEWAKE_ALL);
+
+ /*
+ * Try to restore the logical GPU state to match the continuation
* of the request queue. If we skip the context/PD restore, then
* the next request may try to execute assuming that its context
* is valid and loaded on the GPU and so may try to access invalid
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b3c3f94fc7e4..983617b5b338 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -626,7 +626,23 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
if (!dev_priv->uncore.funcs.force_wake_get)
return;
- WARN_ON(dev_priv->uncore.fw_domains_active);
+ WARN(dev_priv->uncore.fw_domains_active,
+ "Expected all fw_domains to be inactive, but %08x are still on\n",
+ dev_priv->uncore.fw_domains_active);
+}
+
+void assert_forcewakes_active(struct drm_i915_private *dev_priv,
+ enum forcewake_domains fw_domains)
+{
+ if (!dev_priv->uncore.funcs.force_wake_get)
+ return;
+
+ assert_rpm_wakelock_held(dev_priv);
+
+ fw_domains &= dev_priv->uncore.fw_domains;
+ WARN(fw_domains & ~dev_priv->uncore.fw_domains_active,
+ "Expected %08x fw_domains to be active, but %08x are off\n",
+ fw_domains, fw_domains & ~dev_priv->uncore.fw_domains_active);
}
/* We give fast paths for the really cool registers */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 66eae2ce2f29..582771251b57 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -137,6 +137,8 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv);
u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
+void assert_forcewakes_active(struct drm_i915_private *dev_priv,
+ enum forcewake_domains fw_domains);
const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
enum forcewake_domains
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests
2017-10-09 10:14 ` Mika Kuoppala
@ 2017-10-09 10:28 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-10-09 10:28 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2017-10-09 11:14:44)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Resetting the engine requires us to hold the forcewake wakeref to
> > prevent RC6 trying to happen in the middle of the reset sequence.
> > Normally, this is taken by i915_handle_error(), but as we are calling
>
> There is no explicit forcewake grab on i915_handle_error().
> What I am missing in here?
No, you are right, I was thinking of the rpm wakeref. Ok, that means I
should push the problem into the reset sequence. First thought is
i915_gem_reset_prepare, but we want transparency between global/engine
resets. Maybe i915_gem_reset_prepare_engine and
i915_gem_reset_finish_engine.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone (rev2)
2017-10-09 9:32 [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone Chris Wilson
` (3 preceding siblings ...)
2017-10-09 10:22 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone Patchwork
@ 2017-10-09 12:01 ` Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-10-09 12:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone (rev2)
URL : https://patchwork.freedesktop.org/series/31581/
State : warning
== Summary ==
Series 31581v2 series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone
https://patchwork.freedesktop.org/api/1.0/series/31581/revisions/2/mbox/
Test gem_exec_fence:
Subgroup await-hang-default:
pass -> DMESG-WARN (fi-snb-2520m)
pass -> DMESG-WARN (fi-ivb-3520m)
Test drv_module_reload:
Subgroup basic-no-display:
pass -> DMESG-WARN (fi-cfl-s) fdo#103022
fdo#103022 https://bugs.freedesktop.org/show_bug.cgi?id=103022
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:456s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:472s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:390s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:564s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:283s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:521s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:520s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:532s
fi-cfl-s total:289 pass:255 dwarn:2 dfail:0 fail:0 skip:32 time:566s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:610s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:424s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:594s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:438s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:415s
fi-ivb-3520m total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:507s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:475s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:508s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:580s
fi-kbl-7567u total:289 pass:265 dwarn:4 dfail:0 fail:0 skip:20 time:486s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:595s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:652s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:472s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:658s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:530s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:508s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:470s
fi-snb-2520m total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:582s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:423s
fi-byt-n2820 failed to connect after reboot
7b01dd7095721ed58b8f15d98069f99095f338e8 drm-tip: 2017y-10m-09d-10h-57m-19s UTC integration manifest
127f07dc6705 drm/i915: Provide an assert for when we expect forcewake to be held
ba4af78ebec3 drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests
ff80378b3f4f drm/i915/selftests: Pretty print engine state when requests fail to start
53143341df75 drm/i915: Make i915_engine_info pretty printer to standalone
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5949/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-10-09 12:01 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-09 9:32 [PATCH 1/4] drm/i915: Make i915_engine_info pretty printer to standalone Chris Wilson
2017-10-09 9:32 ` [PATCH 2/4] drm/i915/selftests: Pretty print engine state when requests fail to start Chris Wilson
2017-10-09 9:32 ` [PATCH 3/4] drm/i915/selftests: Hold the rpm/forcewake wakeref for the reset tests Chris Wilson
2017-10-09 10:14 ` Mika Kuoppala
2017-10-09 10:28 ` Chris Wilson
2017-10-09 9:32 ` [PATCH 4/4] drm/i915: Provide an assert for when we expect forcewake to be held Chris Wilson
2017-10-09 10:05 ` Mika Kuoppala
2017-10-09 10:14 ` Chris Wilson
2017-10-09 10:24 ` [PATCH v2] " Chris Wilson
2017-10-09 10:22 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone Patchwork
2017-10-09 12:01 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Make i915_engine_info pretty printer to standalone (rev2) Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox