* [PATCH 02/16] drm/i915: Drop assertion that ce->pin_mutex guards state updates
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-22 12:25 ` Mika Kuoppala
2019-10-21 8:02 ` [PATCH 03/16] drm/i915/selftests: Add coverage of mocs registers Chris Wilson
` (17 subsequent siblings)
18 siblings, 1 reply; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
The actual conditions are that we know the GPU is not accessing the
context, and we hold a pin on the context image to allow CPU access. We
used a fake lock on ce->pin_mutex so that we could try and use lockdep
to assert that access is serialised, but the various different
hardirq/softirq contexts where we need to *fake* holding the pin_mutex
are causing more trouble.
Still it would be nice if we did have a way to reassure ourselves that
the direct update to the context image is serialised with GPU execution.
In the meantime, stop lockdep complaining about false irq inversions.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111923
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 16 ----------------
drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 -----
drivers/gpu/drm/i915/i915_perf.c | 1 -
3 files changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index d0088d020220..f9f3e985bb79 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -235,16 +235,6 @@ static void execlists_init_reg_state(u32 *reg_state,
const struct intel_ring *ring,
bool close);
-static void __context_pin_acquire(struct intel_context *ce)
-{
- mutex_acquire(&ce->pin_mutex.dep_map, 2, 0, _RET_IP_);
-}
-
-static void __context_pin_release(struct intel_context *ce)
-{
- mutex_release(&ce->pin_mutex.dep_map, 0, _RET_IP_);
-}
-
static void mark_eio(struct i915_request *rq)
{
if (i915_request_completed(rq))
@@ -2792,9 +2782,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
ce = rq->hw_context;
GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
- /* Proclaim we have exclusive access to the context image! */
- __context_pin_acquire(ce);
-
rq = active_request(rq);
if (!rq) {
/* Idle context; tidy up the ring so we can restart afresh */
@@ -2860,7 +2847,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
__execlists_reset_reg_state(ce, engine);
__execlists_update_reg_state(ce, engine);
ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */
- __context_pin_release(ce);
unwind:
/* Push back any incomplete requests for replay after the reset. */
@@ -4502,7 +4488,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
bool scrub)
{
GEM_BUG_ON(!intel_context_is_pinned(ce));
- __context_pin_acquire(ce);
/*
* We want a simple context + ring to execute the breadcrumb update.
@@ -4528,7 +4513,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
intel_ring_update_space(ce->ring);
__execlists_update_reg_state(ce, engine);
- __context_pin_release(ce);
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 5dc679781a08..7516d1c90925 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -168,12 +168,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio)
}
GEM_BUG_ON(!ce[1]->ring->size);
intel_ring_reset(ce[1]->ring, ce[1]->ring->size / 2);
-
- local_irq_disable(); /* appease lockdep */
- __context_pin_acquire(ce[1]);
__execlists_update_reg_state(ce[1], engine);
- __context_pin_release(ce[1]);
- local_irq_enable();
rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK);
if (IS_ERR(rq[0])) {
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index d2ac51fe4f04..3130b0c7ed83 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2615,7 +2615,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
struct i915_perf_stream *stream;
/* perf.exclusive_stream serialised by gen8_configure_all_contexts() */
- lockdep_assert_held(&ce->pin_mutex);
if (engine->class != RENDER_CLASS)
return;
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 02/16] drm/i915: Drop assertion that ce->pin_mutex guards state updates
2019-10-21 8:02 ` [PATCH 02/16] drm/i915: Drop assertion that ce->pin_mutex guards state updates Chris Wilson
@ 2019-10-22 12:25 ` Mika Kuoppala
0 siblings, 0 replies; 26+ messages in thread
From: Mika Kuoppala @ 2019-10-22 12:25 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> The actual conditions are that we know the GPU is not accessing the
> context, and we hold a pin on the context image to allow CPU access. We
> used a fake lock on ce->pin_mutex so that we could try and use lockdep
> to assert that access is serialised, but the various different
> hardirq/softirq contexts where we need to *fake* holding the pin_mutex
> are causing more trouble.
>
> Still it would be nice if we did have a way to reassure ourselves that
> the direct update to the context image is serialised with GPU execution.
> In the meantime, stop lockdep complaining about false irq inversions.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111923
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
A tad sad that the price/payout didn't provide.
Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 16 ----------------
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 -----
> drivers/gpu/drm/i915/i915_perf.c | 1 -
> 3 files changed, 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index d0088d020220..f9f3e985bb79 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -235,16 +235,6 @@ static void execlists_init_reg_state(u32 *reg_state,
> const struct intel_ring *ring,
> bool close);
>
> -static void __context_pin_acquire(struct intel_context *ce)
> -{
> - mutex_acquire(&ce->pin_mutex.dep_map, 2, 0, _RET_IP_);
> -}
> -
> -static void __context_pin_release(struct intel_context *ce)
> -{
> - mutex_release(&ce->pin_mutex.dep_map, 0, _RET_IP_);
> -}
> -
> static void mark_eio(struct i915_request *rq)
> {
> if (i915_request_completed(rq))
> @@ -2792,9 +2782,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
> ce = rq->hw_context;
> GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
>
> - /* Proclaim we have exclusive access to the context image! */
> - __context_pin_acquire(ce);
> -
> rq = active_request(rq);
> if (!rq) {
> /* Idle context; tidy up the ring so we can restart afresh */
> @@ -2860,7 +2847,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
> __execlists_reset_reg_state(ce, engine);
> __execlists_update_reg_state(ce, engine);
> ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */
> - __context_pin_release(ce);
>
> unwind:
> /* Push back any incomplete requests for replay after the reset. */
> @@ -4502,7 +4488,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
> bool scrub)
> {
> GEM_BUG_ON(!intel_context_is_pinned(ce));
> - __context_pin_acquire(ce);
>
> /*
> * We want a simple context + ring to execute the breadcrumb update.
> @@ -4528,7 +4513,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
> intel_ring_update_space(ce->ring);
>
> __execlists_update_reg_state(ce, engine);
> - __context_pin_release(ce);
> }
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 5dc679781a08..7516d1c90925 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -168,12 +168,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio)
> }
> GEM_BUG_ON(!ce[1]->ring->size);
> intel_ring_reset(ce[1]->ring, ce[1]->ring->size / 2);
> -
> - local_irq_disable(); /* appease lockdep */
> - __context_pin_acquire(ce[1]);
> __execlists_update_reg_state(ce[1], engine);
> - __context_pin_release(ce[1]);
> - local_irq_enable();
>
> rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK);
> if (IS_ERR(rq[0])) {
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index d2ac51fe4f04..3130b0c7ed83 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -2615,7 +2615,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
> struct i915_perf_stream *stream;
>
> /* perf.exclusive_stream serialised by gen8_configure_all_contexts() */
> - lockdep_assert_held(&ce->pin_mutex);
>
> if (engine->class != RENDER_CLASS)
> return;
> --
> 2.24.0.rc0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 03/16] drm/i915/selftests: Add coverage of mocs registers
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
2019-10-21 8:02 ` [PATCH 02/16] drm/i915: Drop assertion that ce->pin_mutex guards state updates Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 04/16] drm/i915/selftests: Teach igt_flush_test and igt_live_test to take intel_gt Chris Wilson
` (16 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Probe the mocs registers for new contexts and across GPU resets. Similar
to intel_workarounds, we have tables of what register values we expect
to see, so verify that user contexts are affected by them. In the
future, we should add tests similar to intel_sseu to cover dynamic
reconfigurations.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/gt/intel_mocs.c | 4 +
drivers/gpu/drm/i915/gt/selftest_mocs.c | 431 ++++++++++++++++++
.../drm/i915/selftests/i915_live_selftests.h | 1 +
3 files changed, 436 insertions(+)
create mode 100644 drivers/gpu/drm/i915/gt/selftest_mocs.c
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 5bac3966906b..f5a239640553 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -490,3 +490,7 @@ void intel_mocs_init(struct intel_gt *gt)
if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
intel_mocs_init_global(gt);
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_mocs.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
new file mode 100644
index 000000000000..94c1c638621b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -0,0 +1,431 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "gt/intel_engine_pm.h"
+#include "i915_selftest.h"
+
+#include "gem/selftests/mock_context.h"
+#include "selftests/igt_reset.h"
+#include "selftests/igt_spinner.h"
+
+struct live_mocs {
+ struct drm_i915_mocs_table table;
+ struct i915_gem_context *ctx;
+ struct i915_vma *scratch;
+ void *vaddr;
+};
+
+static int request_add_sync(struct i915_request *rq, int err)
+{
+ i915_request_get(rq);
+ i915_request_add(rq);
+ if (i915_request_wait(rq, 0, HZ / 5) < 0)
+ err = -ETIME;
+ i915_request_put(rq);
+
+ return err;
+}
+
+static int request_add_spin(struct i915_request *rq, struct igt_spinner *spin)
+{
+ int err = 0;
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+ if (spin && !igt_wait_for_spinner(spin, rq))
+ err = -ETIME;
+ i915_request_put(rq);
+
+ return err;
+}
+
+static struct i915_vma *create_scratch(struct intel_gt *gt)
+{
+ struct drm_i915_gem_object *obj;
+ struct i915_vma *vma;
+ int err;
+
+ obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return ERR_CAST(obj);
+
+ i915_gem_object_set_cache_coherency(obj, I915_CACHING_CACHED);
+
+ vma = i915_vma_instance(obj, >->ggtt->vm, NULL);
+ if (IS_ERR(vma)) {
+ i915_gem_object_put(obj);
+ return vma;
+ }
+
+ err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL);
+ if (err) {
+ i915_gem_object_put(obj);
+ return ERR_PTR(err);
+ }
+
+ return vma;
+}
+
+static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
+{
+ int err;
+
+ if (!get_mocs_settings(gt->i915, &arg->table))
+ return -EINVAL;
+
+ arg->ctx = kernel_context(gt->i915);
+ if (!arg->ctx)
+ return -ENOMEM;
+
+ arg->scratch = create_scratch(gt);
+ if (IS_ERR(arg->scratch)) {
+ err = PTR_ERR(arg->scratch);
+ goto err_ctx;
+ }
+
+ arg->vaddr = i915_gem_object_pin_map(arg->scratch->obj, I915_MAP_WB);
+ if (IS_ERR(arg->vaddr)) {
+ err = PTR_ERR(arg->vaddr);
+ goto err_scratch;
+ }
+
+ return 0;
+
+err_scratch:
+ i915_vma_unpin_and_release(&arg->scratch, 0);
+err_ctx:
+ kernel_context_close(arg->ctx);
+ return err;
+}
+
+static void live_mocs_fini(struct live_mocs *arg)
+{
+ i915_vma_unpin_and_release(&arg->scratch, I915_VMA_RELEASE_MAP);
+ kernel_context_close(arg->ctx);
+}
+
+static int read_mocs_table(struct i915_request *rq,
+ const struct drm_i915_mocs_table *table,
+ struct i915_vma *vma, int *offset)
+{
+ unsigned int i;
+ u32 *cs;
+
+ cs = intel_ring_begin(rq, 4 * table->n_entries);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+
+ for (i = 0; i < table->n_entries; i++) {
+ *cs++ = MI_STORE_REGISTER_MEM_GEN8 | MI_USE_GGTT;
+ *cs++ = i915_mmio_reg_offset(HAS_GLOBAL_MOCS_REGISTERS(rq->i915) ?
+ GEN12_GLOBAL_MOCS(i) :
+ mocs_register(rq->engine, i));
+ *cs++ = i915_ggtt_offset(vma) + i * sizeof(u32) + *offset;
+ *cs++ = 0;
+ }
+
+ intel_ring_advance(rq, cs);
+ *offset += i * sizeof(u32);
+
+ return 0;
+}
+
+static int read_l3cc_table(struct i915_request *rq,
+ const struct drm_i915_mocs_table *table,
+ struct i915_vma *vma, int *offset)
+{
+ unsigned int i;
+ u32 *cs;
+
+ cs = intel_ring_begin(rq, 4 * table->n_entries / 2);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+
+ /* XXX can't read the MCR range 0xb00 directly */
+ for (i = 0; i < table->n_entries / 2; i++) {
+ *cs++ = MI_STORE_REGISTER_MEM_GEN8 | MI_USE_GGTT;
+ *cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
+ *cs++ = i915_ggtt_offset(vma) + i * sizeof(u32) + *offset;
+ *cs++ = 0;
+ }
+
+ intel_ring_advance(rq, cs);
+ *offset += i * sizeof(u32);
+
+ return 0;
+}
+
+static int check_mocs_table(struct intel_engine_cs *engine,
+ const struct drm_i915_mocs_table *table,
+ const u32 *vaddr, int *offset)
+{
+ unsigned int i;
+ u32 expect;
+
+ for (i = 0; i < table->size; i++) {
+ if (HAS_GLOBAL_MOCS_REGISTERS(engine->i915))
+ expect = table->table[i].control_value;
+ else
+ expect = get_entry_control(table, i);
+ if (vaddr[*offset] != expect) {
+ pr_err("%s: Invalid MOCS[%d] entry, found %08x, expected %08x\n",
+ engine->name, i, vaddr[*offset], expect);
+ return -EINVAL;
+ }
+ ++*offset;
+ }
+
+ /* All remaining entries are default */
+ if (HAS_GLOBAL_MOCS_REGISTERS(engine->i915))
+ expect = table->table[0].control_value;
+ else
+ expect = table->table[I915_MOCS_PTE].control_value;
+ for (; i < table->n_entries; i++) {
+ if (vaddr[*offset] != expect) {
+ pr_err("%s: Invalid MOCS[%d*] entry, found %08x, expected %08x\n",
+ engine->name, i, vaddr[*offset], expect);
+ return -EINVAL;
+ }
+ ++*offset;
+ }
+
+ return 0;
+}
+
+static int check_l3cc_table(struct intel_engine_cs *engine,
+ const struct drm_i915_mocs_table *table,
+ const u32 *vaddr, int *offset)
+{
+ u16 unused_value = table->table[I915_MOCS_PTE].l3cc_value;
+ unsigned int i;
+ u32 expect;
+
+ if (1) { /* XXX skip MCR read back */
+ *offset += table->n_entries / 2;
+ return 0;
+ }
+
+ for (i = 0; i < table->size / 2; i++) {
+ u16 low = get_entry_l3cc(table, 2 * i);
+ u16 high = get_entry_l3cc(table, 2 * i + 1);
+
+ expect = l3cc_combine(table, low, high);
+ if (vaddr[*offset] != expect) {
+ pr_err("%s: Invalid L3CC[%d] entry, found %08x, expected %08x\n",
+ engine->name, i, vaddr[*offset], expect);
+ return -EINVAL;
+ }
+ ++*offset;
+ }
+
+ /* Odd table size - 1 left over */
+ if (table->size & 1) {
+ u16 low = get_entry_l3cc(table, 2 * i);
+
+ expect = l3cc_combine(table, low, unused_value);
+ if (vaddr[*offset] != expect) {
+ pr_err("%s: Invalid L3CC[%d] entry, found %08x, expected %08x\n",
+ engine->name, i, vaddr[*offset], expect);
+ return -EINVAL;
+ }
+ ++*offset;
+ i++;
+ }
+
+ /* All remaining entries are also unused */
+ for (; i < table->n_entries / 2; i++) {
+ expect = l3cc_combine(table, unused_value, unused_value);
+ if (vaddr[*offset] != expect) {
+ pr_err("%s: Invalid L3CC[%d] entry, found %08x, expected %08x\n",
+ engine->name, i, vaddr[*offset], expect);
+ return -EINVAL;
+ }
+ ++*offset;
+ }
+
+ return 0;
+}
+
+static int check_mocs_engine(struct live_mocs *arg,
+ struct intel_context *ce)
+{
+ struct i915_vma *vma = arg->scratch;
+ struct i915_request *rq;
+ int offset;
+ int err;
+
+ memset32(arg->vaddr, STACK_MAGIC, PAGE_SIZE / sizeof(u32));
+
+ rq = intel_context_create_request(ce);
+ if (IS_ERR(rq))
+ return PTR_ERR(rq);
+
+ i915_vma_lock(vma);
+ err = i915_request_await_object(rq, vma->obj, true);
+ if (!err)
+ err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
+ i915_vma_unlock(vma);
+
+ offset = 0;
+ if (!err)
+ err = read_mocs_table(rq, &arg->table, vma, &offset);
+ if (!err && ce->engine->class == RENDER_CLASS)
+ err = read_l3cc_table(rq, &arg->table, vma, &offset);
+
+ err = request_add_sync(rq, err);
+ if (err)
+ return err;
+
+ offset = 0;
+ if (!err)
+ err = check_mocs_table(ce->engine, &arg->table,
+ arg->vaddr, &offset);
+ if (!err && ce->engine->class == RENDER_CLASS)
+ err = check_l3cc_table(ce->engine, &arg->table,
+ arg->vaddr, &offset);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static int live_mocs_clean(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ struct live_mocs mocs;
+ int err;
+
+ err = live_mocs_init(&mocs, gt);
+ if (err)
+ return err;
+
+ for_each_engine(engine, gt, id) {
+ struct intel_context *ce;
+
+ ce = intel_context_create(engine->kernel_context->gem_context,
+ engine);
+ if (IS_ERR(ce)) {
+ err = PTR_ERR(ce);
+ break;
+ }
+
+ err = check_mocs_engine(&mocs, ce);
+ intel_context_put(ce);
+ if (err)
+ break;
+ }
+
+ live_mocs_fini(&mocs);
+
+ return err;
+}
+
+static int active_engine_reset(struct intel_context *ce,
+ const char *reason)
+{
+ struct igt_spinner spin;
+ struct i915_request *rq;
+ int err;
+
+ err = igt_spinner_init(&spin, ce->engine->gt);
+ if (err)
+ return err;
+
+ rq = igt_spinner_create_request(&spin, ce, MI_NOOP);
+ if (IS_ERR(rq)) {
+ igt_spinner_fini(&spin);
+ return PTR_ERR(rq);
+ }
+
+ err = request_add_spin(rq, &spin);
+ if (err == 0)
+ err = intel_engine_reset(ce->engine, reason);
+
+ igt_spinner_end(&spin);
+ igt_spinner_fini(&spin);
+
+ return err;
+}
+
+static int __live_mocs_reset(struct live_mocs *mocs,
+ struct intel_context *ce)
+{
+ int err;
+
+ err = intel_engine_reset(ce->engine, "mocs");
+ if (err)
+ return err;
+
+ err = check_mocs_engine(mocs, ce);
+ if (err)
+ return err;
+
+ err = active_engine_reset(ce, "mocs");
+ if (err)
+ return err;
+
+ err = check_mocs_engine(mocs, ce);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static int live_mocs_reset(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ struct live_mocs mocs;
+ int err = 0;
+
+ if (!intel_has_reset_engine(gt))
+ return 0;
+
+ err = live_mocs_init(&mocs, gt);
+ if (err)
+ return err;
+
+ igt_global_reset_lock(gt);
+ for_each_engine(engine, gt, id) {
+ struct intel_context *ce;
+
+ ce = intel_context_create(engine->kernel_context->gem_context,
+ engine);
+ if (IS_ERR(ce)) {
+ err = PTR_ERR(ce);
+ break;
+ }
+
+ intel_engine_pm_get(engine);
+ err = __live_mocs_reset(&mocs, ce);
+ intel_engine_pm_put(engine);
+
+ intel_context_put(ce);
+ if (err)
+ break;
+ }
+ igt_global_reset_unlock(gt);
+
+ live_mocs_fini(&mocs);
+ return err;
+}
+
+int intel_mocs_live_selftests(struct drm_i915_private *i915)
+{
+ static const struct i915_subtest tests[] = {
+ SUBTEST(live_mocs_clean),
+ SUBTEST(live_mocs_reset),
+ };
+ struct drm_i915_mocs_table table;
+
+ if (!get_mocs_settings(i915, &table))
+ return 0;
+
+ return intel_gt_live_subtests(tests, &i915->gt);
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 6daf6599ec79..1a6abcffce81 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -16,6 +16,7 @@ selftest(gt_engines, intel_engine_live_selftests)
selftest(gt_timelines, intel_timeline_live_selftests)
selftest(gt_contexts, intel_context_live_selftests)
selftest(gt_lrc, intel_lrc_live_selftests)
+selftest(gt_mocs, intel_mocs_live_selftests)
selftest(gt_pm, intel_gt_pm_live_selftests)
selftest(requests, i915_request_live_selftests)
selftest(active, i915_active_live_selftests)
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 04/16] drm/i915/selftests: Teach igt_flush_test and igt_live_test to take intel_gt
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
2019-10-21 8:02 ` [PATCH 02/16] drm/i915: Drop assertion that ce->pin_mutex guards state updates Chris Wilson
2019-10-21 8:02 ` [PATCH 03/16] drm/i915/selftests: Add coverage of mocs registers Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 05/16] drm/i915/selftests: Force ordering of context switches Chris Wilson
` (15 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Both routines operate local to the intel_gt, so pass it along as the
object to work on.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../drm/i915/gem/selftests/i915_gem_context.c | 30 +++++++------
.../drm/i915/gem/selftests/i915_gem_mman.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_context.c | 4 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 16 +++----
drivers/gpu/drm/i915/gt/selftest_lrc.c | 42 +++++++++----------
drivers/gpu/drm/i915/gt/selftest_timeline.c | 6 +--
.../gpu/drm/i915/gt/selftest_workarounds.c | 6 +--
drivers/gpu/drm/i915/selftests/i915_active.c | 4 +-
.../gpu/drm/i915/selftests/i915_gem_evict.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_request.c | 14 ++++---
.../gpu/drm/i915/selftests/i915_selftest.c | 4 +-
.../gpu/drm/i915/selftests/igt_flush_test.c | 3 +-
.../gpu/drm/i915/selftests/igt_flush_test.h | 4 +-
.../gpu/drm/i915/selftests/igt_live_test.c | 28 ++++++-------
.../gpu/drm/i915/selftests/igt_live_test.h | 8 ++--
16 files changed, 91 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index e5c235051ae5..d2d7962a049b 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -93,7 +93,8 @@ static int live_nop_switch(void *arg)
pr_info("Populated %d contexts on %s in %lluns\n",
nctx, engine->name, ktime_to_ns(times[1] - times[0]));
- err = igt_live_test_begin(&t, i915, __func__, engine->name);
+ err = igt_live_test_begin(&t, engine->gt,
+ __func__, engine->name);
if (err)
goto out_file;
@@ -307,7 +308,8 @@ static int live_parallel_switch(void *arg)
struct igt_live_test t;
int n;
- err = igt_live_test_begin(&t, i915, __func__, "");
+ err = igt_live_test_begin(&t, data[0].ce[0]->engine->gt,
+ __func__, "");
if (err)
break;
@@ -614,7 +616,8 @@ static int igt_ctx_exec(void *arg)
if (IS_ERR(file))
return PTR_ERR(file);
- err = igt_live_test_begin(&t, i915, __func__, engine->name);
+ err = igt_live_test_begin(&t, engine->gt,
+ __func__, engine->name);
if (err)
goto out_file;
@@ -739,10 +742,6 @@ static int igt_shared_ctx_exec(void *arg)
goto out_file;
}
- err = igt_live_test_begin(&t, i915, __func__, "");
- if (err)
- goto out_file;
-
for_each_engine(engine, i915, id) {
unsigned long ncontexts, ndwords, dw;
struct drm_i915_gem_object *obj = NULL;
@@ -752,6 +751,11 @@ static int igt_shared_ctx_exec(void *arg)
if (!intel_engine_can_store_dword(engine))
continue;
+ err = igt_live_test_begin(&t, engine->gt,
+ __func__, engine->name);
+ if (err)
+ goto out_file;
+
dw = 0;
ndwords = 0;
ncontexts = 0;
@@ -827,12 +831,14 @@ static int igt_shared_ctx_exec(void *arg)
dw += rem;
}
+ err = igt_live_test_end(&t);
+ if (err)
+ goto out_test;
+
i915_gem_drain_freed_objects(i915);
}
out_test:
throttle_release(tq, ARRAY_SIZE(tq));
- if (igt_live_test_end(&t))
- err = -EIO;
out_file:
mock_file_free(i915, file);
return err;
@@ -1249,7 +1255,7 @@ __igt_ctx_sseu(struct drm_i915_private *i915,
goto out_fail;
out_fail:
- if (igt_flush_test(i915))
+ if (igt_flush_test(&i915->gt))
ret = -EIO;
intel_context_unpin(ce);
@@ -1318,7 +1324,7 @@ static int igt_ctx_readonly(void *arg)
if (IS_ERR(file))
return PTR_ERR(file);
- err = igt_live_test_begin(&t, i915, __func__, "");
+ err = igt_live_test_begin(&t, &i915->gt, __func__, "");
if (err)
goto out_file;
@@ -1667,7 +1673,7 @@ static int igt_vm_isolation(void *arg)
if (IS_ERR(file))
return PTR_ERR(file);
- err = igt_live_test_begin(&t, i915, __func__, "");
+ err = igt_live_test_begin(&t, &i915->gt, __func__, "");
if (err)
goto out_file;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 65d4dbf91999..cdab880c55d6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -578,7 +578,7 @@ static void disable_retire_worker(struct drm_i915_private *i915)
static void restore_retire_worker(struct drm_i915_private *i915)
{
- igt_flush_test(i915);
+ igt_flush_test(&i915->gt);
intel_gt_pm_put(&i915->gt);
i915_gem_driver_register__shrinker(i915);
}
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index f63a26a3e620..be43149373de 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -310,7 +310,7 @@ static int live_active_context(void *arg)
if (err)
break;
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
}
@@ -420,7 +420,7 @@ static int live_remote_context(void *arg)
if (err)
break;
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
}
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 8e0016464325..54e4a962c336 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -295,7 +295,7 @@ static void hang_fini(struct hang *h)
kernel_context_close(h->ctx);
- igt_flush_test(h->gt->i915);
+ igt_flush_test(h->gt);
}
static bool wait_until_running(struct hang *h, struct i915_request *rq)
@@ -431,13 +431,13 @@ static int igt_reset_nop(void *arg)
break;
}
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
} while (time_before(jiffies, end_time));
pr_info("%s: %d resets\n", __func__, count);
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
out:
mock_file_free(gt->i915, file);
if (intel_gt_is_wedged(gt))
@@ -528,12 +528,12 @@ static int igt_reset_nop_engine(void *arg)
if (err)
break;
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
}
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
out:
mock_file_free(gt->i915, file);
if (intel_gt_is_wedged(gt))
@@ -634,7 +634,7 @@ static int __igt_reset_engine(struct intel_gt *gt, bool active)
if (err)
break;
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
}
@@ -951,7 +951,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
if (err)
break;
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
}
@@ -1473,7 +1473,7 @@ static int igt_reset_queue(void *arg)
i915_request_put(prev);
- err = igt_flush_test(gt->i915);
+ err = igt_flush_test(gt);
if (err)
break;
}
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 7516d1c90925..2c9c75166570 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -87,7 +87,7 @@ static int live_sanitycheck(void *arg)
}
igt_spinner_end(&spin);
- if (igt_flush_test(gt->i915)) {
+ if (igt_flush_test(gt)) {
err = -EIO;
goto err_ctx;
}
@@ -135,7 +135,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio)
if (!intel_engine_can_store_dword(engine))
continue;
- if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
+ if (igt_live_test_begin(&t, gt, __func__, engine->name)) {
err = -EIO;
break;
}
@@ -474,7 +474,7 @@ static int live_timeslice_preempt(void *arg)
if (err)
goto err_pin;
- if (igt_flush_test(gt->i915)) {
+ if (igt_flush_test(gt)) {
err = -EIO;
goto err_pin;
}
@@ -695,7 +695,7 @@ static int live_busywait_preempt(void *arg)
if (!intel_engine_can_store_dword(engine))
continue;
- if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
+ if (igt_live_test_begin(&t, gt, __func__, engine->name)) {
err = -EIO;
goto err_vma;
}
@@ -867,7 +867,7 @@ static int live_preempt(void *arg)
if (!intel_engine_has_preemption(engine))
continue;
- if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
+ if (igt_live_test_begin(&t, gt, __func__, engine->name)) {
err = -EIO;
goto err_ctx_lo;
}
@@ -963,7 +963,7 @@ static int live_late_preempt(void *arg)
if (!intel_engine_has_preemption(engine))
continue;
- if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
+ if (igt_live_test_begin(&t, gt, __func__, engine->name)) {
err = -EIO;
goto err_ctx_lo;
}
@@ -1141,7 +1141,7 @@ static int live_nopreempt(void *arg)
goto err_wedged;
}
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
goto err_wedged;
}
@@ -1199,7 +1199,7 @@ static int live_suppress_self_preempt(void *arg)
if (!intel_engine_has_preemption(engine))
continue;
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
goto err_wedged;
intel_engine_pm_get(engine);
@@ -1260,7 +1260,7 @@ static int live_suppress_self_preempt(void *arg)
}
intel_engine_pm_put(engine);
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
goto err_wedged;
}
@@ -1414,7 +1414,7 @@ static int live_suppress_wait_preempt(void *arg)
for (i = 0; i < ARRAY_SIZE(client); i++)
igt_spinner_end(&client[i].spin);
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
goto err_wedged;
if (engine->execlists.preempt_hang.count) {
@@ -1501,7 +1501,7 @@ static int live_chain_preempt(void *arg)
goto err_wedged;
}
- if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
+ if (igt_live_test_begin(&t, gt, __func__, engine->name)) {
err = -EIO;
goto err_wedged;
}
@@ -1679,7 +1679,7 @@ static int live_preempt_hang(void *arg)
igt_spinner_end(&spin_hi);
igt_spinner_end(&spin_lo);
- if (igt_flush_test(gt->i915)) {
+ if (igt_flush_test(gt)) {
err = -EIO;
goto err_ctx_lo;
}
@@ -1914,7 +1914,7 @@ static int live_preempt_smoke(void *arg)
i915_gem_object_flush_map(smoke.batch);
i915_gem_object_unpin_map(smoke.batch);
- if (igt_live_test_begin(&t, smoke.gt->i915, __func__, "all")) {
+ if (igt_live_test_begin(&t, smoke.gt, __func__, "all")) {
err = -EIO;
goto err_batch;
}
@@ -1997,7 +1997,7 @@ static int nop_virtual_engine(struct intel_gt *gt,
}
}
- err = igt_live_test_begin(&t, gt->i915, __func__, ve[0]->engine->name);
+ err = igt_live_test_begin(&t, gt, __func__, ve[0]->engine->name);
if (err)
goto out;
@@ -2066,7 +2066,7 @@ static int nop_virtual_engine(struct intel_gt *gt,
prime, div64_u64(ktime_to_ns(times[1]), prime));
out:
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
for (nc = 0; nc < nctx; nc++) {
@@ -2156,7 +2156,7 @@ static int mask_virtual_engine(struct intel_gt *gt,
if (err)
goto out_put;
- err = igt_live_test_begin(&t, gt->i915, __func__, ve->engine->name);
+ err = igt_live_test_begin(&t, gt, __func__, ve->engine->name);
if (err)
goto out_unpin;
@@ -2203,7 +2203,7 @@ static int mask_virtual_engine(struct intel_gt *gt,
err = igt_live_test_end(&t);
out:
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
for (n = 0; n < nsibling; n++)
@@ -2282,7 +2282,7 @@ static int preserved_virtual_engine(struct intel_gt *gt,
if (err)
goto out_put;
- err = igt_live_test_begin(&t, gt->i915, __func__, ve->engine->name);
+ err = igt_live_test_begin(&t, gt, __func__, ve->engine->name);
if (err)
goto out_unpin;
@@ -2527,7 +2527,7 @@ static int bond_virtual_engine(struct intel_gt *gt,
out:
for (n = 0; !IS_ERR(rq[n]); n++)
i915_request_put(rq[n]);
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
kernel_context_close(ctx);
@@ -2860,7 +2860,7 @@ static int live_lrc_state(void *arg)
break;
}
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
i915_vma_unpin_and_release(&scratch, 0);
@@ -3003,7 +3003,7 @@ static int live_gpr_clear(void *arg)
break;
}
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
i915_vma_unpin_and_release(&scratch, 0);
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index dac86f699a4c..abdcbb3b8632 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -551,7 +551,7 @@ static int live_hwsp_engine(void *arg)
break;
}
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
for (n = 0; n < count; n++) {
@@ -623,7 +623,7 @@ static int live_hwsp_alternate(void *arg)
}
out:
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
for (n = 0; n < count; n++) {
@@ -742,7 +742,7 @@ static int live_hwsp_wrap(void *arg)
}
out:
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
intel_timeline_unpin(tl);
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index ef02920cec29..c4877cc81fce 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -693,7 +693,7 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
break;
}
- if (igt_flush_test(ctx->i915))
+ if (igt_flush_test(&ctx->i915->gt))
err = -EIO;
out_batch:
i915_vma_unpin_and_release(&batch, 0);
@@ -1105,7 +1105,7 @@ static int live_isolated_whitelist(void *arg)
kernel_context_close(client[i].ctx);
}
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
return err;
@@ -1261,7 +1261,7 @@ live_engine_reset_workarounds(void *arg)
igt_global_reset_unlock(gt);
kernel_context_close(ctx);
- igt_flush_test(gt->i915);
+ igt_flush_test(gt);
return ret;
}
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index 268192b5613b..ade5e4a0c12a 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -162,7 +162,7 @@ static int live_active_wait(void *arg)
__live_put(active);
- if (igt_flush_test(i915))
+ if (igt_flush_test(&i915->gt))
err = -EIO;
return err;
@@ -181,7 +181,7 @@ static int live_active_retire(void *arg)
return PTR_ERR(active);
/* waits for & retires all requests */
- if (igt_flush_test(i915))
+ if (igt_flush_test(&i915->gt))
err = -EIO;
if (!READ_ONCE(active->retired)) {
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 0af9a58d011d..300967ea6257 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -520,7 +520,7 @@ static int igt_evict_contexts(void *arg)
mutex_lock(&i915->ggtt.vm.mutex);
out_locked:
- if (igt_flush_test(i915))
+ if (igt_flush_test(&i915->gt))
err = -EIO;
while (reserved) {
struct reserved *next = reserved->next;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index ebe735df6504..6321bf91f5c7 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -2003,7 +2003,7 @@ static int igt_cs_tlb(void *arg)
}
}
end:
- if (igt_flush_test(i915))
+ if (igt_flush_test(&i915->gt))
err = -EIO;
i915_gem_context_unlock_engines(ctx);
i915_gem_object_unpin_map(out);
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
index 30ae34f62176..c9ac3dd4dbda 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -535,7 +535,8 @@ static int live_nop_request(void *arg)
IGT_TIMEOUT(end_time);
ktime_t times[2] = {};
- err = igt_live_test_begin(&t, i915, __func__, engine->name);
+ err = igt_live_test_begin(&t, engine->gt,
+ __func__, engine->name);
if (err)
return err;
@@ -687,7 +688,8 @@ static int live_empty_request(void *arg)
unsigned long n, prime;
ktime_t times[2] = {};
- err = igt_live_test_begin(&t, i915, __func__, engine->name);
+ err = igt_live_test_begin(&t, engine->gt,
+ __func__, engine->name);
if (err)
goto out_batch;
@@ -831,7 +833,7 @@ static int live_all_engines(void *arg)
if (!request)
return -ENOMEM;
- err = igt_live_test_begin(&t, i915, __func__, "");
+ err = igt_live_test_begin(&t, &i915->gt, __func__, "");
if (err)
goto out_free;
@@ -945,7 +947,7 @@ static int live_sequential_engines(void *arg)
if (!request)
return -ENOMEM;
- err = igt_live_test_begin(&t, i915, __func__, "");
+ err = igt_live_test_begin(&t, &i915->gt, __func__, "");
if (err)
goto out_free;
@@ -1142,7 +1144,7 @@ static int live_parallel_engines(void *arg)
struct igt_live_test t;
unsigned int idx;
- err = igt_live_test_begin(&t, i915, __func__, "");
+ err = igt_live_test_begin(&t, &i915->gt, __func__, "");
if (err)
break;
@@ -1278,7 +1280,7 @@ static int live_breadcrumbs_smoketest(void *arg)
}
}
- ret = igt_live_test_begin(&live, i915, __func__, "");
+ ret = igt_live_test_begin(&live, &i915->gt, __func__, "");
if (ret)
goto out_contexts;
diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
index 825a8286cbe8..d83679d4f1c2 100644
--- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
+++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
@@ -263,7 +263,7 @@ int __i915_live_teardown(int err, void *data)
{
struct drm_i915_private *i915 = data;
- if (igt_flush_test(i915))
+ if (igt_flush_test(&i915->gt))
err = -EIO;
i915_gem_drain_freed_objects(i915);
@@ -282,7 +282,7 @@ int __intel_gt_live_teardown(int err, void *data)
{
struct intel_gt *gt = data;
- if (igt_flush_test(gt->i915))
+ if (igt_flush_test(gt))
err = -EIO;
i915_gem_drain_freed_objects(gt->i915);
diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c
index 7b0939e3f007..25ef435af051 100644
--- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c
+++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c
@@ -12,9 +12,8 @@
#include "igt_flush_test.h"
-int igt_flush_test(struct drm_i915_private *i915)
+int igt_flush_test(struct intel_gt *gt)
{
- struct intel_gt *gt = &i915->gt;
int ret = intel_gt_is_wedged(gt) ? -EIO : 0;
cond_resched();
diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.h b/drivers/gpu/drm/i915/selftests/igt_flush_test.h
index 7541fa74e641..d522ce11f0e1 100644
--- a/drivers/gpu/drm/i915/selftests/igt_flush_test.h
+++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.h
@@ -7,8 +7,8 @@
#ifndef IGT_FLUSH_TEST_H
#define IGT_FLUSH_TEST_H
-struct drm_i915_private;
+struct intel_gt;
-int igt_flush_test(struct drm_i915_private *i915);
+int igt_flush_test(struct intel_gt *gt);
#endif /* IGT_FLUSH_TEST_H */
diff --git a/drivers/gpu/drm/i915/selftests/igt_live_test.c b/drivers/gpu/drm/i915/selftests/igt_live_test.c
index 810b60100c2c..12893bffd0a1 100644
--- a/drivers/gpu/drm/i915/selftests/igt_live_test.c
+++ b/drivers/gpu/drm/i915/selftests/igt_live_test.c
@@ -12,58 +12,58 @@
#include "igt_live_test.h"
int igt_live_test_begin(struct igt_live_test *t,
- struct drm_i915_private *i915,
+ struct intel_gt *gt,
const char *func,
const char *name)
{
+ struct i915_gpu_error *error = >->i915->gpu_error;
struct intel_engine_cs *engine;
enum intel_engine_id id;
int err;
- t->i915 = i915;
+ t->gt = gt;
t->func = func;
t->name = name;
- err = intel_gt_wait_for_idle(&i915->gt, MAX_SCHEDULE_TIMEOUT);
+ err = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
if (err) {
pr_err("%s(%s): failed to idle before, with err=%d!",
func, name, err);
return err;
}
- t->reset_global = i915_reset_count(&i915->gpu_error);
+ t->reset_global = i915_reset_count(error);
- for_each_engine(engine, i915, id)
- t->reset_engine[id] =
- i915_reset_engine_count(&i915->gpu_error, engine);
+ for_each_engine(engine, gt, id)
+ t->reset_engine[id] = i915_reset_engine_count(error, engine);
return 0;
}
int igt_live_test_end(struct igt_live_test *t)
{
- struct drm_i915_private *i915 = t->i915;
+ struct i915_gpu_error *error = &t->gt->i915->gpu_error;
struct intel_engine_cs *engine;
enum intel_engine_id id;
- if (igt_flush_test(i915))
+ if (igt_flush_test(t->gt))
return -EIO;
- if (t->reset_global != i915_reset_count(&i915->gpu_error)) {
+ if (t->reset_global != i915_reset_count(error)) {
pr_err("%s(%s): GPU was reset %d times!\n",
t->func, t->name,
- i915_reset_count(&i915->gpu_error) - t->reset_global);
+ i915_reset_count(error) - t->reset_global);
return -EIO;
}
- for_each_engine(engine, i915, id) {
+ for_each_engine(engine, t->gt, id) {
if (t->reset_engine[id] ==
- i915_reset_engine_count(&i915->gpu_error, engine))
+ i915_reset_engine_count(error, engine))
continue;
pr_err("%s(%s): engine '%s' was reset %d times!\n",
t->func, t->name, engine->name,
- i915_reset_engine_count(&i915->gpu_error, engine) -
+ i915_reset_engine_count(error, engine) -
t->reset_engine[id]);
return -EIO;
}
diff --git a/drivers/gpu/drm/i915/selftests/igt_live_test.h b/drivers/gpu/drm/i915/selftests/igt_live_test.h
index c0e9f99d50de..234666e93226 100644
--- a/drivers/gpu/drm/i915/selftests/igt_live_test.h
+++ b/drivers/gpu/drm/i915/selftests/igt_live_test.h
@@ -7,12 +7,12 @@
#ifndef IGT_LIVE_TEST_H
#define IGT_LIVE_TEST_H
-#include "../i915_gem.h"
+#include "gt/intel_engine_types.h" /* I915_NUM_ENGINES */
-struct drm_i915_private;
+struct intel_gt;
struct igt_live_test {
- struct drm_i915_private *i915;
+ struct intel_gt *gt;
const char *func;
const char *name;
@@ -27,7 +27,7 @@ struct igt_live_test {
* e.g. if the GPU was reset.
*/
int igt_live_test_begin(struct igt_live_test *t,
- struct drm_i915_private *i915,
+ struct intel_gt *gt,
const char *func,
const char *name);
int igt_live_test_end(struct igt_live_test *t);
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 05/16] drm/i915/selftests: Force ordering of context switches
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (2 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 04/16] drm/i915/selftests: Teach igt_flush_test and igt_live_test to take intel_gt Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 06/16] drm/i915: Expose engine properties via sysfs Chris Wilson
` (14 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
The parallel switch test has an underlying assumption that its requests
are executed in order of submission, which is only true if the backend
manages to keep up. Ensure the order of execution matches the submission
order by explicit dependencies and so when we wait on the last request,
we know we wait on completion of the entire queue.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../drm/i915/gem/selftests/i915_gem_context.c | 35 +++++++++++++++----
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index d2d7962a049b..dd3728085fd8 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -171,18 +171,24 @@ static int __live_parallel_switch1(void *data)
struct i915_request *rq = NULL;
int err, n;
- for (n = 0; n < ARRAY_SIZE(arg->ce); n++) {
- i915_request_put(rq);
+ err = 0;
+ for (n = 0; !err && n < ARRAY_SIZE(arg->ce); n++) {
+ struct i915_request *prev = rq;
rq = i915_request_create(arg->ce[n]);
- if (IS_ERR(rq))
+ if (IS_ERR(rq)) {
+ i915_request_put(prev);
return PTR_ERR(rq);
+ }
i915_request_get(rq);
+ if (prev) {
+ err = i915_request_await_dma_fence(rq, &prev->fence);
+ i915_request_put(prev);
+ }
+
i915_request_add(rq);
}
-
- err = 0;
if (i915_request_wait(rq, 0, HZ / 5) < 0)
err = -ETIME;
i915_request_put(rq);
@@ -199,6 +205,7 @@ static int __live_parallel_switch1(void *data)
static int __live_parallel_switchN(void *data)
{
struct parallel_switch *arg = data;
+ struct i915_request *rq = NULL;
IGT_TIMEOUT(end_time);
unsigned long count;
int n;
@@ -206,17 +213,31 @@ static int __live_parallel_switchN(void *data)
count = 0;
do {
for (n = 0; n < ARRAY_SIZE(arg->ce); n++) {
- struct i915_request *rq;
+ struct i915_request *prev = rq;
+ int err = 0;
rq = i915_request_create(arg->ce[n]);
- if (IS_ERR(rq))
+ if (IS_ERR(rq)) {
+ i915_request_put(prev);
return PTR_ERR(rq);
+ }
+
+ i915_request_get(rq);
+ if (prev) {
+ err = i915_request_await_dma_fence(rq, &prev->fence);
+ i915_request_put(prev);
+ }
i915_request_add(rq);
+ if (err) {
+ i915_request_put(rq);
+ return err;
+ }
}
count++;
} while (!__igt_timeout(end_time, NULL));
+ i915_request_put(rq);
pr_info("%s: %lu switches (many)\n", arg->ce[0]->engine->name, count);
return 0;
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 06/16] drm/i915: Expose engine properties via sysfs
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (3 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 05/16] drm/i915/selftests: Force ordering of context switches Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 07/16] drm/i915: Expose engine->mmio_base " Chris Wilson
` (13 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Preliminary stub to add engines underneath /sys/class/drm/cardN/, so
that we can expose properties on each engine to the sysadmin.
To start with we have basic analogues of the i915_query ioctl so that we
can pretty print engine discovery from the shell, and flesh out the
directory structure. Later we will add writeable sysadmin properties such
as per-engine timeout controls.
An example tree of the engine properties on Braswell:
/sys/class/drm/card0
└── engine
├── bcs0
│ ├── capabilities
│ ├── class
│ ├── instance
│ ├── known_capabilities
│ └── name
├── rcs0
│ ├── capabilities
│ ├── class
│ ├── instance
│ ├── known_capabilities
│ └── name
├── vcs0
│ ├── capabilities
│ ├── class
│ ├── instance
│ ├── known_capabilities
│ └── name
└── vecs0
├── capabilities
├── class
├── instance
├── known_capabilities
└── name
v2: Include stringified capabilities
v3: Include all known capabilities for futureproofing.
v4: Combine the two caps loops into one
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 205 +++++++++++++++++++
drivers/gpu/drm/i915/gt/intel_engine_sysfs.h | 14 ++
drivers/gpu/drm/i915/i915_sysfs.c | 3 +
4 files changed, 224 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_sysfs.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a16a2daef977..761d9a70afcd 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -78,8 +78,9 @@ gt-y += \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
gt/intel_engine_cs.o \
- gt/intel_engine_pool.o \
gt/intel_engine_pm.o \
+ gt/intel_engine_pool.o \
+ gt/intel_engine_sysfs.o \
gt/intel_engine_user.o \
gt/intel_gt.o \
gt/intel_gt_irq.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
new file mode 100644
index 000000000000..823153e56c67
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -0,0 +1,205 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+
+#include "i915_drv.h"
+#include "intel_engine.h"
+#include "intel_engine_sysfs.h"
+
+struct kobj_engine {
+ struct kobject base;
+ struct intel_engine_cs *engine;
+};
+
+static struct intel_engine_cs *kobj_to_engine(struct kobject *kobj)
+{
+ return container_of(kobj, struct kobj_engine, base)->engine;
+}
+
+static ssize_t
+name_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%s\n", kobj_to_engine(kobj)->name);
+}
+
+static struct kobj_attribute name_attr =
+__ATTR(name, 0444, name_show, NULL);
+
+static ssize_t
+class_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", kobj_to_engine(kobj)->uabi_class);
+}
+
+static struct kobj_attribute class_attr =
+__ATTR(class, 0444, class_show, NULL);
+
+static ssize_t
+inst_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", kobj_to_engine(kobj)->uabi_instance);
+}
+
+static struct kobj_attribute inst_attr =
+__ATTR(instance, 0444, inst_show, NULL);
+
+static const char * const vcs_caps[] = {
+ [ilog2(I915_VIDEO_CLASS_CAPABILITY_HEVC)] = "hevc",
+ [ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc",
+};
+
+static const char * const vecs_caps[] = {
+ [ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc",
+};
+
+static ssize_t repr_trim(char *buf, ssize_t len)
+{
+ /* Trim off the trailing space and replace with a newline */
+ if (len > PAGE_SIZE)
+ len = PAGE_SIZE;
+ if (len > 0)
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
+static ssize_t
+__caps_show(struct intel_engine_cs *engine,
+ typeof(engine->uabi_capabilities) caps,
+ char *buf, bool show_unknown)
+{
+ const char * const *repr;
+ int count, n;
+ ssize_t len;
+
+ switch (engine->class) {
+ case VIDEO_DECODE_CLASS:
+ repr = vcs_caps;
+ count = ARRAY_SIZE(vcs_caps);
+ break;
+
+ case VIDEO_ENHANCEMENT_CLASS:
+ repr = vecs_caps;
+ count = ARRAY_SIZE(vecs_caps);
+ break;
+
+ default:
+ repr = NULL;
+ count = 0;
+ break;
+ }
+ GEM_BUG_ON(count > BITS_PER_TYPE(typeof(caps)));
+
+ len = 0;
+ for_each_set_bit(n,
+ (unsigned long *)&caps,
+ show_unknown ? BITS_PER_TYPE(typeof(caps)) : count) {
+ if (n >= count || !repr[n]) {
+ if (GEM_WARN_ON(show_unknown))
+ len += snprintf(buf + len, PAGE_SIZE - len,
+ "[%x] ", n);
+ } else {
+ len += snprintf(buf + len, PAGE_SIZE - len,
+ "%s ", repr[n]);
+ }
+ if (GEM_WARN_ON(len >= PAGE_SIZE))
+ break;
+ }
+ return repr_trim(buf, len);
+}
+
+static ssize_t
+caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+
+ return __caps_show(engine, engine->uabi_capabilities, buf, true);
+}
+
+static struct kobj_attribute caps_attr =
+__ATTR(capabilities, 0444, caps_show, NULL);
+
+static ssize_t
+all_caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ return __caps_show(kobj_to_engine(kobj), -1, buf, false);
+}
+
+static struct kobj_attribute all_caps_attr =
+__ATTR(known_capabilities, 0444, all_caps_show, NULL);
+
+static void kobj_engine_release(struct kobject *kobj)
+{
+ kfree(kobj);
+}
+
+static struct kobj_type kobj_engine_type = {
+ .release = kobj_engine_release,
+ .sysfs_ops = &kobj_sysfs_ops
+};
+
+static struct kobject *
+kobj_engine(struct kobject *dir, struct intel_engine_cs *engine)
+{
+ struct kobj_engine *ke;
+
+ ke = kzalloc(sizeof(*ke), GFP_KERNEL);
+ if (!ke)
+ return NULL;
+
+ kobject_init(&ke->base, &kobj_engine_type);
+ ke->engine = engine;
+
+ if (kobject_add(&ke->base, dir, "%s", engine->name)) {
+ kobject_put(&ke->base);
+ return NULL;
+ }
+
+ /* xfer ownership to sysfs tree */
+ return &ke->base;
+}
+
+void intel_engines_add_sysfs(struct drm_i915_private *i915)
+{
+ static const struct attribute *files[] = {
+ &name_attr.attr,
+ &class_attr.attr,
+ &inst_attr.attr,
+ &caps_attr.attr,
+ &all_caps_attr.attr,
+ NULL
+ };
+
+ struct device *kdev = i915->drm.primary->kdev;
+ struct intel_engine_cs *engine;
+ struct kobject *dir;
+
+ dir = kobject_create_and_add("engine", &kdev->kobj);
+ if (!dir)
+ return;
+
+ for_each_uabi_engine(engine, i915) {
+ struct kobject *kobj;
+
+ kobj = kobj_engine(dir, engine);
+ if (!kobj)
+ goto err_engine;
+
+ if (sysfs_create_files(kobj, files))
+ goto err_object;
+
+ if (0) {
+err_object:
+ kobject_put(kobj);
+err_engine:
+ dev_err(kdev, "Failed to add sysfs engine '%s'\n",
+ engine->name);
+ break;
+ }
+ }
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.h b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.h
new file mode 100644
index 000000000000..ef44a745b70a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.h
@@ -0,0 +1,14 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef INTEL_ENGINE_SYSFS_H
+#define INTEL_ENGINE_SYSFS_H
+
+struct drm_i915_private;
+
+void intel_engines_add_sysfs(struct drm_i915_private *i915);
+
+#endif /* INTEL_ENGINE_SYSFS_H */
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index bf039b8ba593..7b665f69f301 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -30,6 +30,7 @@
#include <linux/stat.h>
#include <linux/sysfs.h>
+#include "gt/intel_engine_sysfs.h"
#include "gt/intel_rc6.h"
#include "i915_drv.h"
@@ -616,6 +617,8 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
DRM_ERROR("RPS sysfs setup failed\n");
i915_setup_error_capture(kdev);
+
+ intel_engines_add_sysfs(dev_priv);
}
void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 07/16] drm/i915: Expose engine->mmio_base via sysfs
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (4 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 06/16] drm/i915: Expose engine properties via sysfs Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 08/16] drm/i915: Expose timeslice duration to sysfs Chris Wilson
` (12 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Use the per-engine sysfs directory to let userspace discover the
mmio_base of each engine. Prior to recent generations, the user
accessible registers on each engine are at a fixed offset relative to
each engine -- but require absolute addressing. As the absolute address
depends on the actual physical engine, this is not always possible to
determine from userspace (for example icl may expose vcs1 or vcs2 as the
second vcs engine). Make this easy for userspace to discover by
providing the mmio_base in sysfs.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
index 823153e56c67..557ef1e2f84a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -48,6 +48,15 @@ inst_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
static struct kobj_attribute inst_attr =
__ATTR(instance, 0444, inst_show, NULL);
+static ssize_t
+mmio_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ return sprintf(buf, "0x%x\n", kobj_to_engine(kobj)->mmio_base);
+}
+
+static struct kobj_attribute mmio_attr =
+__ATTR(mmio_base, 0444, mmio_show, NULL);
+
static const char * const vcs_caps[] = {
[ilog2(I915_VIDEO_CLASS_CAPABILITY_HEVC)] = "hevc",
[ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc",
@@ -170,6 +179,7 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
&name_attr.attr,
&class_attr.attr,
&inst_attr.attr,
+ &mmio_attr.attr,
&caps_attr.attr,
&all_caps_attr.attr,
NULL
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 08/16] drm/i915: Expose timeslice duration to sysfs
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (5 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 07/16] drm/i915: Expose engine->mmio_base " Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 09/16] drm/i915/execlists: Force preemption Chris Wilson
` (11 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Execlists uses a scheduling quantum (a timeslice) to alternate execution
between ready-to-run contexts of equal priority. This ensures that all
users (though only if they of equal importance) have the opportunity to
run and prevents livelocks where contexts may have implicit ordering due
to userspace semaphores.
The timeslicing mechanism can be compiled out with
./scripts/config --set-val DRM_I915_TIMESLICE_DURATION 0
The timeslice duration can be adjusted per-engine using,
/sys/class/drm/card?/engine/*/timeslice_duration_ms
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/Kconfig.profile | 18 ++++++
drivers/gpu/drm/i915/gt/intel_engine.h | 9 +++
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +
drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 46 +++++++++++++++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ++
drivers/gpu/drm/i915/gt/intel_lrc.c | 59 ++++++++++++++++----
drivers/gpu/drm/i915/gt/selftest_lrc.c | 13 ++++-
7 files changed, 138 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
index 48df8889a88a..b8df80bc0b47 100644
--- a/drivers/gpu/drm/i915/Kconfig.profile
+++ b/drivers/gpu/drm/i915/Kconfig.profile
@@ -25,3 +25,21 @@ config DRM_I915_SPIN_REQUEST
May be 0 to disable the initial spin. In practice, we estimate
the cost of enabling the interrupt (if currently disabled) to be
a few microseconds.
+
+config DRM_I915_TIMESLICE_DURATION
+ int "Scheduling quantum for userspace batches (ms, jiffy granularity)"
+ default 1 # milliseconds
+ help
+ When two user batches of equal priority are executing, we will
+ alternate execution of each batch to ensure forward progress of
+ all users. This is necessary in some cases where there may be
+ an implicit dependency between those batches that requires
+ concurrent execution in order for them to proceed, e.g. they
+ interact with each other via userspace semaphores. Each context
+ is scheduled for execution for the timeslice duration, before
+ switching to the next context.
+
+ This is adjustable via
+ /sys/class/drm/card?/engine/*/timeslice_duration_ms
+
+ May be 0 to disable timeslicing.
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 93ea367fe624..393481374f96 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -525,4 +525,13 @@ void intel_engine_init_active(struct intel_engine_cs *engine,
#define ENGINE_MOCK 1
#define ENGINE_VIRTUAL 2
+static inline bool
+intel_engine_has_timeslices(const struct intel_engine_cs *engine)
+{
+ if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
+ return 0;
+
+ return intel_engine_has_semaphores(engine);
+}
+
#endif /* _INTEL_RINGBUFFER_H_ */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 051734c9b733..b93dbed67fe7 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -308,6 +308,9 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
engine->instance = info->instance;
__sprint_engine_name(engine);
+ engine->props.timeslice_duration_ms =
+ CONFIG_DRM_I915_TIMESLICE_DURATION;
+
/*
* To be overridden by the backend on setup. However to facilitate
* cleanup on error during setup, we always provide the destroy vfunc.
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
index 557ef1e2f84a..55ae81769a8e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -142,6 +142,48 @@ all_caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
static struct kobj_attribute all_caps_attr =
__ATTR(known_capabilities, 0444, all_caps_show, NULL);
+static ssize_t
+timeslice_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+
+ return sprintf(buf, "%lu\n", engine->props.timeslice_duration_ms);
+}
+
+static ssize_t
+timeslice_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+ unsigned long long duration;
+ int err;
+
+ err = kstrtoull(buf, 0, &duration);
+ if (err)
+ return err;
+
+ if (duration > jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
+ return -EINVAL;
+
+ WRITE_ONCE(engine->props.timeslice_duration_ms, duration);
+
+ if (execlists_active(&engine->execlists)) {
+ struct timer_list *t = &engine->execlists.timer;
+
+ if (!duration) {
+ cancel_timer(t);
+ } else {
+ duration = msecs_to_jiffies_timeout(duration);
+ mod_timer(t, jiffies + duration);
+ }
+ }
+
+ return count;
+}
+
+static struct kobj_attribute timeslice_duration_attr =
+__ATTR(timeslice_duration_ms, 0644, timeslice_show, timeslice_store);
+
static void kobj_engine_release(struct kobject *kobj)
{
kfree(kobj);
@@ -203,6 +245,10 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
if (sysfs_create_files(kobj, files))
goto err_object;
+ if (intel_engine_has_timeslices(engine) &&
+ sysfs_create_file(kobj, ×lice_duration_attr.attr))
+ goto err_engine;
+
if (0) {
err_object:
kobject_put(kobj);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 3451be034caf..89a9616e8539 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -542,6 +542,10 @@ struct intel_engine_cs {
*/
ktime_t total;
} stats;
+
+ struct {
+ unsigned long timeslice_duration_ms;
+ } props;
};
static inline bool
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index f9f3e985bb79..30142ebce8fe 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -252,6 +252,26 @@ static inline u32 intel_hws_preempt_address(struct intel_engine_cs *engine)
I915_GEM_HWS_PREEMPT_ADDR);
}
+static void __set_timer(struct timer_list *t, unsigned long timeout)
+{
+ if (!timeout) {
+ cancel_timer(t);
+ return;
+ }
+
+ timeout = msecs_to_jiffies_timeout(timeout);
+
+ /*
+ * Paranoia to make sure the compiler computes the timeout before
+ * loading 'jiffies' as jiffies is volatile and may be updated in
+ * the background by a timer tick. All to reduce the complexity
+ * of the addition and reduce the risk of losing a jiffie.
+ */
+ barrier();
+
+ mod_timer(t, jiffies + timeout);
+}
+
static inline void
ring_set_paused(const struct intel_engine_cs *engine, int state)
{
@@ -1335,7 +1355,7 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
{
int hint;
- if (!intel_engine_has_semaphores(engine))
+ if (!intel_engine_has_timeslices(engine))
return false;
if (list_is_last(&rq->sched.link, &engine->active.requests))
@@ -1356,15 +1376,32 @@ switch_prio(struct intel_engine_cs *engine, const struct i915_request *rq)
return rq_prio(list_next_entry(rq, sched.link));
}
-static bool
-enable_timeslice(const struct intel_engine_execlists *execlists)
+static inline unsigned long
+timeslice(const struct intel_engine_cs *engine)
+{
+ return READ_ONCE(engine->props.timeslice_duration_ms);
+}
+
+static unsigned long
+active_timeslice(const struct intel_engine_cs *engine)
{
- const struct i915_request *rq = *execlists->active;
+ const struct i915_request *rq = *engine->execlists.active;
if (i915_request_completed(rq))
- return false;
+ return 0;
- return execlists->switch_priority_hint >= effective_prio(rq);
+ if (engine->execlists.switch_priority_hint < effective_prio(rq))
+ return 0;
+
+ return timeslice(engine);
+}
+
+static void set_timeslice(struct intel_engine_cs *engine)
+{
+ if (!intel_engine_has_timeslices(engine))
+ return;
+
+ __set_timer(&engine->execlists.timer, active_timeslice(engine));
}
static void record_preemption(struct intel_engine_execlists *execlists)
@@ -1511,8 +1548,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
*/
if (!execlists->timer.expires &&
need_timeslice(engine, last))
- mod_timer(&execlists->timer,
- jiffies + 1);
+ __set_timer(&execlists->timer,
+ timeslice(engine));
+
return;
}
@@ -1934,10 +1972,7 @@ static void process_csb(struct intel_engine_cs *engine)
execlists_num_ports(execlists) *
sizeof(*execlists->pending));
- if (enable_timeslice(execlists))
- mod_timer(&execlists->timer, jiffies + 1);
- else
- cancel_timer(&execlists->timer);
+ set_timeslice(engine);
WRITE_ONCE(execlists->pending[0], NULL);
} else {
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 2c9c75166570..04864d7588a2 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -439,6 +439,8 @@ static int live_timeslice_preempt(void *arg)
* need to preempt the current task and replace it with another
* ready task.
*/
+ if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
+ return 0;
obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
if (IS_ERR(obj))
@@ -513,6 +515,11 @@ static void wait_for_submit(struct intel_engine_cs *engine,
} while (!i915_request_is_active(rq));
}
+static long timeslice_threshold(const struct intel_engine_cs *engine)
+{
+ return 2 * msecs_to_jiffies_timeout(timeslice(engine)) + 1;
+}
+
static int live_timeslice_queue(void *arg)
{
struct intel_gt *gt = arg;
@@ -530,6 +537,8 @@ static int live_timeslice_queue(void *arg)
* ELSP[1] is already occupied, so must rely on timeslicing to
* eject ELSP[0] in favour of the queue.)
*/
+ if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
+ return 0;
obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
if (IS_ERR(obj))
@@ -607,8 +616,8 @@ static int live_timeslice_queue(void *arg)
err = -EINVAL;
}
- /* Timeslice every jiffie, so within 2 we should signal */
- if (i915_request_wait(rq, 0, 3) < 0) {
+ /* Timeslice every jiffy, so within 2 we should signal */
+ if (i915_request_wait(rq, 0, timeslice_threshold(engine)) < 0) {
struct drm_printer p =
drm_info_printer(gt->i915->drm.dev);
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 09/16] drm/i915/execlists: Force preemption
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (6 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 08/16] drm/i915: Expose timeslice duration to sysfs Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 10/16] drm/i915/gt: Introduce barrier pulses along engines Chris Wilson
` (10 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
If the preempted context takes too long to relinquish control, e.g. it
is stuck inside a shader with arbitration disabled, evict that context
with an engine reset. This ensures that preemptions are reasonably
responsive, providing a tighter QoS for the more important context at
the cost of flagging unresponsive contexts more frequently (i.e. instead
of using an ~10s hangcheck, we now evict at ~100ms). The challenge of
lies in picking a timeout that can be reasonably serviced by HW for
typical workloads, balancing the existing clients against the needs for
responsiveness.
Note that coupled with timeslicing, this will lead to rapid GPU "hang"
detection with multiple active contexts vying for GPU time.
The preempt timeout can be adjusted per-engine using,
/sys/class/drm/card?/engine/*/preempt_timeout_ms
v2: Couple in sysfs control of preemption timeout
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>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/Kconfig.profile | 15 +++
drivers/gpu/drm/i915/gt/intel_engine.h | 9 ++
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 5 +-
drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 47 +++++++++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 6 ++
drivers/gpu/drm/i915/gt/intel_lrc.c | 92 +++++++++++++++--
drivers/gpu/drm/i915/gt/selftest_lrc.c | 100 +++++++++++++++++++
drivers/gpu/drm/i915/i915_params.h | 2 +-
8 files changed, 266 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
index b8df80bc0b47..00bd37aa88d3 100644
--- a/drivers/gpu/drm/i915/Kconfig.profile
+++ b/drivers/gpu/drm/i915/Kconfig.profile
@@ -43,3 +43,18 @@ config DRM_I915_TIMESLICE_DURATION
/sys/class/drm/card?/engine/*/timeslice_duration_ms
May be 0 to disable timeslicing.
+
+config DRM_I915_PREEMPT_TIMEOUT
+ int "Preempt timeout (ms, jiffy granularity)"
+ default 100 # milliseconds
+ help
+ How long to wait (in milliseconds) for a preemption event to occur
+ when submitting a new context via execlists. If the current context
+ does not hit an arbitration point and yield to HW before the timer
+ expires, the HW will be reset to allow the more important context
+ to execute.
+
+ This is adjustable via
+ /sys/class/drm/card?/engine/*/preempt_timeout_ms
+
+ May be 0 to disable the timeout.
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 393481374f96..ef4b8fdc0a24 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -534,4 +534,13 @@ intel_engine_has_timeslices(const struct intel_engine_cs *engine)
return intel_engine_has_semaphores(engine);
}
+static inline bool
+intel_engine_has_preempt_reset(const struct intel_engine_cs *engine)
+{
+ if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
+ return 0;
+
+ return intel_engine_has_preemption(engine);
+}
+
#endif /* _INTEL_RINGBUFFER_H_ */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index b93dbed67fe7..7e8d3cc2459c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -308,6 +308,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
engine->instance = info->instance;
__sprint_engine_name(engine);
+ engine->props.preempt_timeout_ms =
+ CONFIG_DRM_I915_PREEMPT_TIMEOUT;
engine->props.timeslice_duration_ms =
CONFIG_DRM_I915_TIMESLICE_DURATION;
@@ -1321,10 +1323,11 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine,
unsigned int idx;
u8 read, write;
- drm_printf(m, "\tExeclist tasklet queued? %s (%s), timeslice? %s\n",
+ drm_printf(m, "\tExeclist tasklet queued? %s (%s), preempt? %s, timeslice? %s\n",
yesno(test_bit(TASKLET_STATE_SCHED,
&engine->execlists.tasklet.state)),
enableddisabled(!atomic_read(&engine->execlists.tasklet.count)),
+ repr_timer(&engine->execlists.preempt),
repr_timer(&engine->execlists.timer));
read = execlists->csb_head;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
index 55ae81769a8e..27c144b77020 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -184,6 +184,49 @@ timeslice_store(struct kobject *kobj, struct kobj_attribute *attr,
static struct kobj_attribute timeslice_duration_attr =
__ATTR(timeslice_duration_ms, 0644, timeslice_show, timeslice_store);
+static ssize_t
+preempt_timeout_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+
+ return sprintf(buf, "%lu\n", engine->props.preempt_timeout_ms);
+}
+
+static ssize_t
+preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+ unsigned long long timeout;
+ int err;
+
+ err = kstrtoull(buf, 0, &timeout);
+ if (err)
+ return err;
+
+ if (timeout > jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
+ return -EINVAL;
+
+ WRITE_ONCE(engine->props.preempt_timeout_ms, timeout);
+
+ if (READ_ONCE(engine->execlists.pending[0])) {
+ struct timer_list *t = &engine->execlists.preempt;
+
+ if (!timeout) {
+ cancel_timer(t);
+ } else {
+ timeout = msecs_to_jiffies_timeout(timeout);
+ mod_timer(t, jiffies + timeout);
+ }
+ }
+
+ return count;
+}
+
+static struct kobj_attribute preempt_timeout_attr =
+__ATTR(preempt_timeout_ms, 0644, preempt_timeout_show, preempt_timeout_store);
+
static void kobj_engine_release(struct kobject *kobj)
{
kfree(kobj);
@@ -249,6 +292,10 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
sysfs_create_file(kobj, ×lice_duration_attr.attr))
goto err_engine;
+ if (intel_engine_has_preempt_reset(engine) &&
+ sysfs_create_file(kobj, &preempt_timeout_attr.attr))
+ goto err_engine;
+
if (0) {
err_object:
kobject_put(kobj);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 89a9616e8539..01926a826d52 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -174,6 +174,11 @@ struct intel_engine_execlists {
*/
struct timer_list timer;
+ /**
+ * @preempt: reset the current context if it fails to give way
+ */
+ struct timer_list preempt;
+
/**
* @default_priolist: priority list for I915_PRIORITY_NORMAL
*/
@@ -544,6 +549,7 @@ struct intel_engine_cs {
} stats;
struct {
+ unsigned long preempt_timeout_ms;
unsigned long timeslice_duration_ms;
} props;
};
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 30142ebce8fe..ec0f7ef88e3e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1409,6 +1409,25 @@ static void record_preemption(struct intel_engine_execlists *execlists)
(void)I915_SELFTEST_ONLY(execlists->preempt_hang.count++);
}
+static unsigned long active_preempt_timeout(struct intel_engine_cs *engine)
+{
+ struct i915_request *rq;
+
+ rq = last_active(&engine->execlists);
+ if (!rq)
+ return 0;
+
+ return READ_ONCE(engine->props.preempt_timeout_ms);
+}
+
+static void set_preempt_timeout(struct intel_engine_cs *engine)
+{
+ if (!intel_engine_has_preempt_reset(engine))
+ return;
+
+ __set_timer(&engine->execlists.preempt, active_preempt_timeout(engine));
+}
+
static void execlists_dequeue(struct intel_engine_cs *engine)
{
struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -1785,6 +1804,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
memset(port + 1, 0, (last_port - port) * sizeof(*port));
execlists_submit_ports(engine);
+
+ set_preempt_timeout(engine);
} else {
skip_submit:
ring_set_paused(engine, 0);
@@ -2022,6 +2043,43 @@ static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
}
}
+static noinline void preempt_reset(struct intel_engine_cs *engine)
+{
+ const unsigned int bit = I915_RESET_ENGINE + engine->id;
+ unsigned long *lock = &engine->gt->reset.flags;
+
+ if (i915_modparams.reset < 3)
+ return;
+
+ if (test_and_set_bit(bit, lock))
+ return;
+
+ /* Mark this tasklet as disabled to avoid waiting for it to complete */
+ tasklet_disable_nosync(&engine->execlists.tasklet);
+
+ GEM_TRACE("%s: preempt timeout %lu+%ums\n",
+ engine->name,
+ engine->props.preempt_timeout_ms,
+ jiffies_to_msecs(jiffies - engine->execlists.preempt.expires));
+ intel_engine_reset(engine, "preemption time out");
+
+ tasklet_enable(&engine->execlists.tasklet);
+ clear_and_wake_up_bit(bit, lock);
+}
+
+static bool preempt_timeout(const struct intel_engine_cs *const engine)
+{
+ const struct timer_list *t = &engine->execlists.preempt;
+
+ if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
+ return false;
+
+ if (!timer_expired(t))
+ return false;
+
+ return READ_ONCE(engine->execlists.pending[0]);
+}
+
/*
* Check the unread Context Status Buffers and manage the submission of new
* contexts to the ELSP accordingly.
@@ -2029,23 +2087,39 @@ static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
static void execlists_submission_tasklet(unsigned long data)
{
struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
- unsigned long flags;
+ bool timeout = preempt_timeout(engine);
process_csb(engine);
- if (!READ_ONCE(engine->execlists.pending[0])) {
+ if (!READ_ONCE(engine->execlists.pending[0]) || timeout) {
+ unsigned long flags;
+
spin_lock_irqsave(&engine->active.lock, flags);
__execlists_submission_tasklet(engine);
spin_unlock_irqrestore(&engine->active.lock, flags);
+
+ /* Recheck after serialising with direct-submission */
+ if (timeout && preempt_timeout(engine))
+ preempt_reset(engine);
}
}
-static void execlists_submission_timer(struct timer_list *timer)
+static void __execlists_kick(struct intel_engine_execlists *execlists)
{
- struct intel_engine_cs *engine =
- from_timer(engine, timer, execlists.timer);
-
/* Kick the tasklet for some interrupt coalescing and reset handling */
- tasklet_hi_schedule(&engine->execlists.tasklet);
+ tasklet_hi_schedule(&execlists->tasklet);
+}
+
+#define execlists_kick(t, member) \
+ __execlists_kick(container_of(t, struct intel_engine_execlists, member))
+
+static void execlists_timeslice(struct timer_list *timer)
+{
+ execlists_kick(timer, timer);
+}
+
+static void execlists_preempt(struct timer_list *timer)
+{
+ execlists_kick(timer, preempt);
}
static void queue_request(struct intel_engine_cs *engine,
@@ -3490,6 +3564,7 @@ gen12_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
static void execlists_park(struct intel_engine_cs *engine)
{
cancel_timer(&engine->execlists.timer);
+ cancel_timer(&engine->execlists.preempt);
}
void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
@@ -3607,7 +3682,8 @@ int intel_execlists_submission_setup(struct intel_engine_cs *engine)
{
tasklet_init(&engine->execlists.tasklet,
execlists_submission_tasklet, (unsigned long)engine);
- timer_setup(&engine->execlists.timer, execlists_submission_timer, 0);
+ timer_setup(&engine->execlists.timer, execlists_timeslice, 0);
+ timer_setup(&engine->execlists.preempt, execlists_preempt, 0);
logical_ring_default_vfuncs(engine);
logical_ring_default_irqs(engine);
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 04864d7588a2..1b228f2b1118 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1706,6 +1706,105 @@ static int live_preempt_hang(void *arg)
return err;
}
+static int live_preempt_timeout(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct i915_gem_context *ctx_hi, *ctx_lo;
+ struct igt_spinner spin_lo;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ int err = -ENOMEM;
+
+ /*
+ * Check that we force preemption to occur by cancelling the previous
+ * context if it refuses to yield the GPU.
+ */
+ if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
+ return 0;
+
+ if (!HAS_LOGICAL_RING_PREEMPTION(gt->i915))
+ return 0;
+
+ if (!intel_has_reset_engine(gt))
+ return 0;
+
+ if (igt_spinner_init(&spin_lo, gt))
+ return -ENOMEM;
+
+ ctx_hi = kernel_context(gt->i915);
+ if (!ctx_hi)
+ goto err_spin_lo;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
+
+ ctx_lo = kernel_context(gt->i915);
+ if (!ctx_lo)
+ goto err_ctx_hi;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
+
+ for_each_engine(engine, gt, id) {
+ unsigned long saved_timeout;
+ struct i915_request *rq;
+
+ if (!intel_engine_has_preemption(engine))
+ continue;
+
+ rq = spinner_create_request(&spin_lo, ctx_lo, engine,
+ MI_NOOP); /* preemption disabled */
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto err_ctx_lo;
+ }
+
+ i915_request_add(rq);
+ if (!igt_wait_for_spinner(&spin_lo, rq)) {
+ intel_gt_set_wedged(gt);
+ err = -EIO;
+ goto err_ctx_lo;
+ }
+
+ rq = igt_request_alloc(ctx_hi, engine);
+ if (IS_ERR(rq)) {
+ igt_spinner_end(&spin_lo);
+ err = PTR_ERR(rq);
+ goto err_ctx_lo;
+ }
+
+ /* Flush the previous CS ack before changing timeouts */
+ while (READ_ONCE(engine->execlists.pending[0]))
+ cpu_relax();
+
+ saved_timeout = engine->props.preempt_timeout_ms;
+ engine->props.preempt_timeout_ms = 1; /* in ms, -> 1 jiffie */
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+
+ intel_engine_flush_submission(engine);
+ engine->props.preempt_timeout_ms = saved_timeout;
+
+ if (i915_request_wait(rq, 0, HZ / 10) < 0) {
+ intel_gt_set_wedged(gt);
+ i915_request_put(rq);
+ err = -ETIME;
+ goto err_ctx_lo;
+ }
+
+ igt_spinner_end(&spin_lo);
+ i915_request_put(rq);
+ }
+
+ err = 0;
+err_ctx_lo:
+ kernel_context_close(ctx_lo);
+err_ctx_hi:
+ kernel_context_close(ctx_hi);
+err_spin_lo:
+ igt_spinner_fini(&spin_lo);
+ return err;
+}
+
static int random_range(struct rnd_state *rnd, int min, int max)
{
return i915_prandom_u32_max_state(max - min, rnd) + min;
@@ -2607,6 +2706,7 @@ int intel_execlists_live_selftests(struct drm_i915_private *i915)
SUBTEST(live_suppress_wait_preempt),
SUBTEST(live_chain_preempt),
SUBTEST(live_preempt_hang),
+ SUBTEST(live_preempt_timeout),
SUBTEST(live_preempt_smoke),
SUBTEST(live_virtual_engine),
SUBTEST(live_virtual_mask),
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index d29ade3b7de6..56058978bb27 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -61,7 +61,7 @@ struct drm_printer;
param(char *, dmc_firmware_path, NULL) \
param(int, mmio_debug, -IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO)) \
param(int, edp_vswing, 0) \
- param(int, reset, 2) \
+ param(int, reset, 3) \
param(unsigned int, inject_load_failure, 0) \
param(int, fastboot, -1) \
param(int, enable_dpcd_backlight, 0) \
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 10/16] drm/i915/gt: Introduce barrier pulses along engines
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (7 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 09/16] drm/i915/execlists: Force preemption Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 11/16] drm/i915/execlists: Cancel banned contexts on schedule-out Chris Wilson
` (9 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
To flush idle barriers, and even inflight requests, we want to send a
preemptive 'pulse' along an engine. We use a no-op request along the
pinned kernel_context at high priority so that it should run or else
kick off the stuck requests. We can use this to ensure idle barriers are
immediately flushed, as part of a context cancellation mechanism, or as
part of a heartbeat mechanism to detect and reset a stuck GPU.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/Makefile | 1 +
.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 56 +++++++++++++++++++
.../gpu/drm/i915/gt/intel_engine_heartbeat.h | 14 +++++
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +-
drivers/gpu/drm/i915/i915_priolist_types.h | 1 +
5 files changed, 73 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 761d9a70afcd..9b3268038535 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -78,6 +78,7 @@ gt-y += \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
gt/intel_engine_cs.o \
+ gt/intel_engine_heartbeat.o \
gt/intel_engine_pm.o \
gt/intel_engine_pool.o \
gt/intel_engine_sysfs.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
new file mode 100644
index 000000000000..2fc413f9d506
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -0,0 +1,56 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "i915_request.h"
+
+#include "intel_context.h"
+#include "intel_engine_heartbeat.h"
+#include "intel_engine_pm.h"
+#include "intel_engine.h"
+#include "intel_gt.h"
+
+static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
+{
+ engine->wakeref_serial = READ_ONCE(engine->serial) + 1;
+ i915_request_add_active_barriers(rq);
+}
+
+int intel_engine_pulse(struct intel_engine_cs *engine)
+{
+ struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER };
+ struct intel_context *ce = engine->kernel_context;
+ struct i915_request *rq;
+ int err = 0;
+
+ if (!intel_engine_has_preemption(engine))
+ return -ENODEV;
+
+ if (!intel_engine_pm_get_if_awake(engine))
+ return 0;
+
+ if (mutex_lock_interruptible(&ce->timeline->mutex))
+ goto out_rpm;
+
+ intel_context_enter(ce);
+ rq = __i915_request_create(ce, GFP_NOWAIT | __GFP_NOWARN);
+ intel_context_exit(ce);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto out_unlock;
+ }
+
+ rq->flags |= I915_REQUEST_SENTINEL;
+ idle_pulse(engine, rq);
+
+ __i915_request_commit(rq);
+ __i915_request_queue(rq, &attr);
+
+out_unlock:
+ mutex_unlock(&ce->timeline->mutex);
+out_rpm:
+ intel_engine_pm_put(engine);
+ return err;
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
new file mode 100644
index 000000000000..b950451b5998
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
@@ -0,0 +1,14 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef INTEL_ENGINE_HEARTBEAT_H
+#define INTEL_ENGINE_HEARTBEAT_H
+
+struct intel_engine_cs;
+
+int intel_engine_pulse(struct intel_engine_cs *engine);
+
+#endif /* INTEL_ENGINE_HEARTBEAT_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 67eb6183648a..7d76611d9df1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -111,7 +111,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
i915_request_add_active_barriers(rq);
/* Install ourselves as a preemption barrier */
- rq->sched.attr.priority = I915_PRIORITY_UNPREEMPTABLE;
+ rq->sched.attr.priority = I915_PRIORITY_BARRIER;
__i915_request_commit(rq);
/* Release our exclusive hold on the engine */
diff --git a/drivers/gpu/drm/i915/i915_priolist_types.h b/drivers/gpu/drm/i915/i915_priolist_types.h
index 21037a2e2038..ae8bb3cb627e 100644
--- a/drivers/gpu/drm/i915/i915_priolist_types.h
+++ b/drivers/gpu/drm/i915/i915_priolist_types.h
@@ -39,6 +39,7 @@ enum {
* active request.
*/
#define I915_PRIORITY_UNPREEMPTABLE INT_MAX
+#define I915_PRIORITY_BARRIER INT_MAX
#define __NO_PREEMPTION (I915_PRIORITY_WAIT)
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 11/16] drm/i915/execlists: Cancel banned contexts on schedule-out
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (8 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 10/16] drm/i915/gt: Introduce barrier pulses along engines Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 12/16] drm/i915/gem: Cancel contexts when hangchecking is disabled Chris Wilson
` (8 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
On schedule-out (CS completion) of a banned context, scrub the context
image so that we do not replay the active payload. The intent is that we
skip banned payloads on request submission so that the timeline
advancement continues on in the background. However, if we are returning
to a preempted request, i915_request_skip() is ineffective and instead we
need to patch up the context image so that it continues from the start
of the next request.
v2: Fixup cancellation so that we only scrub the payload of the active
request and do not short-circuit the breadcrumbs (which might cause
other contexts to execute out of order).
v3: Grammar pass
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 131 ++++++----
drivers/gpu/drm/i915/gt/selftest_lrc.c | 321 +++++++++++++++++++++++++
2 files changed, 411 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index ec0f7ef88e3e..e6ae38832e6e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -234,6 +234,9 @@ static void execlists_init_reg_state(u32 *reg_state,
const struct intel_engine_cs *engine,
const struct intel_ring *ring,
bool close);
+static void
+__execlists_update_reg_state(const struct intel_context *ce,
+ const struct intel_engine_cs *engine);
static void mark_eio(struct i915_request *rq)
{
@@ -246,6 +249,31 @@ static void mark_eio(struct i915_request *rq)
i915_request_mark_complete(rq);
}
+static struct i915_request *active_request(struct i915_request *rq)
+{
+ const struct intel_context * const ce = rq->hw_context;
+ struct i915_request *active = NULL;
+ struct list_head *list;
+
+ if (!i915_request_is_active(rq)) /* unwound, but incomplete! */
+ return rq;
+
+ rcu_read_lock();
+ list = &rcu_dereference(rq->timeline)->requests;
+ list_for_each_entry_from_reverse(rq, list, link) {
+ if (i915_request_completed(rq))
+ break;
+
+ if (rq->hw_context != ce)
+ break;
+
+ active = rq;
+ }
+ rcu_read_unlock();
+
+ return active;
+}
+
static inline u32 intel_hws_preempt_address(struct intel_engine_cs *engine)
{
return (i915_ggtt_offset(engine->status_page.vma) +
@@ -992,6 +1020,58 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
tasklet_schedule(&ve->base.execlists.tasklet);
}
+static void restore_default_state(struct intel_context *ce,
+ struct intel_engine_cs *engine)
+{
+ u32 *regs = ce->lrc_reg_state;
+
+ if (engine->pinned_default_state)
+ memcpy(regs, /* skip restoring the vanilla PPHWSP */
+ engine->pinned_default_state + LRC_STATE_PN * PAGE_SIZE,
+ engine->context_size - PAGE_SIZE);
+
+ execlists_init_reg_state(regs, ce, engine, ce->ring, false);
+}
+
+static void reset_active(struct i915_request *rq,
+ struct intel_engine_cs *engine)
+{
+ struct intel_context * const ce = rq->hw_context;
+
+ /*
+ * The executing context has been cancelled. We want to prevent
+ * further execution along this context and propagate the error on
+ * to anything depending on its results.
+ *
+ * In __i915_request_submit(), we apply the -EIO and remove the
+ * requests' payloads for any banned requests. But first, we must
+ * rewind the context back to the start of the incomplete request so
+ * that we do not jump back into the middle of the batch.
+ *
+ * We preserve the breadcrumbs and semaphores of the incomplete
+ * requests so that inter-timeline dependencies (i.e other timelines)
+ * remain correctly ordered. And we defer to __i915_request_submit()
+ * so that all asynchronous waits are correctly handled.
+ */
+ GEM_TRACE("%s(%s): { rq=%llx:%lld }\n",
+ __func__, engine->name, rq->fence.context, rq->fence.seqno);
+
+ /* On resubmission of the active request, payload will be scrubbed */
+ rq = active_request(rq);
+ if (rq)
+ ce->ring->head = intel_ring_wrap(ce->ring, rq->head);
+ else
+ ce->ring->head = ce->ring->tail;
+ intel_ring_update_space(ce->ring);
+
+ /* Scrub the context image to prevent replaying the previous batch */
+ restore_default_state(ce, engine);
+ __execlists_update_reg_state(ce, engine);
+
+ /* We've switched away, so this should be a no-op, but intent matters */
+ ce->lrc_desc |= CTX_DESC_FORCE_RESTORE;
+}
+
static inline void
__execlists_schedule_out(struct i915_request *rq,
struct intel_engine_cs * const engine)
@@ -1002,6 +1082,9 @@ __execlists_schedule_out(struct i915_request *rq,
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
intel_gt_pm_put(engine->gt);
+ if (unlikely(i915_gem_context_is_banned(ce->gem_context)))
+ reset_active(rq, engine);
+
/*
* If this is part of a virtual engine, its next request may
* have been blocked waiting for access to the active context.
@@ -1417,6 +1500,10 @@ static unsigned long active_preempt_timeout(struct intel_engine_cs *engine)
if (!rq)
return 0;
+ /* Force a fast reset for terminated contexts (ignoring sysfs!) */
+ if (unlikely(i915_gem_context_is_banned(rq->gem_context)))
+ return 1;
+
return READ_ONCE(engine->props.preempt_timeout_ms);
}
@@ -2826,29 +2913,6 @@ static void reset_csb_pointers(struct intel_engine_cs *engine)
&execlists->csb_status[reset_value]);
}
-static struct i915_request *active_request(struct i915_request *rq)
-{
- const struct intel_context * const ce = rq->hw_context;
- struct i915_request *active = NULL;
- struct list_head *list;
-
- if (!i915_request_is_active(rq)) /* unwound, but incomplete! */
- return rq;
-
- list = &i915_request_active_timeline(rq)->requests;
- list_for_each_entry_from_reverse(rq, list, link) {
- if (i915_request_completed(rq))
- break;
-
- if (rq->hw_context != ce)
- break;
-
- active = rq;
- }
-
- return active;
-}
-
static void __execlists_reset_reg_state(const struct intel_context *ce,
const struct intel_engine_cs *engine)
{
@@ -2865,7 +2929,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
struct intel_engine_execlists * const execlists = &engine->execlists;
struct intel_context *ce;
struct i915_request *rq;
- u32 *regs;
mb(); /* paranoia: read the CSB pointers from after the reset */
clflush(execlists->csb_write);
@@ -2941,13 +3004,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
* to recreate its own state.
*/
GEM_BUG_ON(!intel_context_is_pinned(ce));
- regs = ce->lrc_reg_state;
- if (engine->pinned_default_state) {
- memcpy(regs, /* skip restoring the vanilla PPHWSP */
- engine->pinned_default_state + LRC_STATE_PN * PAGE_SIZE,
- engine->context_size - PAGE_SIZE);
- }
- execlists_init_reg_state(regs, ce, engine, ce->ring, false);
+ restore_default_state(ce, engine);
out_replay:
GEM_TRACE("%s replay {head:%04x, tail:%04x\n",
@@ -4608,16 +4665,8 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
* future request will be after userspace has had the opportunity
* to recreate its own state.
*/
- if (scrub) {
- u32 *regs = ce->lrc_reg_state;
-
- if (engine->pinned_default_state) {
- memcpy(regs, /* skip restoring the vanilla PPHWSP */
- engine->pinned_default_state + LRC_STATE_PN * PAGE_SIZE,
- engine->context_size - PAGE_SIZE);
- }
- execlists_init_reg_state(regs, ce, engine, ce->ring, false);
- }
+ if (scrub)
+ restore_default_state(ce, engine);
/* Rerun the request; its payload has been neutered (if guilty). */
ce->ring->head = head;
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 1b228f2b1118..25a41ba6317f 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -7,6 +7,7 @@
#include <linux/prime_numbers.h>
#include "gem/i915_gem_pm.h"
+#include "gt/intel_engine_heartbeat.h"
#include "gt/intel_reset.h"
#include "i915_selftest.h"
@@ -1169,6 +1170,325 @@ static int live_nopreempt(void *arg)
goto err_client_b;
}
+struct live_preempt_cancel {
+ struct intel_engine_cs *engine;
+ struct preempt_client a, b;
+};
+
+static int __cancel_active0(struct live_preempt_cancel *arg)
+{
+ struct i915_request *rq;
+ struct igt_live_test t;
+ int err;
+
+ /* Preempt cancel of ELSP0 */
+ GEM_TRACE("%s(%s)\n", __func__, arg->engine->name);
+ if (igt_live_test_begin(&t, arg->engine->gt,
+ __func__, arg->engine->name))
+ return -EIO;
+
+ clear_bit(CONTEXT_BANNED, &arg->a.ctx->flags);
+ rq = spinner_create_request(&arg->a.spin,
+ arg->a.ctx, arg->engine,
+ MI_ARB_CHECK);
+ if (IS_ERR(rq))
+ return PTR_ERR(rq);
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+ if (!igt_wait_for_spinner(&arg->a.spin, rq)) {
+ err = -EIO;
+ goto out;
+ }
+
+ i915_gem_context_set_banned(arg->a.ctx);
+ err = intel_engine_pulse(arg->engine);
+ if (err)
+ goto out;
+
+ if (i915_request_wait(rq, 0, HZ / 5) < 0) {
+ err = -EIO;
+ goto out;
+ }
+
+ if (rq->fence.error != -EIO) {
+ pr_err("Cancelled inflight0 request did not report -EIO\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+out:
+ i915_request_put(rq);
+ if (igt_live_test_end(&t))
+ err = -EIO;
+ return err;
+}
+
+static int __cancel_active1(struct live_preempt_cancel *arg)
+{
+ struct i915_request *rq[2] = {};
+ struct igt_live_test t;
+ int err;
+
+ /* Preempt cancel of ELSP1 */
+ GEM_TRACE("%s(%s)\n", __func__, arg->engine->name);
+ if (igt_live_test_begin(&t, arg->engine->gt,
+ __func__, arg->engine->name))
+ return -EIO;
+
+ clear_bit(CONTEXT_BANNED, &arg->a.ctx->flags);
+ rq[0] = spinner_create_request(&arg->a.spin,
+ arg->a.ctx, arg->engine,
+ MI_NOOP); /* no preemption */
+ if (IS_ERR(rq[0]))
+ return PTR_ERR(rq[0]);
+
+ i915_request_get(rq[0]);
+ i915_request_add(rq[0]);
+ if (!igt_wait_for_spinner(&arg->a.spin, rq[0])) {
+ err = -EIO;
+ goto out;
+ }
+
+ clear_bit(CONTEXT_BANNED, &arg->b.ctx->flags);
+ rq[1] = spinner_create_request(&arg->b.spin,
+ arg->b.ctx, arg->engine,
+ MI_ARB_CHECK);
+ if (IS_ERR(rq[1])) {
+ err = PTR_ERR(rq[1]);
+ goto out;
+ }
+
+ i915_request_get(rq[1]);
+ err = i915_request_await_dma_fence(rq[1], &rq[0]->fence);
+ i915_request_add(rq[1]);
+ if (err)
+ goto out;
+
+ i915_gem_context_set_banned(arg->b.ctx);
+ err = intel_engine_pulse(arg->engine);
+ if (err)
+ goto out;
+
+ igt_spinner_end(&arg->a.spin);
+ if (i915_request_wait(rq[1], 0, HZ / 5) < 0) {
+ err = -EIO;
+ goto out;
+ }
+
+ if (rq[0]->fence.error != 0) {
+ pr_err("Normal inflight0 request did not complete\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (rq[1]->fence.error != -EIO) {
+ pr_err("Cancelled inflight1 request did not report -EIO\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+out:
+ i915_request_put(rq[1]);
+ i915_request_put(rq[0]);
+ if (igt_live_test_end(&t))
+ err = -EIO;
+ return err;
+}
+
+static int __cancel_queued(struct live_preempt_cancel *arg)
+{
+ struct i915_request *rq[3] = {};
+ struct igt_live_test t;
+ int err;
+
+ /* Full ELSP and one in the wings */
+ GEM_TRACE("%s(%s)\n", __func__, arg->engine->name);
+ if (igt_live_test_begin(&t, arg->engine->gt,
+ __func__, arg->engine->name))
+ return -EIO;
+
+ clear_bit(CONTEXT_BANNED, &arg->a.ctx->flags);
+ rq[0] = spinner_create_request(&arg->a.spin,
+ arg->a.ctx, arg->engine,
+ MI_ARB_CHECK);
+ if (IS_ERR(rq[0]))
+ return PTR_ERR(rq[0]);
+
+ i915_request_get(rq[0]);
+ i915_request_add(rq[0]);
+ if (!igt_wait_for_spinner(&arg->a.spin, rq[0])) {
+ err = -EIO;
+ goto out;
+ }
+
+ clear_bit(CONTEXT_BANNED, &arg->b.ctx->flags);
+ rq[1] = igt_request_alloc(arg->b.ctx, arg->engine);
+ if (IS_ERR(rq[1])) {
+ err = PTR_ERR(rq[1]);
+ goto out;
+ }
+
+ i915_request_get(rq[1]);
+ err = i915_request_await_dma_fence(rq[1], &rq[0]->fence);
+ i915_request_add(rq[1]);
+ if (err)
+ goto out;
+
+ rq[2] = spinner_create_request(&arg->b.spin,
+ arg->a.ctx, arg->engine,
+ MI_ARB_CHECK);
+ if (IS_ERR(rq[2])) {
+ err = PTR_ERR(rq[2]);
+ goto out;
+ }
+
+ i915_request_get(rq[2]);
+ err = i915_request_await_dma_fence(rq[2], &rq[1]->fence);
+ i915_request_add(rq[2]);
+ if (err)
+ goto out;
+
+ i915_gem_context_set_banned(arg->a.ctx);
+ err = intel_engine_pulse(arg->engine);
+ if (err)
+ goto out;
+
+ if (i915_request_wait(rq[2], 0, HZ / 5) < 0) {
+ err = -EIO;
+ goto out;
+ }
+
+ if (rq[0]->fence.error != -EIO) {
+ pr_err("Cancelled inflight0 request did not report -EIO\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (rq[1]->fence.error != 0) {
+ pr_err("Normal inflight1 request did not complete\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (rq[2]->fence.error != -EIO) {
+ pr_err("Cancelled queued request did not report -EIO\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+out:
+ i915_request_put(rq[2]);
+ i915_request_put(rq[1]);
+ i915_request_put(rq[0]);
+ if (igt_live_test_end(&t))
+ err = -EIO;
+ return err;
+}
+
+static int __cancel_hostile(struct live_preempt_cancel *arg)
+{
+ struct i915_request *rq;
+ int err;
+
+ /* Preempt cancel non-preemptible spinner in ELSP0 */
+ if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
+ return 0;
+
+ GEM_TRACE("%s(%s)\n", __func__, arg->engine->name);
+ clear_bit(CONTEXT_BANNED, &arg->a.ctx->flags);
+ rq = spinner_create_request(&arg->a.spin,
+ arg->a.ctx, arg->engine,
+ MI_NOOP); /* preemption disabled */
+ if (IS_ERR(rq))
+ return PTR_ERR(rq);
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+ if (!igt_wait_for_spinner(&arg->a.spin, rq)) {
+ err = -EIO;
+ goto out;
+ }
+
+ i915_gem_context_set_banned(arg->a.ctx);
+ err = intel_engine_pulse(arg->engine); /* force reset */
+ if (err)
+ goto out;
+
+ if (i915_request_wait(rq, 0, HZ / 5) < 0) {
+ err = -EIO;
+ goto out;
+ }
+
+ if (rq->fence.error != -EIO) {
+ pr_err("Cancelled inflight0 request did not report -EIO\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+out:
+ i915_request_put(rq);
+ if (igt_flush_test(arg->engine->gt))
+ err = -EIO;
+ return err;
+}
+
+static int live_preempt_cancel(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct live_preempt_cancel data;
+ enum intel_engine_id id;
+ int err = -ENOMEM;
+
+ /*
+ * To cancel an inflight context, we need to first remove it from the
+ * GPU. That sounds like preemption! Plus a little bit of bookkeeping.
+ */
+
+ if (!HAS_LOGICAL_RING_PREEMPTION(gt->i915))
+ return 0;
+
+ if (preempt_client_init(gt, &data.a))
+ return -ENOMEM;
+ if (preempt_client_init(gt, &data.b))
+ goto err_client_a;
+
+ for_each_engine(data.engine, gt, id) {
+ if (!intel_engine_has_preemption(data.engine))
+ continue;
+
+ err = __cancel_active0(&data);
+ if (err)
+ goto err_wedged;
+
+ err = __cancel_active1(&data);
+ if (err)
+ goto err_wedged;
+
+ err = __cancel_queued(&data);
+ if (err)
+ goto err_wedged;
+
+ err = __cancel_hostile(&data);
+ if (err)
+ goto err_wedged;
+ }
+
+ err = 0;
+err_client_b:
+ preempt_client_fini(&data.b);
+err_client_a:
+ preempt_client_fini(&data.a);
+ return err;
+
+err_wedged:
+ GEM_TRACE_DUMP();
+ igt_spinner_end(&data.b.spin);
+ igt_spinner_end(&data.a.spin);
+ intel_gt_set_wedged(gt);
+ goto err_client_b;
+}
+
static int live_suppress_self_preempt(void *arg)
{
struct intel_gt *gt = arg;
@@ -2702,6 +3022,7 @@ int intel_execlists_live_selftests(struct drm_i915_private *i915)
SUBTEST(live_preempt),
SUBTEST(live_late_preempt),
SUBTEST(live_nopreempt),
+ SUBTEST(live_preempt_cancel),
SUBTEST(live_suppress_self_preempt),
SUBTEST(live_suppress_wait_preempt),
SUBTEST(live_chain_preempt),
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 12/16] drm/i915/gem: Cancel contexts when hangchecking is disabled
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (9 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 11/16] drm/i915/execlists: Cancel banned contexts on schedule-out Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 13/16] drm/i915: Replace hangcheck by heartbeats Chris Wilson
` (7 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Normally, we rely on our hangcheck to prevent persistent batches from
hogging the GPU. However, if the user disables hangcheck, this mechanism
breaks down. Despite our insistence that this is unsafe, the users are
equally insistent that they want to use endless batches and will disable
the hangcheck mechanism. We are looking at replacing hangcheck, in the
next patch, with a softer mechanism, that sends a pulse down the engine
to check if it is well. We can use the same preemptive pulse to flush an
active context off the GPU upon context close, preventing resources
being lost and unkillable requests remaining on the GPU after process
termination.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 141 ++++++++++++++++++++
1 file changed, 141 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7b01f4605f21..b2f042d87be0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -70,6 +70,7 @@
#include <drm/i915_drm.h>
#include "gt/intel_lrc_reg.h"
+#include "gt/intel_engine_heartbeat.h"
#include "gt/intel_engine_user.h"
#include "i915_gem_context.h"
@@ -276,6 +277,135 @@ void i915_gem_context_release(struct kref *ref)
schedule_work(&gc->free_work);
}
+static inline struct i915_gem_engines *
+__context_engines_static(const struct i915_gem_context *ctx)
+{
+ return rcu_dereference_protected(ctx->engines, true);
+}
+
+static bool __reset_engine(struct intel_engine_cs *engine)
+{
+ struct intel_gt *gt = engine->gt;
+ bool success = false;
+
+ if (!intel_has_reset_engine(gt))
+ return false;
+
+ if (!test_and_set_bit(I915_RESET_ENGINE + engine->id,
+ >->reset.flags)) {
+ success = intel_engine_reset(engine, NULL) == 0;
+ clear_and_wake_up_bit(I915_RESET_ENGINE + engine->id,
+ >->reset.flags);
+ }
+
+ return success;
+}
+
+static void __reset_context(struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine)
+{
+ intel_gt_handle_error(engine->gt, engine->mask, 0,
+ "context closure in %s", ctx->name);
+}
+
+static bool __cancel_engine(struct intel_engine_cs *engine)
+{
+ /*
+ * Send a "high priority pulse" down the engine to cause the
+ * current request to be momentarily preempted. (If it fails to
+ * be preempted, it will be reset). As we have marked our context
+ * as banned, any incomplete request, including any running, will
+ * be skipped following the preemption.
+ *
+ * If there is no hangchecking (one of the reasons why we try to
+ * cancel the context) and no forced preemption, there may be no
+ * means by which we reset the GPU and evict the persistent hog.
+ * Ergo if we are unable to inject a preemptive pulse that can
+ * kill the banned context, we fallback to doing a local reset
+ * instead.
+ */
+ if (CONFIG_DRM_I915_PREEMPT_TIMEOUT && !intel_engine_pulse(engine))
+ return true;
+
+ /* If we are unable to send a pulse, try resetting this engine. */
+ return __reset_engine(engine);
+}
+
+static struct intel_engine_cs *
+active_engine(struct dma_fence *fence, struct intel_context *ce)
+{
+ struct i915_request *rq = to_request(fence);
+ struct intel_engine_cs *engine, *locked;
+
+ /*
+ * Serialise with __i915_request_submit() so that it sees
+ * is-banned?, or we know the request is already inflight.
+ */
+ locked = READ_ONCE(rq->engine);
+ spin_lock_irq(&locked->active.lock);
+ while (unlikely(locked != (engine = READ_ONCE(rq->engine)))) {
+ spin_unlock(&locked->active.lock);
+ spin_lock(&engine->active.lock);
+ locked = engine;
+ }
+
+ engine = NULL;
+ if (i915_request_is_active(rq) && !rq->fence.error)
+ engine = rq->engine;
+
+ spin_unlock_irq(&locked->active.lock);
+
+ return engine;
+}
+
+static void kill_context(struct i915_gem_context *ctx)
+{
+ struct i915_gem_engines_iter it;
+ struct intel_context *ce;
+
+ /*
+ * If we are already banned, it was due to a guilty request causing
+ * a reset and the entire context being evicted from the GPU.
+ */
+ if (i915_gem_context_is_banned(ctx))
+ return;
+
+ i915_gem_context_set_banned(ctx);
+
+ /*
+ * Map the user's engine back to the actual engines; one virtual
+ * engine will be mapped to multiple engines, and using ctx->engine[]
+ * the same engine may be have multiple instances in the user's map.
+ * However, we only care about pending requests, so only include
+ * engines on which there are incomplete requests.
+ */
+ for_each_gem_engine(ce, __context_engines_static(ctx), it) {
+ struct intel_engine_cs *engine;
+ struct dma_fence *fence;
+
+ if (!ce->timeline)
+ continue;
+
+ fence = i915_active_fence_get(&ce->timeline->last_request);
+ if (!fence)
+ continue;
+
+ /* Check with the backend if the request is still inflight */
+ engine = active_engine(fence, ce);
+
+ /* First attempt to gracefully cancel the context */
+ if (engine && !__cancel_engine(engine))
+ /*
+ * If we are unable to send a preemptive pulse to bump
+ * the context from the GPU, we have to resort to a full
+ * reset. We hope the collateral damage is worth it.
+ */
+ __reset_context(ctx, engine);
+
+ dma_fence_put(fence);
+ }
+}
+
static void context_close(struct i915_gem_context *ctx)
{
struct i915_address_space *vm;
@@ -298,6 +428,17 @@ static void context_close(struct i915_gem_context *ctx)
lut_close(ctx);
mutex_unlock(&ctx->mutex);
+
+ /*
+ * If the user has disabled hangchecking, we can not be sure that
+ * the batches will ever complete after the context is closed,
+ * keeping the context and all resources pinned forever. So in this
+ * case we opt to forcibly kill off all remaining requests on
+ * context close.
+ */
+ if (!i915_modparams.enable_hangcheck)
+ kill_context(ctx);
+
i915_gem_context_put(ctx);
}
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 13/16] drm/i915: Replace hangcheck by heartbeats
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (10 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 12/16] drm/i915/gem: Cancel contexts when hangchecking is disabled Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 14/16] drm/i915/gem: Make context persistence optional Chris Wilson
` (6 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Replace sampling the engine state every so often with a periodic
heartbeat request to measure the health of an engine. This is coupled
with the forced-preemption to allow long running requests to survive so
long as they do not block other users.
The heartbeat interval can be adjusted per-engine using,
/sys/class/drm/card?/engine/*/heartbeat_interval_ms
v2: Couple in sysfs controls
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/Kconfig.profile | 14 +
drivers/gpu/drm/i915/Makefile | 1 -
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_object.h | 1 -
drivers/gpu/drm/i915/gem/i915_gem_pm.c | 2 -
drivers/gpu/drm/i915/gt/intel_engine.h | 32 --
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 12 +-
.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 161 ++++++++
.../gpu/drm/i915/gt/intel_engine_heartbeat.h | 8 +
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 5 +-
drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 38 ++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 17 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 1 -
drivers/gpu/drm/i915/gt/intel_gt.h | 4 -
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 1 -
drivers/gpu/drm/i915/gt/intel_gt_types.h | 9 -
drivers/gpu/drm/i915/gt/intel_hangcheck.c | 361 ------------------
drivers/gpu/drm/i915/gt/intel_reset.c | 3 +-
.../drm/i915/gt/selftest_engine_heartbeat.c | 198 ++++++++++
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 -
drivers/gpu/drm/i915/i915_debugfs.c | 87 -----
drivers/gpu/drm/i915/i915_drv.c | 3 -
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/i915_gpu_error.c | 33 +-
drivers/gpu/drm/i915/i915_gpu_error.h | 2 -
drivers/gpu/drm/i915/i915_priolist_types.h | 6 +
.../drm/i915/selftests/i915_live_selftests.h | 1 +
27 files changed, 452 insertions(+), 555 deletions(-)
delete mode 100644 drivers/gpu/drm/i915/gt/intel_hangcheck.c
create mode 100644 drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
index 00bd37aa88d3..1bb3970aa3ac 100644
--- a/drivers/gpu/drm/i915/Kconfig.profile
+++ b/drivers/gpu/drm/i915/Kconfig.profile
@@ -58,3 +58,17 @@ config DRM_I915_PREEMPT_TIMEOUT
/sys/class/drm/card?/engine/*/preempt_timeout_ms
May be 0 to disable the timeout.
+
+config DRM_I915_HEARTBEAT_INTERVAL
+ int "Interval between heartbeat pulses (ms)"
+ default 2500 # milliseconds
+ help
+ The driver sends a periodic heartbeat down all active engines to
+ check the health of the GPU and undertake regular house-keeping of
+ internal driver state.
+
+ This is adjustable via
+ /sys/class/drm/card?/engine/*/heartbeat_interval_ms
+
+ May be 0 to disable heartbeats and therefore disable automatic GPU
+ hang detection.
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9b3268038535..a91e0a487a79 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -88,7 +88,6 @@ gt-y += \
gt/intel_gt_pm.o \
gt/intel_gt_pm_irq.o \
gt/intel_gt_requests.o \
- gt/intel_hangcheck.o \
gt/intel_llc.o \
gt/intel_lrc.o \
gt/intel_rc6.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2912abd85148..89a47d1fe364 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14863,7 +14863,7 @@ static void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
static void fb_obj_bump_render_priority(struct drm_i915_gem_object *obj)
{
struct i915_sched_attr attr = {
- .priority = I915_PRIORITY_DISPLAY,
+ .priority = I915_USER_PRIORITY(I915_PRIORITY_DISPLAY),
};
i915_gem_object_wait_priority(obj, 0, &attr);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 85921796851f..117d541ca0e8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -461,6 +461,5 @@ int i915_gem_object_wait(struct drm_i915_gem_object *obj,
int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
unsigned int flags,
const struct i915_sched_attr *attr);
-#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 7987b54fb1f5..0e97520cb1bb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -100,8 +100,6 @@ void i915_gem_suspend(struct drm_i915_private *i915)
intel_gt_suspend(&i915->gt);
intel_uc_suspend(&i915->gt.uc);
- cancel_delayed_work_sync(&i915->gt.hangcheck.work);
-
i915_gem_drain_freed_objects(i915);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index ef4b8fdc0a24..88498452b086 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -89,38 +89,6 @@ struct drm_printer;
/* seqno size is actually only a uint32, but since we plan to use MI_FLUSH_DW to
* do the writes, and that must have qw aligned offsets, simply pretend it's 8b.
*/
-enum intel_engine_hangcheck_action {
- ENGINE_IDLE = 0,
- ENGINE_WAIT,
- ENGINE_ACTIVE_SEQNO,
- ENGINE_ACTIVE_HEAD,
- ENGINE_ACTIVE_SUBUNITS,
- ENGINE_WAIT_KICK,
- ENGINE_DEAD,
-};
-
-static inline const char *
-hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
-{
- switch (a) {
- case ENGINE_IDLE:
- return "idle";
- case ENGINE_WAIT:
- return "wait";
- case ENGINE_ACTIVE_SEQNO:
- return "active seqno";
- case ENGINE_ACTIVE_HEAD:
- return "active head";
- case ENGINE_ACTIVE_SUBUNITS:
- return "active subunits";
- case ENGINE_WAIT_KICK:
- return "wait kick";
- case ENGINE_DEAD:
- return "dead";
- }
-
- return "unknown";
-}
static inline unsigned int
execlists_num_ports(const struct intel_engine_execlists * const execlists)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 7e8d3cc2459c..48173ae8ad83 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -310,6 +310,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
engine->props.preempt_timeout_ms =
CONFIG_DRM_I915_PREEMPT_TIMEOUT;
+ engine->props.heartbeat_interval_ms =
+ CONFIG_DRM_I915_HEARTBEAT_INTERVAL;
engine->props.timeslice_duration_ms =
CONFIG_DRM_I915_TIMESLICE_DURATION;
@@ -607,7 +609,6 @@ static int intel_engine_setup_common(struct intel_engine_cs *engine)
intel_engine_init_active(engine, ENGINE_PHYSICAL);
intel_engine_init_breadcrumbs(engine);
intel_engine_init_execlists(engine);
- intel_engine_init_hangcheck(engine);
intel_engine_init_cmd_parser(engine);
intel_engine_init__pm(engine);
@@ -1453,8 +1454,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
drm_printf(m, "*** WEDGED ***\n");
drm_printf(m, "\tAwake? %d\n", atomic_read(&engine->wakeref.count));
- drm_printf(m, "\tHangcheck: %d ms ago\n",
- jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp));
+
+ rcu_read_lock();
+ rq = READ_ONCE(engine->heartbeat.systole);
+ if (rq)
+ drm_printf(m, "\tHeartbeat: %d ms ago\n",
+ jiffies_to_msecs(jiffies - rq->emitted_jiffies));
+ rcu_read_unlock();
drm_printf(m, "\tReset count: %d (global %d)\n",
i915_reset_engine_count(error, engine),
i915_reset_count(error));
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 2fc413f9d506..5bd6b83d679a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -11,6 +11,30 @@
#include "intel_engine_pm.h"
#include "intel_engine.h"
#include "intel_gt.h"
+#include "intel_reset.h"
+
+/*
+ * While the engine is active, we send a periodic pulse along the engine
+ * to check on its health and to flush any idle-barriers. If that request
+ * is stuck, and we fail to preempt it, we declare the engine hung and
+ * issue a reset -- in the hope that restores progress.
+ */
+
+static bool next_heartbeat(struct intel_engine_cs *engine)
+{
+ long delay;
+
+ delay = READ_ONCE(engine->props.heartbeat_interval_ms);
+ if (!delay)
+ return false;
+
+ delay = msecs_to_jiffies_timeout(delay);
+ if (delay >= HZ)
+ delay = round_jiffies_up_relative(delay);
+ schedule_delayed_work(&engine->heartbeat.work, delay);
+
+ return true;
+}
static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
{
@@ -18,6 +42,139 @@ static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
i915_request_add_active_barriers(rq);
}
+static void show_heartbeat(const struct i915_request *rq,
+ struct intel_engine_cs *engine)
+{
+ struct drm_printer p = drm_debug_printer("heartbeat");
+
+ intel_engine_dump(engine, &p,
+ "%s heartbeat {prio:%d} not ticking\n",
+ engine->name,
+ rq->sched.attr.priority);
+}
+
+static void heartbeat(struct work_struct *wrk)
+{
+ struct i915_sched_attr attr = {
+ .priority = I915_USER_PRIORITY(I915_PRIORITY_MIN),
+ };
+ struct intel_engine_cs *engine =
+ container_of(wrk, typeof(*engine), heartbeat.work.work);
+ struct intel_context *ce = engine->kernel_context;
+ struct i915_request *rq;
+
+ if (!intel_engine_pm_get_if_awake(engine))
+ return;
+
+ rq = engine->heartbeat.systole;
+ if (rq && i915_request_completed(rq)) {
+ i915_request_put(rq);
+ engine->heartbeat.systole = NULL;
+ }
+
+ if (intel_gt_is_wedged(engine->gt))
+ goto out;
+
+ if (engine->heartbeat.systole) {
+ if (engine->schedule &&
+ rq->sched.attr.priority < I915_PRIORITY_BARRIER) {
+ /*
+ * Gradually raise the priority of the heartbeat to
+ * give high priority work [which presumably desires
+ * low latency and no jitter] the chance to naturally
+ * complete before being preempted.
+ */
+ attr.priority = I915_PRIORITY_MASK;
+ if (rq->sched.attr.priority >= attr.priority)
+ attr.priority |= I915_USER_PRIORITY(I915_PRIORITY_HEARTBEAT);
+ if (rq->sched.attr.priority >= attr.priority)
+ attr.priority = I915_PRIORITY_BARRIER;
+
+ local_bh_disable();
+ engine->schedule(rq, &attr);
+ local_bh_enable();
+ } else {
+ if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+ show_heartbeat(rq, engine);
+
+ intel_gt_handle_error(engine->gt, engine->mask,
+ I915_ERROR_CAPTURE,
+ "stopped heartbeat on %s",
+ engine->name);
+ }
+ goto out;
+ }
+
+ if (engine->wakeref_serial == engine->serial)
+ goto out;
+
+ mutex_lock(&ce->timeline->mutex);
+
+ intel_context_enter(ce);
+ rq = __i915_request_create(ce, GFP_NOWAIT | __GFP_NOWARN);
+ intel_context_exit(ce);
+ if (IS_ERR(rq))
+ goto unlock;
+
+ idle_pulse(engine, rq);
+ if (i915_modparams.enable_hangcheck)
+ engine->heartbeat.systole = i915_request_get(rq);
+
+ __i915_request_commit(rq);
+ __i915_request_queue(rq, &attr);
+
+unlock:
+ mutex_unlock(&ce->timeline->mutex);
+out:
+ if (!next_heartbeat(engine))
+ i915_request_put(fetch_and_zero(&engine->heartbeat.systole));
+ intel_engine_pm_put(engine);
+}
+
+void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine)
+{
+ if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
+ return;
+
+ next_heartbeat(engine);
+}
+
+void intel_engine_park_heartbeat(struct intel_engine_cs *engine)
+{
+ cancel_delayed_work(&engine->heartbeat.work);
+ i915_request_put(fetch_and_zero(&engine->heartbeat.systole));
+}
+
+void intel_engine_init_heartbeat(struct intel_engine_cs *engine)
+{
+ INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat);
+}
+
+int intel_engine_set_heartbeat(struct intel_engine_cs *engine,
+ unsigned long delay)
+{
+ int err;
+
+ /* Send one last pulse before to cleanup persistent hogs */
+ if (!delay && CONFIG_DRM_I915_PREEMPT_TIMEOUT) {
+ err = intel_engine_pulse(engine);
+ if (err)
+ return err;
+ }
+
+ WRITE_ONCE(engine->props.heartbeat_interval_ms, delay);
+
+ if (intel_engine_pm_get_if_awake(engine)) {
+ if (delay)
+ intel_engine_unpark_heartbeat(engine);
+ else
+ intel_engine_park_heartbeat(engine);
+ intel_engine_pm_put(engine);
+ }
+
+ return 0;
+}
+
int intel_engine_pulse(struct intel_engine_cs *engine)
{
struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER };
@@ -54,3 +211,7 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
intel_engine_pm_put(engine);
return err;
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_engine_heartbeat.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
index b950451b5998..ba85b8c2d82b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
@@ -9,6 +9,14 @@
struct intel_engine_cs;
+void intel_engine_init_heartbeat(struct intel_engine_cs *engine);
+
+int intel_engine_set_heartbeat(struct intel_engine_cs *engine,
+ unsigned long delay);
+
+void intel_engine_park_heartbeat(struct intel_engine_cs *engine);
+void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine);
+
int intel_engine_pulse(struct intel_engine_cs *engine);
#endif /* INTEL_ENGINE_HEARTBEAT_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 7d76611d9df1..6fbfa2162e54 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -7,6 +7,7 @@
#include "i915_drv.h"
#include "intel_engine.h"
+#include "intel_engine_heartbeat.h"
#include "intel_engine_pm.h"
#include "intel_engine_pool.h"
#include "intel_gt.h"
@@ -34,7 +35,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
if (engine->unpark)
engine->unpark(engine);
- intel_engine_init_hangcheck(engine);
+ intel_engine_unpark_heartbeat(engine);
return 0;
}
@@ -158,6 +159,7 @@ static int __engine_park(struct intel_wakeref *wf)
call_idle_barriers(engine); /* cleanup after wedging */
+ intel_engine_park_heartbeat(engine);
intel_engine_disarm_breadcrumbs(engine);
intel_engine_pool_park(&engine->pool);
@@ -188,6 +190,7 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
struct intel_runtime_pm *rpm = engine->uncore->rpm;
intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
+ intel_engine_init_heartbeat(engine);
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
index 27c144b77020..84a2956c7300 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -9,6 +9,8 @@
#include "i915_drv.h"
#include "intel_engine.h"
+#include "intel_engine_heartbeat.h"
+#include "intel_engine_pm.h"
#include "intel_engine_sysfs.h"
struct kobj_engine {
@@ -227,6 +229,39 @@ preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
static struct kobj_attribute preempt_timeout_attr =
__ATTR(preempt_timeout_ms, 0644, preempt_timeout_show, preempt_timeout_store);
+static ssize_t
+heartbeat_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+
+ return sprintf(buf, "%lu\n", engine->props.heartbeat_interval_ms);
+}
+
+static ssize_t
+heartbeat_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct intel_engine_cs *engine = kobj_to_engine(kobj);
+ unsigned long long delay;
+ int err;
+
+ err = kstrtoull(buf, 0, &delay);
+ if (err)
+ return err;
+
+ if (delay >= jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
+ return -EINVAL;
+
+ err = intel_engine_set_heartbeat(engine, delay);
+ if (err)
+ return err;
+
+ return count;
+}
+
+static struct kobj_attribute heartbeat_interval_attr =
+__ATTR(heartbeat_interval_ms, 0644, heartbeat_show, heartbeat_store);
+
static void kobj_engine_release(struct kobject *kobj)
{
kfree(kobj);
@@ -267,6 +302,9 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
&mmio_attr.attr,
&caps_attr.attr,
&all_caps_attr.attr,
+#if CONFIG_DRM_I915_HEARTBEAT_INTERVAL
+ &heartbeat_interval_attr.attr,
+#endif
NULL
};
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 01926a826d52..b590704f9822 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -15,6 +15,7 @@
#include <linux/rbtree.h>
#include <linux/timer.h>
#include <linux/types.h>
+#include <linux/workqueue.h>
#include "i915_gem.h"
#include "i915_pmu.h"
@@ -76,14 +77,6 @@ struct intel_instdone {
u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
};
-struct intel_engine_hangcheck {
- u64 acthd;
- u32 last_ring;
- u32 last_head;
- unsigned long action_timestamp;
- struct intel_instdone instdone;
-};
-
struct intel_ring {
struct kref ref;
struct i915_vma *vma;
@@ -331,6 +324,11 @@ struct intel_engine_cs {
intel_engine_mask_t saturated; /* submitting semaphores too late? */
+ struct {
+ struct delayed_work work;
+ struct i915_request *systole;
+ } heartbeat;
+
unsigned long serial;
unsigned long wakeref_serial;
@@ -481,8 +479,6 @@ struct intel_engine_cs {
/* status_notifier: list of callbacks for context-switch changes */
struct atomic_notifier_head context_status_notifier;
- struct intel_engine_hangcheck hangcheck;
-
#define I915_ENGINE_NEEDS_CMD_PARSER BIT(0)
#define I915_ENGINE_SUPPORTS_STATS BIT(1)
#define I915_ENGINE_HAS_PREEMPTION BIT(2)
@@ -549,6 +545,7 @@ struct intel_engine_cs {
} stats;
struct {
+ unsigned long heartbeat_interval_ms;
unsigned long preempt_timeout_ms;
unsigned long timeslice_duration_ms;
} props;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 1c4b6c9642ad..35127699591d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -22,7 +22,6 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
INIT_LIST_HEAD(>->closed_vma);
spin_lock_init(>->closed_lock);
- intel_gt_init_hangcheck(gt);
intel_gt_init_reset(gt);
intel_gt_init_requests(gt);
intel_gt_pm_init_early(gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index e6ab0bff0efb..5b6effed3713 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -46,8 +46,6 @@ void intel_gt_clear_error_registers(struct intel_gt *gt,
void intel_gt_flush_ggtt_writes(struct intel_gt *gt);
void intel_gt_chipset_flush(struct intel_gt *gt);
-void intel_gt_init_hangcheck(struct intel_gt *gt);
-
static inline u32 intel_gt_scratch_offset(const struct intel_gt *gt,
enum intel_gt_scratch_field field)
{
@@ -59,6 +57,4 @@ static inline bool intel_gt_is_wedged(struct intel_gt *gt)
return __intel_reset_failed(>->reset);
}
-void intel_gt_queue_hangcheck(struct intel_gt *gt);
-
#endif /* __INTEL_GT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index b866d5b1eee0..d5783522281c 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -52,7 +52,6 @@ static int __gt_unpark(struct intel_wakeref *wf)
i915_pmu_gt_unparked(i915);
- intel_gt_queue_hangcheck(gt);
intel_gt_unpark_requests(gt);
pm_notify(gt, INTEL_GT_UNPARK);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index ae4aaf75ac78..17883072786e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -27,14 +27,6 @@ struct i915_ggtt;
struct intel_engine_cs;
struct intel_uncore;
-struct intel_hangcheck {
- /* For hangcheck timer */
-#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
-#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
-
- struct delayed_work work;
-};
-
struct intel_gt {
struct drm_i915_private *i915;
struct intel_uncore *uncore;
@@ -68,7 +60,6 @@ struct intel_gt {
struct list_head closed_vma;
spinlock_t closed_lock; /* guards the list of closed_vma */
- struct intel_hangcheck hangcheck;
struct intel_reset reset;
/**
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
deleted file mode 100644
index 0fdef00af9e4..000000000000
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ /dev/null
@@ -1,361 +0,0 @@
-/*
- * Copyright © 2016 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- */
-
-#include "i915_drv.h"
-#include "intel_engine.h"
-#include "intel_gt.h"
-#include "intel_reset.h"
-
-struct hangcheck {
- u64 acthd;
- u32 ring;
- u32 head;
- enum intel_engine_hangcheck_action action;
- unsigned long action_timestamp;
- int deadlock;
- struct intel_instdone instdone;
- bool wedged:1;
- bool stalled:1;
-};
-
-static bool instdone_unchanged(u32 current_instdone, u32 *old_instdone)
-{
- u32 tmp = current_instdone | *old_instdone;
- bool unchanged;
-
- unchanged = tmp == *old_instdone;
- *old_instdone |= tmp;
-
- return unchanged;
-}
-
-static bool subunits_stuck(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *dev_priv = engine->i915;
- const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
- struct intel_instdone instdone;
- struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
- bool stuck;
- int slice;
- int subslice;
-
- intel_engine_get_instdone(engine, &instdone);
-
- /* There might be unstable subunit states even when
- * actual head is not moving. Filter out the unstable ones by
- * accumulating the undone -> done transitions and only
- * consider those as progress.
- */
- stuck = instdone_unchanged(instdone.instdone,
- &accu_instdone->instdone);
- stuck &= instdone_unchanged(instdone.slice_common,
- &accu_instdone->slice_common);
-
- for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice) {
- stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
- &accu_instdone->sampler[slice][subslice]);
- stuck &= instdone_unchanged(instdone.row[slice][subslice],
- &accu_instdone->row[slice][subslice]);
- }
-
- return stuck;
-}
-
-static enum intel_engine_hangcheck_action
-head_stuck(struct intel_engine_cs *engine, u64 acthd)
-{
- if (acthd != engine->hangcheck.acthd) {
-
- /* Clear subunit states on head movement */
- memset(&engine->hangcheck.instdone, 0,
- sizeof(engine->hangcheck.instdone));
-
- return ENGINE_ACTIVE_HEAD;
- }
-
- if (!subunits_stuck(engine))
- return ENGINE_ACTIVE_SUBUNITS;
-
- return ENGINE_DEAD;
-}
-
-static enum intel_engine_hangcheck_action
-engine_stuck(struct intel_engine_cs *engine, u64 acthd)
-{
- enum intel_engine_hangcheck_action ha;
- u32 tmp;
-
- ha = head_stuck(engine, acthd);
- if (ha != ENGINE_DEAD)
- return ha;
-
- if (IS_GEN(engine->i915, 2))
- return ENGINE_DEAD;
-
- /* Is the chip hanging on a WAIT_FOR_EVENT?
- * If so we can simply poke the RB_WAIT bit
- * and break the hang. This should work on
- * all but the second generation chipsets.
- */
- tmp = ENGINE_READ(engine, RING_CTL);
- if (tmp & RING_WAIT) {
- intel_gt_handle_error(engine->gt, engine->mask, 0,
- "stuck wait on %s", engine->name);
- ENGINE_WRITE(engine, RING_CTL, tmp);
- return ENGINE_WAIT_KICK;
- }
-
- return ENGINE_DEAD;
-}
-
-static void hangcheck_load_sample(struct intel_engine_cs *engine,
- struct hangcheck *hc)
-{
- hc->acthd = intel_engine_get_active_head(engine);
- hc->ring = ENGINE_READ(engine, RING_START);
- hc->head = ENGINE_READ(engine, RING_HEAD);
-}
-
-static void hangcheck_store_sample(struct intel_engine_cs *engine,
- const struct hangcheck *hc)
-{
- engine->hangcheck.acthd = hc->acthd;
- engine->hangcheck.last_ring = hc->ring;
- engine->hangcheck.last_head = hc->head;
-}
-
-static enum intel_engine_hangcheck_action
-hangcheck_get_action(struct intel_engine_cs *engine,
- const struct hangcheck *hc)
-{
- if (intel_engine_is_idle(engine))
- return ENGINE_IDLE;
-
- if (engine->hangcheck.last_ring != hc->ring)
- return ENGINE_ACTIVE_SEQNO;
-
- if (engine->hangcheck.last_head != hc->head)
- return ENGINE_ACTIVE_SEQNO;
-
- return engine_stuck(engine, hc->acthd);
-}
-
-static void hangcheck_accumulate_sample(struct intel_engine_cs *engine,
- struct hangcheck *hc)
-{
- unsigned long timeout = I915_ENGINE_DEAD_TIMEOUT;
-
- hc->action = hangcheck_get_action(engine, hc);
-
- /* We always increment the progress
- * if the engine is busy and still processing
- * the same request, so that no single request
- * can run indefinitely (such as a chain of
- * batches). The only time we do not increment
- * the hangcheck score on this ring, if this
- * engine is in a legitimate wait for another
- * engine. In that case the waiting engine is a
- * victim and we want to be sure we catch the
- * right culprit. Then every time we do kick
- * the ring, make it as a progress as the seqno
- * advancement might ensure and if not, it
- * will catch the hanging engine.
- */
-
- switch (hc->action) {
- case ENGINE_IDLE:
- case ENGINE_ACTIVE_SEQNO:
- /* Clear head and subunit states on seqno movement */
- hc->acthd = 0;
-
- memset(&engine->hangcheck.instdone, 0,
- sizeof(engine->hangcheck.instdone));
-
- /* Intentional fall through */
- case ENGINE_WAIT_KICK:
- case ENGINE_WAIT:
- engine->hangcheck.action_timestamp = jiffies;
- break;
-
- case ENGINE_ACTIVE_HEAD:
- case ENGINE_ACTIVE_SUBUNITS:
- /*
- * Seqno stuck with still active engine gets leeway,
- * in hopes that it is just a long shader.
- */
- timeout = I915_SEQNO_DEAD_TIMEOUT;
- break;
-
- case ENGINE_DEAD:
- break;
-
- default:
- MISSING_CASE(hc->action);
- }
-
- hc->stalled = time_after(jiffies,
- engine->hangcheck.action_timestamp + timeout);
- hc->wedged = time_after(jiffies,
- engine->hangcheck.action_timestamp +
- I915_ENGINE_WEDGED_TIMEOUT);
-}
-
-static void hangcheck_declare_hang(struct intel_gt *gt,
- intel_engine_mask_t hung,
- intel_engine_mask_t stuck)
-{
- struct intel_engine_cs *engine;
- intel_engine_mask_t tmp;
- char msg[80];
- int len;
-
- /* If some rings hung but others were still busy, only
- * blame the hanging rings in the synopsis.
- */
- if (stuck != hung)
- hung &= ~stuck;
- len = scnprintf(msg, sizeof(msg),
- "%s on ", stuck == hung ? "no progress" : "hang");
- for_each_engine_masked(engine, gt, hung, tmp)
- len += scnprintf(msg + len, sizeof(msg) - len,
- "%s, ", engine->name);
- msg[len-2] = '\0';
-
- return intel_gt_handle_error(gt, hung, I915_ERROR_CAPTURE, "%s", msg);
-}
-
-/*
- * This is called when the chip hasn't reported back with completed
- * batchbuffers in a long time. We keep track per ring seqno progress and
- * if there are no progress, hangcheck score for that ring is increased.
- * Further, acthd is inspected to see if the ring is stuck. On stuck case
- * we kick the ring. If we see no progress on three subsequent calls
- * we assume chip is wedged and try to fix it by resetting the chip.
- */
-static void hangcheck_elapsed(struct work_struct *work)
-{
- struct intel_gt *gt =
- container_of(work, typeof(*gt), hangcheck.work.work);
- intel_engine_mask_t hung = 0, stuck = 0, wedged = 0;
- struct intel_engine_cs *engine;
- enum intel_engine_id id;
- intel_wakeref_t wakeref;
-
- if (!i915_modparams.enable_hangcheck)
- return;
-
- if (!READ_ONCE(gt->awake))
- return;
-
- if (intel_gt_is_wedged(gt))
- return;
-
- wakeref = intel_runtime_pm_get_if_in_use(gt->uncore->rpm);
- if (!wakeref)
- return;
-
- /* As enabling the GPU requires fairly extensive mmio access,
- * periodically arm the mmio checker to see if we are triggering
- * any invalid access.
- */
- intel_uncore_arm_unclaimed_mmio_detection(gt->uncore);
-
- for_each_engine(engine, gt, id) {
- struct hangcheck hc;
-
- intel_engine_breadcrumbs_irq(engine);
-
- hangcheck_load_sample(engine, &hc);
- hangcheck_accumulate_sample(engine, &hc);
- hangcheck_store_sample(engine, &hc);
-
- if (hc.stalled) {
- hung |= engine->mask;
- if (hc.action != ENGINE_DEAD)
- stuck |= engine->mask;
- }
-
- if (hc.wedged)
- wedged |= engine->mask;
- }
-
- if (GEM_SHOW_DEBUG() && (hung | stuck)) {
- struct drm_printer p = drm_debug_printer("hangcheck");
-
- for_each_engine(engine, gt, id) {
- if (intel_engine_is_idle(engine))
- continue;
-
- intel_engine_dump(engine, &p, "%s\n", engine->name);
- }
- }
-
- if (wedged) {
- dev_err(gt->i915->drm.dev,
- "GPU recovery timed out,"
- " cancelling all in-flight rendering.\n");
- GEM_TRACE_DUMP();
- intel_gt_set_wedged(gt);
- }
-
- if (hung)
- hangcheck_declare_hang(gt, hung, stuck);
-
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
-
- /* Reset timer in case GPU hangs without another request being added */
- intel_gt_queue_hangcheck(gt);
-}
-
-void intel_gt_queue_hangcheck(struct intel_gt *gt)
-{
- unsigned long delay;
-
- if (unlikely(!i915_modparams.enable_hangcheck))
- return;
-
- /*
- * Don't continually defer the hangcheck so that it is always run at
- * least once after work has been scheduled on any ring. Otherwise,
- * we will ignore a hung ring if a second ring is kept busy.
- */
-
- delay = round_jiffies_up_relative(DRM_I915_HANGCHECK_JIFFIES);
- queue_delayed_work(system_long_wq, >->hangcheck.work, delay);
-}
-
-void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
-{
- memset(&engine->hangcheck, 0, sizeof(engine->hangcheck));
- engine->hangcheck.action_timestamp = jiffies;
-}
-
-void intel_gt_init_hangcheck(struct intel_gt *gt)
-{
- INIT_DELAYED_WORK(>->hangcheck.work, hangcheck_elapsed);
-}
-
-#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
-#include "selftest_hangcheck.c"
-#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index bf8d1ed4b1d8..f03e000051c1 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1024,8 +1024,6 @@ void intel_gt_reset(struct intel_gt *gt,
if (ret)
goto taint;
- intel_gt_queue_hangcheck(gt);
-
finish:
reset_finish(gt, awake);
unlock:
@@ -1353,4 +1351,5 @@ void __intel_fini_wedge(struct intel_wedge_me *w)
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftest_reset.c"
+#include "selftest_hangcheck.c"
#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
new file mode 100644
index 000000000000..92eafac8af44
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -0,0 +1,198 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include <linux/sort.h>
+
+#include "i915_drv.h"
+
+#include "i915_selftest.h"
+
+static int cmp_u32(const void *_a, const void *_b)
+{
+ const u32 *a = _a, *b = _b;
+
+ return *a - *b;
+}
+
+static int __live_heartbeat_fast(struct intel_engine_cs *engine)
+{
+ struct intel_context *ce;
+ struct i915_request *rq;
+ ktime_t t0, t1;
+ u32 times[5];
+ int err;
+ int i;
+
+ ce = intel_context_create(engine->kernel_context->gem_context,
+ engine);
+ if (IS_ERR(ce))
+ return PTR_ERR(ce);
+
+ intel_engine_pm_get(engine);
+
+ err = intel_engine_set_heartbeat(engine, 1);
+ if (err)
+ goto err_pm;
+
+ for (i = 0; i < ARRAY_SIZE(times); i++) {
+ /* Manufacture a tick */
+ do {
+ while (READ_ONCE(engine->heartbeat.systole))
+ flush_delayed_work(&engine->heartbeat.work);
+
+ engine->serial++; /* quick, pretend we are not idle! */
+ flush_delayed_work(&engine->heartbeat.work);
+ if (!delayed_work_pending(&engine->heartbeat.work)) {
+ pr_err("%s: heartbeat did not start\n",
+ engine->name);
+ err = -EINVAL;
+ goto err_pm;
+ }
+
+ rcu_read_lock();
+ rq = READ_ONCE(engine->heartbeat.systole);
+ if (rq)
+ rq = i915_request_get_rcu(rq);
+ rcu_read_unlock();
+ } while (!rq);
+
+ t0 = ktime_get();
+ while (rq == READ_ONCE(engine->heartbeat.systole))
+ yield(); /* work is on the local cpu! */
+ t1 = ktime_get();
+
+ i915_request_put(rq);
+ times[i] = ktime_us_delta(t1, t0);
+ }
+
+ sort(times, ARRAY_SIZE(times), sizeof(times[0]), cmp_u32, NULL);
+
+ pr_info("%s: Heartbeat delay: %uus [%u, %u]\n",
+ engine->name,
+ times[ARRAY_SIZE(times) / 2],
+ times[0],
+ times[ARRAY_SIZE(times) - 1]);
+
+ /* Min work delay is 2 * 2 (worst), +1 for scheduling, +1 for slack */
+ if (times[ARRAY_SIZE(times) / 2] > jiffies_to_usecs(6)) {
+ pr_err("%s: Heartbeat delay was %uus, expected less than %dus\n",
+ engine->name,
+ times[ARRAY_SIZE(times) / 2],
+ jiffies_to_usecs(6));
+ err = -EINVAL;
+ }
+
+ intel_engine_set_heartbeat(engine, CONFIG_DRM_I915_HEARTBEAT_INTERVAL);
+err_pm:
+ intel_engine_pm_put(engine);
+ intel_context_put(ce);
+ return err;
+}
+
+static int live_heartbeat_fast(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ int err;
+
+ /* Check that the heartbeat ticks at the desired rate. */
+
+ for_each_engine(engine, gt, id) {
+ err = __live_heartbeat_fast(engine);
+ if (err)
+ break;
+ }
+
+ return err;
+}
+
+static int __live_heartbeat_off(struct intel_engine_cs *engine)
+{
+ int err;
+
+ intel_engine_pm_get(engine);
+
+ engine->serial++;
+ flush_delayed_work(&engine->heartbeat.work);
+ if (!delayed_work_pending(&engine->heartbeat.work)) {
+ pr_err("%s: heartbeat not running\n",
+ engine->name);
+ err = -EINVAL;
+ goto err_pm;
+ }
+
+ err = intel_engine_set_heartbeat(engine, 0);
+ if (err)
+ goto err_pm;
+
+ engine->serial++;
+ flush_delayed_work(&engine->heartbeat.work);
+ if (delayed_work_pending(&engine->heartbeat.work)) {
+ pr_err("%s: heartbeat still running\n",
+ engine->name);
+ err = -EINVAL;
+ goto err_beat;
+ }
+
+ if (READ_ONCE(engine->heartbeat.systole)) {
+ pr_err("%s: heartbeat still allocated\n",
+ engine->name);
+ err = -EINVAL;
+ goto err_beat;
+ }
+
+err_beat:
+ intel_engine_set_heartbeat(engine, CONFIG_DRM_I915_HEARTBEAT_INTERVAL);
+err_pm:
+ intel_engine_pm_put(engine);
+ return err;
+}
+
+static int live_heartbeat_off(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ int err;
+
+ /* Check that we can turn off heartbeat and not interrupt VIP */
+
+ for_each_engine(engine, gt, id) {
+ if (!intel_engine_has_preemption(engine))
+ continue;
+
+ err = __live_heartbeat_off(engine);
+ if (err)
+ break;
+ }
+
+ return err;
+}
+
+int intel_heartbeat_live_selftests(struct drm_i915_private *i915)
+{
+ static const struct i915_subtest tests[] = {
+ SUBTEST(live_heartbeat_fast),
+ SUBTEST(live_heartbeat_off),
+ };
+ int saved_hangcheck;
+ int err;
+
+ if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
+ return 0;
+
+ if (intel_gt_is_wedged(&i915->gt))
+ return 0;
+
+ saved_hangcheck = i915_modparams.enable_hangcheck;
+ i915_modparams.enable_hangcheck = INT_MAX;
+
+ err = intel_gt_live_subtests(tests, &i915->gt);
+
+ i915_modparams.enable_hangcheck = saved_hangcheck;
+ return err;
+}
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 54e4a962c336..b8096ff1cbed 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -1686,7 +1686,6 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
};
struct intel_gt *gt = &i915->gt;
intel_wakeref_t wakeref;
- bool saved_hangcheck;
int err;
if (!intel_has_gpu_reset(gt))
@@ -1696,12 +1695,9 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
return -EIO; /* we're long past hope of a successful reset */
wakeref = intel_runtime_pm_get(gt->uncore->rpm);
- saved_hangcheck = fetch_and_zero(&i915_modparams.enable_hangcheck);
- drain_delayed_work(>->hangcheck.work); /* flush param */
err = intel_gt_live_subtests(tests, gt);
- i915_modparams.enable_hangcheck = saved_hangcheck;
intel_runtime_pm_put(gt->uncore->rpm, wakeref);
return err;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ada57eee914a..f529a658b9ce 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1011,92 +1011,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
return ret;
}
-static void i915_instdone_info(struct drm_i915_private *dev_priv,
- struct seq_file *m,
- struct intel_instdone *instdone)
-{
- const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
- int slice;
- int subslice;
-
- seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
- instdone->instdone);
-
- if (INTEL_GEN(dev_priv) <= 3)
- return;
-
- seq_printf(m, "\t\tSC_INSTDONE: 0x%08x\n",
- instdone->slice_common);
-
- if (INTEL_GEN(dev_priv) <= 6)
- return;
-
- for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
- seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
- slice, subslice, instdone->sampler[slice][subslice]);
-
- for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
- seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
- slice, subslice, instdone->row[slice][subslice]);
-}
-
-static int i915_hangcheck_info(struct seq_file *m, void *unused)
-{
- struct drm_i915_private *i915 = node_to_i915(m->private);
- struct intel_gt *gt = &i915->gt;
- struct intel_engine_cs *engine;
- intel_wakeref_t wakeref;
- enum intel_engine_id id;
-
- seq_printf(m, "Reset flags: %lx\n", gt->reset.flags);
- if (test_bit(I915_WEDGED, >->reset.flags))
- seq_puts(m, "\tWedged\n");
- if (test_bit(I915_RESET_BACKOFF, >->reset.flags))
- seq_puts(m, "\tDevice (global) reset in progress\n");
-
- if (!i915_modparams.enable_hangcheck) {
- seq_puts(m, "Hangcheck disabled\n");
- return 0;
- }
-
- if (timer_pending(>->hangcheck.work.timer))
- seq_printf(m, "Hangcheck active, timer fires in %dms\n",
- jiffies_to_msecs(gt->hangcheck.work.timer.expires -
- jiffies));
- else if (delayed_work_pending(>->hangcheck.work))
- seq_puts(m, "Hangcheck active, work pending\n");
- else
- seq_puts(m, "Hangcheck inactive\n");
-
- seq_printf(m, "GT active? %s\n", yesno(gt->awake));
-
- with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
- for_each_engine(engine, i915, id) {
- struct intel_instdone instdone;
-
- seq_printf(m, "%s: %d ms ago\n",
- engine->name,
- jiffies_to_msecs(jiffies -
- engine->hangcheck.action_timestamp));
-
- seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
- (long long)engine->hangcheck.acthd,
- intel_engine_get_active_head(engine));
-
- intel_engine_get_instdone(engine, &instdone);
-
- seq_puts(m, "\tinstdone read =\n");
- i915_instdone_info(i915, m, &instdone);
-
- seq_puts(m, "\tinstdone accu =\n");
- i915_instdone_info(i915, m,
- &engine->hangcheck.instdone);
- }
- }
-
- return 0;
-}
-
static int ironlake_drpc_info(struct seq_file *m)
{
struct drm_i915_private *i915 = node_to_i915(m->private);
@@ -4339,7 +4253,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_guc_stage_pool", i915_guc_stage_pool, 0},
{"i915_huc_load_status", i915_huc_load_status_info, 0},
{"i915_frequency_info", i915_frequency_info, 0},
- {"i915_hangcheck_info", i915_hangcheck_info, 0},
{"i915_drpc_info", i915_drpc_info, 0},
{"i915_ring_freq_table", i915_ring_freq_table, 0},
{"i915_frontbuffer_tracking", i915_frontbuffer_tracking, 0},
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 21b36a6143b9..7418f81b3cd5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1548,10 +1548,7 @@ void i915_driver_remove(struct drm_i915_private *i915)
i915_driver_modeset_remove(i915);
- /* Free error state after interrupts are fully disabled. */
- cancel_delayed_work_sync(&i915->gt.hangcheck.work);
i915_reset_error_state(i915);
-
i915_gem_driver_remove(i915);
intel_power_domains_driver_remove(i915);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 16b85c7f7f21..f9feddfddbe4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1846,7 +1846,6 @@ void i915_driver_remove(struct drm_i915_private *i915);
int i915_resume_switcheroo(struct drm_i915_private *i915);
int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
-void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 5cf4eed5add8..47239df653f2 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -534,10 +534,6 @@ static void error_print_engine(struct drm_i915_error_state_buf *m,
}
err_printf(m, " ring->head: 0x%08x\n", ee->cpu_ring_head);
err_printf(m, " ring->tail: 0x%08x\n", ee->cpu_ring_tail);
- err_printf(m, " hangcheck timestamp: %dms (%lu%s)\n",
- jiffies_to_msecs(ee->hangcheck_timestamp - epoch),
- ee->hangcheck_timestamp,
- ee->hangcheck_timestamp == epoch ? "; epoch" : "");
err_printf(m, " engine reset count: %u\n", ee->reset_count);
for (n = 0; n < ee->num_ports; n++) {
@@ -679,11 +675,8 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
ts = ktime_to_timespec64(error->uptime);
err_printf(m, "Uptime: %lld s %ld us\n",
(s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
- err_printf(m, "Epoch: %lu jiffies (%u HZ)\n", error->epoch, HZ);
- err_printf(m, "Capture: %lu jiffies; %d ms ago, %d ms after epoch\n",
- error->capture,
- jiffies_to_msecs(jiffies - error->capture),
- jiffies_to_msecs(error->capture - error->epoch));
+ err_printf(m, "Capture: %lu jiffies; %d ms ago\n",
+ error->capture, jiffies_to_msecs(jiffies - error->capture));
for (ee = error->engine; ee; ee = ee->next)
err_printf(m, "Active process (on ring %s): %s [%d]\n",
@@ -742,7 +735,7 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
err_printf(m, "GTT_CACHE_EN: 0x%08x\n", error->gtt_cache);
for (ee = error->engine; ee; ee = ee->next)
- error_print_engine(m, ee, error->epoch);
+ error_print_engine(m, ee, error->capture);
for (ee = error->engine; ee; ee = ee->next) {
const struct drm_i915_error_object *obj;
@@ -770,7 +763,7 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
for (j = 0; j < ee->num_requests; j++)
error_print_request(m, " ",
&ee->requests[j],
- error->epoch);
+ error->capture);
}
print_error_obj(m, ee->engine, "ringbuffer", ee->ringbuffer);
@@ -1144,8 +1137,6 @@ static void error_record_engine_registers(struct i915_gpu_state *error,
}
ee->idle = intel_engine_is_idle(engine);
- if (!ee->idle)
- ee->hangcheck_timestamp = engine->hangcheck.action_timestamp;
ee->reset_count = i915_reset_engine_count(&dev_priv->gpu_error,
engine);
@@ -1657,20 +1648,6 @@ static void capture_params(struct i915_gpu_state *error)
i915_params_copy(&error->params, &i915_modparams);
}
-static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
-{
- const struct drm_i915_error_engine *ee;
- unsigned long epoch = error->capture;
-
- for (ee = error->engine; ee; ee = ee->next) {
- if (ee->hangcheck_timestamp &&
- time_before(ee->hangcheck_timestamp, epoch))
- epoch = ee->hangcheck_timestamp;
- }
-
- return epoch;
-}
-
static void capture_finish(struct i915_gpu_state *error)
{
struct i915_ggtt *ggtt = &error->i915->ggtt;
@@ -1722,8 +1699,6 @@ i915_capture_gpu_state(struct drm_i915_private *i915)
error->overlay = intel_overlay_capture_error_state(i915);
error->display = intel_display_capture_error_state(i915);
- error->epoch = capture_find_epoch(error);
-
capture_finish(error);
compress_fini(&compress);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
index 7f1cd0b1fef7..4dc36d6ee3a2 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -34,7 +34,6 @@ struct i915_gpu_state {
ktime_t boottime;
ktime_t uptime;
unsigned long capture;
- unsigned long epoch;
struct drm_i915_private *i915;
@@ -86,7 +85,6 @@ struct i915_gpu_state {
/* Software tracked state */
bool idle;
- unsigned long hangcheck_timestamp;
int num_requests;
u32 reset_count;
diff --git a/drivers/gpu/drm/i915/i915_priolist_types.h b/drivers/gpu/drm/i915/i915_priolist_types.h
index ae8bb3cb627e..732aad148881 100644
--- a/drivers/gpu/drm/i915/i915_priolist_types.h
+++ b/drivers/gpu/drm/i915/i915_priolist_types.h
@@ -16,6 +16,12 @@ enum {
I915_PRIORITY_MIN = I915_CONTEXT_MIN_USER_PRIORITY - 1,
I915_PRIORITY_NORMAL = I915_CONTEXT_DEFAULT_PRIORITY,
I915_PRIORITY_MAX = I915_CONTEXT_MAX_USER_PRIORITY + 1,
+
+ /* A preemptive pulse used to monitor the health of each engine */
+ I915_PRIORITY_HEARTBEAT,
+
+ /* Interactive workload, scheduled for immediate pageflipping */
+ I915_PRIORITY_DISPLAY,
};
#define I915_USER_PRIORITY_SHIFT 2
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 1a6abcffce81..c23d06bca09e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -18,6 +18,7 @@ selftest(gt_contexts, intel_context_live_selftests)
selftest(gt_lrc, intel_lrc_live_selftests)
selftest(gt_mocs, intel_mocs_live_selftests)
selftest(gt_pm, intel_gt_pm_live_selftests)
+selftest(gt_heartbeat, intel_heartbeat_live_selftests)
selftest(requests, i915_request_live_selftests)
selftest(active, i915_active_live_selftests)
selftest(objects, i915_gem_object_live_selftests)
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 14/16] drm/i915/gem: Make context persistence optional
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (11 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 13/16] drm/i915: Replace hangcheck by heartbeats Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:02 ` [PATCH 15/16] drm/i915/gem: Distinguish each object type Chris Wilson
` (5 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
Our existing behaviour is to allow contexts and their GPU requests to
persist past the point of closure until the requests are complete. This
allows clients to operate in a 'fire-and-forget' manner where they can
setup a rendering pipeline and hand it over to the display server and
immediately exiting. As the rendering pipeline is kept alive until
completion, the display server (or other consumer) can use the results
in the future and present them to the user.
However, not all clients want this persistent behaviour and would prefer
that the contexts are cleaned up immediately upon closure. This ensures
that when clients are run without hangchecking, any GPU hang is
terminated with the process and does not continue to hog resources.
By defining a context property to allow clients to control persistence
explicitly, we can remove the blanket advice to disable hangchecking
that seems to be far too prevalent.
The default behaviour for new controls is the legacy persistence mode.
New clients will have to opt out for immediate cleanup on context
closure. If the hangchecking modparam is disabled, so is persistent
context support -- all contexts will be terminated on closure.
Testcase: igt/gem_ctx_persistence
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 50 ++++++++++++++++++-
drivers/gpu/drm/i915/gem/i915_gem_context.h | 15 ++++++
.../gpu/drm/i915/gem/i915_gem_context_types.h | 1 +
.../gpu/drm/i915/gem/selftests/mock_context.c | 2 +
include/uapi/drm/i915_drm.h | 15 ++++++
5 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index b2f042d87be0..f92fa04c8e4a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -436,12 +436,39 @@ static void context_close(struct i915_gem_context *ctx)
* case we opt to forcibly kill off all remaining requests on
* context close.
*/
- if (!i915_modparams.enable_hangcheck)
+ if (!i915_gem_context_is_persistent(ctx) ||
+ !i915_modparams.enable_hangcheck)
kill_context(ctx);
i915_gem_context_put(ctx);
}
+static int __context_set_persistence(struct i915_gem_context *ctx, bool state)
+{
+ if (i915_gem_context_is_persistent(ctx) == state)
+ return 0;
+
+ if (state) {
+ /*
+ * Only contexts that are short-lived [that will expire or be
+ * reset] are allowed to survive past termination. We require
+ * hangcheck to ensure that the persistent requests are healthy.
+ */
+ if (!i915_modparams.enable_hangcheck)
+ return -EINVAL;
+
+ i915_gem_context_set_persistence(ctx);
+ } else {
+ /* To cancel a context we use "preempt-to-idle" */
+ if (!(ctx->i915->caps.scheduler & I915_SCHEDULER_CAP_PREEMPTION))
+ return -ENODEV;
+
+ i915_gem_context_clear_persistence(ctx);
+ }
+
+ return 0;
+}
+
static struct i915_gem_context *
__create_context(struct drm_i915_private *i915)
{
@@ -476,6 +503,7 @@ __create_context(struct drm_i915_private *i915)
i915_gem_context_set_bannable(ctx);
i915_gem_context_set_recoverable(ctx);
+ __context_set_persistence(ctx, true /* cgroup hook? */);
for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
@@ -632,6 +660,7 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
return ctx;
i915_gem_context_clear_bannable(ctx);
+ i915_gem_context_set_persistence(ctx);
ctx->sched.priority = I915_USER_PRIORITY(prio);
GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
@@ -1742,6 +1771,16 @@ get_engines(struct i915_gem_context *ctx,
return err;
}
+static int
+set_persistence(struct i915_gem_context *ctx,
+ const struct drm_i915_gem_context_param *args)
+{
+ if (args->size)
+ return -EINVAL;
+
+ return __context_set_persistence(ctx, args->value);
+}
+
static int ctx_setparam(struct drm_i915_file_private *fpriv,
struct i915_gem_context *ctx,
struct drm_i915_gem_context_param *args)
@@ -1819,6 +1858,10 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv,
ret = set_engines(ctx, args);
break;
+ case I915_CONTEXT_PARAM_PERSISTENCE:
+ ret = set_persistence(ctx, args);
+ break;
+
case I915_CONTEXT_PARAM_BAN_PERIOD:
default:
ret = -EINVAL;
@@ -2271,6 +2314,11 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
ret = get_engines(ctx, args);
break;
+ case I915_CONTEXT_PARAM_PERSISTENCE:
+ args->size = 0;
+ args->value = i915_gem_context_is_persistent(ctx);
+ break;
+
case I915_CONTEXT_PARAM_BAN_PERIOD:
default:
ret = -EINVAL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index cfe80590f0ed..18e50a769a6e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -76,6 +76,21 @@ static inline void i915_gem_context_clear_recoverable(struct i915_gem_context *c
clear_bit(UCONTEXT_RECOVERABLE, &ctx->user_flags);
}
+static inline bool i915_gem_context_is_persistent(const struct i915_gem_context *ctx)
+{
+ return test_bit(UCONTEXT_PERSISTENCE, &ctx->user_flags);
+}
+
+static inline void i915_gem_context_set_persistence(struct i915_gem_context *ctx)
+{
+ set_bit(UCONTEXT_PERSISTENCE, &ctx->user_flags);
+}
+
+static inline void i915_gem_context_clear_persistence(struct i915_gem_context *ctx)
+{
+ clear_bit(UCONTEXT_PERSISTENCE, &ctx->user_flags);
+}
+
static inline bool i915_gem_context_is_banned(const struct i915_gem_context *ctx)
{
return test_bit(CONTEXT_BANNED, &ctx->flags);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index fe97b8ba4fda..861d7d92fe9f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -137,6 +137,7 @@ struct i915_gem_context {
#define UCONTEXT_NO_ERROR_CAPTURE 1
#define UCONTEXT_BANNABLE 2
#define UCONTEXT_RECOVERABLE 3
+#define UCONTEXT_PERSISTENCE 4
/**
* @flags: small set of booleans
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 74ddd682c9cd..29b8984f0e47 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -22,6 +22,8 @@ mock_context(struct drm_i915_private *i915,
INIT_LIST_HEAD(&ctx->link);
ctx->i915 = i915;
+ i915_gem_context_set_persistence(ctx);
+
mutex_init(&ctx->engines_mutex);
e = default_engines(ctx);
if (IS_ERR(e))
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 63d40cba97e0..b5fd5e4bd16e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1572,6 +1572,21 @@ struct drm_i915_gem_context_param {
* i915_context_engines_bond (I915_CONTEXT_ENGINES_EXT_BOND)
*/
#define I915_CONTEXT_PARAM_ENGINES 0xa
+
+/*
+ * I915_CONTEXT_PARAM_PERSISTENCE:
+ *
+ * Allow the context and active rendering to survive the process until
+ * completion. Persistence allows fire-and-forget clients to queue up a
+ * bunch of work, hand the output over to a display server and the quit.
+ * If the context is not marked as persistent, upon closing (either via
+ * an explicit DRM_I915_GEM_CONTEXT_DESTROY or implicitly from file closure
+ * or process termination), the context and any outstanding requests will be
+ * cancelled (and exported fences for cancelled requests marked as -EIO).
+ *
+ * By default, new contexts allow persistence.
+ */
+#define I915_CONTEXT_PARAM_PERSISTENCE 0xb
/* Must be kept compact -- no holes and well documented */
__u64 value;
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 15/16] drm/i915/gem: Distinguish each object type
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (12 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 14/16] drm/i915/gem: Make context persistence optional Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-22 14:30 ` Matthew Auld
2019-10-21 8:02 ` [PATCH 16/16] drm/i915: Flush idle barriers when waiting Chris Wilson
` (4 subsequent siblings)
18 siblings, 1 reply; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Separate each object class into a separate lock type to avoid lockdep
cross-contamination between paths (i.e. userptr!).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_object.c | 5 +++--
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 ++-
drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c | 3 ++-
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 8 +++++---
drivers/gpu/drm/i915/gvt/dmabuf.c | 3 ++-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 ++-
drivers/gpu/drm/i915/selftests/mock_region.c | 3 ++-
12 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 96ce95c8ac5a..eaea49d08eb5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -256,6 +256,7 @@ static const struct drm_i915_gem_object_ops i915_gem_object_dmabuf_ops = {
struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf)
{
+ static struct lock_class_key lock_class;
struct dma_buf_attachment *attach;
struct drm_i915_gem_object *obj;
int ret;
@@ -287,7 +288,7 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
}
drm_gem_private_object_init(dev, &obj->base, dma_buf->size);
- i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops);
+ i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops, &lock_class);
obj->base.import_attach = attach;
obj->base.resv = dma_buf->resv;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index 5ae694c24df4..9cfb0e41ff06 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -164,6 +164,7 @@ struct drm_i915_gem_object *
i915_gem_object_create_internal(struct drm_i915_private *i915,
phys_addr_t size)
{
+ static struct lock_class_key lock_class;
struct drm_i915_gem_object *obj;
unsigned int cache_level;
@@ -178,7 +179,7 @@ i915_gem_object_create_internal(struct drm_i915_private *i915,
return ERR_PTR(-ENOMEM);
drm_gem_private_object_init(&i915->drm, &obj->base, size);
- i915_gem_object_init(obj, &i915_gem_object_internal_ops);
+ i915_gem_object_init(obj, &i915_gem_object_internal_ops, &lock_class);
/*
* Mark the object as volatile, such that the pages are marked as
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index dbf9be9a79f4..a50296cce0d8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -47,9 +47,10 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj)
}
void i915_gem_object_init(struct drm_i915_gem_object *obj,
- const struct drm_i915_gem_object_ops *ops)
+ const struct drm_i915_gem_object_ops *ops,
+ struct lock_class_key *key)
{
- mutex_init(&obj->mm.lock);
+ __mutex_init(&obj->mm.lock, "obj->mm.lock", key);
spin_lock_init(&obj->vma.lock);
INIT_LIST_HEAD(&obj->vma.list);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 117d541ca0e8..458cd51331f1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -23,7 +23,8 @@ struct drm_i915_gem_object *i915_gem_object_alloc(void);
void i915_gem_object_free(struct drm_i915_gem_object *obj);
void i915_gem_object_init(struct drm_i915_gem_object *obj,
- const struct drm_i915_gem_object_ops *ops);
+ const struct drm_i915_gem_object_ops *ops,
+ struct lock_class_key *key);
struct drm_i915_gem_object *
i915_gem_object_create_shmem(struct drm_i915_private *i915,
resource_size_t size);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index be68b76e13b3..4d69c3fc3439 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -465,6 +465,7 @@ create_shmem(struct intel_memory_region *mem,
resource_size_t size,
unsigned int flags)
{
+ static struct lock_class_key lock_class;
struct drm_i915_private *i915 = mem->i915;
struct drm_i915_gem_object *obj;
struct address_space *mapping;
@@ -491,7 +492,7 @@ create_shmem(struct intel_memory_region *mem,
mapping_set_gfp_mask(mapping, mask);
GEM_BUG_ON(!(mapping_gfp_mask(mapping) & __GFP_RECLAIM));
- i915_gem_object_init(obj, &i915_gem_shmem_ops);
+ i915_gem_object_init(obj, &i915_gem_shmem_ops, &lock_class);
obj->write_domain = I915_GEM_DOMAIN_CPU;
obj->read_domains = I915_GEM_DOMAIN_CPU;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 57cd8bc2657c..a2d49c04e6a4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -556,6 +556,7 @@ __i915_gem_object_create_stolen(struct drm_i915_private *dev_priv,
struct drm_mm_node *stolen,
struct intel_memory_region *mem)
{
+ static struct lock_class_key lock_class;
struct drm_i915_gem_object *obj;
unsigned int cache_level;
int err = -ENOMEM;
@@ -565,7 +566,7 @@ __i915_gem_object_create_stolen(struct drm_i915_private *dev_priv,
goto err;
drm_gem_private_object_init(&dev_priv->drm, &obj->base, stolen->size);
- i915_gem_object_init(obj, &i915_gem_object_stolen_ops);
+ i915_gem_object_init(obj, &i915_gem_object_stolen_ops, &lock_class);
obj->stolen = stolen;
obj->read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 4f970474013f..1e045c337044 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -725,6 +725,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file)
{
+ static struct lock_class_key lock_class;
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_i915_gem_userptr *args = data;
struct drm_i915_gem_object *obj;
@@ -769,7 +770,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -ENOMEM;
drm_gem_private_object_init(dev, &obj->base, args->user_size);
- i915_gem_object_init(obj, &i915_gem_userptr_ops);
+ i915_gem_object_init(obj, &i915_gem_userptr_ops, &lock_class);
obj->read_domains = I915_GEM_DOMAIN_CPU;
obj->write_domain = I915_GEM_DOMAIN_CPU;
i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c b/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c
index 3c5d17b2b670..892d12db6c49 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c
@@ -96,6 +96,7 @@ huge_gem_object(struct drm_i915_private *i915,
phys_addr_t phys_size,
dma_addr_t dma_size)
{
+ static struct lock_class_key lock_class;
struct drm_i915_gem_object *obj;
unsigned int cache_level;
@@ -111,7 +112,7 @@ huge_gem_object(struct drm_i915_private *i915,
return ERR_PTR(-ENOMEM);
drm_gem_private_object_init(&i915->drm, &obj->base, dma_size);
- i915_gem_object_init(obj, &huge_ops);
+ i915_gem_object_init(obj, &huge_ops, &lock_class);
obj->read_domains = I915_GEM_DOMAIN_CPU;
obj->write_domain = I915_GEM_DOMAIN_CPU;
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index f27772f6779a..dac8344507c1 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -149,6 +149,7 @@ huge_pages_object(struct drm_i915_private *i915,
u64 size,
unsigned int page_mask)
{
+ static struct lock_class_key lock_class;
struct drm_i915_gem_object *obj;
GEM_BUG_ON(!size);
@@ -165,7 +166,7 @@ huge_pages_object(struct drm_i915_private *i915,
return ERR_PTR(-ENOMEM);
drm_gem_private_object_init(&i915->drm, &obj->base, size);
- i915_gem_object_init(obj, &huge_page_ops);
+ i915_gem_object_init(obj, &huge_page_ops, &lock_class);
i915_gem_object_set_volatile(obj);
@@ -295,6 +296,7 @@ static const struct drm_i915_gem_object_ops fake_ops_single = {
static struct drm_i915_gem_object *
fake_huge_pages_object(struct drm_i915_private *i915, u64 size, bool single)
{
+ static struct lock_class_key lock_class;
struct drm_i915_gem_object *obj;
GEM_BUG_ON(!size);
@@ -313,9 +315,9 @@ fake_huge_pages_object(struct drm_i915_private *i915, u64 size, bool single)
drm_gem_private_object_init(&i915->drm, &obj->base, size);
if (single)
- i915_gem_object_init(obj, &fake_ops_single);
+ i915_gem_object_init(obj, &fake_ops_single, &lock_class);
else
- i915_gem_object_init(obj, &fake_ops);
+ i915_gem_object_init(obj, &fake_ops, &lock_class);
i915_gem_object_set_volatile(obj);
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
index 13044c027f27..a816aef6142b 100644
--- a/drivers/gpu/drm/i915/gvt/dmabuf.c
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -152,6 +152,7 @@ static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = {
static struct drm_i915_gem_object *vgpu_create_gem(struct drm_device *dev,
struct intel_vgpu_fb_info *info)
{
+ static struct lock_class_key lock_class;
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_i915_gem_object *obj;
@@ -161,7 +162,7 @@ static struct drm_i915_gem_object *vgpu_create_gem(struct drm_device *dev,
drm_gem_private_object_init(dev, &obj->base,
roundup(info->size, PAGE_SIZE));
- i915_gem_object_init(obj, &intel_vgpu_gem_ops);
+ i915_gem_object_init(obj, &intel_vgpu_gem_ops, &lock_class);
obj->read_domains = I915_GEM_DOMAIN_GTT;
obj->write_domain = 0;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 6321bf91f5c7..399ade164b58 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -104,6 +104,7 @@ static const struct drm_i915_gem_object_ops fake_ops = {
static struct drm_i915_gem_object *
fake_dma_object(struct drm_i915_private *i915, u64 size)
{
+ static struct lock_class_key lock_class;
struct drm_i915_gem_object *obj;
GEM_BUG_ON(!size);
@@ -117,7 +118,7 @@ fake_dma_object(struct drm_i915_private *i915, u64 size)
goto err;
drm_gem_private_object_init(&i915->drm, &obj->base, size);
- i915_gem_object_init(obj, &fake_ops);
+ i915_gem_object_init(obj, &fake_ops, &lock_class);
i915_gem_object_set_volatile(obj);
diff --git a/drivers/gpu/drm/i915/selftests/mock_region.c b/drivers/gpu/drm/i915/selftests/mock_region.c
index 7b0c99ddc2d5..b2ad41c27e67 100644
--- a/drivers/gpu/drm/i915/selftests/mock_region.c
+++ b/drivers/gpu/drm/i915/selftests/mock_region.c
@@ -19,6 +19,7 @@ mock_object_create(struct intel_memory_region *mem,
resource_size_t size,
unsigned int flags)
{
+ static struct lock_class_key lock_class;
struct drm_i915_private *i915 = mem->i915;
struct drm_i915_gem_object *obj;
@@ -30,7 +31,7 @@ mock_object_create(struct intel_memory_region *mem,
return ERR_PTR(-ENOMEM);
drm_gem_private_object_init(&i915->drm, &obj->base, size);
- i915_gem_object_init(obj, &mock_region_obj_ops);
+ i915_gem_object_init(obj, &mock_region_obj_ops, &lock_class);
obj->read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT;
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 15/16] drm/i915/gem: Distinguish each object type
2019-10-21 8:02 ` [PATCH 15/16] drm/i915/gem: Distinguish each object type Chris Wilson
@ 2019-10-22 14:30 ` Matthew Auld
2019-11-04 17:51 ` [Intel-gfx] " Daniel Vetter
0 siblings, 1 reply; 26+ messages in thread
From: Matthew Auld @ 2019-10-22 14:30 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics Development, Matthew Auld
On Mon, 21 Oct 2019 at 09:03, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Separate each object class into a separate lock type to avoid lockdep
> cross-contamination between paths (i.e. userptr!).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
As per the explanation you gave on IRC, makes sense.
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 15/16] drm/i915/gem: Distinguish each object type
@ 2019-11-04 17:51 ` Daniel Vetter
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Vetter @ 2019-11-04 17:51 UTC (permalink / raw)
To: Matthew Auld; +Cc: Intel Graphics Development, Matthew Auld
On Tue, Oct 22, 2019 at 03:30:13PM +0100, Matthew Auld wrote:
> On Mon, 21 Oct 2019 at 09:03, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Separate each object class into a separate lock type to avoid lockdep
> > cross-contamination between paths (i.e. userptr!).
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld@intel.com>
>
> As per the explanation you gave on IRC, makes sense.
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
It would be really lovely if such explanations would make it from
ephemeral irc onto something more permanent like mailing lists, or even
better, commit messages :-)
So, why do we need this?
I'm wondering since aside from userptr we'll have to unify how locking for
bo works longer term (with all the dma_buf/ww_mutex stuff going on), not
make it more distinct.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Intel-gfx] [PATCH 15/16] drm/i915/gem: Distinguish each object type
@ 2019-11-04 17:51 ` Daniel Vetter
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Vetter @ 2019-11-04 17:51 UTC (permalink / raw)
To: Matthew Auld; +Cc: Intel Graphics Development, Matthew Auld
On Tue, Oct 22, 2019 at 03:30:13PM +0100, Matthew Auld wrote:
> On Mon, 21 Oct 2019 at 09:03, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Separate each object class into a separate lock type to avoid lockdep
> > cross-contamination between paths (i.e. userptr!).
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld@intel.com>
>
> As per the explanation you gave on IRC, makes sense.
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
It would be really lovely if such explanations would make it from
ephemeral irc onto something more permanent like mailing lists, or even
better, commit messages :-)
So, why do we need this?
I'm wondering since aside from userptr we'll have to unify how locking for
bo works longer term (with all the dma_buf/ww_mutex stuff going on), not
make it more distinct.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 16/16] drm/i915: Flush idle barriers when waiting
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (13 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 15/16] drm/i915/gem: Distinguish each object type Chris Wilson
@ 2019-10-21 8:02 ` Chris Wilson
2019-10-21 8:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Patchwork
` (3 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 8:02 UTC (permalink / raw)
To: intel-gfx
If we do find ourselves with an idle barrier inside our active while
waiting, attempt to flush it by emitting a pulse using the kernel
context.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 14 +++++++++++++
.../gpu/drm/i915/gt/intel_engine_heartbeat.h | 1 +
drivers/gpu/drm/i915/i915_active.c | 21 +++++++++++++++++--
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 5bd6b83d679a..a73ffb0937c8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -212,6 +212,20 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
return err;
}
+int intel_engine_flush_barriers(struct intel_engine_cs *engine)
+{
+ struct i915_request *rq;
+
+ rq = i915_request_create(engine->kernel_context);
+ if (IS_ERR(rq))
+ return PTR_ERR(rq);
+
+ idle_pulse(engine, rq);
+ i915_request_add(rq);
+
+ return 0;
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftest_engine_heartbeat.c"
#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
index ba85b8c2d82b..a7b8c0f9e005 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
@@ -18,5 +18,6 @@ void intel_engine_park_heartbeat(struct intel_engine_cs *engine);
void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine);
int intel_engine_pulse(struct intel_engine_cs *engine);
+int intel_engine_flush_barriers(struct intel_engine_cs *engine);
#endif /* INTEL_ENGINE_HEARTBEAT_H */
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 7927b1a0c7a6..da805742bf62 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -6,6 +6,7 @@
#include <linux/debugobjects.h>
+#include "gt/intel_engine_heartbeat.h"
#include "gt/intel_engine_pm.h"
#include "i915_drv.h"
@@ -435,6 +436,21 @@ static void enable_signaling(struct i915_active_fence *active)
dma_fence_put(fence);
}
+static int flush_barrier(struct active_node *it)
+{
+ struct intel_engine_cs *engine;
+
+ if (!is_barrier(&it->base))
+ return 0;
+
+ engine = __barrier_to_engine(it);
+ smp_rmb(); /* serialise with add_active_barriers */
+ if (!is_barrier(&it->base))
+ return 0;
+
+ return intel_engine_flush_barriers(engine);
+}
+
int i915_active_wait(struct i915_active *ref)
{
struct active_node *it, *n;
@@ -448,8 +464,9 @@ int i915_active_wait(struct i915_active *ref)
/* Flush lazy signals */
enable_signaling(&ref->excl);
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
- if (is_barrier(&it->base)) /* unconnected idle barrier */
- continue;
+ err = flush_barrier(it); /* unconnected idle barrier? */
+ if (err)
+ break;
enable_signaling(&it->base);
}
--
2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (14 preceding siblings ...)
2019-10-21 8:02 ` [PATCH 16/16] drm/i915: Flush idle barriers when waiting Chris Wilson
@ 2019-10-21 8:54 ` Patchwork
2019-10-21 9:01 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-10-21 8:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
URL : https://patchwork.freedesktop.org/series/68295/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
48cdac321482 drm/i915: Don't set queue_priority_hint if we don't kick the submission
c04b1d31305a drm/i915: Drop assertion that ce->pin_mutex guards state updates
9435f35a8e2b drm/i915/selftests: Add coverage of mocs registers
-:29: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#29:
new file mode 100644
-:34: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#34: FILE: drivers/gpu/drm/i915/gt/selftest_mocs.c:1:
+/*
-:35: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#35: FILE: drivers/gpu/drm/i915/gt/selftest_mocs.c:2:
+ * SPDX-License-Identifier: MIT
total: 0 errors, 3 warnings, 0 checks, 445 lines checked
7383a7e9b6bf drm/i915/selftests: Teach igt_flush_test and igt_live_test to take intel_gt
09ffac131a0d drm/i915/selftests: Force ordering of context switches
5751429a6359 drm/i915: Expose engine properties via sysfs
-:73: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#73:
new file mode 100644
-:78: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#78: FILE: drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:1:
+/*
-:79: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#79: FILE: drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:2:
+ * SPDX-License-Identifier: MIT
-:178: CHECK:SPACING: No space is necessary after a cast
#178: FILE: drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:101:
+ show_unknown ? BITS_PER_TYPE(typeof(caps)) : count) {
-:289: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#289: FILE: drivers/gpu/drm/i915/gt/intel_engine_sysfs.h:1:
+/*
-:290: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#290: FILE: drivers/gpu/drm/i915/gt/intel_engine_sysfs.h:2:
+ * SPDX-License-Identifier: MIT
total: 0 errors, 5 warnings, 1 checks, 244 lines checked
2aeae13e0baa drm/i915: Expose engine->mmio_base via sysfs
dc571802eb53 drm/i915: Expose timeslice duration to sysfs
cda9f6b21f0f drm/i915/execlists: Force preemption
263213d1e0dd drm/i915/gt: Introduce barrier pulses along engines
-:29: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#29:
new file mode 100644
-:34: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#34: FILE: drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c:1:
+/*
-:35: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#35: FILE: drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c:2:
+ * SPDX-License-Identifier: MIT
-:96: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#96: FILE: drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h:1:
+/*
-:97: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#97: FILE: drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h:2:
+ * SPDX-License-Identifier: MIT
total: 0 errors, 5 warnings, 0 checks, 92 lines checked
092761273758 drm/i915/execlists: Cancel banned contexts on schedule-out
ab9bbc283596 drm/i915/gem: Cancel contexts when hangchecking is disabled
3d0b01cdae61 drm/i915: Replace hangcheck by heartbeats
-:271: WARNING:EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to using 'heartbeat', this function's name, in a string
#271: FILE: drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c:102:
+ "stopped heartbeat on %s",
-:605: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#605:
deleted file mode 100644
-:996: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#996: FILE: drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c:1:
+/*
-:997: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#997: FILE: drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c:2:
+ * SPDX-License-Identifier: MIT
-:1059: WARNING:YIELD: Using yield() is generally wrong. See yield() kernel-doc (sched/core.c)
#1059: FILE: drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c:64:
+ yield(); /* work is on the local cpu! */
total: 0 errors, 5 warnings, 0 checks, 939 lines checked
b2bbf76bf802 drm/i915/gem: Make context persistence optional
f07fa82b62c1 drm/i915/gem: Distinguish each object type
e4eda090fa42 drm/i915: Flush idle barriers when waiting
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread* ✗ Fi.CI.SPARSE: warning for series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (15 preceding siblings ...)
2019-10-21 8:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Patchwork
@ 2019-10-21 9:01 ` Patchwork
2019-10-21 9:18 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-10-21 9:49 ` [PATCH 01/16] " Mika Kuoppala
18 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-10-21 9:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
URL : https://patchwork.freedesktop.org/series/68295/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915: Don't set queue_priority_hint if we don't kick the submission
Okay!
Commit: drm/i915: Drop assertion that ce->pin_mutex guards state updates
Okay!
Commit: drm/i915/selftests: Add coverage of mocs registers
Okay!
Commit: drm/i915/selftests: Teach igt_flush_test and igt_live_test to take intel_gt
Okay!
Commit: drm/i915/selftests: Force ordering of context switches
Okay!
Commit: drm/i915: Expose engine properties via sysfs
-
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:52:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:53:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:57:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:73:20: error: undefined identifier 'engine'
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:96:9: error: cannot size expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:99:9: error: cannot size expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:99:9: error: cannot size expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:99:9: error: cannot size expression
Commit: drm/i915: Expose engine->mmio_base via sysfs
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:61:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:62:10: error: bad integer constant expression
-O:drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:52:10: error: bad integer constant expression
-O:drivers/gpu/drm/i915/gt/intel_engine_sysfs.c:53:10: error: bad integer constant expression
Commit: drm/i915: Expose timeslice duration to sysfs
Okay!
Commit: drm/i915/execlists: Force preemption
Okay!
Commit: drm/i915/gt: Introduce barrier pulses along engines
Okay!
Commit: drm/i915/execlists: Cancel banned contexts on schedule-out
Okay!
Commit: drm/i915/gem: Cancel contexts when hangchecking is disabled
Okay!
Commit: drm/i915: Replace hangcheck by heartbeats
Okay!
Commit: drm/i915/gem: Make context persistence optional
Okay!
Commit: drm/i915/gem: Distinguish each object type
Okay!
Commit: drm/i915: Flush idle barriers when waiting
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (16 preceding siblings ...)
2019-10-21 9:01 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-10-21 9:18 ` Patchwork
2019-10-21 9:49 ` [PATCH 01/16] " Mika Kuoppala
18 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-10-21 9:18 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
URL : https://patchwork.freedesktop.org/series/68295/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7138 -> Patchwork_14900
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_14900 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_14900, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_14900:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live_execlists:
- fi-kbl-x1275: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-kbl-x1275/igt@i915_selftest@live_execlists.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-kbl-x1275/igt@i915_selftest@live_execlists.html
* igt@i915_selftest@live_gem_contexts:
- fi-bsw-n3050: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-bsw-n3050/igt@i915_selftest@live_gem_contexts.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-bsw-n3050/igt@i915_selftest@live_gem_contexts.html
New tests
---------
New tests have been introduced between CI_DRM_7138 and Patchwork_14900:
### New IGT tests (2) ###
* igt@i915_selftest@live_gt_heartbeat:
- Statuses : 44 pass(s)
- Exec time: [0.43, 2.46] s
* igt@i915_selftest@live_gt_mocs:
- Statuses : 44 pass(s)
- Exec time: [0.40, 2.54] s
Known issues
------------
Here are the changes found in Patchwork_14900 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_chamelium@dp-edid-read:
- fi-kbl-7500u: [PASS][5] -> [WARN][6] ([fdo#109483])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
* igt@prime_busy@basic-wait-before-default:
- fi-icl-u3: [PASS][7] -> [DMESG-WARN][8] ([fdo#107724]) +2 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-icl-u3/igt@prime_busy@basic-wait-before-default.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-icl-u3/igt@prime_busy@basic-wait-before-default.html
#### Possible fixes ####
* igt@gem_ctx_switch@legacy-render:
- fi-bxt-dsi: [INCOMPLETE][9] ([fdo#103927] / [fdo#111381]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
* igt@gem_flink_basic@double-flink:
- fi-icl-u3: [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12] +2 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-icl-u3/igt@gem_flink_basic@double-flink.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-icl-u3/igt@gem_flink_basic@double-flink.html
* igt@i915_selftest@live_execlists:
- fi-cml-u: [INCOMPLETE][13] ([fdo#110566]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-cml-u/igt@i915_selftest@live_execlists.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-cml-u/igt@i915_selftest@live_execlists.html
- {fi-cml-s}: [INCOMPLETE][15] ([fdo#110566]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-cml-s/igt@i915_selftest@live_execlists.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-cml-s/igt@i915_selftest@live_execlists.html
- fi-skl-6260u: [INCOMPLETE][17] ([fdo#111934]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-skl-6260u/igt@i915_selftest@live_execlists.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-skl-6260u/igt@i915_selftest@live_execlists.html
* igt@kms_busy@basic-flip-a:
- {fi-tgl-u2}: [DMESG-WARN][19] ([fdo#111600]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-tgl-u2/igt@kms_busy@basic-flip-a.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-tgl-u2/igt@kms_busy@basic-flip-a.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][21] ([fdo#111045] / [fdo#111096]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7138/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
[fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
[fdo#111600]: https://bugs.freedesktop.org/show_bug.cgi?id=111600
[fdo#111934]: https://bugs.freedesktop.org/show_bug.cgi?id=111934
Participating hosts (52 -> 44)
------------------------------
Missing (8): fi-ilk-m540 fi-tgl-u fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7138 -> Patchwork_14900
CI-20190529: 20190529
CI_DRM_7138: 5365225cba2945e74299464b9b1c522da1049b64 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5234: 1205552397bd8a19dc6e5abdaa727cc091dabbfe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14900: e4eda090fa4254a4b6a7abcaec3c3d6441ff9a9f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
e4eda090fa42 drm/i915: Flush idle barriers when waiting
f07fa82b62c1 drm/i915/gem: Distinguish each object type
b2bbf76bf802 drm/i915/gem: Make context persistence optional
3d0b01cdae61 drm/i915: Replace hangcheck by heartbeats
ab9bbc283596 drm/i915/gem: Cancel contexts when hangchecking is disabled
092761273758 drm/i915/execlists: Cancel banned contexts on schedule-out
263213d1e0dd drm/i915/gt: Introduce barrier pulses along engines
cda9f6b21f0f drm/i915/execlists: Force preemption
dc571802eb53 drm/i915: Expose timeslice duration to sysfs
2aeae13e0baa drm/i915: Expose engine->mmio_base via sysfs
5751429a6359 drm/i915: Expose engine properties via sysfs
09ffac131a0d drm/i915/selftests: Force ordering of context switches
7383a7e9b6bf drm/i915/selftests: Teach igt_flush_test and igt_live_test to take intel_gt
9435f35a8e2b drm/i915/selftests: Add coverage of mocs registers
c04b1d31305a drm/i915: Drop assertion that ce->pin_mutex guards state updates
48cdac321482 drm/i915: Don't set queue_priority_hint if we don't kick the submission
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14900/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
2019-10-21 8:02 [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission Chris Wilson
` (17 preceding siblings ...)
2019-10-21 9:18 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-10-21 9:49 ` Mika Kuoppala
2019-10-21 9:56 ` Chris Wilson
18 siblings, 1 reply; 26+ messages in thread
From: Mika Kuoppala @ 2019-10-21 9:49 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> If we change the priority of the active context, then it has no impact
> on the decision of whether to preempt the active context -- we don't
> preempt the context with itself. In this situation, we elide the tasklet
> rescheduling and should *not* be marking up the queue_priority_hint as
> that may mask a later submission where we decide we don't have to kick
> the tasklet as a higher priority submission is pending (spoiler alert,
> it was not).
>
> 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/i915_scheduler.c | 37 ++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index 0ca40f6bf08c..d2edb527dcb8 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -189,22 +189,34 @@ static inline bool need_preempt(int prio, int active)
> return prio >= max(I915_PRIORITY_NORMAL, active);
> }
>
> -static void kick_submission(struct intel_engine_cs *engine, int prio)
> +static void kick_submission(struct intel_engine_cs *engine,
> + const struct i915_request *rq,
> + int prio)
> {
> - const struct i915_request *inflight =
> - execlists_active(&engine->execlists);
> + const struct i915_request *inflight;
> +
> + /*
> + * We only need to kick the tasklet once for the high priority
> + * new context we add into the queue.
> + */
> + if (prio <= engine->execlists.queue_priority_hint)
> + return;
> +
> + /* Nothing currently active? We're overdue for a submission! */
> + inflight = execlists_active(&engine->execlists);
> + if (!inflight)
> + return;
>
> /*
> * If we are already the currently executing context, don't
> - * bother evaluating if we should preempt ourselves, or if
> - * we expect nothing to change as a result of running the
> - * tasklet, i.e. we have not change the priority queue
> - * sufficiently to oust the running context.
> + * bother evaluating if we should preempt ourselves.
> */
> - if (!inflight || !need_preempt(prio, rq_prio(inflight)))
> + if (inflight->hw_context == rq->hw_context)
If there is a tail update at this moment, does the hardware
take it into account or do we need to kick?
-Mika
> return;
>
> - tasklet_hi_schedule(&engine->execlists.tasklet);
> + engine->execlists.queue_priority_hint = prio;
> + if (need_preempt(prio, rq_prio(inflight)))
> + tasklet_hi_schedule(&engine->execlists.tasklet);
> }
>
> static void __i915_schedule(struct i915_sched_node *node,
> @@ -330,13 +342,8 @@ static void __i915_schedule(struct i915_sched_node *node,
> list_move_tail(&node->link, cache.priolist);
> }
>
> - if (prio <= engine->execlists.queue_priority_hint)
> - continue;
> -
> - engine->execlists.queue_priority_hint = prio;
> -
> /* Defer (tasklet) submission until after all of our updates. */
> - kick_submission(engine, prio);
> + kick_submission(engine, node_to_request(node), prio);
> }
>
> spin_unlock(&engine->active.lock);
> --
> 2.24.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
2019-10-21 9:49 ` [PATCH 01/16] " Mika Kuoppala
@ 2019-10-21 9:56 ` Chris Wilson
2019-10-21 10:01 ` Mika Kuoppala
0 siblings, 1 reply; 26+ messages in thread
From: Chris Wilson @ 2019-10-21 9:56 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2019-10-21 10:49:14)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > If we change the priority of the active context, then it has no impact
> > on the decision of whether to preempt the active context -- we don't
> > preempt the context with itself. In this situation, we elide the tasklet
> > rescheduling and should *not* be marking up the queue_priority_hint as
> > that may mask a later submission where we decide we don't have to kick
> > the tasklet as a higher priority submission is pending (spoiler alert,
> > it was not).
> >
> > 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/i915_scheduler.c | 37 ++++++++++++++++-----------
> > 1 file changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> > index 0ca40f6bf08c..d2edb527dcb8 100644
> > --- a/drivers/gpu/drm/i915/i915_scheduler.c
> > +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> > @@ -189,22 +189,34 @@ static inline bool need_preempt(int prio, int active)
> > return prio >= max(I915_PRIORITY_NORMAL, active);
> > }
> >
> > -static void kick_submission(struct intel_engine_cs *engine, int prio)
> > +static void kick_submission(struct intel_engine_cs *engine,
> > + const struct i915_request *rq,
> > + int prio)
> > {
> > - const struct i915_request *inflight =
> > - execlists_active(&engine->execlists);
> > + const struct i915_request *inflight;
> > +
> > + /*
> > + * We only need to kick the tasklet once for the high priority
> > + * new context we add into the queue.
> > + */
> > + if (prio <= engine->execlists.queue_priority_hint)
> > + return;
> > +
> > + /* Nothing currently active? We're overdue for a submission! */
> > + inflight = execlists_active(&engine->execlists);
> > + if (!inflight)
> > + return;
> >
> > /*
> > * If we are already the currently executing context, don't
> > - * bother evaluating if we should preempt ourselves, or if
> > - * we expect nothing to change as a result of running the
> > - * tasklet, i.e. we have not change the priority queue
> > - * sufficiently to oust the running context.
> > + * bother evaluating if we should preempt ourselves.
> > */
> > - if (!inflight || !need_preempt(prio, rq_prio(inflight)))
> > + if (inflight->hw_context == rq->hw_context)
>
> If there is a tail update at this moment, does the hardware
> take it into account or do we need to kick?
We are holding the engine->active.lock, so we can't submit at this
moment. If we are inside process_csb (which is outside of the lock),
then this stale value if of no consequence as we are inside the tasklet
already. So if we suppress the kick, we are inside the tasklet and
didn't need the kick. The other result of giving a kick even though the
HW as about ready, is just one kick too many. We are just trying to
reduce the number of unnecessary tasklet executions, ideal is 0 false
kicks, but any small number is better than kicking on every loop through
the priority node updates.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission
2019-10-21 9:56 ` Chris Wilson
@ 2019-10-21 10:01 ` Mika Kuoppala
0 siblings, 0 replies; 26+ messages in thread
From: Mika Kuoppala @ 2019-10-21 10:01 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2019-10-21 10:49:14)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>>
>> > If we change the priority of the active context, then it has no impact
>> > on the decision of whether to preempt the active context -- we don't
>> > preempt the context with itself. In this situation, we elide the tasklet
>> > rescheduling and should *not* be marking up the queue_priority_hint as
>> > that may mask a later submission where we decide we don't have to kick
>> > the tasklet as a higher priority submission is pending (spoiler alert,
>> > it was not).
>> >
>> > 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/i915_scheduler.c | 37 ++++++++++++++++-----------
>> > 1 file changed, 22 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
>> > index 0ca40f6bf08c..d2edb527dcb8 100644
>> > --- a/drivers/gpu/drm/i915/i915_scheduler.c
>> > +++ b/drivers/gpu/drm/i915/i915_scheduler.c
>> > @@ -189,22 +189,34 @@ static inline bool need_preempt(int prio, int active)
>> > return prio >= max(I915_PRIORITY_NORMAL, active);
>> > }
>> >
>> > -static void kick_submission(struct intel_engine_cs *engine, int prio)
>> > +static void kick_submission(struct intel_engine_cs *engine,
>> > + const struct i915_request *rq,
>> > + int prio)
>> > {
>> > - const struct i915_request *inflight =
>> > - execlists_active(&engine->execlists);
>> > + const struct i915_request *inflight;
>> > +
>> > + /*
>> > + * We only need to kick the tasklet once for the high priority
>> > + * new context we add into the queue.
>> > + */
>> > + if (prio <= engine->execlists.queue_priority_hint)
>> > + return;
>> > +
>> > + /* Nothing currently active? We're overdue for a submission! */
>> > + inflight = execlists_active(&engine->execlists);
>> > + if (!inflight)
>> > + return;
>> >
>> > /*
>> > * If we are already the currently executing context, don't
>> > - * bother evaluating if we should preempt ourselves, or if
>> > - * we expect nothing to change as a result of running the
>> > - * tasklet, i.e. we have not change the priority queue
>> > - * sufficiently to oust the running context.
>> > + * bother evaluating if we should preempt ourselves.
>> > */
>> > - if (!inflight || !need_preempt(prio, rq_prio(inflight)))
>> > + if (inflight->hw_context == rq->hw_context)
>>
>> If there is a tail update at this moment, does the hardware
>> take it into account or do we need to kick?
>
> We are holding the engine->active.lock, so we can't submit at this
> moment. If we are inside process_csb (which is outside of the lock),
> then this stale value if of no consequence as we are inside the tasklet
> already. So if we suppress the kick, we are inside the tasklet and
> didn't need the kick. The other result of giving a kick even though the
> HW as about ready, is just one kick too many. We are just trying to
> reduce the number of unnecessary tasklet executions, ideal is 0 false
> kicks, but any small number is better than kicking on every loop through
> the priority node updates.
Ok, can't submit nor can't change prio. My prime concern was one
kick too little.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread