[Mesa-dev] [PATCH 14/36] i965/blorp: Refactor interleaved multisample destination handling

[Pohjolainen, Topi]

On Fri, Jul 01, 2016 at 09:56:10AM +0300, Pohjolainen, Topi wrote:
> On Wed, Jun 29, 2016 at 05:37:33PM -0700, Jason Ekstrand wrote:
> > We put all of the code for fake IMS together.  This requires moving a bit
> > of the program key setup code further down so that it gets the right values
> > out of the final surface.
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 71 +++++++++++++---------------
> >  1 file changed, 34 insertions(+), 37 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > index 9a0b9bb..c253412 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > @@ -1695,28 +1695,6 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
> >        unreachable("Unrecognized blorp format");
> >     }
> >  
> > -   if (brw->gen > 6) {
> > -      /* Gen7's rendering hardware only supports the IMS layout for depth and
> > -       * stencil render targets.  Blorp always maps its destination surface as
> > -       * a color render target (even if it's actually a depth or stencil
> > -       * buffer).  So if the destination is IMS, we'll have to map it as a
> > -       * single-sampled texture and interleave the samples ourselves.
> > -       */
> > -      if (dst_mt->msaa_layout == INTEL_MSAA_LAYOUT_IMS) {
> 
> This should have been switched to isl enum in patch 12 (Use isl_msaa_layout
> instead of intel_msaa_layout)?
> 
> > -         params.dst.surf.samples = 1;
> > -         params.dst.surf.msaa_layout = ISL_MSAA_LAYOUT_NONE;
> > -      }
> > -   }
> > -
> > -   if (params.src.surf.samples > 0 && params.dst.surf.samples > 1) {
> > -      /* We are blitting from a multisample buffer to a multisample buffer, so
> > -       * we must preserve samples within a pixel.  This means we have to
> > -       * arrange for the WM program to run once per sample rather than once
> > -       * per pixel.
> > -       */
> > -      wm_prog_key.persample_msaa_dispatch = true;
> > -   }
> > -
> >     /* Scaled blitting or not. */
> >     wm_prog_key.blit_scaled =
> >        ((dst_x1 - dst_x0) == (src_x1 - src_x0) &&
> > @@ -1756,20 +1734,8 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
> >     wm_prog_key.src_samples = src_mt->num_samples;
> >     wm_prog_key.dst_samples = dst_mt->num_samples;
> >  
> > -   /* tex_samples and rt_samples are the sample counts that are set up in
> > -    * SURFACE_STATE.
> > -    */
> > -   wm_prog_key.tex_samples = params.src.surf.samples;
> > -   wm_prog_key.rt_samples  = params.dst.surf.samples;
> > -
> >     wm_prog_key.tex_aux_usage = params.src.aux_usage;
> >  
> > -   /* tex_layout and rt_layout indicate the MSAA layout the GPU pipeline will
> > -    * use to access the source and destination surfaces.
> > -    */
> > -   wm_prog_key.tex_layout = params.src.surf.msaa_layout;
> > -   wm_prog_key.rt_layout = params.dst.surf.msaa_layout;
> > -
> >     /* src_layout and dst_layout indicate the true MSAA layout used by src and
> >      * dst.
> >      */
> > @@ -1805,7 +1771,7 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
> >        params.wm_push_consts.src_z = 0;
> >     }
> >  
> > -   if (params.dst.surf.samples <= 1 && dst_mt->num_samples > 1) {
> > +   if (brw->gen > 6 && dst_mt->msaa_layout == INTEL_MSAA_LAYOUT_IMS) {
> 
> And here we should have ISL enum, right?

Otherwise the patch looks right:

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