[Mesa-dev] [PATCH v2 26/35] i965/blorp: Rework hiz rect alignment calculations

[Pohjolainen, Topi]

On Thu, Jul 28, 2016 at 10:54:43AM +0300, Pohjolainen, Topi wrote:
> On Tue, Jul 26, 2016 at 03:02:17PM -0700, Jason Ekstrand wrote:
> > At the moment, the minify operation does nothing because
> > params.depth.view.base_level is always zero.  However, as soon as we start
> > using actual base miplevels and array slices, we are going to need the
> > minification.  Also, we only need to align the surface dimensions in the
> > case where we are operating on miplevel 0.  Previously, it didn't matter
> > because it aligned on miplevel 0 and, for all other miplevels, the miptree
> > code guaranteed that the level was already aligned.
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c | 23 +++++++++++++++--------
> >  1 file changed, 15 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index 2cf0f99..bc26e41 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -593,14 +593,21 @@ gen6_blorp_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt,
> >      * not 8. But commit 1f112cc increased the alignment from 4 to 8, which
> >      * prevents the clobbering.
> >      */
> > -   params.dst.surf.samples = MAX2(mt->num_samples, 1);
> > -   params.depth.surf.logical_level0_px.width =
> > -      ALIGN(params.depth.surf.logical_level0_px.width, 8);
> > -   params.depth.surf.logical_level0_px.height =
> > -      ALIGN(params.depth.surf.logical_level0_px.height, 4);
> > -
> > -   params.x1 = params.depth.surf.logical_level0_px.width;
> > -   params.y1 = params.depth.surf.logical_level0_px.height;
> > +   params.x1 = minify(params.depth.surf.logical_level0_px.width,
> > +                      params.depth.view.base_level);
> > +   params.y1 = minify(params.depth.surf.logical_level0_px.height,
> > +                      params.depth.view.base_level);
> > +   params.x1 = ALIGN(params.x1, 8);
> > +   params.y1 = ALIGN(params.y1, 4);
> > +
> > +   if (params.depth.view.base_level == 0) {
> > +      /* TODO: What about MSAA? */
> 
> What were you thinking is missing here? At least we don't need any stomping of
> msaa to single sampled with hiz, do we?

Right, after discussing offline a bit. We seem to have everything in place,
but agreed to leave this TODO here for now.

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

> 
> > +      params.depth.surf.logical_level0_px.width = params.x1;
> > +      params.depth.surf.logical_level0_px.height = params.y1;
> > +   }
> > +
> > +   params.dst.surf.samples = params.depth.surf.samples;
> > +   params.dst.surf.logical_level0_px = params.depth.surf.logical_level0_px;
> >  
> >     assert(intel_miptree_level_has_hiz(mt, level));
> >  
> > -- 
> > 2.5.0.400.gff86faf
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev