On 26 October 2011 11:45, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">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"> On Wed, 26 Oct 2011 10:11:15 -0700, Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> wrote: Non-text part: multipart/alternative <div>> On 24 October 2011 14:17, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote: > > > 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. </div><div><div></div><div>> > 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; > > > > 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></div>I mentioned it in the commit message :) </blockquote></div> Oops, so you did. Would you mind mentioning it in the commit message for patch 15/33 also? I failed to notice because I reviewed patch 15/33 first :)