On 09/04/2018 12:13, Chris Wilson wrote:
We can refine our current execlists->queue_priority if we inspect ELSP rather than the head of the unsubmitted queue. Currently, we use the unsubmitted queue and say that if a subsequent request is more than important than the current queue, we will rerun the submission tasklet
s/more than important/more important/
to evaluate the need for preemption. However, we only want to preempt if we need to jump ahead of a currently executing request in ELSP. The second reason for running the submission tasklet is amalgamate requests into the active context on ELSP to avoid a stall when ELSP drains. (Though repeatedly amalgamating requests into the active context and triggering many lite-restore is off question gain, the goal really is to put a context into ELSP to cover the interrupt.) So if instead of looking at the head of the queue, we look at the context in ELSP we can answer both of the questions more accurately -- we don't need to rerun the submission tasklet unless our new request is important enough to feed into, at least, ELSP. References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiar...@intel.com> Cc: Michel Thierry <michel.thie...@intel.com> Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 3592288e4696..b47d53d284ca 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -713,7 +713,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) kmem_cache_free(engine->i915->priorities, p); } done: - execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN; + execlists->queue_priority = + port != execlists->port ? rq_prio(last) : INT_MIN;
Please bear with my questions since I am not 100% up to date with preemption.
Should this be rq_prio("port") for future proofing? Or you really mean last port? In which case it wouldn't be the highest pending priority as kerneldoc for execlists->queue_priority says.
So this patch changes the meaning of "pending". From pending == "not submitted to ELSP" to pending == "not submitted to ELSP". Which seems to make sense, although it is not the easiest job to figure out the consequences.
It even feels like a bugfix since it prevents tasklet scheduling when all ports are filled with higher priority requests than the new one.
Although I failed to understand what do we do in both cases if a new request arrives of higher prio than the one in ELSP. Looks like nothing? Wait until GPU moves it to ELSP and preempt then? Is this so because we can't safely or I misread something?
Also, don't you need to manage execlists->queue_priority after CSB processing now? So that it correctly reflects the priority of request in ELSP after ELSP gets completed? It seems that without it would get stuck at the previous value and then submission could decide to skip scheduling the tasklet if new priority is lower than what was in ELSP before, and so would fail to fill ELSP.
execlists->first = rb; if (submit) port_assign(port, last);
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intelemail@example.com https://lists.freedesktop.org/mailman/listinfo/intel-gfx