drm/i915/gt: Distinguish the virtual breadcrumbs from the irq breadcrumbs
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 31 Jul 2020 15:48:34 +0000 (16:48 +0100)
committerJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Mon, 7 Sep 2020 11:23:55 +0000 (14:23 +0300)
On the virtual engines, we only use the intel_breadcrumbs for tracking
signaling of stale breadcrumbs from the irq_workers. They do not have
any associated interrupt handling, active requests are passed to a
physical engine and associated breadcrumb interrupt handler. This causes
issues for us as we need to ensure that we do not actually try and
enable interrupts and the powermanagement required for them on the
virtual engine, as they will never be disabled. Instead, let's
specify the physical engine used for interrupt handler on a particular
breadcrumb.

v2: Drop b->irq_armed = true mocking for no interrupt HW

Fixes: 4fe6abb8f513 ("drm/i915/gt: Ignore irq enabling on the virtual engines")
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>
Link: https://patchwork.freedesktop.org/patch/msgid/20200731154834.8378-4-chris@chris-wilson.co.uk
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
16 files changed:
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
drivers/gpu/drm/i915/gt/intel_breadcrumbs.h [new file with mode: 0644]
drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h [new file with mode: 0644]
drivers/gpu/drm/i915/gt/intel_engine.h
drivers/gpu/drm/i915/gt/intel_engine_cs.c
drivers/gpu/drm/i915/gt/intel_engine_pm.c
drivers/gpu/drm/i915/gt/intel_engine_types.h
drivers/gpu/drm/i915/gt/intel_gt_irq.c
drivers/gpu/drm/i915/gt/intel_lrc.c
drivers/gpu/drm/i915/gt/intel_reset.c
drivers/gpu/drm/i915/gt/intel_ring_submission.c
drivers/gpu/drm/i915/gt/intel_rps.c
drivers/gpu/drm/i915/gt/mock_engine.c
drivers/gpu/drm/i915/i915_irq.c
drivers/gpu/drm/i915/i915_request.c
drivers/gpu/drm/i915/i915_request.h

index fbdc465..2ffd47a 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include "intel_breadcrumbs.h"
 #include "intel_gt_pm.h"
 #include "intel_gt_requests.h"
 
@@ -55,30 +56,28 @@ static void irq_disable(struct intel_engine_cs *engine)
 
 static void __intel_breadcrumbs_disarm_irq(struct intel_breadcrumbs *b)
 {
-       struct intel_engine_cs *engine =
-               container_of(b, struct intel_engine_cs, breadcrumbs);
-
        lockdep_assert_held(&b->irq_lock);
 
+       if (!b->irq_engine || !b->irq_armed)
+               return;
+
        GEM_BUG_ON(!b->irq_enabled);
        if (!--b->irq_enabled)
-               irq_disable(engine);
+               irq_disable(b->irq_engine);
 
        WRITE_ONCE(b->irq_armed, false);
-       intel_gt_pm_put_async(engine->gt);
+       intel_gt_pm_put_async(b->irq_engine->gt);
 }
 
-void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
+void intel_breadcrumbs_park(struct intel_breadcrumbs *b)
 {
-       struct intel_breadcrumbs *b = &engine->breadcrumbs;
        unsigned long flags;
 
        if (!READ_ONCE(b->irq_armed))
                return;
 
        spin_lock_irqsave(&b->irq_lock, flags);
-       if (b->irq_armed)
-               __intel_breadcrumbs_disarm_irq(b);
+       __intel_breadcrumbs_disarm_irq(b);
        spin_unlock_irqrestore(&b->irq_lock, flags);
 }
 
@@ -133,13 +132,8 @@ __dma_fence_signal__notify(struct dma_fence *fence,
 
 static void add_retire(struct intel_breadcrumbs *b, struct intel_timeline *tl)
 {
-       struct intel_engine_cs *engine =
-               container_of(b, struct intel_engine_cs, breadcrumbs);
-
-       if (unlikely(intel_engine_is_virtual(engine)))
-               engine = intel_virtual_engine_get_sibling(engine, 0);
-
-       intel_engine_add_retire(engine, tl);
+       if (b->irq_engine)
+               intel_engine_add_retire(b->irq_engine, tl);
 }
 
 static bool __signal_request(struct i915_request *rq, struct list_head *signals)
@@ -164,7 +158,7 @@ static void signal_irq_work(struct irq_work *work)
 
        spin_lock(&b->irq_lock);
 
-       if (b->irq_armed && list_empty(&b->signalers))
+       if (list_empty(&b->signalers))
                __intel_breadcrumbs_disarm_irq(b);
 
        list_splice_init(&b->signaled_requests, &signal);
@@ -222,14 +216,12 @@ static void signal_irq_work(struct irq_work *work)
 
 static void __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
 {
-       struct intel_engine_cs *engine =
-               container_of(b, struct intel_engine_cs, breadcrumbs);
-
        lockdep_assert_held(&b->irq_lock);
-       if (b->irq_armed)
+
+       if (!b->irq_engine || b->irq_armed)
                return;
 
-       if (!intel_gt_pm_get_if_awake(engine->gt))
+       if (!intel_gt_pm_get_if_awake(b->irq_engine->gt))
                return;
 
        /*
@@ -249,37 +241,49 @@ static void __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
         */
 
        if (!b->irq_enabled++)
-               irq_enable(engine);
+               irq_enable(b->irq_engine);
 }
 
-void intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
+struct intel_breadcrumbs *
+intel_breadcrumbs_create(struct intel_engine_cs *irq_engine)
 {
-       struct intel_breadcrumbs *b = &engine->breadcrumbs;
+       struct intel_breadcrumbs *b;
+
+       b = kzalloc(sizeof(*b), GFP_KERNEL);
+       if (!b)
+               return NULL;
 
        spin_lock_init(&b->irq_lock);
        INIT_LIST_HEAD(&b->signalers);
        INIT_LIST_HEAD(&b->signaled_requests);
 
        init_irq_work(&b->irq_work, signal_irq_work);
+
+       b->irq_engine = irq_engine;
+
+       return b;
 }
 
-void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
+void intel_breadcrumbs_reset(struct intel_breadcrumbs *b)
 {
-       struct intel_breadcrumbs *b = &engine->breadcrumbs;
        unsigned long flags;
 
+       if (!b->irq_engine)
+               return;
+
        spin_lock_irqsave(&b->irq_lock, flags);
 
        if (b->irq_enabled)
-               irq_enable(engine);
+               irq_enable(b->irq_engine);
        else
-               irq_disable(engine);
+               irq_disable(b->irq_engine);
 
        spin_unlock_irqrestore(&b->irq_lock, flags);
 }
 
-void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
+void intel_breadcrumbs_free(struct intel_breadcrumbs *b)
 {
+       kfree(b);
 }
 
 static void insert_breadcrumb(struct i915_request *rq,
@@ -369,11 +373,11 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
         * request submit/unsubmit path, and so we must be more careful to
         * acquire the right lock.
         */
-       b = &READ_ONCE(rq->engine)->breadcrumbs;
+       b = READ_ONCE(rq->engine)->breadcrumbs;
        spin_lock(&b->irq_lock);
-       while (unlikely(b != &READ_ONCE(rq->engine)->breadcrumbs)) {
+       while (unlikely(b != READ_ONCE(rq->engine)->breadcrumbs)) {
                spin_unlock(&b->irq_lock);
-               b = &READ_ONCE(rq->engine)->breadcrumbs;
+               b = READ_ONCE(rq->engine)->breadcrumbs;
                spin_lock(&b->irq_lock);
        }
 
@@ -394,7 +398,7 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
 
 void i915_request_cancel_breadcrumb(struct i915_request *rq)
 {
-       struct intel_breadcrumbs *b = &rq->engine->breadcrumbs;
+       struct intel_breadcrumbs *b = rq->engine->breadcrumbs;
 
        /*
         * We must wait for b->irq_lock so that we know the interrupt handler
@@ -418,11 +422,11 @@ void i915_request_cancel_breadcrumb(struct i915_request *rq)
 void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
                                    struct drm_printer *p)
 {
-       struct intel_breadcrumbs *b = &engine->breadcrumbs;
+       struct intel_breadcrumbs *b = engine->breadcrumbs;
        struct intel_context *ce;
        struct i915_request *rq;
 
-       if (list_empty(&b->signalers))
+       if (!b || list_empty(&b->signalers))
                return;
 
        drm_printf(p, "Signals:\n");
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.h b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.h
new file mode 100644 (file)
index 0000000..ed3d1de
--- /dev/null
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __INTEL_BREADCRUMBS__
+#define __INTEL_BREADCRUMBS__
+
+#include <linux/irq_work.h>
+
+#include "intel_engine_types.h"
+
+struct drm_printer;
+struct i915_request;
+struct intel_breadcrumbs;
+
+struct intel_breadcrumbs *
+intel_breadcrumbs_create(struct intel_engine_cs *irq_engine);
+void intel_breadcrumbs_free(struct intel_breadcrumbs *b);
+
+void intel_breadcrumbs_reset(struct intel_breadcrumbs *b);
+void intel_breadcrumbs_park(struct intel_breadcrumbs *b);
+
+static inline void
+intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine)
+{
+       irq_work_queue(&engine->breadcrumbs->irq_work);
+}
+
+void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
+                                   struct drm_printer *p);
+
+bool i915_request_enable_breadcrumb(struct i915_request *request);
+void i915_request_cancel_breadcrumb(struct i915_request *request);
+
+#endif /* __INTEL_BREADCRUMBS__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h b/drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h
new file mode 100644 (file)
index 0000000..8e53b99
--- /dev/null
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __INTEL_BREADCRUMBS_TYPES__
+#define __INTEL_BREADCRUMBS_TYPES__
+
+#include <linux/irq_work.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+/*
+ * Rather than have every client wait upon all user interrupts,
+ * with the herd waking after every interrupt and each doing the
+ * heavyweight seqno dance, we delegate the task (of being the
+ * bottom-half of the user interrupt) to the first client. After
+ * every interrupt, we wake up one client, who does the heavyweight
+ * coherent seqno read and either goes back to sleep (if incomplete),
+ * or wakes up all the completed clients in parallel, before then
+ * transferring the bottom-half status to the next client in the queue.
+ *
+ * Compared to walking the entire list of waiters in a single dedicated
+ * bottom-half, we reduce the latency of the first waiter by avoiding
+ * a context switch, but incur additional coherent seqno reads when
+ * following the chain of request breadcrumbs. Since it is most likely
+ * that we have a single client waiting on each seqno, then reducing
+ * the overhead of waking that client is much preferred.
+ */
+struct intel_breadcrumbs {
+       spinlock_t irq_lock; /* protects the lists used in hardirq context */
+
+       /* Not all breadcrumbs are attached to physical HW */
+       struct intel_engine_cs *irq_engine;
+
+       struct list_head signalers;
+       struct list_head signaled_requests;
+
+       struct irq_work irq_work; /* for use from inside irq_lock */
+
+       unsigned int irq_enabled;
+
+       bool irq_armed;
+};
+
+#endif /* __INTEL_BREADCRUMBS_TYPES__ */
index faf00a3..08e2c00 100644 (file)
@@ -223,23 +223,6 @@ void intel_engine_get_instdone(const struct intel_engine_cs *engine,
 
 void intel_engine_init_execlists(struct intel_engine_cs *engine);
 
-void intel_engine_init_breadcrumbs(struct intel_engine_cs *engine);
-void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
-
-void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
-
-static inline void
-intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine)
-{
-       irq_work_queue(&engine->breadcrumbs.irq_work);
-}
-
-void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine);
-void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
-
-void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
-                                   struct drm_printer *p);
-
 static inline u32 *__gen8_emit_pipe_control(u32 *batch, u32 flags0, u32 flags1, u32 offset)
 {
        memset(batch, 0, 6 * sizeof(u32));
index 86651bb..ea4ba2a 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "i915_drv.h"
 
+#include "intel_breadcrumbs.h"
 #include "intel_context.h"
 #include "intel_engine.h"
 #include "intel_engine_pm.h"
@@ -700,8 +701,13 @@ static int engine_setup_common(struct intel_engine_cs *engine)
        if (err)
                return err;
 
+       engine->breadcrumbs = intel_breadcrumbs_create(engine);
+       if (!engine->breadcrumbs) {
+               err = -ENOMEM;
+               goto err_status;
+       }
+
        intel_engine_init_active(engine, ENGINE_PHYSICAL);
-       intel_engine_init_breadcrumbs(engine);
        intel_engine_init_execlists(engine);
        intel_engine_init_cmd_parser(engine);
        intel_engine_init__pm(engine);
@@ -716,6 +722,10 @@ static int engine_setup_common(struct intel_engine_cs *engine)
        intel_engine_init_ctx_wa(engine);
 
        return 0;
+
+err_status:
+       cleanup_status_page(engine);
+       return err;
 }
 
 struct measure_breadcrumb {
@@ -914,9 +924,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
        tasklet_kill(&engine->execlists.tasklet); /* flush the callback */
 
        cleanup_status_page(engine);
+       intel_breadcrumbs_free(engine->breadcrumbs);
 
        intel_engine_fini_retire(engine);
-       intel_engine_fini_breadcrumbs(engine);
        intel_engine_cleanup_cmd_parser(engine);
 
        if (engine->default_state)
index 8ec3eec..f7b2e07 100644 (file)
@@ -6,6 +6,7 @@
 
 #include "i915_drv.h"
 
+#include "intel_breadcrumbs.h"
 #include "intel_context.h"
 #include "intel_engine.h"
 #include "intel_engine_heartbeat.h"
@@ -247,7 +248,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_breadcrumbs_park(engine->breadcrumbs);
 
        /* Must be reset upon idling, or we may miss the busy wakeup. */
        GEM_BUG_ON(engine->execlists.queue_priority_hint != INT_MIN);
index 8de92fd..c400aaa 100644 (file)
@@ -22,6 +22,7 @@
 #include "i915_pmu.h"
 #include "i915_priolist_types.h"
 #include "i915_selftest.h"
+#include "intel_breadcrumbs_types.h"
 #include "intel_sseu.h"
 #include "intel_timeline_types.h"
 #include "intel_uncore.h"
@@ -373,34 +374,8 @@ struct intel_engine_cs {
         */
        struct ewma__engine_latency latency;
 
-       /* Rather than have every client wait upon all user interrupts,
-        * with the herd waking after every interrupt and each doing the
-        * heavyweight seqno dance, we delegate the task (of being the
-        * bottom-half of the user interrupt) to the first client. After
-        * every interrupt, we wake up one client, who does the heavyweight
-        * coherent seqno read and either goes back to sleep (if incomplete),
-        * or wakes up all the completed clients in parallel, before then
-        * transferring the bottom-half status to the next client in the queue.
-        *
-        * Compared to walking the entire list of waiters in a single dedicated
-        * bottom-half, we reduce the latency of the first waiter by avoiding
-        * a context switch, but incur additional coherent seqno reads when
-        * following the chain of request breadcrumbs. Since it is most likely
-        * that we have a single client waiting on each seqno, then reducing
-        * the overhead of waking that client is much preferred.
-        */
-       struct intel_breadcrumbs {
-               spinlock_t irq_lock;
-               struct list_head signalers;
-
-               struct list_head signaled_requests;
-
-               struct irq_work irq_work; /* for use from inside irq_lock */
-
-               unsigned int irq_enabled;
-
-               bool irq_armed;
-       } breadcrumbs;
+       /* Keep track of all the seqno used, a trail of breadcrumbs */
+       struct intel_breadcrumbs *breadcrumbs;
 
        struct intel_engine_pmu {
                /**
index b05da68..257063a 100644 (file)
@@ -8,6 +8,7 @@
 
 #include "i915_drv.h"
 #include "i915_irq.h"
+#include "intel_breadcrumbs.h"
 #include "intel_gt.h"
 #include "intel_gt_irq.h"
 #include "intel_uncore.h"
index e5a37d1..801ebe1 100644 (file)
 #include "i915_perf.h"
 #include "i915_trace.h"
 #include "i915_vgpu.h"
+#include "intel_breadcrumbs.h"
 #include "intel_context.h"
 #include "intel_engine_pm.h"
 #include "intel_gt.h"
@@ -4112,7 +4113,7 @@ static int execlists_resume(struct intel_engine_cs *engine)
 {
        intel_mocs_init_engine(engine);
 
-       intel_engine_reset_breadcrumbs(engine);
+       intel_breadcrumbs_reset(engine->breadcrumbs);
 
        if (GEM_SHOW_DEBUG() && unexpected_starting_state(engine)) {
                struct drm_printer p = drm_debug_printer(__func__);
@@ -5714,9 +5715,7 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
        snprintf(ve->base.name, sizeof(ve->base.name), "virtual");
 
        intel_engine_init_active(&ve->base, ENGINE_VIRTUAL);
-       intel_engine_init_breadcrumbs(&ve->base);
        intel_engine_init_execlists(&ve->base);
-       ve->base.breadcrumbs.irq_armed = true; /* fake HW, used for irq_work */
 
        ve->base.cops = &virtual_context_ops;
        ve->base.request_alloc = execlists_request_alloc;
@@ -5733,6 +5732,12 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
 
        intel_context_init(&ve->context, &ve->base);
 
+       ve->base.breadcrumbs = intel_breadcrumbs_create(NULL);
+       if (!ve->base.breadcrumbs) {
+               err = -ENOMEM;
+               goto err_put;
+       }
+
        for (n = 0; n < count; n++) {
                struct intel_engine_cs *sibling = siblings[n];
 
index 46a5cef..ac36b67 100644 (file)
@@ -15,6 +15,7 @@
 #include "i915_drv.h"
 #include "i915_gpu_error.h"
 #include "i915_irq.h"
+#include "intel_breadcrumbs.h"
 #include "intel_engine_pm.h"
 #include "intel_gt.h"
 #include "intel_gt_pm.h"
index 87cef6d..84da20f 100644 (file)
@@ -32,6 +32,7 @@
 #include "gen6_ppgtt.h"
 #include "gen7_renderclear.h"
 #include "i915_drv.h"
+#include "intel_breadcrumbs.h"
 #include "intel_context.h"
 #include "intel_gt.h"
 #include "intel_reset.h"
@@ -255,7 +256,7 @@ static int xcs_resume(struct intel_engine_cs *engine)
        else
                ring_setup_status_page(engine);
 
-       intel_engine_reset_breadcrumbs(engine);
+       intel_breadcrumbs_reset(engine->breadcrumbs);
 
        /* Enforce ordering by reading HEAD register back */
        ENGINE_POSTING_READ(engine, RING_HEAD);
index 97ba14a..e6a00ee 100644 (file)
@@ -7,6 +7,7 @@
 #include <drm/i915_drm.h>
 
 #include "i915_drv.h"
+#include "intel_breadcrumbs.h"
 #include "intel_gt.h"
 #include "intel_gt_clock_utils.h"
 #include "intel_gt_irq.h"
index 027de53..7976430 100644 (file)
@@ -260,11 +260,12 @@ static void mock_engine_release(struct intel_engine_cs *engine)
 
        GEM_BUG_ON(timer_pending(&mock->hw_delay));
 
+       intel_breadcrumbs_free(engine->breadcrumbs);
+
        intel_context_unpin(engine->kernel_context);
        intel_context_put(engine->kernel_context);
 
        intel_engine_fini_retire(engine);
-       intel_engine_fini_breadcrumbs(engine);
 }
 
 struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
@@ -322,11 +323,14 @@ int mock_engine_init(struct intel_engine_cs *engine)
        struct intel_context *ce;
 
        intel_engine_init_active(engine, ENGINE_MOCK);
-       intel_engine_init_breadcrumbs(engine);
        intel_engine_init_execlists(engine);
        intel_engine_init__pm(engine);
        intel_engine_init_retire(engine);
 
+       engine->breadcrumbs = intel_breadcrumbs_create(NULL);
+       if (!engine->breadcrumbs)
+               return -ENOMEM;
+
        ce = create_kernel_context(engine);
        if (IS_ERR(ce))
                goto err_breadcrumbs;
@@ -338,7 +342,7 @@ int mock_engine_init(struct intel_engine_cs *engine)
        return 0;
 
 err_breadcrumbs:
-       intel_engine_fini_breadcrumbs(engine);
+       intel_breadcrumbs_free(engine->breadcrumbs);
        return -ENOMEM;
 }
 
index 1fa6770..f113fe4 100644 (file)
@@ -41,6 +41,7 @@
 #include "display/intel_lpe_audio.h"
 #include "display/intel_psr.h"
 
+#include "gt/intel_breadcrumbs.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_irq.h"
 #include "gt/intel_gt_pm_irq.h"
index e9d5ce2..fcb00b2 100644 (file)
@@ -31,6 +31,7 @@
 #include <linux/sched/signal.h>
 
 #include "gem/i915_gem_context.h"
+#include "gt/intel_breadcrumbs.h"
 #include "gt/intel_context.h"
 #include "gt/intel_ring.h"
 #include "gt/intel_rps.h"
index fc18378..16b7210 100644 (file)
@@ -361,10 +361,6 @@ void i915_request_submit(struct i915_request *request);
 void __i915_request_unsubmit(struct i915_request *request);
 void i915_request_unsubmit(struct i915_request *request);
 
-/* Note: part of the intel_breadcrumbs family */
-bool i915_request_enable_breadcrumb(struct i915_request *request);
-void i915_request_cancel_breadcrumb(struct i915_request *request);
-
 long i915_request_wait(struct i915_request *rq,
                       unsigned int flags,
                       long timeout)