Re: [PATCH 6/9] drm/i915/execlists: Reset CSB write pointer after reset

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

On 27/06/2018 22:07, Chris Wilson wrote:
> On HW reset, the HW clears the write pointer (to 0). But since it also
> writes its first CSB entry to slot 0, we need to reset the write pointer
> back to the element before (so the first entry we read is 0).
> 
> This is required for the next patch, where we trust the CSB completely!
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 368a8c74d11d..8b111a268697 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -884,6 +884,21 @@ static void reset_irq(struct intel_engine_cs *engine)
>   	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>   }
>   
> +static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> +{
> +	/*
> +	 * After a reset, the HW starts writing into CSB entry [0]. We
> +	 * therefore have to set our HEAD pointer back one entry so that
> +	 * the *first* entry we check is entry 0. To complicate this further,
> +	 * as we don't wait for the first interrupt after reset, we have to
> +	 * fake the HW write to point back to the last entry so that our
> +	 * inline comparison of our cached head position against the last HW
> +	 * write works even before the first interrupt.
> +	 */
> +	execlists->csb_head = GEN8_CSB_ENTRIES - 1;
> +	WRITE_ONCE(*execlists->csb_write, (GEN8_CSB_ENTRIES - 1) | 0xff << 16);

Use _MASKED_FIELD and GEN8_CSB_WRITE_PTR_MASK?

> +}
> +
>   static void execlists_cancel_requests(struct intel_engine_cs *engine)
>   {
>   	struct intel_engine_execlists * const execlists = &engine->execlists;
> @@ -1953,7 +1968,7 @@ static void execlists_reset(struct intel_engine_cs *engine,
>   	__unwind_incomplete_requests(engine);
>   
>   	/* Following the reset, we need to reload the CSB read/write pointers */
> -	engine->execlists.csb_head = GEN8_CSB_ENTRIES - 1;
> +	reset_csb_pointers(&engine->execlists);
>   
>   	spin_unlock_irqrestore(&engine->timeline.lock, flags);
>   
> @@ -2452,7 +2467,6 @@ static int logical_ring_init(struct intel_engine_cs *engine)
>   			upper_32_bits(ce->lrc_desc);
>   	}
>   
> -	execlists->csb_head = GEN8_CSB_ENTRIES - 1;
>   	execlists->csb_read =
>   		i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
>   	if (csb_force_mmio(i915)) {
> @@ -2467,6 +2481,7 @@ static int logical_ring_init(struct intel_engine_cs *engine)
>   		execlists->csb_write =
>   			&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
>   	}
> +	reset_csb_pointers(execlists);
>   
>   	return 0;
>   
> 

Regards,

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