On Wednesday, July 13, 2016 10:10:51 AM PDT Topi Pohjolainen wrote: > This is partial revert of commit cc2d0e64. > > It looks that even though blorp disables a stage the corresponding > 3DSTATE_CONSTANT_XS packet is needed to be programmed. Hardware > seems to try to fetch the constants even for disabled stages. > Therefore care needs to be taken that the constant buffer is > set up properly. Blorp will continue to trash it into non-existing > such as before. > It is possible that this could be omitted on SKL where the > constant buffer is considered when the corresponding binding table > settings are changed. Bspec: > > "The 3DSTATE_CONSTANT_* command is not committed to the shader > unit until the corresponding (same shader) > 3DSTATE_BINDING_TABLE_POINTER_* command is parsed." > > However, as CONSTANT_XS packet itself does not seem to stall on its > own, it is safer to emit the packets for SKL also. > > Possible alternative to blorp trashing could have been to setup > defaults in the beginning of each batch buffer. However, hardware > doesn't seem to tolerate these packets being programmed multiple > times per primitive. Bspec for IVB: > > "It is invalid to execute this command more than once between > 3D_PRIMITIVE commands." > > Signed-off-by: Topi Pohjolainen <[email protected]> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96878 > CC: Jason Ekstrand <[email protected]> > CC: Kenneth Graunke <[email protected]> > CC: [email protected] > CC: [email protected] > --- > src/mesa/drivers/dri/i965/gen7_blorp.c | 24 ++++++++++++++++++++++++ > src/mesa/drivers/dri/i965/gen8_blorp.c | 28 ++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.c > b/src/mesa/drivers/dri/i965/gen7_blorp.c > index 0afd76b..dc62a93 100644 > --- a/src/mesa/drivers/dri/i965/gen7_blorp.c > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.c > @@ -228,6 +228,24 @@ gen7_blorp_emit_surface_state(struct brw_context *brw, > return wm_surf_offset; > } > > +/* Hardware seems to try to fetch the constants even though the corresponding > + * stage gets disables. Therefore make sure the settings for the constant > + * buffer are valid. > + */ > +static void > +gen7_blorp_emit_disable_constant_state(struct brw_context *brw, > + unsigned opcode)
As is, this is: Reviewed-by: Kenneth Graunke <[email protected]> A couple of small suggestions: - Why not use this for disabling PS constants too? Otherwise, you've basically got the same function twice...for no real benefit. - "emit disable" sounds a little odd to me, perhaps just call it "gen7_blorp_disable_constant_state" (either way is fine) > +{ > + BEGIN_BATCH(7); > + OUT_BATCH(opcode << 16 | (7 - 2)); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + ADVANCE_BATCH(); > +} > > /* 3DSTATE_VS > * > @@ -760,6 +778,12 @@ gen7_blorp_exec(struct brw_context *brw, > gen7_blorp_emit_blend_state_pointer(brw, cc_blend_state_offset); > gen7_blorp_emit_cc_state_pointer(brw, cc_state_offset); > } > + > + gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_VS); > + gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_HS); > + gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_DS); > + gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_GS); > + > depthstencil_offset = gen6_blorp_emit_depth_stencil_state(brw, params); > gen7_blorp_emit_depth_stencil_state_pointers(brw, depthstencil_offset); > if (brw->use_resource_streamer) > diff --git a/src/mesa/drivers/dri/i965/gen8_blorp.c > b/src/mesa/drivers/dri/i965/gen8_blorp.c > index 7ca24a8..bc113ec 100644 > --- a/src/mesa/drivers/dri/i965/gen8_blorp.c > +++ b/src/mesa/drivers/dri/i965/gen8_blorp.c > @@ -156,6 +156,29 @@ gen8_blorp_emit_blend_state(struct brw_context *brw, > return blend_state_offset; > } > > +/* Hardware seems to try to fetch the constants even though the corresponding > + * stage gets disables. Therefore make sure the settings for the constant > + * buffer are valid. > + */ > +static void > +gen8_blorp_emit_disable_constant_state(struct brw_context *brw, > + unsigned opcode) > +{ > + BEGIN_BATCH(11); > + OUT_BATCH(opcode << 16 | (11 - 2)); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + OUT_BATCH(0); > + ADVANCE_BATCH(); > +} > + > /* 3DSTATE_VS > * > * Disable vertex shader. > @@ -657,6 +680,11 @@ gen8_blorp_exec(struct brw_context *brw, const struct > brw_blorp_params *params) > const uint32_t cc_state_offset = gen6_blorp_emit_cc_state(brw); > gen7_blorp_emit_cc_state_pointer(brw, cc_state_offset); > > + gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_VS); > + gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_HS); > + gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_DS); > + gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_GS); > + > gen8_blorp_emit_disable_constant_ps(brw); > wm_bind_bo_offset = gen8_blorp_emit_surface_states(brw, params); > >
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
