From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: Mika Kuoppala <mika.kuoppala@intel.com>,
Ben Widawsky <benjamin.widawsky@intel.com>
Subject: Re: [PATCH v3 12/13] drm/i915/execlists: Preemption!
Date: Fri, 29 Sep 2017 16:14:10 +0300 [thread overview]
Message-ID: <1506690850.4729.93.camel@linux.intel.com> (raw)
In-Reply-To: <150667992005.27384.17678506487224981105@mail.alporthouse.com>
On Fri, 2017-09-29 at 11:12 +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-09-29 08:29:57)
> > On Thu, 2017-09-28 at 20:39 +0100, Chris Wilson wrote:
> > > When we write to ELSP, it triggers a context preemption at the earliest
> > > arbitration point (3DPRIMITIVE, some PIPECONTROLs, a few other
> > > operations and the explicit MI_ARB_CHECK). If this is to the same
> > > context, it triggers a LITE_RESTORE where the RING_TAIL is merely
> > > updated (used currently to chain requests from the same context
> > > together, avoiding bubbles). However, if it is to a different context, a
> > > full context-switch is performed and it will start to execute the new
> > > context saving the image of the old for later execution.
> > >
> > > Previously we avoided preemption by only submitting a new context when
> > > the old was idle. But now we wish embrace it, and if the new request has
> > > a higher priority than the currently executing request, we write to the
> > > ELSP regardless, thus triggering preemption, but we tell the GPU to
> > > switch to our special preemption context (not the target). In the
> > > context-switch interrupt handler, we know that the previous contexts
> > > have finished execution and so can unwind all the incomplete requests
> > > and compute the new highest priority request to execute.
> > >
> > > It would be feasible to avoid the switch-to-idle intermediate by
> > > programming the ELSP with the target context. The difficulty is in
> > > tracking which request that should be whilst maintaining the dependency
> > > change, the error comes in with coalesced requests. As we only track the
> > > most recent request and its priority, we may run into the issue of being
> > > tricked in preempting a high priority request that was followed by a
> > > low priority request from the same context (e.g. for PI); worse still
> > > that earlier request may be our own dependency and the order then broken
> > > by preemption. By injecting the switch-to-idle and then recomputing the
> > > priority queue, we avoid the issue with tracking in-flight coalesced
> > > requests. Having tried the preempt-to-busy approach, and failed to find
> > > a way around the coalesced priority issue, Michal's original proposal to
> > > inject an idle context (based on handling GuC preemption) succeeds.
> > >
> > > The current heuristic for deciding when to preempt are only if the new
> > > request is of higher priority, and has the privileged priority of
> > > greater than 0. Note that the scheduler remains unfair!
> > >
> > > v2: Disable for gen8 (bdw/bsw) as we need additional w/a for GPGPU.
> > > Since, the feature is now conditional and not always available when we
> > > have a scheduler, make it known via the HAS_SCHEDULER GETPARAM (now a
> > > capability mask).
> > > v3: Stylistic tweaks.
> > >
> > > Suggested-by: Michal Winiarski <michal.winiarski@intel.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Cc: Zhi Wang <zhi.a.wang@intel.com>
> >
> > I'm still voting for "preempting" variable name + kerneldoc.
>
> I'm open to suggestions, or even a list of questions that you would like
> answered for @preempt[ing].
>
> That goes for anything. If at any time anyone sees something odd or not
> clear from the context while reading the code, just send a patch adding a
> question. Knowing at what point the confusion arose gives us the perfect
> place to address that for the next reader.
My point of confusion was that "if (execlists->preempt)" looks lot like
a callback hook. So I was expecting execlists->preempt()... It's not a
huge point.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-09-29 13:14 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 19:38 [PATCH v3 01/13] drm/i915: Inherit Kabylake platform features from Skylake Chris Wilson
2017-09-28 19:38 ` [PATCH v3 02/13] drm/i915/execlists: Move request unwinding to a separate function Chris Wilson
2017-09-28 19:39 ` [PATCH v3 03/13] drm/i915: Give the invalid priority a magic name Chris Wilson
2017-09-29 6:39 ` Mika Kuoppala
2017-09-29 7:01 ` Joonas Lahtinen
2017-09-28 19:39 ` [PATCH v3 04/13] drm/i915/execlists: Cache the last priolist lookup Chris Wilson
2017-09-29 11:45 ` Chris Wilson
2017-09-28 19:39 ` [PATCH v3 05/13] drm/i915/preempt: Fix WaEnablePreemptionGranularityControlByUMD Chris Wilson
2017-09-28 19:39 ` [PATCH v3 06/13] drm/i915/preempt: Default to disabled mid-command preemption levels Chris Wilson
2017-09-29 7:18 ` Joonas Lahtinen
2017-09-28 19:39 ` [PATCH v3 07/13] drm/i915/execlists: Distinguish the incomplete context notifies Chris Wilson
2017-09-28 19:39 ` [PATCH v3 08/13] drm/i915: Introduce a preempt context Chris Wilson
2017-09-29 7:23 ` Joonas Lahtinen
2017-09-28 19:39 ` [PATCH v3 09/13] drm/i915/execlists: Move bdw GPGPU w/a to emit_bb Chris Wilson
2017-09-28 19:39 ` [PATCH v3 10/13] drm/i915/execlists: Keep request->priority for its lifetime Chris Wilson
2017-09-28 19:39 ` [PATCH v3 11/13] drm/i915: Expand I915_PARAM_HAS_SCHEDULER into a capability bitmask Chris Wilson
2017-09-28 19:39 ` [PATCH v3 12/13] drm/i915/execlists: Preemption! Chris Wilson
2017-09-29 6:55 ` Mika Kuoppala
2017-09-29 10:07 ` Chris Wilson
2017-09-29 7:29 ` Joonas Lahtinen
2017-09-29 10:12 ` Chris Wilson
2017-09-29 13:14 ` Joonas Lahtinen [this message]
2017-09-28 19:39 ` [PATCH v3 13/13] drm/i915/scheduler: Support user-defined priorities Chris Wilson
2017-09-29 7:32 ` Joonas Lahtinen
2017-09-28 19:58 ` [PATCH v3 01/13] drm/i915: Inherit Kabylake platform features from Skylake Rodrigo Vivi
2017-09-28 20:05 ` Chris Wilson
2017-09-28 21:04 ` ✗ Fi.CI.BAT: failure for series starting with [v3,01/13] " Patchwork
2017-09-29 10:29 ` ✓ Fi.CI.BAT: success " Patchwork
2017-09-29 11:20 ` [PATCH v3 01/13] " David Weinehall
2017-09-29 11:24 ` ✗ Fi.CI.IGT: failure for series starting with [v3,01/13] " Patchwork
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=1506690850.4729.93.camel@linux.intel.com \
--to=joonas.lahtinen@linux.intel.com \
--cc=benjamin.widawsky@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
--cc=mika.kuoppala@intel.com \
/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