[Mesa-dev] [PATCH 12/12] radeonsi: turn SDMA IBs into de-facto preambles of GFX IBs

[Nicolai Hähnle]

That Deus Ex number is impressive. For the series:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

On 02.01.2017 23:54, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> Draw calls no longer flush SDMA IBs. r600_need_dma_space is
> responsible for synchronizing execution between both IBs.
>
> Initial buffer clears and fast clears will stay unflushed in the SDMA IB
> (up to 64 MB) as long as the GFX IB isn't flushed either.
> ---
>  src/gallium/drivers/radeon/r600_pipe_common.c |  4 ++--
>  src/gallium/drivers/radeonsi/si_hw_context.c  | 20 ++++++++++++++++----
>  2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c b/src/gallium/drivers/radeon/r600_pipe_common.c
> index 4d8bb74..28bb791 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.c
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.c
> @@ -352,23 +352,23 @@ static void r600_flush_from_st(struct pipe_context *ctx,
>  	unsigned rflags = 0;
>  	struct pipe_fence_handle *gfx_fence = NULL;
>  	struct pipe_fence_handle *sdma_fence = NULL;
>  	bool deferred_fence = false;
>
>  	if (flags & PIPE_FLUSH_END_OF_FRAME)
>  		rflags |= RADEON_FLUSH_END_OF_FRAME;
>  	if (flags & PIPE_FLUSH_DEFERRED)
>  		rflags |= RADEON_FLUSH_ASYNC;
>
> -	if (rctx->dma.cs) {
> +	/* DMA IBs are preambles to gfx IBs, therefore must be flushed first. */
> +	if (rctx->dma.cs)
>  		rctx->dma.flush(rctx, rflags, fence ? &sdma_fence : NULL);
> -	}
>
>  	if (!radeon_emitted(rctx->gfx.cs, rctx->initial_gfx_cs_size)) {
>  		if (fence)
>  			ws->fence_reference(&gfx_fence, rctx->last_gfx_fence);
>  		if (!(rflags & RADEON_FLUSH_ASYNC))
>  			ws->cs_sync_flush(rctx->gfx.cs);
>  	} else {
>  		/* Instead of flushing, create a deferred fence. Constraints:
>  		 * - The state tracker must allow a deferred flush.
>  		 * - The state tracker must request a fence.
> diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c b/src/gallium/drivers/radeonsi/si_hw_context.c
> index 87ba316..57eaac9 100644
> --- a/src/gallium/drivers/radeonsi/si_hw_context.c
> +++ b/src/gallium/drivers/radeonsi/si_hw_context.c
> @@ -55,25 +55,27 @@ static unsigned si_ce_needed_cs_space(void)
>  	space += 2;
>
>  	return space;
>  }
>
>  /* initialize */
>  void si_need_cs_space(struct si_context *ctx)
>  {
>  	struct radeon_winsys_cs *cs = ctx->b.gfx.cs;
>  	struct radeon_winsys_cs *ce_ib = ctx->ce_ib;
> -	struct radeon_winsys_cs *dma = ctx->b.dma.cs;
>
> -	/* Flush the DMA IB if it's not empty. */
> -	if (radeon_emitted(dma, 0))
> -		ctx->b.dma.flush(ctx, RADEON_FLUSH_ASYNC, NULL);
> +	/* There is no need to flush the DMA IB here, because
> +	 * r600_need_dma_space always flushes the GFX IB if there is
> +	 * a conflict, which means any unflushed DMA commands automatically
> +	 * precede the GFX IB (= they had no dependency on the GFX IB when
> +	 * they were submitted).
> +	 */
>
>  	/* There are two memory usage counters in the winsys for all buffers
>  	 * that have been added (cs_add_buffer) and two counters in the pipe
>  	 * driver for those that haven't been added yet.
>  	 */
>  	if (unlikely(!radeon_cs_memory_below_limit(ctx->b.screen, ctx->b.gfx.cs,
>  						   ctx->b.vram, ctx->b.gtt))) {
>  		ctx->b.gtt = 0;
>  		ctx->b.vram = 0;
>  		ctx->b.gfx.flush(ctx, RADEON_FLUSH_ASYNC, NULL);
> @@ -99,20 +101,30 @@ void si_context_gfx_flush(void *context, unsigned flags,
>
>  	if (ctx->gfx_flush_in_progress)
>  		return;
>
>  	if (!radeon_emitted(cs, ctx->b.initial_gfx_cs_size))
>  		return;
>
>  	if (r600_check_device_reset(&ctx->b))
>  		return;
>
> +	/* If the state tracker is flushing the GFX IB, r600_flush_from_st is
> +	 * responsible for flushing the DMA IB and merging the fences from both.
> +	 * This code is only needed when the driver flushes the GFX IB
> +	 * internally, and it never asks for a fence handle.
> +	 */
> +	if (radeon_emitted(ctx->b.dma.cs, 0)) {
> +		assert(fence == NULL); /* internal flushes only */
> +		ctx->b.dma.flush(ctx, flags, NULL);
> +	}
> +
>  	ctx->gfx_flush_in_progress = true;
>
>  	r600_preflush_suspend_features(&ctx->b);
>
>  	ctx->b.flags |= SI_CONTEXT_CS_PARTIAL_FLUSH |
>  			SI_CONTEXT_PS_PARTIAL_FLUSH;
>
>  	/* DRM 3.1.0 doesn't flush TC for VI correctly. */
>  	if (ctx->b.chip_class == VI && ctx->b.screen->info.drm_minor <= 1)
>  		ctx->b.flags |= SI_CONTEXT_INV_GLOBAL_L2 |
>