[Mesa-stable] [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-stable mailing list