On 17/01/2019 14:34, Chris Wilson wrote:
Since commit d4ccceb05591 ("drm/i915/icl: Ringbuffer interrupt handling")
we have required a mechanism to avoid touching the interrupt hardware
for breadcrumbs, superseding our mock interface for selftests.
References: d4ccceb05591 ("drm/i915/icl: Ringbuffer interrupt handling")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 39 ++++++++------------
drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++----
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
drivers/gpu/drm/i915/selftests/mock_engine.c | 1 -
4 files changed, 20 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 4ed7105d7ff5..7b517bf83507 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -158,6 +158,9 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t)
static void irq_enable(struct intel_engine_cs *engine)
{
+ if (!engine->irq_enable)
+ return;
+
/*
* FIXME: Ideally we want this on the API boundary, but for the
* sake of testing with mock breadcrumbs (no HW so unable to
Okay I think I misunderstood this patch in the last round. So you want
to avoid the GEM_BUG_ON below _and_ a dedicated boolean only for the
mock engine.
I only wonder on the remaining merit of this comment and actually a
GEM_BUG_ON, which will be hit and miss depending on the platform now.
Gut feeling says something is still not ideal here. Selftests variable
does actually feel better in this sense.
mock_engine seems only used from mock_gem_device, so could an
alternative be to set i915->runtime_pm.irqs_enabled there and keep the
GEM_BUG_ON in irq_enable above the !engine->irq_enable early return?
That would still provide the unconditional assert on the state of the
driver outside selftests.
Regards,
Tvrtko
@@ -167,21 +170,20 @@ static void irq_enable(struct intel_engine_cs *engine)
GEM_BUG_ON(!intel_irqs_enabled(engine->i915));
/* Caller disables interrupts */
- if (engine->irq_enable) {
- spin_lock(&engine->i915->irq_lock);
- engine->irq_enable(engine);
- spin_unlock(&engine->i915->irq_lock);
- }
+ spin_lock(&engine->i915->irq_lock);
+ engine->irq_enable(engine);
+ spin_unlock(&engine->i915->irq_lock);
}
static void irq_disable(struct intel_engine_cs *engine)
{
+ if (!engine->irq_disable)
+ return;
+
/* Caller disables interrupts */
- if (engine->irq_disable) {
- spin_lock(&engine->i915->irq_lock);
- engine->irq_disable(engine);
- spin_unlock(&engine->i915->irq_lock);
- }
+ spin_lock(&engine->i915->irq_lock);
+ engine->irq_disable(engine);
+ spin_unlock(&engine->i915->irq_lock);
}
void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
@@ -293,25 +295,16 @@ static bool __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
if (b->irq_armed)
return false;
- /* The breadcrumb irq will be disarmed on the interrupt after the
+ /*
+ * The breadcrumb irq will be disarmed on the interrupt after the
* waiters are signaled. This gives us a single interrupt window in
* which we can add a new waiter and avoid the cost of re-enabling
* the irq.
*/
b->irq_armed = true;
- if (I915_SELFTEST_ONLY(b->mock)) {
- /* For our mock objects we want to avoid interaction
- * with the real hardware (which is not set up). So
- * we simply pretend we have enabled the powerwell
- * and the irq, and leave it up to the mock
- * implementation to call intel_engine_wakeup()
- * itself when it wants to simulate a user interrupt,
- */
- return true;
- }
-
- /* Since we are waiting on a request, the GPU should be busy
+ /*
+ * Since we are waiting on a request, the GPU should be busy
* and should have its own rpm reference. This is tracked
* by i915->gt.awake, we can forgo holding our own wakref
* for the interrupt as before i915->gt.awake is released (when
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index e2f65c59d6e8..fc52737751e7 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -917,6 +917,9 @@ static bool ring_is_idle(struct intel_engine_cs *engine)
intel_wakeref_t wakeref;
bool idle = true;
+ if (I915_SELFTEST_ONLY(!engine->mmio_base))
+ return true;
+
/* If the whole device is asleep, the engine must be idle */
wakeref = intel_runtime_pm_get_if_in_use(dev_priv);
if (!wakeref)
@@ -955,9 +958,6 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
if (!intel_engine_signaled(engine, intel_engine_last_submit(engine)))
return false;
- if (I915_SELFTEST_ONLY(engine->breadcrumbs.mock))
- return true;
-
/* Waiting to drain ELSP? */
if (READ_ONCE(engine->execlists.active)) {
struct tasklet_struct *t = &engine->execlists.tasklet;
@@ -983,10 +983,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
return false;
/* Ring stopped? */
- if (!ring_is_idle(engine))
- return false;
-
- return true;
+ return ring_is_idle(engine);
}
bool intel_engines_are_idle(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1adf9845710c..17e05d11ee34 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -397,7 +397,6 @@ struct intel_engine_cs {
unsigned int irq_count;
bool irq_armed : 1;
- I915_SELFTEST_DECLARE(bool mock : 1);
} breadcrumbs;
struct {
diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index 50e1a0b1af7e..9fe5b2c8f8d4 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -201,7 +201,6 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
i915_timeline_set_subclass(&engine->base.timeline, TIMELINE_ENGINE);
intel_engine_init_breadcrumbs(&engine->base);
- engine->base.breadcrumbs.mock = true; /* prevent touching HW for irqs */
/* fake hw queue */
spin_lock_init(&engine->hw_lock);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
