[Mesa-dev] [PATCH] r600/eg: use texture target to pick array size not view target (v2)

[Roland Scheidegger]

Am 02.02.2018 um 06:33 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
> 
> This fixes a few CTS cases in :
> KHR-GL45.texture_view.view_sampling
> 
> some multisample cases are still broken, but not sure this is
> the same problem.
> 
> v2: fix more cases
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/gallium/drivers/r600/evergreen_state.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index 63a39a23f8..90f05c06d3 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -811,18 +811,21 @@ static int evergreen_fill_tex_resource_words(struct r600_context *rctx,
>  	}
>  	nbanks = eg_num_banks(rscreen->b.info.r600_num_banks);
>  
> -	if (params->target == PIPE_TEXTURE_1D_ARRAY) {
> -	        height = 1;
> -		depth = texture->array_size;
> -	} else if (params->target == PIPE_TEXTURE_2D_ARRAY) {
> -		depth = texture->array_size;
> -	} else if (params->target == PIPE_TEXTURE_CUBE_ARRAY)
> -		depth = texture->array_size / 6;
>  
>  	va = tmp->resource.gpu_address;
>  
>  	/* array type views and views into array types need to use layer offset */
>  	dim = r600_tex_dim(tmp, params->target, texture->nr_samples);
> +
> +	if (dim == V_030000_SQ_TEX_DIM_1D_ARRAY) {
> +	        height = 1;
> +		depth = texture->array_size;
I know this is unchanged, but is this actually correct? The docs say:
TEX_HEIGHT 13:0 0x0
Height of the texture minus 1; number of stacks minus 1,
for 1D arrays.
TEX_DEPTH 26:14 0x0
Depth of the texture minus 1; number of stacks minus 1,
for 2D arrays.

So if some weirdness is going on here I think some comment would be nice.


> +	} else if (dim == V_030000_SQ_TEX_DIM_2D_ARRAY ||
> +		   dim == V_030000_SQ_TEX_DIM_2D_ARRAY_MSAA) {
> +		depth = texture->array_size;
> +	} else if (dim == V_030000_SQ_TEX_DIM_CUBEMAP)
> +		depth = texture->array_size / 6;
> +
>  	tex_resource_words[0] = (S_030000_DIM(dim) |
>  				 S_030000_PITCH((pitch / 8) - 1) |
>  				 S_030000_TEX_WIDTH(width - 1));
> 

This looks otherwise alright to me.
Reviewed-by: Roland Scheidegger <sroland at vmware.com>

FWIW there's some bits just below which also contradict the docs:

	if (params->target != texture->target && depth == 1) {
		last_layer = params->first_layer;
	}
So, if params->target was CUBEMAP_ARRAY (or just cubemap) and
texture->target was 2D_ARRAY with a array size of 6, then depth would be
1 and we'd smash last_layer to first_layer. The docs say "For cubemaps
and cubemap arrays, LAST_ARRAY must be programmed with BASE_ARRAY +
(N*6) - 1, where N is the number of cubemaps in the array, or N=1 for a
single cubemap." So I have no idea what this bit here is supposed to do,
but it doesn't look right.

Maybe should fix these issues on r600 too? Albeit we don't support
ARB_texture_view there so I suppose it's not really needed. (Though
afaik the chip should be able to support it, since d3d10 requires it,
but it's always possible there's some gl specific bits which don't work,
in particular view format classes compatibility is quite different, but
maybe fixing those issues there might be all that's needed?)
Albeit r600 is quite ancient at this point :-).

Roland