On Thu, Aug 20, 2015 at 11:20:31AM +0300, Pohjolainen, Topi wrote: > On Wed, Aug 19, 2015 at 05:34:14PM +0300, Francisco Jerez wrote: > > Topi Pohjolainen <topi.pohjolai...@intel.com> writes: > > > > > All generations do the same exact dispatch and it could be > > > therefore done in the hardware independent stage. > > > > > > v2: Rebased where there are still duplicate calls in gen7 and gen8 > > > handlers. These will be dropped in subsequent patches. > > > > > > Reviewed-by: Matt Turner <matts...@gmail.com> (v1) > > > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> (v1) > > > Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > --- > > > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 28 > > > ++++++++++++++---------- > > > 1 file changed, 16 insertions(+), 12 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 73aa719..dca67e8 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > > @@ -319,12 +319,6 @@ brw_update_texture_surface(struct gl_context *ctx, > > > struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit); > > > uint32_t *surf; > > > > > > - /* BRW_NEW_TEXTURE_BUFFER */ > > > - if (tObj->Target == GL_TEXTURE_BUFFER) { > > > - brw_update_buffer_texture_surface(brw, tObj, surf_offset); > > > - return; > > > - } > > > - > > > surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, > > > 6 * 4, 32, surf_offset); > > > > > > @@ -819,14 +813,24 @@ update_stage_texture_surfaces(struct brw_context > > > *brw, > > > for (unsigned s = 0; s < num_samplers; s++) { > > > surf_offset[s] = 0; > > > > > > - if (prog->SamplersUsed & (1 << s)) { > > > - const unsigned unit = prog->SamplerUnits[s]; > > > + if (!(prog->SamplersUsed & (1 << s))) > > > + continue; > > > > > > - /* _NEW_TEXTURE */ > > > - if (ctx->Texture.Unit[unit]._Current) { > > > - brw->vtbl.update_texture_surface(ctx, unit, surf_offset + s, > > > for_gather); > > > - } > > > + const unsigned unit = prog->SamplerUnits[s]; > > > + struct gl_texture_object *tex = ctx->Texture.Unit[unit]._Current; > > > + > > > + if (!tex) > > > + continue; > > > + > > > + /* BRW_NEW_TEXTURE_BUFFER */ > > > + if (tex->Target == GL_TEXTURE_BUFFER) { > > > + brw_update_buffer_texture_surface(brw, tex, surf_offset); > > > > You probably didn't mean to always pass the first surface state entry > > (missing "+ s"?). > > Oh, that's really bad. Many thanks for the careful review. I wonder how I > got clean piglit run, I need to recheck this.
I tried with jenkins and piglit doesn't get upset about it. I guess this is a path that just doesn't get tickled. Therefore I'm even more grateful for the careful review :) > > > > > > + continue; > > > } > > > + > > > + /* _NEW_TEXTURE */ > > > + brw->vtbl.update_texture_surface(ctx, unit, > > > + surf_offset + s, for_gather); > > > > I'd keep the control flow structured here instead of adding a jump, > > like: > > > > | if (tex->Target == GL_TEXTURE_BUFFER) { > > | // Handle buffer textures. > > | } else { > > | // Handle non-buffer textures. > > | } > > I fully agree, thanks. Well, actually I had a reason for the jump. The latter case will get augmented quite a bit by the rest of the series and I wanted to keep the indentation level (instead of growing the else-branch). I'd rather have it this way if you don't mind. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev