[Intel-gfx] [PATCH 3/3] drm/i915: Priority boost switching to an idle ring

[Tvrtko Ursulin]

On 21/01/2017 09:25, Chris Wilson wrote:
> In order to maximise concurrency between engines, if we queue a request
> to a current idle ring, reorder its dependencies to execute that request
> as early as possible and ideally improve occupancy of multiple engines
> simultaneously.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.h | 5 +++--
>  drivers/gpu/drm/i915/intel_lrc.c        | 3 +++
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index ba83c507613b..7ba9cc53abe9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -74,8 +74,9 @@ struct i915_priotree {
>  };
>
>  enum {
> -	I915_PRIORITY_LOCKED = I915_PRIORITY_MAX,
> -	I915_PRIORITY_DISPLAY
> +	I915_PRIORITY_STALL = I915_PRIORITY_MAX,
> +	I915_PRIORITY_LOCKED,
> +	I915_PRIORITY_DISPLAY,
>  };
>
>  struct i915_gem_capture_list {
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 50bec759989f..b46cb1bb32b8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -691,6 +691,9 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio)
>  	struct i915_dependency stack;
>  	LIST_HEAD(dfs);
>
> +	if (execlists_elsp_ready(request->engine))
> +		prio = max(prio, I915_PRIORITY_STALL);
> +

It would have to be execlists_elsp_idle for it to match with the commit 
message.

But even then the idea worries me sufficiently that I would refrain from 
adding it.

I don't like the name of I915_PRIORITY_STALL either, since I think about 
stalls as ELSP transitioning to idle and no runnable requests. It could 
be I915_PRIORITY_SUBMIT, but I just can't find a good story for when 
this might be a good idea.

Perhaps if we combined it with the no other runnable requests on the 
engine it might be passable?

Regards,

Tvrtko

>  	if (prio <= READ_ONCE(request->priotree.priority))
>  		return;
>
>