[Mesa-dev] [PATCH 5/5] r600: set the number type correctly for float rts in cb setup

[Roland Scheidegger]

Ping (for the series)?

Roland

Am 09.11.2017 um 20:00 schrieb sroland at vmware.com:
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Float rts were always set as unorm instead of float.
> Not sure of the consequences, but at least it looks like the blend clamp
> would have been enabled, which is against the rules (only eg really bothered
> to even attempt to specify this correctly, r600 always used clamp anyway).
> Albeit r600 (not r700) setup still looks bugged to me due to never setting
> BLEND_FLOAT32 which must be set according to docs...
> Not sure if the hw really cares, no piglit change (on eg/juniper).
> ---
>  src/gallium/drivers/r600/evergreen_state.c |  7 ++++++-
>  src/gallium/drivers/r600/r600_state.c      | 10 +++++++++-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index ef323bf4f6..e724cb157f 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -1042,7 +1042,7 @@ static void evergreen_set_color_surface_buffer(struct r600_context *rctx,
>  		}
>  	}
>  	ntype = V_028C70_NUMBER_UNORM;
> -		if (desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB)
> +	if (desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB)
>  		ntype = V_028C70_NUMBER_SRGB;
>  	else if (desc->channel[i].type == UTIL_FORMAT_TYPE_SIGNED) {
>  		if (desc->channel[i].normalized)
> @@ -1054,7 +1054,10 @@ static void evergreen_set_color_surface_buffer(struct r600_context *rctx,
>  			ntype = V_028C70_NUMBER_UNORM;
>  		else if (desc->channel[i].pure_integer)
>  			ntype = V_028C70_NUMBER_UINT;
> +	} else if (desc->channel[i].type == UTIL_FORMAT_TYPE_FLOAT) {
> +		ntype = V_028C70_NUMBER_FLOAT;
>  	}
> +
>  	pitch = (pitch / 8) - 1;
>  	color->pitch = S_028C64_PITCH_TILE_MAX(pitch);
>  
> @@ -1180,6 +1183,8 @@ static void evergreen_set_color_surface_common(struct r600_context *rctx,
>  			ntype = V_028C70_NUMBER_UNORM;
>  		else if (desc->channel[i].pure_integer)
>  			ntype = V_028C70_NUMBER_UINT;
> +	} else if (desc->channel[i].type == UTIL_FORMAT_TYPE_FLOAT) {
> +		ntype = V_028C70_NUMBER_FLOAT;
>  	}
>  
>  	if (R600_BIG_ENDIAN)
> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
> index db3d6db70b..f024987a30 100644
> --- a/src/gallium/drivers/r600/r600_state.c
> +++ b/src/gallium/drivers/r600/r600_state.c
> @@ -817,7 +817,7 @@ static void r600_init_color_surface(struct r600_context *rctx,
>  	unsigned offset;
>  	const struct util_format_description *desc;
>  	int i;
> -	bool blend_bypass = 0, blend_clamp = 1, do_endian_swap = FALSE;
> +	bool blend_bypass = 0, blend_clamp = 0, do_endian_swap = FALSE;
>  
>  	if (rtex->db_compatible && !r600_can_sample_zs(rtex, false)) {
>  		r600_init_flushed_depth_texture(&rctx->b.b, surf->base.texture, NULL);
> @@ -869,6 +869,8 @@ static void r600_init_color_surface(struct r600_context *rctx,
>  			ntype = V_0280A0_NUMBER_UNORM;
>  		else if (desc->channel[i].pure_integer)
>  			ntype = V_0280A0_NUMBER_UINT;
> +	} else if (desc->channel[i].type == UTIL_FORMAT_TYPE_FLOAT) {
> +		ntype = V_0280A0_NUMBER_FLOAT;
>  	}
>  
>  	if (R600_BIG_ENDIAN)
> @@ -883,6 +885,11 @@ static void r600_init_color_surface(struct r600_context *rctx,
>  
>  	endian = r600_colorformat_endian_swap(format, do_endian_swap);
>  
> +	/* blend clamp should be set for all NORM/SRGB types */
> +	if (ntype == V_0280A0_NUMBER_UNORM || ntype == V_0280A0_NUMBER_SNORM ||
> +	    ntype == V_0280A0_NUMBER_SRGB)
> +		blend_clamp = 1;
> +
>  	/* set blend bypass according to docs if SINT/UINT or
>  	   8/24 COLOR variants */
>  	if (ntype == V_0280A0_NUMBER_UINT || ntype == V_0280A0_NUMBER_SINT ||
> @@ -916,6 +923,7 @@ static void r600_init_color_surface(struct r600_context *rctx,
>  		     ntype != V_0280A0_NUMBER_UINT &&
>  		     ntype != V_0280A0_NUMBER_SINT) &&
>  		    G_0280A0_BLEND_CLAMP(color_info) &&
> +		    /* XXX this condition is always true since BLEND_FLOAT32 is never set (bug?). */
>  		    !G_0280A0_BLEND_FLOAT32(color_info)) {
>  			color_info |= S_0280A0_SOURCE_FORMAT(V_0280A0_EXPORT_NORM);
>  			surf->export_16bpc = true;
>