On Wed, May 03, 2017 at 11:28:31PM -0700, Kenneth Graunke wrote:
> On Wednesday, May 3, 2017 9:59:03 PM PDT Pohjolainen, Topi wrote:
> > On Mon, May 01, 2017 at 06:43:06PM -0700, Rafael Antognolli wrote:
> > > +#if GEN_GEN == 6
> > > + brw_batch_emit(brw, GENX(3DSTATE_CC_STATE_POINTERS), ptr) {
> > > + ptr.PointertoDEPTH_STENCIL_STATE = ds_offset;
> > > + ptr.DEPTH_STENCIL_STATEChange = true;
> > > + }
> > > +#elif GEN_GEN == 7
> > > + brw_batch_emit(brw, GENX(3DSTATE_DEPTH_STENCIL_STATE_POINTERS), ptr) {
> > > + ptr.PointertoDEPTH_STENCIL_STATE = ds_offset;
> >
> > Don't we need here also:
> >
> > ptr.DEPTH_STENCIL_STATEChange = true;
>
> No - in gen7.xml bit 32 is marked "mbo" or "Must Be One". The genxml
> packing magic will automatically set that for us. It's a bit
> surprising, but there's actually no named field, so you can't set it
> explicitly.
Ok. Thanks for explaining.
>
> > > + }
> > > +#endif
> > > +}
> > > +
> > > +static const struct brw_tracked_state genX(depth_stencil_state) = {
> > > + .dirty = {
> > > + .mesa = _NEW_BUFFERS |
> > > + _NEW_DEPTH |
> > > + _NEW_STENCIL,
> > > + .brw = BRW_NEW_BLORP |
> > > + (GEN_GEN >= 8 ? BRW_NEW_CONTEXT
> >
> > Shouldn't this be >= 6 or am I missing something?
>
> Gen6+ uses DEPTH_STENCIL_STATE which has to be re-emitted on every batch
> buffer (BRW_NEW_BATCH). On Gen8+ it's 3DSTATE_WM_DEPTH_STENCIL (an
> actual packet) which doesn't need to be re-emitted every batch. We use
> BRW_NEW_CONTEXT.
>
> BRW_NEW_CONTEXT is unnecessary if you listen to BRW_NEW_BATCH. It's
> signalled on context creation (but we signal everything, so...not sure
> what the point is)...and on new batch buffers when there isn't a hw_ctx
> (but we already signal BRW_NEW_BATCH). Honestly, I've thought about
> just canning the bit entirely...
Right. This is fine then.
>
> >
> > > + : BRW_NEW_BATCH |
> > > + BRW_NEW_STATE_BASE_ADDRESS),
> > > + },
> > > + .emit = genX(upload_depth_stencil_state),
> > > +};
> > > +
> > > +/*
> > > ---------------------------------------------------------------------- */
> > > +
> > > +#endif
> > > +
> > > void
> > > genX(init_atoms)(struct brw_context *brw)
> > > {
> > > @@ -250,7 +349,7 @@ genX(init_atoms)(struct brw_context *brw)
> >
> > Isn't this leaving gen6 with:
> >
> > &gen6_depth_stencil_state, /* must do before cc unit */
>
> Yeah, but &genX(depth_stencil_state) is &gen6_depth_stencil_state so
> it's OK. We should probably change it nonetheless...
Ok.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
