Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx>
> {
> - /* 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
