[Mesa-dev] [PATCH 5/5] radeonsi: calculate optimal GS ring sizes to fix GS hangs on Tonga

Nicolai Hähnle nhaehnle at gmail.com
Mon Nov 9 02:16:24 PST 2015


On 08.11.2015 23:00, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> I discovered that increasing the ESGS ring size fixes GS hangs on Tonga,
> so let's do it properly.
>
> There is now a separate init_config_gs_rings state that is not immutable,
> because GS rings are resized when needed.
>
> This also saves some memory. Most apps won't need more than 1MB
> per ring per shader engine.
> ---
>   src/gallium/drivers/radeonsi/si_hw_context.c    |   2 +
>   src/gallium/drivers/radeonsi/si_pipe.c          |   2 +
>   src/gallium/drivers/radeonsi/si_pipe.h          |   1 +
>   src/gallium/drivers/radeonsi/si_shader.h        |   1 +
>   src/gallium/drivers/radeonsi/si_state_shaders.c | 153 ++++++++++++++++--------
>   5 files changed, 112 insertions(+), 47 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c b/src/gallium/drivers/radeonsi/si_hw_context.c
> index f28c11c..baa0229 100644
> --- a/src/gallium/drivers/radeonsi/si_hw_context.c
> +++ b/src/gallium/drivers/radeonsi/si_hw_context.c
> @@ -165,6 +165,8 @@ void si_begin_new_cs(struct si_context *ctx)
>
>   	/* The CS initialization should be emitted before everything else. */
>   	si_pm4_emit(ctx, ctx->init_config);
> +	if (ctx->init_config_gs_rings)
> +		si_pm4_emit(ctx, ctx->init_config_gs_rings);
>
>   	ctx->framebuffer.dirty_cbufs = (1 << 8) - 1;
>   	ctx->framebuffer.dirty_zsbuf = true;
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
> index 2b9784f..1cbee8d 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -50,6 +50,8 @@ static void si_destroy_context(struct pipe_context *context)
>   	sctx->b.ws->fence_reference(&sctx->last_gfx_fence, NULL);
>
>   	si_pm4_free_state(sctx, sctx->init_config, ~0);
> +	if (sctx->init_config_gs_rings)
> +		si_pm4_free_state(sctx, sctx->init_config_gs_rings, ~0);
>   	for (i = 0; i < Elements(sctx->vgt_shader_config); i++)
>   		si_pm4_delete_state(sctx, vgt_shader_config, sctx->vgt_shader_config[i]);
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
> index 6e742fc..05d52fe 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -202,6 +202,7 @@ struct si_context {
>
>   	/* Precomputed states. */
>   	struct si_pm4_state		*init_config;
> +	struct si_pm4_state		*init_config_gs_rings;
>   	bool				init_config_has_vgt_flush;
>   	struct si_pm4_state		*vgt_shader_config[4];
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.h b/src/gallium/drivers/radeonsi/si_shader.h
> index 1f4f0de..3400a03 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.h
> +++ b/src/gallium/drivers/radeonsi/si_shader.h
> @@ -202,6 +202,7 @@ struct si_shader_selector {
>   	bool		forces_persample_interp_for_linear;
>
>   	unsigned	esgs_itemsize;
> +	unsigned	gs_input_verts_per_prim;
>   	unsigned	gs_output_prim;
>   	unsigned	gs_max_out_vertices;
>   	unsigned	gs_num_invocations;
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index c402ce2..b543971 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -33,6 +33,7 @@
>   #include "tgsi/tgsi_parse.h"
>   #include "tgsi/tgsi_ureg.h"
>   #include "util/u_memory.h"
> +#include "util/u_prim.h"
>   #include "util/u_simple_shaders.h"
>
>   static void si_set_tesseval_regs(struct si_shader *shader,
> @@ -703,6 +704,9 @@ static void *si_create_shader_selector(struct pipe_context *ctx,
>   		for (i = 0; i < sel->so.num_outputs; i++)
>   			sel->max_gs_stream = MAX2(sel->max_gs_stream,
>   						  sel->so.output[i].stream);
> +
> +		sel->gs_input_verts_per_prim =
> +			u_vertices_per_prim(sel->info.properties[TGSI_PROPERTY_GS_INPUT_PRIM]);
>   		break;
>
>   	case PIPE_SHADER_VERTEX:
> @@ -1054,6 +1058,7 @@ static void si_init_config_add_vgt_flush(struct si_context *sctx)
>   	if (sctx->init_config_has_vgt_flush)
>   		return;
>
> +	/* VGT_FLUSH is required even if VGT is idle. It resets VGT pointers. */
>   	si_pm4_cmd_begin(sctx->init_config, PKT3_EVENT_WRITE);
>   	si_pm4_cmd_add(sctx->init_config, EVENT_TYPE(V_028A90_VGT_FLUSH) | EVENT_INDEX(0));
>   	si_pm4_cmd_end(sctx->init_config, false);
> @@ -1061,62 +1066,119 @@ static void si_init_config_add_vgt_flush(struct si_context *sctx)
>   }
>
>   /* Initialize state related to ESGS / GSVS ring buffers */
> -static void si_init_gs_rings(struct si_context *sctx)
> +static bool si_update_gs_ring_buffers(struct si_context *sctx)
>   {
> -	unsigned esgs_ring_size = 128 * 1024;
> -	unsigned gsvs_ring_size = 60 * 1024 * 1024;
> +	struct si_shader_selector *es =
> +		sctx->tes_shader.cso ? sctx->tes_shader.cso : sctx->vs_shader.cso;
> +	struct si_shader_selector *gs = sctx->gs_shader.cso;
> +	struct si_pm4_state *pm4;
>
> -	assert(!sctx->esgs_ring && !sctx->gsvs_ring);
> +	/* Chip constants. */
> +	unsigned num_se = sctx->screen->b.info.max_se;
> +	unsigned wave_size = 64;
> +	unsigned max_gs_waves = 32 * num_se; /* max 32 per SE on GCN */
> +	unsigned gs_vertex_reuse = 16 * num_se; /* GS_VERTEX_REUSE register (per SE) */
> +	unsigned alignment = 256 * num_se;
> +	/* The maximum size is 63.999 MB per SE. */
> +	unsigned max_size = ((unsigned)(63.999 * 1024 * 1024) & ~255) * num_se;
> +
> +	/* Calculate the minimum size. */
> +	unsigned min_esgs_ring_size = align(es->esgs_itemsize * gs_vertex_reuse *
> +					    wave_size, alignment);
> +
> +	/* These are recommended sizes, not minimum sizes. */
> +	unsigned esgs_ring_size = max_gs_waves * 2 * wave_size *
> +				  es->esgs_itemsize * gs->gs_input_verts_per_prim;
> +	unsigned gsvs_ring_size = max_gs_waves * 2 * wave_size *
> +				  gs->max_gsvs_emit_size * (gs->max_gs_stream + 1);
> +
> +	min_esgs_ring_size = align(min_esgs_ring_size, alignment);
> +	esgs_ring_size = align(esgs_ring_size, alignment);
> +	gsvs_ring_size = align(gsvs_ring_size, alignment);
> +
> +	esgs_ring_size = CLAMP(esgs_ring_size, min_esgs_ring_size, max_size);
> +	gsvs_ring_size = MIN2(gsvs_ring_size, max_size);
> +
> +	/* Some rings don't have to be allocated if shaders don't use them.
> +	 * (e.g. no varyings between ES and GS or GS and PS)
> +	 */
> +	bool update_esgs = esgs_ring_size &&
> +			   (!sctx->esgs_ring ||
> +			    sctx->esgs_ring->width0 < esgs_ring_size);
> +	bool update_gsvs = gsvs_ring_size &&
> +			   (!sctx->gsvs_ring ||
> +			    sctx->gsvs_ring->width0 < gsvs_ring_size);

I take it the comment above should be "or GS and _VS_".

With this, the series is

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

>
> -	sctx->esgs_ring = pipe_buffer_create(sctx->b.b.screen, PIPE_BIND_CUSTOM,
> -				       PIPE_USAGE_DEFAULT, esgs_ring_size);
> -	if (!sctx->esgs_ring)
> -		return;
> +	if (!update_esgs && !update_gsvs)
> +		return true;
>
> -	sctx->gsvs_ring = pipe_buffer_create(sctx->b.b.screen, PIPE_BIND_CUSTOM,
> -					     PIPE_USAGE_DEFAULT, gsvs_ring_size);
> -	if (!sctx->gsvs_ring) {
> +	if (update_esgs) {
>   		pipe_resource_reference(&sctx->esgs_ring, NULL);
> -		return;
> +		sctx->esgs_ring = pipe_buffer_create(sctx->b.b.screen, PIPE_BIND_CUSTOM,
> +						     PIPE_USAGE_DEFAULT,
> +						     esgs_ring_size);
> +		if (!sctx->esgs_ring)
> +			return false;
>   	}
>
> -	si_init_config_add_vgt_flush(sctx);
> +	if (update_gsvs) {
> +		pipe_resource_reference(&sctx->gsvs_ring, NULL);
> +		sctx->gsvs_ring = pipe_buffer_create(sctx->b.b.screen, PIPE_BIND_CUSTOM,
> +						     PIPE_USAGE_DEFAULT,
> +						     gsvs_ring_size);
> +		if (!sctx->gsvs_ring)
> +			return false;
> +	}
> +
> +	/* Create the "init_config_gs_rings" state. */
> +	pm4 = CALLOC_STRUCT(si_pm4_state);
> +	if (!pm4)
> +		return false;
>
> -	/* Append these registers to the init config state. */
>   	if (sctx->b.chip_class >= CIK) {
> -		if (sctx->b.chip_class >= VI) {
> -			/* The maximum sizes are 63.999 MB on VI, because
> -			 * the register fields only have 18 bits. */
> -			assert(esgs_ring_size / 256 < (1 << 18));
> -			assert(gsvs_ring_size / 256 < (1 << 18));
> -		}
> -		si_pm4_set_reg(sctx->init_config, R_030900_VGT_ESGS_RING_SIZE,
> -			       esgs_ring_size / 256);
> -		si_pm4_set_reg(sctx->init_config, R_030904_VGT_GSVS_RING_SIZE,
> -			       gsvs_ring_size / 256);
> +		if (sctx->esgs_ring)
> +			si_pm4_set_reg(pm4, R_030900_VGT_ESGS_RING_SIZE,
> +				       sctx->esgs_ring->width0 / 256);
> +		if (sctx->gsvs_ring)
> +			si_pm4_set_reg(pm4, R_030904_VGT_GSVS_RING_SIZE,
> +				       sctx->gsvs_ring->width0 / 256);
>   	} else {
> -		si_pm4_set_reg(sctx->init_config, R_0088C8_VGT_ESGS_RING_SIZE,
> -			       esgs_ring_size / 256);
> -		si_pm4_set_reg(sctx->init_config, R_0088CC_VGT_GSVS_RING_SIZE,
> -			       gsvs_ring_size / 256);
> +		if (sctx->esgs_ring)
> +			si_pm4_set_reg(pm4, R_0088C8_VGT_ESGS_RING_SIZE,
> +				       sctx->esgs_ring->width0 / 256);
> +		if (sctx->gsvs_ring)
> +			si_pm4_set_reg(pm4, R_0088CC_VGT_GSVS_RING_SIZE,
> +				       sctx->gsvs_ring->width0 / 256);
>   	}
>
> -	/* Flush the context to re-emit the init_config state.
> -	 * This is done only once in a lifetime of a context.
> -	 */
> -	si_pm4_upload_indirect_buffer(sctx, sctx->init_config);
> +	/* Set the state. */
> +	if (sctx->init_config_gs_rings)
> +		si_pm4_free_state(sctx, sctx->init_config_gs_rings, ~0);
> +	sctx->init_config_gs_rings = pm4;
> +
> +	if (!sctx->init_config_has_vgt_flush) {
> +		si_init_config_add_vgt_flush(sctx);
> +		si_pm4_upload_indirect_buffer(sctx, sctx->init_config);
> +	}
> +
> +	/* Flush the context to re-emit the state. */
>   	sctx->b.initial_gfx_cs_size = 0; /* force flush */
>   	si_context_gfx_flush(sctx, RADEON_FLUSH_ASYNC, NULL);
>
> -	si_set_ring_buffer(&sctx->b.b, PIPE_SHADER_VERTEX, SI_RING_ESGS,
> -			   sctx->esgs_ring, 0, esgs_ring_size,
> -			   true, true, 4, 64, 0);
> -	si_set_ring_buffer(&sctx->b.b, PIPE_SHADER_GEOMETRY, SI_RING_ESGS,
> -			   sctx->esgs_ring, 0, esgs_ring_size,
> -			   false, false, 0, 0, 0);
> -	si_set_ring_buffer(&sctx->b.b, PIPE_SHADER_VERTEX, SI_RING_GSVS,
> -			   sctx->gsvs_ring, 0, gsvs_ring_size,
> -			   false, false, 0, 0, 0);
> +	/* Set ring bindings. */
> +	if (sctx->esgs_ring) {
> +		si_set_ring_buffer(&sctx->b.b, PIPE_SHADER_VERTEX, SI_RING_ESGS,
> +				   sctx->esgs_ring, 0, sctx->esgs_ring->width0,
> +				   true, true, 4, 64, 0);
> +		si_set_ring_buffer(&sctx->b.b, PIPE_SHADER_GEOMETRY, SI_RING_ESGS,
> +				   sctx->esgs_ring, 0, sctx->esgs_ring->width0,
> +				   false, false, 0, 0, 0);
> +	}
> +	if (sctx->gsvs_ring)
> +		si_set_ring_buffer(&sctx->b.b, PIPE_SHADER_VERTEX, SI_RING_GSVS,
> +				   sctx->gsvs_ring, 0, sctx->gsvs_ring->width0,
> +				   false, false, 0, 0, 0);
> +	return true;
>   }
>
>   static void si_update_gsvs_ring_bindings(struct si_context *sctx)
> @@ -1124,7 +1186,7 @@ static void si_update_gsvs_ring_bindings(struct si_context *sctx)
>   	unsigned gsvs_itemsize = sctx->gs_shader.cso->max_gsvs_emit_size;
>   	uint64_t offset;
>
> -	if (gsvs_itemsize == sctx->last_gsvs_itemsize)
> +	if (!sctx->gsvs_ring || gsvs_itemsize == sctx->last_gsvs_itemsize)
>   		return;
>
>   	sctx->last_gsvs_itemsize = gsvs_itemsize;
> @@ -1485,11 +1547,8 @@ bool si_update_shaders(struct si_context *sctx)
>   		si_pm4_bind_state(sctx, vs, sctx->gs_shader.current->gs_copy_shader->pm4);
>   		si_update_so(sctx, sctx->gs_shader.cso);
>
> -		if (!sctx->gsvs_ring) {
> -			si_init_gs_rings(sctx);
> -			if (!sctx->gsvs_ring)
> -				return false;
> -		}
> +		if (!si_update_gs_ring_buffers(sctx))
> +			return false;
>
>   		si_update_gsvs_ring_bindings(sctx);
>   	} else {
>



More information about the mesa-dev mailing list