[Mesa-dev] [PATCH 05/10] i965/meta/gen9: Individually fast clear color attachments

[Pohjolainen, Topi]

On Wed, Oct 14, 2015 at 11:39:03AM +0200, Neil Roberts wrote:
> Ben Widawsky <benjamin.widawsky at intel.com> writes:
> 
> > The impetus for this patch comes from a seemingly benign statement within the
> > spec (quoted within the patch). For me, this patch was at some point critical
> > for getting stable piglit results (though this did not seem to be the case on a
> > branch Chad was working on).
> >
> > It is very important for clearing multiple color buffer attachments and can be
> > observed in the following piglit tests:
> > spec/arb_framebuffer_object/fbo-drawbuffers-none glclear
> > spec/ext_framebuffer_multisample/blit-multiple-render-targets 0
> >
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 97 +++++++++++++++++++++----
> >  1 file changed, 84 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> > index 7bf52f0..9e6711e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> > +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> > @@ -427,6 +427,74 @@ use_rectlist(struct brw_context *brw, bool enable)
> >     brw->ctx.NewDriverState |= BRW_NEW_FRAGMENT_PROGRAM;
> >  }
> >  
> > +/**
> > + * Individually fast clear each color buffer attachment. On previous gens this
> > + * isn't required. The motivation for this comes from one line (which seems to
> > + * be specific to SKL+). The list item is in section titled _MCS Buffer for
> > + * Render Target(s)_
> > + *
> > + *   "Since only one RT is bound with a clear pass, only one RT can be cleared
> > + *   at a time. To clear multiple RTs, multiple clear passes are required."
> 
> This sentence also appears in the HSW PRM so it seems a bit odd if it's
> only causing problems on SKL. I guess if we get Piglit regressions
> without it then it makes sense to have the patch. It might be worth just
> double checking whether this patch is completely necessary. The wording
> in the commit message seems a little unsure.

The spec seems to be missing something as the section discussing "Render
Target Fast Clear" seems to suggest the opposite:

"The render target(s) is/are bound as they normally would be, with the MCS
 surface defined in SURFACE_STATE."