Re: [Mesa-dev] [PATCH 3a/3] i965: Correctly set RENDER_SURFACE_STATE::Depth for cube map textures

2016-07-18 Thread Pohjolainen, Topi 
On Mon, Jul 18, 2016 at 10:16:53PM -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 remoe the shader hacks we've always done

remove

> since they were only needed because we were setting the depth field six
> times too large.
> 
> Signed-off-by: Jason Ekstrand 
> Cc: "12.0 11.2 11.1" 
> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp  | 21 +
>  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 ++-
>  4 files changed, 14 insertions(+), 19 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 , 
> 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_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);
> +
> surf[3] = (brw_get_surface_tiling_bits(mt->tiling) |
> -   (mt->logical_depth0 - 1) << BRW_SURFACE_DEPTH_SHIFT |
> +   (depth - 1) << BRW_SURFACE_DEPTH_SHIFT |
> (mt->pitch - 1) << BRW_SURFACE_PITCH_SHIFT);
>  
> const unsigned min_lod = tObj->MinLevel + tObj->BaseLevel - 
> mt->first_level;
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 932e62e..f4a88f3 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -276,7 +276,8 @@ gen7_emit_texture_surface_state(struct brw_context *brw,
>  int surf_index /* unused */,
>  bool rw, bool for_gather)
>  {
> -   const unsigned depth = max_layer - min_layer;
> +   const unsigned depth = (max_layer - min_layer) /
> +  (_mesa_is_cube_map_texture(target) ? 6 : 1);
> uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
>  8 * 4, 32, surf_offset);
>  
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
>

[Mesa-dev] [PATCH 3a/3] i965: Correctly set RENDER_SURFACE_STATE::Depth for cube map textures

2016-07-18 Thread Jason Ekstrand 
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 remoe the shader hacks we've always done
since they were only needed because we were setting the depth field six
times too large.

Signed-off-by: Jason Ekstrand 
Cc: "12.0 11.2 11.1" 
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp  | 21 +
 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 ++-
 4 files changed, 14 insertions(+), 19 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 , 
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_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);
+
surf[3] = (brw_get_surface_tiling_bits(mt->tiling) |
- (mt->logical_depth0 - 1) << BRW_SURFACE_DEPTH_SHIFT |
+ (depth - 1) << BRW_SURFACE_DEPTH_SHIFT |
  (mt->pitch - 1) << BRW_SURFACE_PITCH_SHIFT);
 
const unsigned min_lod = tObj->MinLevel + tObj->BaseLevel - mt->first_level;
diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
index 932e62e..f4a88f3 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
@@ -276,7 +276,8 @@ gen7_emit_texture_surface_state(struct brw_context *brw,
 int surf_index /* unused */,
 bool rw, bool for_gather)
 {
-   const unsigned depth = max_layer - min_layer;
+   const unsigned depth = (max_layer - min_layer) /
+  (_mesa_is_cube_map_texture(target) ? 6 : 1);
uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
 8 * 4, 32, surf_offset);
 
diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index bd9e2a1..89ea8cc 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -235,7 +235,8 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
 uint32_t *surf_offset, int