On 24 October 2011 14:17, Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> This rearranges the code a bit, and makes the upload of the binding table take only as many surfaces as there are in use. --- src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 62 +++++++++------------- 1 files changed, 25 insertions(+), 37 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c index bfc4f5d..0237b58 100644 --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c @@ -138,22 +138,6 @@ brw_update_vs_constant_surface( struct gl_context *ctx, } } - -static void -prepare_vs_surfaces(struct brw_context *brw) -{ - int nr_surfaces = 0; - - if (brw->vs.const_bo) { - nr_surfaces = 1; - } - - if (brw->vs.nr_surfaces != nr_surfaces) { - brw->state.dirty.brw |= BRW_NEW_NR_VS_SURFACES; - brw->vs.nr_surfaces = nr_surfaces; - } -} - /** * Vertex shader surfaces (constant buffer). * @@ -161,36 +145,41 @@ prepare_vs_surfaces(struct brw_context *brw) * to be updated, and produces BRW_NEW_NR_VS_SURFACES for the VS unit and * CACHE_NEW_SURF_BIND for the binding table upload. */ -static void upload_vs_surfaces(struct brw_context *brw) +static void +brw_upload_vs_surfaces(struct brw_context *brw) { struct gl_context *ctx = &brw->intel.ctx; uint32_t *bind; int i; + int nr_surfaces = 0; + + /* BRW_NEW_VS_CONSTBUF */ + if (brw->vs.const_bo) { + nr_surfaces = 1; + brw_update_vs_constant_surface(ctx, SURF_INDEX_VERT_CONST_BUFFER); + } + + if (nr_surfaces != 0) { </blockquote><div> This reads a little strange to me. The only way nr_surfaces can be nonzero is if the previous if test succeeded, so why not just merge the two if statements? </div> <blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> + bind = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, + sizeof(uint32_t) * nr_surfaces, + 32, &brw->vs.bind_bo_offset); - /* BRW_NEW_NR_VS_SURFACES */ - if (brw->vs.nr_surfaces == 0) { + for (i = 0; i < nr_surfaces; i++) { + /* BRW_NEW_VS_CONSTBUF */ + bind[i] = brw->vs.surf_offset[i]; + } </blockquote><div> This for loop also seems weird, since at this point in the code nr_surfaces is known to be equal to 1. Why not just do "bind[0] = brw->vs.surf_offset[0];"? </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> + brw->state.dirty.brw |= BRW_NEW_VS_BINDING_TABLE; + } else { if (brw->vs.bind_bo_offset) { brw->state.dirty.brw |= BRW_NEW_VS_BINDING_TABLE; + brw->vs.bind_bo_offset = 0; } - brw->vs.bind_bo_offset = 0; - return; } - brw_update_vs_constant_surface(ctx, SURF_INDEX_VERT_CONST_BUFFER); - - /* Might want to calculate nr_surfaces first, to avoid taking up so much - * space for the binding table. (once we have vs samplers) - */ - bind = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, - sizeof(uint32_t) * BRW_VS_MAX_SURF, - 32, &brw->vs.bind_bo_offset); - - for (i = 0; i < BRW_VS_MAX_SURF; i++) { - /* BRW_NEW_VS_CONSTBUF */ - bind[i] = brw->vs.surf_offset[i]; + if (brw->vs.nr_surfaces != nr_surfaces) { + brw->state.dirty.brw |= BRW_NEW_NR_VS_SURFACES; + brw->vs.nr_surfaces = nr_surfaces; } - - brw->state.dirty.brw |= BRW_NEW_VS_BINDING_TABLE; } const struct brw_tracked_state brw_vs_surfaces = { @@ -201,6 +190,5 @@ const struct brw_tracked_state brw_vs_surfaces = { BRW_NEW_BATCH), .cache = 0 }, - .prepare = prepare_vs_surfaces, - .emit = upload_vs_surfaces, + .emit = brw_upload_vs_surfaces, }; -- 1.7.7 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>