Re: [PATCH] drm/i915/gt: Try an extra flush on the Haswell blitter

From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/gt: Try an extra flush on the Haswell blitter
Date: Tue, 12 Nov 2019 15:58:36 +0200	[thread overview]
Message-ID: <87sgmt42nn.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20191111120957.17732-1-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On gen7, including Haswell, the MI_FLUSH_DW command is not synchronous
> with the command streamer nor is there an option to make it so. To hide
> this we add a large delay on the CS so that the breadcrumb should always
> be visible before the interrupt. However, that does not seem to be
> enough to ensure the memory is actually coherent with the read of the
> breadcrumb. The breadcrumb update is a post-sync op... Throw in a
> preliminary MI_FLUSH_DW before the breadcrumb update in the hope that
> helps.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112147

If we are not sure, References

> Testcase: igt/i915_selftest/live_blt
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_ring_submission.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> index a47d5a7c32c9..fc29df712810 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -454,6 +454,10 @@ static u32 *gen7_xcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
>  	GEM_BUG_ON(i915_request_active_timeline(rq)->hwsp_ggtt != rq->engine->status_page.vma);
>  	GEM_BUG_ON(offset_in_page(i915_request_active_timeline(rq)->hwsp_offset) != I915_GEM_HWS_SEQNO_ADDR);
>  
> +	*cs++ = MI_FLUSH_DW;
> +	*cs++ = 0;
> +	*cs++ = 0;

I see this as wmb wrt seqno write.

Massive hammer would be to include timestamp write in here with tlb
flush?

But one can always hope.

> +
>  	*cs++ = MI_FLUSH_DW | MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_STORE_INDEX;
>  	*cs++ = I915_GEM_HWS_SEQNO_ADDR | MI_FLUSH_DW_USE_GTT;
>  	*cs++ = rq->fence.seqno;
> @@ -469,7 +473,6 @@ static u32 *gen7_xcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
>  	*cs++ = 0;
>  
>  	*cs++ = MI_USER_INTERRUPT;
> -	*cs++ = MI_NOOP;

Evens,
Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>  
>  	rq->tail = intel_ring_offset(rq, cs);
>  	assert_ring_tail_valid(rq->ring, rq->tail);
> -- 
> 2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx