[Mesa-dev] [PATCH 03/10] i965: Set width, height, and tiling properly for null render targets.

[Paul Berry]

The HW docs say that the width and height of null render targets need
to match the width and height of the corresponding depth and/or
stencil buffers, and that they need to be marked as Y-tiled.  Although
leaving these values at 0 doesn't seem to cause any ill effects, it
seems wise to follow the documented requirements.
---
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |   33 +++++++++++++++++++-
 src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |   29 ++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

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 82e44f9..c2e629c 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -961,9 +961,31 @@ const struct brw_tracked_state brw_wm_pull_constants = {
 static void
 brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
 {
+   /* From the Sandy bridge PRM, Vol4 Part1 p71 (Surface Type: Programming
+    * Notes):
+    *
+    *     A null surface will be used in instances where an actual surface is
+    *     not bound. When a write message is generated to a null surface, no
+    *     actual surface is written to. When a read message (including any
+    *     sampling engine message) is generated to a null surface, the result
+    *     is all zeros. Note that a null surface type is allowed to be used
+    *     with all messages, even if it is not specificially indicated as
+    *     supported. All of the remaining fields in surface state are ignored
+    *     for null surfaces, with the following exceptions:
+    *
+    *     - [DevSNB+]: Width, Height, Depth, and LOD fields must match the
+    *       depth buffer’s corresponding state for all render target surfaces,
+    *       including null.
+    *
+    *     - Surface Format must be R8G8B8A8_UNORM.
+    */
    struct intel_context *intel = &brw->intel;
+   struct gl_context *ctx = &intel->ctx;
    uint32_t *surf;
 
+   /* _NEW_BUFFERS */
+   const struct gl_framebuffer *fb = ctx->DrawBuffer;
+
    surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
 			  6 * 4, 32, &brw->wm.surf_offset[unit]);
 
@@ -976,8 +998,15 @@ brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
 		  1 << BRW_SURFACE_WRITEDISABLE_A_SHIFT);
    }
    surf[1] = 0;
-   surf[2] = 0;
-   surf[3] = 0;
+   surf[2] = ((fb->Width - 1) << BRW_SURFACE_WIDTH_SHIFT |
+              (fb->Height - 1) << BRW_SURFACE_HEIGHT_SHIFT);
+
+   /* From Sandy bridge PRM, Vol4 Part1 p82 (Tiled Surface: Programming
+    * Notes):
+    *
+    *     If Surface Type is SURFTYPE_NULL, this field must be TRUE
+    */
+   surf[3] = BRW_SURFACE_TILED | BRW_SURFACE_TILED_Y;
    surf[4] = 0;
    surf[5] = 0;
 }
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 869f943..2522276 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
@@ -409,8 +409,28 @@ gen7_create_constant_surface(struct brw_context *brw,
 static void
 gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
 {
+   /* From the Ivy bridge PRM, Vol4 Part1 p62 (Surface Type: Programming
+    * Notes):
+    *
+    *     A null surface is used in instances where an actual surface is not
+    *     bound. When a write message is generated to a null surface, no
+    *     actual surface is written to. When a read message (including any
+    *     sampling engine message) is generated to a null surface, the result
+    *     is all zeros. Note that a null surface type is allowed to be used
+    *     with all messages, even if it is not specificially indicated as
+    *     supported. All of the remaining fields in surface state are ignored
+    *     for null surfaces, with the following exceptions: Width, Height,
+    *     Depth, LOD, and Render Target View Extent fields must match the
+    *     depth buffer’s corresponding state for all render target surfaces,
+    *     including null.
+    */
+   struct intel_context *intel = &brw->intel;
+   struct gl_context *ctx = &intel->ctx;
    struct gen7_surface_state *surf;
 
+   /* _NEW_BUFFERS */
+   const struct gl_framebuffer *fb = ctx->DrawBuffer;
+
    surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
 			  sizeof(*surf), 32, &brw->wm.surf_offset[unit]);
    memset(surf, 0, sizeof(*surf));
@@ -418,6 +438,15 @@ gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
    surf->ss0.surface_type = BRW_SURFACE_NULL;
    surf->ss0.surface_format = BRW_SURFACEFORMAT_B8G8R8A8_UNORM;
 
+   surf->ss2.width = fb->Width - 1;
+   surf->ss2.height = fb->Height - 1;
+
+   /* From the Ivy bridge PRM, Vol4 Part1 p65 (Tiled Surface: Programming Notes):
+    *
+    *     If Surface Type is SURFTYPE_NULL, this field must be TRUE.
+    */
+   gen7_set_surface_tiling(surf, I915_TILING_Y);
+
    gen7_check_surface_setup(surf, true /* is_render_target */);
 }
 
-- 
1.7.7.6