On Wed, Oct 14, 2015 at 09:54:43AM -0700, Ben Widawsky wrote: > On Wed, Oct 14, 2015 at 02:43:24PM +0300, Pohjolainen, Topi wrote: > > On Wed, Oct 14, 2015 at 11:39:03AM +0200, Neil Roberts wrote: > > > Ben Widawsky <benjamin.widaw...@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 <b...@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." > > I am aware of all this. Neil, yes it is completely necessary for piglit (I > don't > know if anything in the real world does this or not). > > You are both asking to me to provide something which may be impossible, an > explanation of why the docs and/or hardware are behaving this way. Let me > respond in kind, please provide an alternate patch which fixes: > spec/ext_framebuffer_multisample/blit-multiple-render-targets 0 > spec/arb_framebuffer_object/fbo-drawbuffers-none glclear (all subtests) > > FWIW Topi, it's also contradicted in 3DSTATE_PS definition.
You misunderstood me I think, I'm not questioning your patch or your interpretation, or asking you to provide some information that just isn't there in the spec. We talked about this quite a bit. I'm just saying that I feel that something is missing in the spec. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
