[Mesa-dev] radeonsi: add support for ARB_texture_view

Submitted by Marek Olšák on Oct. 17, 2015, 1:09 p.m.

Details

Message ID 1445087353-24246-1-git-send-email-maraeo@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Marek Olšák Oct. 17, 2015, 1:09 p.m.
From: Marek Olšák <marek.olsak@amd.com>

All tests pass. We don't need to do much - just set CUBE if the view
target is CUBE or CUBE_ARRAY, otherwise set the resource target.

The reason this is so simple can be that texture instructions
have a greater effect on the target than the sampler view.

Thanks Glenn for the piglit test.
---
 src/gallium/drivers/radeonsi/si_pipe.c  |  2 +-
 src/gallium/drivers/radeonsi/si_state.c | 27 +++++++++++++++++++++------
 2 files changed, 22 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index a0283b7..9faba69 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -292,6 +292,7 @@  static int si_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
 	case PIPE_CAP_TEXTURE_FLOAT_LINEAR:
 	case PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR:
 	case PIPE_CAP_DEPTH_BOUNDS_TEST:
+	case PIPE_CAP_SAMPLER_VIEW_TARGET:
 	case PIPE_CAP_TEXTURE_QUERY_LOD:
 	case PIPE_CAP_TEXTURE_GATHER_SM5:
 	case PIPE_CAP_TGSI_TXQS:
@@ -335,7 +336,6 @@  static int si_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
 	case PIPE_CAP_USER_VERTEX_BUFFERS:
 	case PIPE_CAP_FAKE_SW_MSAA:
 	case PIPE_CAP_TEXTURE_GATHER_OFFSETS:
-	case PIPE_CAP_SAMPLER_VIEW_TARGET:
 	case PIPE_CAP_VERTEXID_NOBASE:
 		return 0;
 
diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
index 00d4bc1..95ac183 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -1533,9 +1533,14 @@  static unsigned si_tex_compare(unsigned compare)
 	}
 }
 
-static unsigned si_tex_dim(unsigned dim, unsigned nr_samples)
+static unsigned si_tex_dim(unsigned res_target, unsigned view_target,
+			   unsigned nr_samples)
 {
-	switch (dim) {
+	if (view_target == PIPE_TEXTURE_CUBE ||
+	    view_target == PIPE_TEXTURE_CUBE_ARRAY)
+		res_target = view_target;
+
+	switch (res_target) {
 	default:
 	case PIPE_TEXTURE_1D:
 		return V_008F1C_SQ_RSRC_IMG_1D;
@@ -2389,6 +2394,7 @@  si_create_sampler_view_custom(struct pipe_context *ctx,
 	struct radeon_surf_level *surflevel;
 	int first_non_void;
 	uint64_t va;
+	unsigned last_layer = state->u.tex.last_layer;
 
 	if (view == NULL)
 		return NULL;
@@ -2594,6 +2600,13 @@  si_create_sampler_view_custom(struct pipe_context *ctx,
 	} else if (texture->target == PIPE_TEXTURE_CUBE_ARRAY)
 		depth = texture->array_size / 6;
 
+	/* This is not needed if state trackers set last_layer correctly. */
+	if (state->target == PIPE_TEXTURE_1D ||
+	    state->target == PIPE_TEXTURE_2D ||
+	    state->target == PIPE_TEXTURE_RECT ||
+	    state->target == PIPE_TEXTURE_CUBE)
+		last_layer = state->u.tex.first_layer;
+
 	va = tmp->resource.gpu_address + surflevel[base_level].offset;
 
 	view->state[0] = va >> 8;
@@ -2613,10 +2626,11 @@  si_create_sampler_view_custom(struct pipe_context *ctx,
 						      last_level) |
 			  S_008F1C_TILING_INDEX(si_tile_mode_index(tmp, base_level, false)) |
 			  S_008F1C_POW2_PAD(texture->last_level > 0) |
-			  S_008F1C_TYPE(si_tex_dim(texture->target, texture->nr_samples)));
+			  S_008F1C_TYPE(si_tex_dim(texture->target, state->target,
+						   texture->nr_samples)));
 	view->state[4] = (S_008F20_DEPTH(depth - 1) | S_008F20_PITCH(pitch - 1));
 	view->state[5] = (S_008F24_BASE_ARRAY(state->u.tex.first_layer) |
-			  S_008F24_LAST_ARRAY(state->u.tex.last_layer));
+			  S_008F24_LAST_ARRAY(last_layer));
 	view->state[6] = 0;
 	view->state[7] = 0;
 
@@ -2651,11 +2665,12 @@  si_create_sampler_view_custom(struct pipe_context *ctx,
 				       S_008F1C_DST_SEL_Z(V_008F1C_SQ_SEL_X) |
 				       S_008F1C_DST_SEL_W(V_008F1C_SQ_SEL_X) |
 				       S_008F1C_TILING_INDEX(tmp->fmask.tile_mode_index) |
-				       S_008F1C_TYPE(si_tex_dim(texture->target, 0));
+				       S_008F1C_TYPE(si_tex_dim(texture->target,
+								state->target, 0));
 		view->fmask_state[4] = S_008F20_DEPTH(depth - 1) |
 				       S_008F20_PITCH(tmp->fmask.pitch - 1);
 		view->fmask_state[5] = S_008F24_BASE_ARRAY(state->u.tex.first_layer) |
-				       S_008F24_LAST_ARRAY(state->u.tex.last_layer);
+				       S_008F24_LAST_ARRAY(last_layer);
 		view->fmask_state[6] = 0;
 		view->fmask_state[7] = 0;
 	}

Comments

Le Saturday 17 October 2015, 15:09:13 Marek Olšák a écrit :
> From: Marek Olšák <marek.olsak@amd.com>
> 
> All tests pass. We don't need to do much - just set CUBE if the view
> target is CUBE or CUBE_ARRAY, otherwise set the resource target.
> 
> The reason this is so simple can be that texture instructions
> have a greater effect on the target than the sampler view.
> 
> Thanks Glenn for the piglit test.

gl3.txt update is missing
On Sat, Oct 17, 2015 at 6:45 PM, Laurent Carlier <lordheavym@gmail.com> wrote:
> Le Saturday 17 October 2015, 15:09:13 Marek Olšák a écrit :
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> All tests pass. We don't need to do much - just set CUBE if the view
>> target is CUBE or CUBE_ARRAY, otherwise set the resource target.
>>
>> The reason this is so simple can be that texture instructions
>> have a greater effect on the target than the sampler view.
>>
>> Thanks Glenn for the piglit test.
>
> gl3.txt update is missing

Yes, I'll update it before pushing.

Thanks.

Marek
On 17.10.2015 22:09, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
> 
> All tests pass. We don't need to do much - just set CUBE if the view
> target is CUBE or CUBE_ARRAY, otherwise set the resource target.
> 
> The reason this is so simple can be that texture instructions
> have a greater effect on the target than the sampler view.

I think you meant to swap "is" and "can be": "The reason this can be so
simple is that texture instructions have a greater effect on the target
than the sampler view."

BTW, is that statement really accurate? Isn't it rather because the
format and mip level clamping was already handled via the sampler view
before?


Anyway, if you fix at least the switcheroo above,

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
On Tue, Oct 20, 2015 at 9:42 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 17.10.2015 22:09, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> All tests pass. We don't need to do much - just set CUBE if the view
>> target is CUBE or CUBE_ARRAY, otherwise set the resource target.
>>
>> The reason this is so simple can be that texture instructions
>> have a greater effect on the target than the sampler view.
>
> I think you meant to swap "is" and "can be": "The reason this can be so
> simple is that texture instructions have a greater effect on the target
> than the sampler view."
>
> BTW, is that statement really accurate? Isn't it rather because the
> format and mip level clamping was already handled via the sampler view
> before?

I didn't consider the format and clamping to be anything new here.

The only thing that is new with ARB_texture_view is that the texture
target can be overriden by sampler views. The thing is the hardware
doesn't care much :) setting the correct target in the shader already
had the desired effect for all types except Cube.

Marek
On Sat, Oct 17, 2015 at 9:09 AM, Marek Olšák <maraeo@gmail.com> wrote:
> +       /* This is not needed if state trackers set last_layer correctly. */
> +       if (state->target == PIPE_TEXTURE_1D ||
> +           state->target == PIPE_TEXTURE_2D ||
> +           state->target == PIPE_TEXTURE_RECT ||
> +           state->target == PIPE_TEXTURE_CUBE)
> +               last_layer = state->u.tex.first_layer;
> +

Did you observe state trackers messing this one up, or just being
defensive? Also for cube, normally last layer would be first_layer +
5...

  -ilia
On Tue, Oct 20, 2015 at 5:26 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Sat, Oct 17, 2015 at 9:09 AM, Marek Olšák <maraeo@gmail.com> wrote:
>> +       /* This is not needed if state trackers set last_layer correctly. */
>> +       if (state->target == PIPE_TEXTURE_1D ||
>> +           state->target == PIPE_TEXTURE_2D ||
>> +           state->target == PIPE_TEXTURE_RECT ||
>> +           state->target == PIPE_TEXTURE_CUBE)
>> +               last_layer = state->u.tex.first_layer;
>> +
>
> Did you observe state trackers messing this one up, or just being
> defensive? Also for cube, normally last layer would be first_layer +
> 5...

I'm just being defensive.

The hardware does the right thing if target == CUBE. first_layer ==
last_layer has no effect on cubemaps.

Marek