[Intel-gfx] [PATCH 2/4] drm/i915/gt: Check for arbitration after writing start seqno

[Tvrtko Ursulin]

On 11/01/2021 10:57, Chris Wilson wrote:
> On the off chance that we need to arbitrate before launching the
> payload, perform the check after we signal the request is ready to
> start. Assuming instantaneous processing of the CS event, the request
> will then be treated as having started when we make the decisions as to
> how to process that CS event.

What is the wider context with this change?

> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 2e36e0a9d8a6..9a182652a35e 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -361,19 +361,19 @@ int gen8_emit_init_breadcrumb(struct i915_request *rq)
>   	if (IS_ERR(cs))
>   		return PTR_ERR(cs);
>   
> +	*cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
> +	*cs++ = hwsp_offset(rq);
> +	*cs++ = 0;
> +	*cs++ = rq->fence.seqno - 1;
> +

Strictly this movement even makes the existing comment (below) more correct.

>   	/*
>   	 * Check if we have been preempted before we even get started.
>   	 *
>   	 * After this point i915_request_started() reports true, even if
>   	 * we get preempted and so are no longer running.
>   	 */
> -	*cs++ = MI_ARB_CHECK;
>   	*cs++ = MI_NOOP;
> -
> -	*cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
> -	*cs++ = hwsp_offset(rq);
> -	*cs++ = 0;
> -	*cs++ = rq->fence.seqno - 1;
> +	*cs++ = MI_ARB_CHECK;

Special reason to have NOOP before MI_ARB_CHECK or would more common 
NOOP padding at the end be suitable?

Regards,

Tvrtko

>   
>   	intel_ring_advance(rq, cs);
>   
>