<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Oct 7, 2017 at 11:42 AM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Saturday, October 7, 2017 8:03:54 AM PDT Jason Ekstrand wrote:<br>
> On October 6, 2017 10:25:55 PM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
><br>
> > On Friday, October 6, 2017 8:09:33 PM PDT Jason Ekstrand wrote:<br>
> >> On October 6, 2017 8:00:04 PM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
> >> > diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> >> > b/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> >> > index c7ed7284501..fcb194dbe86 100644<br>
> >> > --- a/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> >> > +++ b/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> >> > @@ -336,21 +336,39 @@ brw_merge_inputs(struct brw_context *brw,<br>
> >> >     }<br>
> >> >  }<br>
> >> ><br>
> >> > +/* Disable auxiliary buffers if a renderbuffer is also bound as a texture<br>
> >> > + * or shader image.  This causes a self-dependency, where both rendering<br>
> >> > + * and sampling may concurrently read or write the CCS buffer, causing<br>
> >> > + * incorrect pixels.<br>
> >> > + *<br>
> >> > + * We don't support sampling from CCS_D, so this only matters for CCS_E.<br>
> >> > + */<br>
> >> >  static bool<br>
> >> > -intel_disable_rb_aux_buffer(<wbr>struct brw_context *brw, const struct brw_bo<br>
> >> *bo)<br>
> >> > +intel_disable_rb_aux_buffer(<wbr>struct brw_context *brw,<br>
> >> > +                            struct intel_mipmap_tree *tex_mt,<br>
> >> > +                            const char *usage)<br>
> >> >  {<br>
> >> >     const struct gl_framebuffer *fb = brw->ctx.DrawBuffer;<br>
> >> >     bool found = false;<br>
> >> ><br>
> >> > +   /* Nothing to disable, don't bother looking */<br>
> >> > +   if (tex_mt->aux_usage != ISL_AUX_USAGE_CCS_E)<br>
> >> > +      return false;<br>
> >><br>
> >> As I mentioned in the office today, I'm not convinced this is actually<br>
> >> needed.  When it isn't CCS_E, we'll resolve anyway so passing disable_aux =<br>
> >> true won't hurt anything.<br>
> ><br>
> > Yeah, that's true - but I figured we could avoid the overhead of the<br>
> > loop in other cases, since it doesn't really matter either way.<br>
> ><br>
> > Make sense?  Should we leave it?  Or would you rather drop it?<br>
><br>
> Leave it in but for a completely different reason than the one you stated.<br>
> :-)  I don't care all that much about trying to avoid a loop whose number<br>
> of iterations was bounded above by 8.<br>
<br>
</div></div>Well, it is nested inside a loop bounded by 32... :)<span class=""><br></span></blockquote><div><br></div><div>Ok, fair enough.  But intel_miptree_prepare can do some nasty looping too.  In any case checking for certain aux_usages is a better plan than I thought.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> If the have a multisampled self dependency, it should work fine because<br>
> compression is limited to a single pixel and we dispatch all the samples<br>
> for a given pixel together.<br>
<br>
</span>Probably not for 16x MSAA and SIMD8 shaders, but...it's probably really<br>
uncommon...<br><div class="HOEnZb"><div class="h5"></div></div></blockquote><div><br></div><div>And given that we don't have MSAA resolve code, it's probably a good idea to not ask for a full resolve.  That would end badly. :-)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> We shouldn't force a resolve so we don't want<br>
> to do this of its MCS.  In the case of CCS_D, I'm not sure if it's safe.<br>
> We will potentially try to sample from a CCS_D image as CCS_E on gen9+ so<br>
> it's probably best to disable it for that too.<br>
</div></div></blockquote></div><br></div></div>