On 14 December 2011 02:59, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 12/13/2011 03:35 PM, Paul Berry wrote: > > Previous to this patch, the function intel_batchbuffer_emit_mi_flush() > > was a bit of a misnomer. On Gen4+, when not using the blit engine, it > > didn't actually flush the pipeline--it simply generated a > > _3DSTATE_PIPE_CONTROL command with the necessary bits set to flush GPU > > It's actually just called "PIPE_CONTROL", never 3DSTATE_PIPE_CONTROL. > (Checks the docs). Hmm, you're right. For some reason we call it _3DSTATE_PIPE_CONTROL in our #defines (see intel_reg.h). Still, it seems better for the commit message to match the documentation. I'll change the commit message. > > > caches. This was usually sufficient, since in most situations where > > intel_batchbuffer_emit_mi_flush() wass called, all we really care > > "... was called, all we really cared" (typo, tense) > Heh, oops. > > This makes a lot of sense. At some point, I think we ought to convert > some of the callers of this function to emit the proper PIPE_CONTROL > directly (with only the necessary bits and workarounds). But this is > the right way to start. Nice work figuring this out. > Agreed. As my CS professor Bob Keller once said, "get it right first, then make it fast." > > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > > about was ensuring cache coherency. > > > > However, with the advent of OpenGL 3.0, there are two cases in which > > data output by one stage of the pipeline might be consumed, in a later > > draw operation, by an earlier stage of the pipeline: > > > > (a) When using textures in the vertex shader. > > > > (b) When using drawing with a vertex buffer that was previously > > generated using transform feedback. > > > > This patch addresses case (a) by changing > > intel_batchbuffer_emit_mi_flush() so that on Gen6+, it sets the > > PIPE_CONTROL_CS_STALL bit (this forces the pipeline to actually > > flush). (Case (b) will be addressed by the next patch in the series). > > > > This is not an ideal solution--in a perfect world, the driver would > > have some buffer dependency tracking so that we would only have to > > flush the pipeline in the two cases above. Until that dependency > > tracking is implemented, however, it seems prudent to have > > intel_batchbuffer_emit_mi_flush() actually flush the pipeline, so that > > we get correct rendering, at the expense of a (hopefully small) > > performance hit. > > > > The change is only applied to Gen6+, since at the moment only Gen6+ > > supports the OpenGL 3.0 features that make a full pipeline flush > > necessary. > > --- > > src/mesa/drivers/dri/intel/intel_batchbuffer.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c > b/src/mesa/drivers/dri/intel/intel_batchbuffer.c > > index 6991db8..4ff098a 100644 > > --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c > > +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c > > @@ -461,7 +461,8 @@ intel_batchbuffer_emit_mi_flush(struct intel_context > *intel) > > PIPE_CONTROL_WRITE_FLUSH | > > PIPE_CONTROL_DEPTH_CACHE_FLUSH | > > PIPE_CONTROL_TC_FLUSH | > > - PIPE_CONTROL_NO_WRITE); > > + PIPE_CONTROL_NO_WRITE | > > + PIPE_CONTROL_CS_STALL); > > OUT_BATCH(0); /* write address */ > > OUT_BATCH(0); /* write data */ > > ADVANCE_BATCH(); > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
