[Mesa-dev] [PATCH v2 1/6] i965: Use SURF_INDEX_DRAW() for drawbuffer binding table indices.

Kenneth Graunke kenneth at whitecape.org
Sun Aug 18 22:15:22 PDT 2013


SURF_INDEX_DRAW() has been the identity function since the dawn of time,
and both the shader code and binding table upload code relied on that,
simply using X rather than SURF_INDEX_DRAW(X).

Even if that continues to be true, using the macro clarifies the code.

The comment about draw buffers needing to be first in order for
headerless render target writes to work turned out to be wrong; with
this change, SURF_INDEX_DRAW can be changed to arbitrary indices and
everything continues working.

The confusion was over the "Render Target Index" field in the FB write
message header.  If it were a binding table index, then RT 0 would have
to be at index 0 for headerless FB writes to work.  However, it's
actually an index into the blend state table, so there's no problem.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Cc: Paul Berry <stereotype441 at gmail.com>
---
 src/mesa/drivers/dri/i965/brw_context.h           |  4 ----
 src/mesa/drivers/dri/i965/brw_fs_emit.cpp         |  2 +-
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 12 ++++++------
 src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 14 ++++++++------
 4 files changed, 15 insertions(+), 17 deletions(-)

In order to fix the comment as Paul requested, I had to do some digging.
The end result was discovering that the comment was entirely wrong - the
only reason SURF_INDEX_DRAW had to be the identity function was that we
weren't calling it.  With that fixed, it can be arbitrary, so there's
no need for the comment at all.

I did try totally scrambling the order of the binding table indexes
after applying the patch, and there were no Piglit regressions on IVB.
So I'm confident that it works (at least on Gen7+).

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index acd8e3f..e0deee0 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -602,10 +602,6 @@ struct brw_vs_prog_data {
  *    |   : |     :                   |
  *    |  63 | SOL Binding 63          |
  *    +-----+-------------------------+
- *
- * Note that nothing actually uses the SURF_INDEX_DRAW macro, so it has to be
- * the identity function or things will break.  We do want to keep draw buffers
- * first so we can use headerless render target writes for RT 0.
  */
 #define SURF_INDEX_DRAW(d)           (d)
 #define SURF_INDEX_FRAG_CONST_BUFFER (BRW_MAX_DRAW_BUFFERS + 1)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index 4225922..b90cf0f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -170,7 +170,7 @@ fs_generator::generate_fb_write(fs_inst *inst)
 		inst->base_mrf,
 		implied_header,
 		msg_control,
-		inst->target,
+		SURF_INDEX_DRAW(inst->target),
 		inst->mlen,
 		0,
 		eot,
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 8847f91..16579f9 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -533,8 +533,8 @@ brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
    /* _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]);
+   surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 6 * 4, 32,
+                          &brw->wm.surf_offset[SURF_INDEX_DRAW(unit)]);
 
    if (fb->Visual.samples > 1) {
       /* On Gen6, null render targets seem to cause GPU hangs when
@@ -587,7 +587,7 @@ brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
 
    if (bo) {
       drm_intel_bo_emit_reloc(brw->batch.bo,
-                              brw->wm.surf_offset[unit] + 4,
+                              brw->wm.surf_offset[SURF_INDEX_DRAW(unit)] + 4,
                               bo, 0,
                               I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER);
    }
@@ -635,8 +635,8 @@ brw_update_renderbuffer_surface(struct brw_context *brw,
 
    region = irb->mt->region;
 
-   surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
-			  6 * 4, 32, &brw->wm.surf_offset[unit]);
+   surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 6 * 4, 32,
+                          &brw->wm.surf_offset[SURF_INDEX_DRAW(unit)]);
 
    format = brw->render_target_format[rb_format];
    if (unlikely(!brw->format_supported_as_render_target[rb_format])) {
@@ -692,7 +692,7 @@ brw_update_renderbuffer_surface(struct brw_context *brw,
    }
 
    drm_intel_bo_emit_reloc(brw->batch.bo,
-			   brw->wm.surf_offset[unit] + 4,
+			   brw->wm.surf_offset[SURF_INDEX_DRAW(unit)] + 4,
 			   region->bo,
 			   surf[1] - region->bo->offset,
 			   I915_GEM_DOMAIN_RENDER,
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 34cf63b..cdd2242 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
@@ -475,8 +475,8 @@ gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
    /* _NEW_BUFFERS */
    const struct gl_framebuffer *fb = ctx->DrawBuffer;
 
-   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
-			            8 * 4, 32, &brw->wm.surf_offset[unit]);
+   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 8 * 4, 32,
+                                    &brw->wm.surf_offset[SURF_INDEX_DRAW(unit)]);
    memset(surf, 0, 8 * 4);
 
    /* From the Ivybridge PRM, Volume 4, Part 1, page 65,
@@ -518,8 +518,10 @@ gen7_update_renderbuffer_surface(struct brw_context *brw,
    GLenum gl_target = rb->TexImage ?
                          rb->TexImage->TexObject->Target : GL_TEXTURE_2D;
 
-   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
-                                    8 * 4, 32, &brw->wm.surf_offset[unit]);
+   uint32_t surf_index = SURF_INDEX_DRAW(unit);
+
+   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 8 * 4, 32,
+                                    &brw->wm.surf_offset[surf_index]);
    memset(surf, 0, 8 * 4);
 
    intel_miptree_used_for_rendering(irb->mt);
@@ -588,7 +590,7 @@ gen7_update_renderbuffer_surface(struct brw_context *brw,
              (depth - 1) << GEN7_SURFACE_RENDER_TARGET_VIEW_EXTENT_SHIFT;
 
    if (irb->mt->mcs_mt) {
-      gen7_set_surface_mcs_info(brw, surf, brw->wm.surf_offset[unit],
+      gen7_set_surface_mcs_info(brw, surf, brw->wm.surf_offset[surf_index],
                                 irb->mt->mcs_mt, true /* is RT */);
    }
 
@@ -602,7 +604,7 @@ gen7_update_renderbuffer_surface(struct brw_context *brw,
    }
 
    drm_intel_bo_emit_reloc(brw->batch.bo,
-			   brw->wm.surf_offset[unit] + 4,
+			   brw->wm.surf_offset[surf_index] + 4,
 			   region->bo,
 			   surf[1] - region->bo->offset,
 			   I915_GEM_DOMAIN_RENDER,
-- 
1.8.3.4



More information about the mesa-dev mailing list