Kenneth Graunke <kenn...@whitecape.org> writes: > On Saturday, January 2, 2016 10:48:02 PM PST Francisco Jerez wrote: >> Switching the current pipeline while it's not completely idle or the >> read and write caches aren't flushed can lead to corruption. Fixes >> misrendering of at least the following Khronos CTS test: >> >> ES31-CTS.shader_image_load_store.basic-allTargets-store-fs >> >> The stall and flushes are no longer required on Gen8+. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93323 >> --- >> src/mesa/drivers/dri/i965/brw_misc_state.c | 28 +++++++++++++++++++++++++++ > + >> 1 file changed, 28 insertions(+) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/ > dri/i965/brw_misc_state.c >> index 7d53d18..75540c1 100644 >> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c >> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c >> @@ -886,6 +886,34 @@ brw_emit_select_pipeline(struct brw_context *brw, enum > brw_pipeline pipeline) >> >> brw->ctx.NewDriverState |= BRW_NEW_CC_STATE; >> } >> + >> + } else if (brw->gen >= 6) { >> + /* From "BXML » GT » MI » vol1a GPU Overview » [Instruction] >> + * PIPELINE_SELECT [DevBWR+]": > > Can we cite the public docs? >
The public docs for PIPELINE_SELECT seemed rather inaccurate. The IVB version I have in front of me right now is missing this one workaround, and the BDW version mentions it incorrectly. Sigh... >> + * >> + * Project: DEVSNB+ >> + * >> + * Software must ensure all the write caches are flushed through a >> + * stalling PIPE_CONTROL command followed by another PIPE_CONTROL >> + * command to invalidate read only caches prior to programming >> + * MI_PIPELINE_SELECT command to change the Pipeline Select Mode. >> + */ >> + const unsigned dc_flush = >> + brw->gen >= 7 ? PIPE_CONTROL_DATA_CACHE_INVALIDATE : 0; > > I was going to suggest doing a brw_emit_post_sync_nonzero_flush first > on Sandybridge, but I forgot that we now just emit that at the start > of every state upload. Fairly moot anyway since we don't do GPGPU on > Sandybridge anyway. > Hmm, that sounds very sensible to me, it would be rather fragile for this function to rely on a flush with post-sync op having been done previously, even if at this point this will only be called once at context creation on SNB -- Although for the same reason it seems rather fragile for brw_emit_pipe_control_flush() to assume that the workaround has been applied already. I'd be inclined to change brw_emit_pipe_control_flush() to emit the post-sync op when needed on SNB just like we do for other PIPE_CONTROL workarounds on Gen7 and Gen8. >> + >> + brw_emit_pipe_control_flush(brw, >> + PIPE_CONTROL_RENDER_TARGET_FLUSH | >> + PIPE_CONTROL_DEPTH_CACHE_FLUSH | >> + dc_flush | >> + PIPE_CONTROL_NO_WRITE | >> + PIPE_CONTROL_CS_STALL); > > Why RENDER_TARGET_FLUSH, DEPTH_CACHE_FLUSH, DATA_CACHE_INVALIDATE, > and NO_WRITE? The cited workaround explains a CS Stall and the RO > invalidations below, but I'm not seeing why the others are needed. > It also says that "software must ensure all the write caches are flushed". >> + >> + brw_emit_pipe_control_flush(brw, >> + PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE | >> + PIPE_CONTROL_CONST_CACHE_INVALIDATE | >> + PIPE_CONTROL_STATE_CACHE_INVALIDATE | >> + PIPE_CONTROL_INSTRUCTION_INVALIDATE | >> + PIPE_CONTROL_NO_WRITE); >> } >> >> /* Select the pipeline */ >>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
