Re: [Mesa-dev] [PATCH 2/2] i965: Use 8x4 aligned rectangles for HiZ operations on Broadwell.

2014-06-16 Thread Jordan Justen
On Fri, Jun 13, 2014 at 5:26 PM, Kenneth Graunke kenn...@whitecape.org wrote:
 Like on Haswell, we need to use 8x4 aligned rectangle primitives for
 hierarchical depth buffer resolves and depth clears.  See the comments
 in brw_blorp.cpp's brw_hiz_op_params() constructor.  (The Broadwell
 documentation confirms that this is still necessary.)

 This patch makes the Broadwell code follow the same behavior as Chad and
 Jordan's Gen7 BLORP code.  Based on a patch by Topi Pohjolainen.

 This fixes es3conform's framebuffer_blit_functionality_scissor_blit
 test, with no Piglit regressions.

 Signed-off-by: Kenneth Graunke kenn...@whitecape.org
 Cc: 10.2 mesa-sta...@lists.freedesktop.org
 ---
  src/mesa/drivers/dri/i965/gen8_depth_state.c | 20 
  1 file changed, 16 insertions(+), 4 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c 
 b/src/mesa/drivers/dri/i965/gen8_depth_state.c
 index 8c70c62..f538d23 100644
 --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
 +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
 @@ -225,6 +225,13 @@ gen8_hiz_exec(struct brw_context *brw, struct 
 intel_mipmap_tree *mt,
 assert(mt-first_level == 0);
 assert(mt-logical_depth0 = 1);

 +   /* If we're operating on LOD 0, align to 8x4 to meet the alignment
 +* requirements for most HiZ operations.  Otherwise, use the actual size
 +* to allow the hardware to calculate the miplevel offsets correctly.
 +*/
 +   uint32_t surface_width  = ALIGN(mt-logical_width0,  level == 0 ? 8 : 1);
 +   uint32_t surface_height = ALIGN(mt-logical_height0, level == 0 ? 4 : 1);
 +
 /* The basic algorithm is:
  * - If needed, emit 3DSTATE_{DEPTH,HIER_DEPTH,STENCIL}_BUFFER and
  *   3DSTATE_CLEAR_PARAMS packets to set up the relevant buffers.
 @@ -239,14 +246,19 @@ gen8_hiz_exec(struct brw_context *brw, struct 
 intel_mipmap_tree *mt,
true, /* depth writes */
NULL, false, 0, /* no stencil for now */
true, /* hiz */
 -  mt-logical_width0,
 -  mt-logical_height0,
 +  surface_width,
 +  surface_height,
mt-logical_depth0,
level,
layer); /* min_array_element */

 -   unsigned rect_width = minify(mt-logical_width0, level);
 -   unsigned rect_height = minify(mt-logical_height0, level);
 +   /* Depth buffer clears and HiZ resolves must use an 8x4 aligned rectangle.
 +* Note that intel_miptree_level_enable_hiz disables HiZ for miplevels  0
 +* which aren't 8x4 aligned, so expanding the size is safe - it'll just
 +* draw into empty padding space.
 +*/

How about ... size is safe. For LOD 0 we may expand the size (by
aligning), but it is safe because at LOD 0 it overdraws into empty
padding space.

Series Reviewed-by: Jordan Justen jordan.l.jus...@intel.com

 +   unsigned rect_width = ALIGN(minify(mt-logical_width0, level), 8);
 +   unsigned rect_height = ALIGN(minify(mt-logical_height0, level), 4);

 BEGIN_BATCH(4);
 OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE  16 | (4 - 2));
 --
 1.9.1

 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] i965: Use 8x4 aligned rectangles for HiZ operations on Broadwell.

2014-06-13 Thread Kenneth Graunke
Like on Haswell, we need to use 8x4 aligned rectangle primitives for
hierarchical depth buffer resolves and depth clears.  See the comments
in brw_blorp.cpp's brw_hiz_op_params() constructor.  (The Broadwell
documentation confirms that this is still necessary.)

This patch makes the Broadwell code follow the same behavior as Chad and
Jordan's Gen7 BLORP code.  Based on a patch by Topi Pohjolainen.

This fixes es3conform's framebuffer_blit_functionality_scissor_blit
test, with no Piglit regressions.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
Cc: 10.2 mesa-sta...@lists.freedesktop.org
---
 src/mesa/drivers/dri/i965/gen8_depth_state.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c 
b/src/mesa/drivers/dri/i965/gen8_depth_state.c
index 8c70c62..f538d23 100644
--- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
@@ -225,6 +225,13 @@ gen8_hiz_exec(struct brw_context *brw, struct 
intel_mipmap_tree *mt,
assert(mt-first_level == 0);
assert(mt-logical_depth0 = 1);
 
+   /* If we're operating on LOD 0, align to 8x4 to meet the alignment
+* requirements for most HiZ operations.  Otherwise, use the actual size
+* to allow the hardware to calculate the miplevel offsets correctly.
+*/
+   uint32_t surface_width  = ALIGN(mt-logical_width0,  level == 0 ? 8 : 1);
+   uint32_t surface_height = ALIGN(mt-logical_height0, level == 0 ? 4 : 1);
+
/* The basic algorithm is:
 * - If needed, emit 3DSTATE_{DEPTH,HIER_DEPTH,STENCIL}_BUFFER and
 *   3DSTATE_CLEAR_PARAMS packets to set up the relevant buffers.
@@ -239,14 +246,19 @@ gen8_hiz_exec(struct brw_context *brw, struct 
intel_mipmap_tree *mt,
   true, /* depth writes */
   NULL, false, 0, /* no stencil for now */
   true, /* hiz */
-  mt-logical_width0,
-  mt-logical_height0,
+  surface_width,
+  surface_height,
   mt-logical_depth0,
   level,
   layer); /* min_array_element */
 
-   unsigned rect_width = minify(mt-logical_width0, level);
-   unsigned rect_height = minify(mt-logical_height0, level);
+   /* Depth buffer clears and HiZ resolves must use an 8x4 aligned rectangle.
+* Note that intel_miptree_level_enable_hiz disables HiZ for miplevels  0
+* which aren't 8x4 aligned, so expanding the size is safe - it'll just
+* draw into empty padding space.
+*/
+   unsigned rect_width = ALIGN(minify(mt-logical_width0, level), 8);
+   unsigned rect_height = ALIGN(minify(mt-logical_height0, level), 4);
 
BEGIN_BATCH(4);
OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE  16 | (4 - 2));
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev