[Mesa-dev] [PATCH 15/15] radeonsi: emit VGT_REUSE_OFF in the right place

[Nicolai Hähnle]

I have sent comments on patches 2 & 9. Apart from those, the series is

Reviewed-by: Marek Olšák <marek.olsak at amd.com>


On 21.08.2017 23:54, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> clip_regs aren't marked dirty when writes_viewport_index is changed.
> 
> Cc: 17.2 <mesa-stable at lists.freedesktop.org>
> ---
>   src/gallium/drivers/radeonsi/si_state.c         |  6 ------
>   src/gallium/drivers/radeonsi/si_state_shaders.c | 11 +++++++++--
>   2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
> index e571201..17e2dc5 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -712,26 +712,20 @@ static void si_emit_clip_regs(struct si_context *sctx, struct r600_atom *atom)
>   
>   	radeon_set_context_reg(cs, R_02881C_PA_CL_VS_OUT_CNTL,
>   		vs_sel->pa_cl_vs_out_cntl |
>   		S_02881C_VS_OUT_CCDIST0_VEC_ENA((total_mask & 0x0F) != 0) |
>   		S_02881C_VS_OUT_CCDIST1_VEC_ENA((total_mask & 0xF0) != 0) |
>   		clipdist_mask | (culldist_mask << 8));
>   	radeon_set_context_reg(cs, R_028810_PA_CL_CLIP_CNTL,
>   		rs->pa_cl_clip_cntl |
>   		ucp_mask |
>   		S_028810_CLIP_DISABLE(window_space));
> -
> -	if (sctx->b.chip_class <= VI) {
> -		/* reuse needs to be set off if we write oViewport */
> -		radeon_set_context_reg(cs, R_028AB4_VGT_REUSE_OFF,
> -				       S_028AB4_REUSE_OFF(info->writes_viewport_index));
> -	}
>   }
>   
>   /*
>    * inferred state between framebuffer and rasterizer
>    */
>   static void si_update_poly_offset_state(struct si_context *sctx)
>   {
>   	struct si_state_rasterizer *rs = sctx->queued.named.rasterizer;
>   
>   	if (!rs || !rs->uses_poly_offset || !sctx->framebuffer.state.zsbuf) {
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 66cd902..d34f2f6 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -829,28 +829,29 @@ static void si_shader_gs(struct si_screen *sscreen, struct si_shader *shader)
>   /**
>    * Compute the state for \p shader, which will run as a vertex shader on the
>    * hardware.
>    *
>    * If \p gs is non-NULL, it points to the geometry shader for which this shader
>    * is the copy shader.
>    */
>   static void si_shader_vs(struct si_screen *sscreen, struct si_shader *shader,
>                            struct si_shader_selector *gs)
>   {
> +	const struct tgsi_shader_info *info = &shader->selector->info;
>   	struct si_pm4_state *pm4;
>   	unsigned num_user_sgprs;
>   	unsigned nparams, vgpr_comp_cnt;
>   	uint64_t va;
>   	unsigned oc_lds_en;
>   	unsigned window_space =
> -	   shader->selector->info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION];
> -	bool enable_prim_id = shader->key.mono.u.vs_export_prim_id || shader->selector->info.uses_primid;
> +	   info->properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION];
> +	bool enable_prim_id = shader->key.mono.u.vs_export_prim_id || info->uses_primid;
>   
>   	pm4 = si_get_shader_pm4_state(shader);
>   	if (!pm4)
>   		return;
>   
>   	/* We always write VGT_GS_MODE in the VS state, because every switch
>   	 * between different shader pipelines involving a different GS or no
>   	 * GS at all involves a switch of the VS (different GS use different
>   	 * copy shaders). On the other hand, when the API switches from a GS to
>   	 * no GS and then back to the same GS used originally, the GS state is
> @@ -863,20 +864,26 @@ static void si_shader_vs(struct si_screen *sscreen, struct si_shader *shader,
>   		if (enable_prim_id)
>   			mode = V_028A40_GS_SCENARIO_A;
>   
>   		si_pm4_set_reg(pm4, R_028A40_VGT_GS_MODE, S_028A40_MODE(mode));
>   		si_pm4_set_reg(pm4, R_028A84_VGT_PRIMITIVEID_EN, enable_prim_id);
>   	} else {
>   		si_pm4_set_reg(pm4, R_028A40_VGT_GS_MODE, si_vgt_gs_mode(gs));
>   		si_pm4_set_reg(pm4, R_028A84_VGT_PRIMITIVEID_EN, 0);
>   	}
>   
> +	if (sscreen->b.chip_class <= VI) {
> +		/* Reuse needs to be set off if we write oViewport. */
> +		si_pm4_set_reg(pm4, R_028AB4_VGT_REUSE_OFF,
> +			       S_028AB4_REUSE_OFF(info->writes_viewport_index));
> +	}
> +
>   	va = shader->bo->gpu_address;
>   	si_pm4_add_bo(pm4, shader->bo, RADEON_USAGE_READ, RADEON_PRIO_SHADER_BINARY);
>   
>   	if (gs) {
>   		vgpr_comp_cnt = 0; /* only VertexID is needed for GS-COPY. */
>   		num_user_sgprs = SI_GSCOPY_NUM_USER_SGPR;
>   	} else if (shader->selector->type == PIPE_SHADER_VERTEX) {
>   		/* VGPR0-3: (VertexID, InstanceID / StepRate0, PrimID, InstanceID)
>   		 * If PrimID is disabled. InstanceID / StepRate1 is loaded instead.
>   		 * StepRate0 is set to 1. so that VGPR3 doesn't have to be loaded.
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.