[Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces

[Jordan Justen]

On 2014-11-18 00:56:02, Kenneth Graunke wrote:
> On Tuesday, November 18, 2014 09:49:53 PM Chris Forbes wrote:
> > Two things were broken here:
> > - The depth/stencil surface dimensions were broken for MSAA.
> > - Sample count was programmed incorrectly.
> > 
> > Result was the depth resolve didn't work correctly on MSAA surfaces, and
> > so sampling the surface later produced garbage.
> > 
> > Fixes the new piglit test arb_texture_multisample-sample-depth, and
> > various artifacts in 'tesseract' with msaa=4 glineardepth=0.
> > 
> > Not observed any piglit regressions on Haswell.
> > 
> > v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
> >     helper function (Ken).
> > 
> > Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.cpp  |  1 +
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp | 11 +++++++++++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp 
> b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > index 20ce7b7..0ecf330 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > @@ -323,6 +323,7 @@ brw_hiz_op_params::brw_hiz_op_params(struct 
> intel_mipmap_tree *mt,
> >      */
> >     depth.width = ALIGN(depth.width, 8);
> >     depth.height = ALIGN(depth.height, 4);
> > +   dst.num_samples = mt->num_samples;
> >  
> >     x1 = depth.width;
> >     y1 = depth.height;
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index 206a6ff..03fc9c8 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -663,6 +663,17 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context 
> *brw,
> >         */
> >        surfwidth = params->depth.width;
> >        surfheight = params->depth.height;
> > +
> > +      if (params->dst.num_samples > 1) {
> > +         /* If this is an MSAA + HIZ op, we need to program the
> > +          * aligned logical size of the depth surface.
> > +          */
> > +         surfwidth = ALIGN(params->depth.mt->logical_width0, 8);
> > +         surfheight = ALIGN(params->depth.mt->logical_height0, 4);

Would something like this cover all cases?

   surfwidth = params->depth.mt->logical_width0;
   surfheight = params->depth.mt->logical_height0;

   if (params->hiz_op != GEN6_HIZ_OP_NONE && lod == 0) {
       surfwidth = ALIGN(surfwidth, 8);
       surfheight = ALIGN(surfheight, 4);
   }

> > +      } else {
> > +         surfwidth = params->depth.width;
> > +         surfheight = params->depth.height;

Duplicate of the code just above.

-Jordan

> > +      }
> >     } else {
> >        surfwidth = params->depth.mt->logical_width0;
> >        surfheight = params->depth.mt->logical_height0;
> > 
> 
> Jordan, Chad...could one of you take a look at this?  It looks reasonable, but 
> I always get confused by when/where we do rounding in BLORP.
> 
> Chris - we might want to Cc this to stable too.
> 
> --Ken
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev