[Intel-gfx] [PATCH 1/2] drm/i915/gt: Expose more parameters for emitting writes into the ring

[Mika Kuoppala]

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

> Add another lower level to emit_ggtt_write so that the GGTT nature of
> the write is not hardcoded into the emitter.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h | 55 ++++++++++++++++----------
>  1 file changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 7c3a1012e702..760fefdfe392 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -245,22 +245,14 @@ static inline u32 *gen12_emit_pipe_control(u32 *batch, u32 flags0, u32 flags1, u
>  }
>  
>  static inline u32 *
> -__gen8_emit_ggtt_write_rcs(u32 *cs, u32 value, u32 gtt_offset, u32 flags0, u32 flags1)
> +__gen8_emit_write_rcs(u32 *cs, u32 value, u32 offset, u32 flags0, u32 flags1)

Opportunity to swap the offset/value to be in line with the actual qw
write. Just an observation rather than a value add proposal.

Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

>  {
> -	/* We're using qword write, offset should be aligned to 8 bytes. */
> -	GEM_BUG_ON(!IS_ALIGNED(gtt_offset, 8));
> -
> -	/* w/a for post sync ops following a GPGPU operation we
> -	 * need a prior CS_STALL, which is emitted by the flush
> -	 * following the batch.
> -	 */
>  	*cs++ = GFX_OP_PIPE_CONTROL(6) | flags0;
> -	*cs++ = flags1 | PIPE_CONTROL_QW_WRITE | PIPE_CONTROL_GLOBAL_GTT_IVB;
> -	*cs++ = gtt_offset;
> +	*cs++ = flags1 | PIPE_CONTROL_QW_WRITE;
> +	*cs++ = offset;
>  	*cs++ = 0;
>  	*cs++ = value;
> -	/* We're thrashing one dword of HWS. */
> -	*cs++ = 0;
> +	*cs++ = 0; /* We're thrashing one extra dword. */
>  
>  	return cs;
>  }
> @@ -268,13 +260,38 @@ __gen8_emit_ggtt_write_rcs(u32 *cs, u32 value, u32 gtt_offset, u32 flags0, u32 f
>  static inline u32*
>  gen8_emit_ggtt_write_rcs(u32 *cs, u32 value, u32 gtt_offset, u32 flags)
>  {
> -	return __gen8_emit_ggtt_write_rcs(cs, value, gtt_offset, 0, flags);
> +	/* We're using qword write, offset should be aligned to 8 bytes. */
> +	GEM_BUG_ON(!IS_ALIGNED(gtt_offset, 8));
> +
> +	return __gen8_emit_write_rcs(cs,
> +				     value,
> +				     gtt_offset,
> +				     0,
> +				     flags | PIPE_CONTROL_GLOBAL_GTT_IVB);
>  }
>  
>  static inline u32*
>  gen12_emit_ggtt_write_rcs(u32 *cs, u32 value, u32 gtt_offset, u32 flags0, u32 flags1)
>  {
> -	return __gen8_emit_ggtt_write_rcs(cs, value, gtt_offset, flags0, flags1);
> +	/* We're using qword write, offset should be aligned to 8 bytes. */
> +	GEM_BUG_ON(!IS_ALIGNED(gtt_offset, 8));
> +
> +	return __gen8_emit_write_rcs(cs,
> +				     value,
> +				     gtt_offset,
> +				     flags0,
> +				     flags1 | PIPE_CONTROL_GLOBAL_GTT_IVB);
> +}
> +
> +static inline u32 *
> +__gen8_emit_flush_dw(u32 *cs, u32 value, u32 gtt_offset, u32 flags)
> +{
> +	*cs++ = (MI_FLUSH_DW + 1) | flags;
> +	*cs++ = gtt_offset;
> +	*cs++ = 0;
> +	*cs++ = value;
> +
> +	return cs;
>  }
>  
>  static inline u32 *
> @@ -285,12 +302,10 @@ gen8_emit_ggtt_write(u32 *cs, u32 value, u32 gtt_offset, u32 flags)
>  	/* Offset should be aligned to 8 bytes for both (QW/DW) write types */
>  	GEM_BUG_ON(!IS_ALIGNED(gtt_offset, 8));
>  
> -	*cs++ = (MI_FLUSH_DW + 1) | MI_FLUSH_DW_OP_STOREDW | flags;
> -	*cs++ = gtt_offset | MI_FLUSH_DW_USE_GTT;
> -	*cs++ = 0;
> -	*cs++ = value;
> -
> -	return cs;
> +	return __gen8_emit_flush_dw(cs,
> +				    value,
> +				    gtt_offset | MI_FLUSH_DW_USE_GTT,
> +				    flags | MI_FLUSH_DW_OP_STOREDW);
>  }
>  
>  static inline void __intel_engine_reset(struct intel_engine_cs *engine,
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx