[Mesa-dev] [PATCH v2 3a/3] i965: Correctly set RENDER_SURFACE_STATE::Depth for cube map textures
Pohjolainen, Topi
topi.pohjolainen at intel.com
Fri Jul 22 07:30:53 UTC 2016
On Tue, Jul 19, 2016 at 08:26:12AM -0700, Jason Ekstrand wrote:
> From the Sky Lake PRM:
>
> "For SURFTYPE_CUBE: For Sampling Engine Surfaces and Typed Data Port
> Surfaces, the range of this field is [0,340], indicating the number of
> cube array elements (equal to the number of underlying 2D array elements
> divided by 6). For other surfaces, this field must be zero."
>
> In other words, the depth field for cube maps is in number of cubes not
> number of 2-D slices so we need to divide by 6. It appears as if we've
> been doing this wrong ever since we first added cube map arrays for Sandy
> Bridge. Also, we now need to remove the shader hacks we've always done
> since they were only needed because we were setting the depth field six
> times too large.
>
> v2: Fix the vec4 backend as well (not sure how I missed this).
>
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> Cc: "12.0 11.2 11.1" <mesa-stable at lists.freedesktop.org>
> ---
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 21 +++++----------------
> src/mesa/drivers/dri/i965/brw_vec4.h | 1 -
> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 8 +-------
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 15 ++++-----------
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 +++++-
> src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 3 ++-
> src/mesa/drivers/dri/i965/gen8_surface_state.c | 3 ++-
> 7 files changed, 19 insertions(+), 38 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 129984a..eeec0e2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -4423,26 +4423,15 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
> for (unsigned i = 0; i < dest_size; i++)
> nir_dest[i] = offset(dst, bld, i);
>
> - bool is_cube_array = instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE &&
> - instr->is_array;
> -
> if (instr->op == nir_texop_query_levels) {
> /* # levels is in .w */
> nir_dest[0] = offset(dst, bld, 3);
> - } else if (instr->op == nir_texop_txs && dest_size >= 3 &&
> - (devinfo->gen < 7 || is_cube_array)) {
> + } else if (instr->op == nir_texop_txs &&
> + dest_size >= 3 && devinfo->gen < 7) {
> + /* Gen4-6 return 0 instead of 1 for single layer surfaces. */
> fs_reg depth = offset(dst, bld, 2);
> - fs_reg fixed_depth = vgrf(glsl_type::int_type);
> -
> - if (is_cube_array) {
> - /* fixup #layers for cube map arrays */
> - bld.emit(SHADER_OPCODE_INT_QUOTIENT, fixed_depth, depth, brw_imm_d(6));
> - } else if (devinfo->gen < 7) {
> - /* Gen4-6 return 0 instead of 1 for single layer surfaces. */
> - bld.emit_minmax(fixed_depth, depth, brw_imm_d(1), BRW_CONDITIONAL_GE);
> - }
> -
> - nir_dest[2] = fixed_depth;
> + nir_dest[2] = vgrf(glsl_type::int_type);
> + bld.emit_minmax(nir_dest[2], depth, brw_imm_d(1), BRW_CONDITIONAL_GE);
> }
>
> bld.LOAD_PAYLOAD(get_nir_dest(instr->dest), nir_dest, dest_size, 0);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 3043147..d544e71 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -259,7 +259,6 @@ public:
> uint32_t constant_offset,
> src_reg offset_value,
> src_reg mcs,
> - bool is_cube_array,
> uint32_t surface, src_reg surface_reg,
> uint32_t sampler, src_reg sampler_reg);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index f3b4528..853a12b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -1875,16 +1875,10 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
>
> ir_texture_opcode op = ir_texture_opcode_for_nir_texop(instr->op);
>
> - bool is_cube_array =
> - instr->op == nir_texop_txs &&
> - instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE &&
> - instr->is_array;
> -
> emit_texture(op, dest, dest_type, coordinate, instr->coord_components,
> shadow_comparitor,
> lod, lod2, sample_index,
> - constant_offset, offset_value,
> - mcs, is_cube_array,
> + constant_offset, offset_value, mcs,
> texture, texture_reg, sampler, sampler_reg);
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 652b453..b87d0a6 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -907,7 +907,6 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
> uint32_t constant_offset,
> src_reg offset_value,
> src_reg mcs,
> - bool is_cube_array,
> uint32_t surface,
> src_reg surface_reg,
> uint32_t sampler,
> @@ -1095,16 +1094,10 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
> /* fixup num layers (z) for cube arrays: hardware returns faces * layers;
> * spec requires layers.
> */
> - if (op == ir_txs) {
> - if (is_cube_array) {
> - emit_math(SHADER_OPCODE_INT_QUOTIENT,
> - writemask(inst->dst, WRITEMASK_Z),
> - src_reg(inst->dst), brw_imm_d(6));
> - } else if (devinfo->gen < 7) {
> - /* Gen4-6 return 0 instead of 1 for single layer surfaces. */
> - emit_minmax(BRW_CONDITIONAL_GE, writemask(inst->dst, WRITEMASK_Z),
> - src_reg(inst->dst), brw_imm_d(1));
> - }
> + if (op == ir_txs && devinfo->gen < 7) {
> + /* Gen4-6 return 0 instead of 1 for single layer surfaces. */
> + emit_minmax(BRW_CONDITIONAL_GE, writemask(inst->dst, WRITEMASK_Z),
> + src_reg(inst->dst), brw_imm_d(1));
> }
>
> if (devinfo->gen == 6 && op == ir_tg4) {
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index c101e05..a96eae5 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -33,6 +33,7 @@
> #include "main/context.h"
> #include "main/blend.h"
> #include "main/mtypes.h"
> +#include "main/teximage.h"
> #include "main/samplerobj.h"
> #include "main/shaderimage.h"
> #include "program/prog_parameter.h"
> @@ -360,8 +361,11 @@ brw_update_texture_surface(struct gl_context *ctx,
> (mt->logical_width0 - 1) << BRW_SURFACE_WIDTH_SHIFT |
> (mt->logical_height0 - 1) << BRW_SURFACE_HEIGHT_SHIFT);
>
> + const unsigned depth = mt->logical_depth0 /
> + (_mesa_is_cube_map_texture(tObj->Target) ? 6 : 1);
In general, it starts to sound to me that logical_depth0 for cubes should
also be the number of cubes instead of the number of faces. But I suppose
this depends on point of view...
More information about the mesa-dev
mailing list