[Mesa-dev] [PATCH] radeonsi: don't set DATA_FORMAT if ADD_TID_ENABLE is set on VI (v2)

[Michel Dänzer]

On 01.10.2015 04:11, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> This can cause incorrect address calculations and hangs.
> 
> v2: do it properly
> 
> Cc: mesa-stable at lists.freedesktop.org
> Tested-and-Reviewed-by: Christian König <christian.koenig at amd.com>
> ---
>  src/gallium/drivers/radeonsi/si_descriptors.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
> index b07ab3b..b56219a 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -619,11 +619,18 @@ void si_set_ring_buffer(struct pipe_context *ctx, uint shader, uint slot,
>  			  S_008F0C_DST_SEL_Z(V_008F0C_SQ_SEL_Z) |
>  			  S_008F0C_DST_SEL_W(V_008F0C_SQ_SEL_W) |
>  			  S_008F0C_NUM_FORMAT(V_008F0C_BUF_NUM_FORMAT_FLOAT) |
> -			  S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32) |
>  			  S_008F0C_ELEMENT_SIZE(element_size) |
>  			  S_008F0C_INDEX_STRIDE(index_stride) |
>  			  S_008F0C_ADD_TID_ENABLE(add_tid);
>  
> +		/* If ADD_TID_ENABLE is set on VI, DATA_FORMAT specifies
> +		 * STRIDE bits [14:17]
> +		 */
> +		if (sctx->b.chip_class >= VI && add_tid)
> +			desc[3] |= S_008F0C_DATA_FORMAT(stride >> 14);
> +		else
> +			desc[3] |= S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32);

The beginning of the function has:

	/* The stride field in the resource descriptor has 14 bits */
	assert(stride < (1 << 14));

So the if-branch is dead code in a non-release build. Would be nice if
that could be reconciled somehow, but I'm fine with doing it in a
follow-up change. Either way, this change is

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer