[Intel-gfx] [PATCH v2] drm/i915: add schedule out notification of preempted but completed request

[Tvrtko Ursulin]

On 24/02/2018 02:59, Weinan Li wrote:
> There is one corner case missing schedule out notification of the preempted
> request. The preempted request is just completed when preemption happen,
> then it will be canceled and won't be resubmitted later, GVT-g will lost
> the schedule out notification.
> 
> Here add schedule out notification if found the preempted request has been
> completed.
> 
> v2:
> - refine description, add completed check and notification in
>    execlists_cancel_port_requests. (Chris)
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index e781c91..24a6e68 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -657,10 +657,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   
>   	while (num_ports-- && port_isset(port)) {
>   		struct i915_request *rq = port_request(port);
> +		unsigned int notify;
>   
>   		GEM_BUG_ON(!execlists->active);
>   		intel_engine_context_out(rq->engine);
> -		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_PREEMPTED);
> +
> +		notify = INTEL_CONTEXT_SCHEDULE_PREEMPTED;
> +		if (i915_request_completed(rq))
> +			notify = INTEL_CONTEXT_SCHEDULE_OUT;
> +		execlists_context_status_change(rq, notify);
> +
>   		i915_request_put(rq);
>   
>   		memset(port, 0, sizeof(*port));
> 

I hope seqno in HWS cannot change between execlists_cancel_port_requests 
and __unwind_incomplete_requests? Some sort of delay in memory 
transaction vs the interrupt? No idea, could be talking nonsense.

 From style point of view I would probably simplify with an inline 
ternary conditional in execlists_context_status_change.

Regards,

Tvrtko