From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 10/31] drm/i915: Fair low-latency scheduling
Date: Mon, 8 Feb 2021 16:03:03 +0000 [thread overview]
Message-ID: <6f648ec5-e28b-513c-d29a-2e5dd4924ab6@linux.intel.com> (raw)
In-Reply-To: <161279816578.9448.5547982919972033105@build.alporthouse.com>
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-02-08 16:04 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-08 10:52 [Intel-gfx] [PATCH 01/31] drm/i915/gt: Ratelimit heartbeat completion probing Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 02/31] drm/i915: Move context revocation to scheduler Chris Wilson
2021-02-08 11:18 ` Tvrtko Ursulin
2021-02-08 10:52 ` [Intel-gfx] [PATCH 03/31] drm/i915: Introduce the scheduling mode Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 04/31] drm/i915: Move timeslicing flag to scheduler Chris Wilson
2021-02-08 11:43 ` Tvrtko Ursulin
2021-02-08 10:52 ` [Intel-gfx] [PATCH 05/31] drm/i915/gt: Declare when we enabled timeslicing Chris Wilson
2021-02-08 11:44 ` Tvrtko Ursulin
2021-02-08 10:52 ` [Intel-gfx] [PATCH 06/31] drm/i915: Move busywaiting control to the scheduler Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 07/31] drm/i915: Move preempt-reset flag " Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 08/31] drm/i915: Fix the iterative dfs for defering requests Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 09/31] drm/i915: Replace priolist rbtree with a skiplist Chris Wilson
2021-02-08 12:29 ` Tvrtko Ursulin
2021-02-08 12:46 ` Chris Wilson
2021-02-08 15:10 ` Tvrtko Ursulin
2021-02-08 15:23 ` Tvrtko Ursulin
2021-02-08 16:19 ` Chris Wilson
2021-02-09 16:11 ` Tvrtko Ursulin
2021-02-08 10:52 ` [Intel-gfx] [PATCH 10/31] drm/i915: Fair low-latency scheduling Chris Wilson
2021-02-08 14:56 ` Tvrtko Ursulin
2021-02-08 15:29 ` Chris Wilson
2021-02-08 16:03 ` Tvrtko Ursulin [this message]
2021-02-08 16:11 ` Chris Wilson
2021-02-09 9:37 ` Tvrtko Ursulin
2021-02-09 10:31 ` Chris Wilson
2021-02-09 10:40 ` Tvrtko Ursulin
2021-02-08 10:52 ` [Intel-gfx] [PATCH 11/31] drm/i915/gt: Specify a deadline for the heartbeat Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 12/31] drm/i915: Extend the priority boosting for the display with a deadline Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 13/31] drm/i915/gt: Support virtual engine queues Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 14/31] drm/i915: Move saturated workload detection back to the context Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 15/31] drm/i915: Bump default timeslicing quantum to 5ms Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 16/31] drm/i915/gt: Delay taking irqoff for execlists submission Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 17/31] drm/i915/gt: Convert the legacy ring submission to use the scheduling interface Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 18/31] drm/i915/gt: Wrap intel_timeline.has_initial_breadcrumb Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 19/31] drm/i915/gt: Track timeline GGTT offset separately from subpage offset Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 20/31] drm/i915/gt: Add timeline "mode" Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 21/31] drm/i915/gt: Use indices for writing into relative timelines Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 22/31] drm/i915/selftests: Exercise relative timeline modes Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 23/31] drm/i915/gt: Use ppHWSP for unshared non-semaphore related timelines Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 24/31] Restore "drm/i915: drop engine_pin/unpin_breadcrumbs_irq" Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 25/31] drm/i915/gt: Support creation of 'internal' rings Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 26/31] drm/i915/gt: Use client timeline address for seqno writes Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 27/31] drm/i915/gt: Infrastructure for ring scheduling Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 28/31] drm/i915/gt: Implement ring scheduler for gen4-7 Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 29/31] drm/i915/gt: Enable ring scheduling for gen5-7 Chris Wilson
2021-02-08 10:52 ` [Intel-gfx] [PATCH 30/31] drm/i915: Support secure dispatch on gen6/gen7 Chris Wilson
2021-02-08 20:55 ` Dave Airlie
2021-02-08 22:49 ` Chris Wilson
2021-02-09 11:02 ` Tvrtko Ursulin
2021-02-08 10:52 ` [Intel-gfx] [PATCH 31/31] drm/i915/gt: Limit C-states while waiting for requests Chris Wilson
2021-02-08 15:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/31] drm/i915/gt: Ratelimit heartbeat completion probing Patchwork
2021-02-08 15:45 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-02-08 16:13 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-02-09 17:52 ` [Intel-gfx] [PATCH 01/31] " Mika Kuoppala
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6f648ec5-e28b-513c-d29a-2e5dd4924ab6@linux.intel.com \
--to=tvrtko.ursulin@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).