+ Matt to check on how this fits with GuC. This patch and a few before
it in this series.
The split between physical and scheduling engine (i915_sched_engine)
makes sense to me. Gut feeling says it should work for GuC as well, in
principle.
A small comment or two below:
On 25/01/2021 14:01, Chris Wilson wrote:
Move the scheduling tasklists out of the execlists backend into the
per-engine scheduling bookkeeping.
Signed-off-by: Chris Wilson <[email protected]>
---
drivers/gpu/drm/i915/gt/intel_engine.h | 14 ----
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 ++--
drivers/gpu/drm/i915/gt/intel_engine_types.h | 5 --
.../drm/i915/gt/intel_execlists_submission.c | 65 +++++++++----------
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_execlists.c | 16 ++---
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_lrc.c | 6 +-
drivers/gpu/drm/i915/gt/selftest_reset.c | 2 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 18 ++---
drivers/gpu/drm/i915/i915_scheduler.c | 14 ++--
drivers/gpu/drm/i915/i915_scheduler.h | 20 ++++++
drivers/gpu/drm/i915/i915_scheduler_types.h | 6 ++
.../gpu/drm/i915/selftests/i915_scheduler.c | 16 ++---
14 files changed, 99 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
b/drivers/gpu/drm/i915/gt/intel_engine.h
index 20974415e7d8..801ae54cf60d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -122,20 +122,6 @@ execlists_active(const struct intel_engine_execlists
*execlists)
return active;
}
-static inline void
-execlists_active_lock_bh(struct intel_engine_execlists *execlists)
-{
- local_bh_disable(); /* prevent local softirq and lock recursion */
- tasklet_lock(&execlists->tasklet);
-}
-
-static inline void
-execlists_active_unlock_bh(struct intel_engine_execlists *execlists)
-{
- tasklet_unlock(&execlists->tasklet);
- local_bh_enable(); /* restore softirq, and kick ksoftirqd! */
-}
-
static inline u32
intel_read_status_page(const struct intel_engine_cs *engine, int reg)
{
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index ef225da35399..cdd07aeada05 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -902,7 +902,6 @@ int intel_engines_init(struct intel_gt *gt)
void intel_engine_cleanup_common(struct intel_engine_cs *engine)
{
i915_sched_fini_engine(&engine->active);
- tasklet_kill(&engine->execlists.tasklet); /* flush the callback */
intel_breadcrumbs_free(engine->breadcrumbs);
@@ -1187,7 +1186,7 @@ static bool ring_is_idle(struct intel_engine_cs *engine)
void __intel_engine_flush_submission(struct intel_engine_cs *engine, bool sync)
{
- struct tasklet_struct *t = &engine->execlists.tasklet;
+ struct tasklet_struct *t = &engine->active.tasklet;
if (!t->func)
return;
@@ -1454,8 +1453,8 @@ static void intel_engine_print_registers(struct
intel_engine_cs *engine,
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)),
+ &engine->active.tasklet.state)),
+
enableddisabled(!atomic_read(&engine->active.tasklet.count)),
repr_timer(&engine->execlists.preempt),
repr_timer(&engine->execlists.timer));
@@ -1479,7 +1478,7 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine,
idx, hws[idx * 2], hws[idx * 2 + 1]);
}
- execlists_active_lock_bh(execlists);
+ i915_sched_lock_bh(&engine->active);
rcu_read_lock();
for (port = execlists->active; (rq = *port); port++) {
char hdr[160];
@@ -1510,7 +1509,7 @@ static void intel_engine_print_registers(struct
intel_engine_cs *engine,
i915_request_show(m, rq, hdr, 0);
}
rcu_read_unlock();
- execlists_active_unlock_bh(execlists);
+ i915_sched_unlock_bh(&engine->active);
} else if (INTEL_GEN(dev_priv) > 6) {
drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
ENGINE_READ(engine, RING_PP_DIR_BASE));
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index c46d70b7e484..76d561c2c6aa 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -138,11 +138,6 @@ struct st_preempt_hang {
* driver and the hardware state for execlist mode of submission.
*/
struct intel_engine_execlists {
- /**
- * @tasklet: softirq tasklet for bottom handler
- */
- struct tasklet_struct tasklet;
-
/**
* @timer: kick the current context if its timeslice expires
*/
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 756ac388a4a8..1103c8a00af1 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -513,7 +513,7 @@ static void kick_siblings(struct i915_request *rq, struct
intel_context *ce)
resubmit_virtual_request(rq, ve);
if (READ_ONCE(ve->request))
- tasklet_hi_schedule(&ve->base.execlists.tasklet);
+ i915_sched_kick(&ve->base.active);
i915_sched_ or i915_sched_engine_ ?
}
static void __execlists_schedule_out(struct i915_request * const rq,
@@ -679,10 +679,9 @@ trace_ports(const struct intel_engine_execlists *execlists,
dump_port(p1, sizeof(p1), ", ", ports[1]));
}
-static bool
-reset_in_progress(const struct intel_engine_execlists *execlists)
+static bool reset_in_progress(const struct intel_engine_cs *engine)
{
- return unlikely(!__tasklet_is_enabled(&execlists->tasklet));
+ return unlikely(!__tasklet_is_enabled(&engine->active.tasklet));
}
static __maybe_unused noinline bool
@@ -699,7 +698,7 @@ assert_pending_valid(const struct intel_engine_execlists
*execlists,
trace_ports(execlists, msg, execlists->pending);
/* We may be messing around with the lists during reset, lalala */
- if (reset_in_progress(execlists))
+ if (reset_in_progress(engine))
return true;
if (!execlists->pending[0]) {
@@ -1084,7 +1083,7 @@ static void start_timeslice(struct intel_engine_cs
*engine)
* its timeslice, so recheck.
*/
if (!timer_pending(&el->timer))
- tasklet_hi_schedule(&el->tasklet);
+ i915_sched_kick(&engine->active);
return;
}
@@ -1664,8 +1663,8 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive)
* access. Either we are inside the tasklet, or the tasklet is disabled
* and we assume that is only inside the reset paths and so serialised.
*/
- GEM_BUG_ON(!tasklet_is_locked(&execlists->tasklet) &&
- !reset_in_progress(execlists));
+ GEM_BUG_ON(!tasklet_is_locked(&engine->active.tasklet) &&
+ !reset_in_progress(engine));
GEM_BUG_ON(!intel_engine_in_execlists_submission_mode(engine));
/*
@@ -2077,13 +2076,13 @@ static noinline void execlists_reset(struct
intel_engine_cs *engine)
ENGINE_TRACE(engine, "reset for %s\n", msg);
/* Mark this tasklet as disabled to avoid waiting for it to complete */
- tasklet_disable_nosync(&engine->execlists.tasklet);
+ tasklet_disable_nosync(&engine->active.tasklet);
ring_set_paused(engine, 1); /* Freeze the current request in place */
execlists_capture(engine);
intel_engine_reset(engine, msg);
- tasklet_enable(&engine->execlists.tasklet);
+ tasklet_enable(&engine->active.tasklet);
Maybe all access to the tasklet from the backend should go via
i915_sched_ helpers to complete the separation? And with some generic
naming in case we don't want to trumpet it is a tasklet but instead some
higher level concept. Like schedule_enable/disable I don't know..
Depends also how this plugs in the GuC.
Just this really, code itself looks clean enough.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
