[Intel-gfx] [PATCH 04/17] drm/i915: Tweak scheduler's kick_submission()

[Tvrtko Ursulin]

On 06/03/2020 13:38, Chris Wilson wrote:
> Skip useless priority bumping on adding a new dependency, but otherwise
> prevent tasklet scheduling until we have completed all the potential
> rescheduling.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_scheduler.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index 52f71e83e088..603cba36d6a4 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -209,6 +209,8 @@ static void kick_submission(struct intel_engine_cs *engine,
>   	if (!inflight)
>   		goto unlock;
>   
> +	engine->execlists.queue_priority_hint = prio;
> +

What is the significance of moving this up? I couldn't correlate it to 
the commit message.

>   	/*
>   	 * If we are already the currently executing context, don't
>   	 * bother evaluating if we should preempt ourselves.
> @@ -216,7 +218,6 @@ static void kick_submission(struct intel_engine_cs *engine,
>   	if (inflight->context == rq->context)
>   		goto unlock;
>   
> -	engine->execlists.queue_priority_hint = prio;
>   	if (need_preempt(prio, rq_prio(inflight)))
>   		tasklet_hi_schedule(&engine->execlists.tasklet);
>   
> @@ -463,11 +464,15 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
>   	if (!dep)
>   		return -ENOMEM;
>   
> +	local_bh_disable();
> +
>   	if (!__i915_sched_node_add_dependency(node, signal, dep,
>   					      I915_DEPENDENCY_EXTERNAL |
>   					      I915_DEPENDENCY_ALLOC))
>   		i915_dependency_free(dep);
>   
> +	local_bh_enable(); /* kick submission tasklet */
> +

And this presumably postpones the tasklet until __bump_priority -> 
__i915_schedule is finished. But then why the request submission path 
which calls __i915_sched_node_add_dependency directly does not need this 
treatment?

Regards,

Tvrtko

>   	return 0;
>   }
>   
>