On 08/02/2021 15:29, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2021-02-08 14:56:31)
On 08/02/2021 10:52, Chris Wilson wrote:
+static bool need_preempt(const struct intel_engine_cs *engine,
const struct i915_request *rq)
{
const struct i915_sched *se = &engine->sched;
- int last_prio;
+ const struct i915_request *first = NULL;
+ const struct i915_request *next;
if (!i915_sched_use_busywait(se))
return false;
/*
- * Check if the current priority hint merits a preemption attempt.
- *
- * We record the highest value priority we saw during rescheduling
- * prior to this dequeue, therefore we know that if it is strictly
- * less than the current tail of ESLP[0], we do not need to force
- * a preempt-to-idle cycle.
- *
- * However, the priority hint is a mere hint that we may need to
- * preempt. If that hint is stale or we may be trying to preempt
- * ourselves, ignore the request.
- *
- * More naturally we would write
- * prio >= max(0, last);
- * except that we wish to prevent triggering preemption at the same
- * priority level: the task that is running should remain running
- * to preserve FIFO ordering of dependencies.
+ * If this request is special and must not be interrupted at any
+ * cost, so be it. Note we are only checking the most recent request
+ * in the context and so may be masking an earlier vip request. It
+ * is hoped that under the conditions where nopreempt is used, this
+ * will not matter (i.e. all requests to that context will be
+ * nopreempt for as long as desired).
*/
- last_prio = max(effective_prio(rq), I915_PRIORITY_NORMAL - 1);
- if (engine->execlists.queue_priority_hint <= last_prio)
+ if (i915_request_has_nopreempt(rq))
return false;
/*
* Check against the first request in ELSP[1], it will, thanks to the
* power of PI, be the highest priority of that context.
*/
- if (!list_is_last(&rq->sched.link, &se->requests) &&
- rq_prio(list_next_entry(rq, sched.link)) > last_prio)
- return true;
+ next = next_elsp_request(se, rq);
+ if (dl_before(next, first))
Here first is always NULL so dl_before always returns true, meaning it
appears redundant to call it.
I was applying a pattern :)
Yeah, thought so. It's fine.
+ first = next;
/*
* If the inflight context did not trigger the preemption, then maybe
@@ -356,8 +343,31 @@ static bool need_preempt(struct intel_engine_cs *engine,
* ELSP[0] or ELSP[1] as, thanks again to PI, if it was the same
* context, it's priority would not exceed ELSP[0] aka last_prio.
*/
- return max(virtual_prio(&engine->execlists),
- queue_prio(se)) > last_prio;
+ next = first_request(se);
+ if (dl_before(next, first))
+ first = next; > +
+ next = first_virtual(engine);
+ if (dl_before(next, first))
+ first = next;
+
+ if (!dl_before(first, rq))
+ return false;
Ends up earliest deadline between list of picks: elsp[1] (or maybe next
in context, depends on coalescing criteria), first in the priolist,
first virtual.
Virtual has a separate queue so that's understandable, but can "elsp[1]"
really have an earlier deadling than first_request() (head of thepriolist)?
elsp[1] could have been promoted and thus now have an earlier deadline
than elsp[0]. Consider the heartbeat as a trivial example that is first
submitted at very low priority, but by the end has absolute priority.
The tree is not kept sorted at all times, or at least at the time
need_preempt peeks at it?
+static u64 virtual_deadline(u64 kt, int priority)
+{
+ return i915_sched_to_ticks(kt + prio_slice(priority));
+}
+
+u64 i915_scheduler_next_virtual_deadline(int priority)
+{
+ return virtual_deadline(ktime_get_mono_fast_ns(), priority);
+}
This helpers becomes a bit odd in that the only two callers are rewind
and defer. And it queries ktime, while before deadline was set based on
signalers.
Where is the place which set the ktime based deadline (converted to
ticks) for requests with no signalers?
signal_deadline() with no signalers returns now. So the first request in
a sequence is queued with virtual_deadline(now() + prio_slice()).
Ah ok.
void i915_request_enqueue(struct i915_request *rq)
{
- struct intel_engine_cs *engine = rq->engine;
- struct i915_sched *se = intel_engine_get_scheduler(engine);
+ struct i915_sched *se = i915_request_get_scheduler(rq);
+ u64 dl = earliest_deadline(se, rq);
unsigned long flags;
bool kick = false;
@@ -880,11 +1107,11 @@ void i915_request_enqueue(struct i915_request *rq)
list_add_tail(&rq->sched.link, &se->hold);
i915_request_set_hold(rq);
} else {
- queue_request(se, rq);
-
+ set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
+ kick = __i915_request_set_deadline(se, rq,
+ min(dl, rq_deadline(rq)));
What is this min for? Dl has been computed above based on rq, so I
wonder why rq_deadline has to be considered again.
earliest_deadline() only looks at the signalers (or now if none) and
picks the next deadline in that sequence. However, some requests we may
set the deadline explicitly (e.g. heartbeat has a known deadline, vblank
rendering we can approximate a deadline) and so we also consider what
deadline has already been specified.
Because earliest_deadline does not actually consider rq->sched.deadline?
So conceptually earliest_deadline would be described as what?
sequence_deadline() ?
earliest_deadline_for_this_sequence() ?
Don't know really. Don't think it's a matter of names just me building a
good image of the operation.
But as earliest does imply earliest, which then gets potentially
overwritten with something even earlier, hm.. baseline? :) Default?
Nah.. Scheduling_deadline? Tree deadline? Sorted deadline?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
