Re: [RESEND 3/6] drm/i915/execlists: Always clear preempt status on cancelling all

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 16/07/2018 09:03, Chris Wilson wrote:
> On reset/wedging, we cancel all pending replies from the HW and we also
> want to cancel an outstanding preemption event. Since we use the same
> function to cancel the pending replies for reset and for a preemption
> event, we can simply clear the active tracking for all.
> 
> v2: Keep execlists_user_end() markup for wedging
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Presumably there is some commit to cite as being fixed with this?

> ---
>   drivers/gpu/drm/i915/intel_guc_submission.c | 2 --
>   drivers/gpu/drm/i915/intel_lrc.c            | 5 ++---
>   2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index de57cf6085d1..3290d77b0194 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -636,8 +636,6 @@ static void complete_preempt_context(struct intel_engine_cs *engine)
>   
>   	wait_for_guc_preempt_report(engine);
>   	intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
> -
> -	execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 703b76dcfcd2..4ef4439ff438 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -566,8 +566,6 @@ static void complete_preempt_context(struct intel_engine_execlists *execlists)
>   	__unwind_incomplete_requests(container_of(execlists,
>   						  struct intel_engine_cs,
>   						  execlists));
> -
> -	execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT);
>   }
>   
>   static void execlists_dequeue(struct intel_engine_cs *engine)
> @@ -795,7 +793,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>   		port++;
>   	}
>   
> -	execlists_user_end(execlists);
> +	execlists->active = 0;

We end up with a mish-mash of direct access and helpers to 
execlists->active. :(

Do two helpers in a row result in optimized code?

>   }
>   
>   static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> @@ -841,6 +839,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
>   
>   	/* Cancel the requests on the HW and clear the ELSP tracker. */
>   	execlists_cancel_port_requests(execlists);
> +	execlists_user_end(execlists);

And here execlists_cancel_port_requests already zeroed the whole field.

>   
>   	/* Mark all executing requests as skipped. */
>   	list_for_each_entry(rq, &engine->timeline.requests, link) {
> 

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx