| Message ID | 20170328180029.1073-2-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 28, 2017 at 08:00:27PM +0200, Michał Winiarski wrote: > Normally when we're inserting requests with equal priority, we're using > FIFO ordering. However, when we're resubmitting requests it's helpful > to be able to ensure that they're executed before their dependencies, > meaning that we need to use LIFO ordering instead. You shouldn't be requeueing before you've resubmitted its dependencies. FIFO should still be holding. This doesn't make sense without seeing the other side, and I suspect that other side isn't right :-p -Chris
On Tue, Mar 28, 2017 at 08:00:27PM +0200, Michał Winiarski wrote: > Normally when we're inserting requests with equal priority, we're using > FIFO ordering. However, when we're resubmitting requests it's helpful > to be able to ensure that they're executed before their dependencies, > meaning that we need to use LIFO ordering instead. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 301ae7c..107cf91 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -611,12 +611,15 @@ static void intel_lrc_irq_handler(unsigned long data) > intel_uncore_forcewake_put(dev_priv, engine->fw_domains); > } > > -static bool insert_request(struct i915_priotree *pt, struct rb_root *root) > +static bool insert_request(struct i915_priotree *pt, struct rb_root *root, > + bool reinsert) I would just use a different insertion routine for unsubmit. The most likely insertion point is execlist_first and so we can optimise the reinsert to begin it's search there (we'll only have to search if there were higher (than it) priority requests added that didn't trigger a preemption). I am not keen on making insert_request more complicated, semantically at least. execlists_schedule() is built around the idea that the dependency graphs are always have an order to their priority (no dependent request can have a greater priority than their dependencies). insert_request() is tightly coupled into that picture. I need more coaxing, I have this nagging feeling that there is a simpler way. -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 301ae7c..107cf91 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -611,12 +611,15 @@ static void intel_lrc_irq_handler(unsigned long data) intel_uncore_forcewake_put(dev_priv, engine->fw_domains); } -static bool insert_request(struct i915_priotree *pt, struct rb_root *root) +static bool insert_request(struct i915_priotree *pt, struct rb_root *root, + bool reinsert) { struct rb_node **p, *rb; bool first = true; - /* most positive priority is scheduled first, equal priorities fifo */ + /* most positive priority is scheduled first, + * equal priorities - fifo, except when we're reinserting - lifo + */ rb = NULL; p = &root->rb_node; while (*p) { @@ -624,7 +627,8 @@ static bool insert_request(struct i915_priotree *pt, struct rb_root *root) rb = *p; pos = rb_entry(rb, typeof(*pos), node); - if (pt->priority > pos->priority) { + if ((pt->priority > pos->priority) || + ((pt->priority == pos->priority) && reinsert)) { p = &rb->rb_left; } else { p = &rb->rb_right; @@ -645,7 +649,8 @@ static void execlists_submit_request(struct drm_i915_gem_request *request) /* Will be called from irq-context when using foreign fences. */ spin_lock_irqsave(&engine->timeline->lock, flags); - if (insert_request(&request->priotree, &engine->execlist_queue)) { + if (insert_request(&request->priotree, + &engine->execlist_queue, false)) { engine->execlist_first = &request->priotree.node; if (execlists_elsp_ready(engine)) tasklet_hi_schedule(&engine->irq_tasklet); @@ -742,7 +747,7 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio) pt->priority = prio; rb_erase(&pt->node, &engine->execlist_queue); - if (insert_request(pt, &engine->execlist_queue)) + if (insert_request(pt, &engine->execlist_queue, false)) engine->execlist_first = &pt->node; }
Normally when we're inserting requests with equal priority, we're using FIFO ordering. However, when we're resubmitting requests it's helpful to be able to ensure that they're executed before their dependencies, meaning that we need to use LIFO ordering instead. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)