Re: [PATCH] drm/i915/execlists: Drop preemption arbitrations points along the ring

[Michel Thierry <michel.thierry@intel.com>]

On 5/3/2018 12:54 PM, Chris Wilson wrote:
> Limit the arbitration (where preemption may occur) to inside the batch,
> and prevent it from happening on the pipecontrols/flushes we use to
> write the breadcrumb seqno. Once the user batch is complete, we have
> nothing left to do but serialise and emit the breadcrumb; switching
> contexts at this point is futile so don't.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> 
> Michał and Michel,
>    please take a look and see if you can think of any objections.

No objections, I was only thinking what would happen if we arm the 
watchdog (and we decide timeout != reset the engine) and forgot to write 
a reply. For the record, I think it'd be ok anyway.

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

> -Chris
> 
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 3d747d1c3d4d..9f3cce022b2d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1933,7 +1933,7 @@ static int gen8_emit_bb_start(struct i915_request *rq,
>   		rq->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(rq->engine);
>   	}
>   
> -	cs = intel_ring_begin(rq, 4);
> +	cs = intel_ring_begin(rq, 6);
>   	if (IS_ERR(cs))
>   		return PTR_ERR(cs);
>   
> @@ -1962,6 +1962,9 @@ static int gen8_emit_bb_start(struct i915_request *rq,
>   		(flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);
>   	*cs++ = lower_32_bits(offset);
>   	*cs++ = upper_32_bits(offset);
> +
> +	*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
> +	*cs++ = MI_NOOP;
>   	intel_ring_advance(rq, cs);
>   
>   	return 0;
> @@ -2104,7 +2107,7 @@ static void gen8_emit_breadcrumb(struct i915_request *request, u32 *cs)
>   	cs = gen8_emit_ggtt_write(cs, request->global_seqno,
>   				  intel_hws_seqno_address(request->engine));
>   	*cs++ = MI_USER_INTERRUPT;
> -	*cs++ = MI_NOOP;
> +	*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
>   	request->tail = intel_ring_offset(request, cs);
>   	assert_ring_tail_valid(request->ring, request->tail);
>   
> @@ -2120,7 +2123,7 @@ static void gen8_emit_breadcrumb_rcs(struct i915_request *request, u32 *cs)
>   	cs = gen8_emit_ggtt_write_rcs(cs, request->global_seqno,
>   				      intel_hws_seqno_address(request->engine));
>   	*cs++ = MI_USER_INTERRUPT;
> -	*cs++ = MI_NOOP;
> +	*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
>   	request->tail = intel_ring_offset(request, cs);
>   	assert_ring_tail_valid(request->ring, request->tail);
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx