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