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;"> These produce BRW_NEW_SURFACES (used by binding table emit()) and BRW_NEW_NR_WM_SURFACES (used by WM unit emit()). Fixes a bug where with no texturing and no color buffer, we wouldn't consider the null renderbuffer in nr_surfaces. This was harmless because nr_surfaces is only used for the prefetch info in the unit state. --- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 54 +++++++-------------- 1 files changed, 18 insertions(+), 36 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 bb7fd2e..782efd5 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c @@ -541,47 +541,16 @@ brw_update_renderbuffer_surface(struct brw_context *brw, I915_GEM_DOMAIN_RENDER); } -static void -prepare_wm_surfaces(struct brw_context *brw) -{ - struct gl_context *ctx = &brw->intel.ctx; - int i; - int nr_surfaces = 0; - - for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) { - nr_surfaces = SURF_INDEX_DRAW(i) + 1; - } - - if (brw->wm.const_bo) { - nr_surfaces = SURF_INDEX_FRAG_CONST_BUFFER + 1; - } - - for (i = 0; i < BRW_MAX_TEX_UNIT; i++) { - const struct gl_texture_unit *texUnit = &ctx->Texture.Unit[i]; - - if (texUnit->_ReallyEnabled) { - nr_surfaces = SURF_INDEX_TEXTURE(i) + 1; - } - } - - /* Have to update this in our prepare, since the unit's prepare - * relies on it. - */ - if (brw->wm.nr_surfaces != nr_surfaces) { - brw->wm.nr_surfaces = nr_surfaces; - brw->state.dirty.brw |= BRW_NEW_NR_WM_SURFACES; - } -} - /** * Constructs the set of surface state objects pointed to by the * binding table. */ static void -upload_wm_surfaces(struct brw_context *brw) +brw_upload_wm_surfaces(struct brw_context *brw) { struct gl_context *ctx = &brw->intel.ctx; GLuint i; + int nr_surfaces = 0; /* _NEW_BUFFERS | _NEW_COLOR */ /* Update surfaces for drawing buffers */ @@ -595,8 +564,15 @@ upload_wm_surfaces(struct brw_context *brw) brw_update_null_renderbuffer_surface(brw, i); } } + nr_surfaces = SURF_INDEX_DRAW(ctx->DrawBuffer->_NumColorDrawBuffers); } else { brw_update_null_renderbuffer_surface(brw, 0); + nr_surfaces = SURF_INDEX_DRAW(0) + 1; </blockquote><div> This looks like a behavioral change to me. The old prepare_wm_surfaces() code would have left nr_surfaces = 0 in this case. Was this an intentional change? Was the old code buggy? I have similar questions about patch 15/33. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> + } + + /* BRW_NEW_WM_CONSTBUF */ + if (brw->wm.const_bo) { + nr_surfaces = SURF_INDEX_FRAG_CONST_BUFFER + 1; } /* Update surfaces for textures */ @@ -607,11 +583,17 @@ upload_wm_surfaces(struct brw_context *brw) /* _NEW_TEXTURE */ if (texUnit->_ReallyEnabled) { brw_update_texture_surface(ctx, i); + nr_surfaces = SURF_INDEX_TEXTURE(i) + 1; } else { brw->wm.surf_offset[surf] = 0; } } + if (brw->wm.nr_surfaces != nr_surfaces) { + brw->wm.nr_surfaces = nr_surfaces; + brw->state.dirty.brw |= BRW_NEW_NR_WM_SURFACES; + } + brw->state.dirty.brw |= BRW_NEW_WM_SURFACES; } @@ -620,11 +602,11 @@ const struct brw_tracked_state brw_wm_surfaces = { .mesa = (_NEW_COLOR | _NEW_TEXTURE | _NEW_BUFFERS), - .brw = (BRW_NEW_BATCH), + .brw = (BRW_NEW_BATCH | + BRW_NEW_WM_CONSTBUF), .cache = 0 }, - .prepare = prepare_wm_surfaces, - .emit = upload_wm_surfaces, + .emit = brw_upload_wm_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>