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.
>
> > + 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.
>
> Anyway, just nitpicking. With these fixed:
>
> Reviewed-by: Francisco Jerez <curroje...@riseup.net>
>
> > }
> > }
> >
> > --
> > 1.9.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
