[Intel-gfx] [PATCH v2] drm/i915/execlists: Set queue priority from secondary port

[Chris Wilson]

Quoting Chris Wilson (2018-04-11 10:55:30)
> We can refine our current execlists->queue_priority if we inspect
> ELSP[1] rather than the head of the unsubmitted queue. Currently, we use
> the unsubmitted queue and say that if a subsequent request is more
> important than the current queue, we will rerun the submission tasklet
> 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[0] to avoid a stall when ELSP[0] 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[1] to cover the interrupt.) So if instead of
> looking at the head of the queue, we look at the context in ELSP[1] 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[1].
> 
> v2: Add some comments from the discussion with Tvrtko.
> 
> References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Aieee, I've realised that the test cases that demonstrated the bug for
the original patch still haven't been reviewed!
-Chris

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c |  3 +++
>  drivers/gpu/drm/i915/intel_lrc.c       | 16 +++++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 12486d8f534b..a217b3fe5f0b 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1687,6 +1687,9 @@ void intel_engines_park(struct drm_i915_private *i915)
>                         intel_engine_dump(engine, &p, NULL);
>                 }
>  
> +               /* Must be reset upon idling, or we may miss the busy wakeup. */
> +               GEM_BUG_ON(engine->execlists.queue_priority != INT_MIN);
> +
>                 if (engine->park)
>                         engine->park(engine);
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 02b25bf2378a..84c29c7b0183 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -713,8 +713,22 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>                 if (p->priority != I915_PRIORITY_NORMAL)
>                         kmem_cache_free(engine->i915->priorities, p);
>         }
> +
>  done:
> -       execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN;
> +       /*
> +        * Here be a bit of magic! Or sleight-of-hand, whichever you prefer.
> +        *
> +        * We choose queue_priority such that if we add a request of greater
> +        * priority than this, we kick the submission tasklet to decide on
> +        * the right order of submitting the requests to hardware. We must
> +        * also be prepared to reorder requests as they are in-flight on the
> +        * HW. We derive the queue_priority then as the first "hole" in
> +        * the HW submission ports and if there are no available slots,
> +        * the priority of the lowest executing request, i.e. last.
> +        */
> +       execlists->queue_priority =
> +               port != execlists->port ? rq_prio(last) : INT_MIN;
> +
>         execlists->first = rb;
>         if (submit)
>                 port_assign(port, last);
> -- 
> 2.17.0
>