On Sat, Oct 7, 2017 at 11:42 AM, Kenneth Graunke <[email protected]> wrote:
> On Saturday, October 7, 2017 8:03:54 AM PDT Jason Ekstrand wrote: > > On October 6, 2017 10:25:55 PM Kenneth Graunke <[email protected]> > wrote: > > > > > On Friday, October 6, 2017 8:09:33 PM PDT Jason Ekstrand wrote: > > >> On October 6, 2017 8:00:04 PM Kenneth Graunke <[email protected]> > wrote: > > >> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c > > >> > b/src/mesa/drivers/dri/i965/brw_draw.c > > >> > index c7ed7284501..fcb194dbe86 100644 > > >> > --- a/src/mesa/drivers/dri/i965/brw_draw.c > > >> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c > > >> > @@ -336,21 +336,39 @@ brw_merge_inputs(struct brw_context *brw, > > >> > } > > >> > } > > >> > > > >> > +/* Disable auxiliary buffers if a renderbuffer is also bound as a > texture > > >> > + * or shader image. This causes a self-dependency, where both > rendering > > >> > + * and sampling may concurrently read or write the CCS buffer, > causing > > >> > + * incorrect pixels. > > >> > + * > > >> > + * We don't support sampling from CCS_D, so this only matters for > CCS_E. > > >> > + */ > > >> > static bool > > >> > -intel_disable_rb_aux_buffer(struct brw_context *brw, const struct > brw_bo > > >> *bo) > > >> > +intel_disable_rb_aux_buffer(struct brw_context *brw, > > >> > + struct intel_mipmap_tree *tex_mt, > > >> > + const char *usage) > > >> > { > > >> > const struct gl_framebuffer *fb = brw->ctx.DrawBuffer; > > >> > bool found = false; > > >> > > > >> > + /* Nothing to disable, don't bother looking */ > > >> > + if (tex_mt->aux_usage != ISL_AUX_USAGE_CCS_E) > > >> > + return false; > > >> > > >> As I mentioned in the office today, I'm not convinced this is actually > > >> needed. When it isn't CCS_E, we'll resolve anyway so passing > disable_aux = > > >> true won't hurt anything. > > > > > > Yeah, that's true - but I figured we could avoid the overhead of the > > > loop in other cases, since it doesn't really matter either way. > > > > > > Make sense? Should we leave it? Or would you rather drop it? > > > > Leave it in but for a completely different reason than the one you > stated. > > :-) I don't care all that much about trying to avoid a loop whose number > > of iterations was bounded above by 8. > > Well, it is nested inside a loop bounded by 32... :) > 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. > > If the have a multisampled self dependency, it should work fine because > > compression is limited to a single pixel and we dispatch all the samples > > for a given pixel together. > > Probably not for 16x MSAA and SIMD8 shaders, but...it's probably really > uncommon... > 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. :-) > > We shouldn't force a resolve so we don't want > > to do this of its MCS. In the case of CCS_D, I'm not sure if it's safe. > > We will potentially try to sample from a CCS_D image as CCS_E on gen9+ so > > it's probably best to disable it for that too. >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
