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.
> > + }
> > +#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...
>
> > + : 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...
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
