<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, May 15, 2017 at 8:12 AM, Pohjolainen, Topi <span dir="ltr"><<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">On Mon, May 15, 2017 at 07:55:46AM -0700, Jason Ekstrand wrote:<br>
> We've discovered in the Vulkan driver that simply doing the end-of-pipe<br>
> sync afterwards is insufficient.  The specific requirement stated in the<br>
> PRM is that you have to do one every time you transition between the<br>
> tree modes of "clear", "render", and "resolve".  This is GL, so we could<br>
> track it but any attempt to do so would most likely get it wrong.  For<br>
> now, it's easier to just assume that every fast-clear op is an island<br>
> and do the sync both before and after.<br>
><br>
> This also removes the unneeded flush and stall after slow-clear<br>
> operations.<br>
><br>
> Cc: "17.0 17.1" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.<wbr>freedesktop.org</a>><br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_<wbr>blorp.c | 56 ++++++++++++++++++++++++------<wbr>-----<br>
>  1 file changed, 38 insertions(+), 18 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c b/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c<br>
> index b69cb4f..ebc4612 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c<br>
> @@ -876,6 +876,22 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb,<br>
>        DBG("%s (fast) to mt %p level %d layers %d+%d\n", __FUNCTION__,<br>
>            irb->mt, irb->mt_level, irb->mt_layer, num_layers);<br>
><br>
> +      /* Ivybrigde PRM Vol 2, Part 1, "11.7 MCS Buffer for Render Target(s)":<br>
> +       *<br>
> +       *    "Any transition from any value in {Clear, Render, Resolve} to a<br>
> +       *    different value in {Clear, Render, Resolve} requires end of pipe<br>
> +       *    synchronization."<br>
<br>
</div></div>I've been meaning to ask should we add some clarification when we refer to<br>
end-of-pipe sync but don't actually do exactly what the spec says. According<br>
to spec there should be also a write just after the flush. I have quite a bit<br>
of details in:<br></blockquote><div><br></div><div>Yeah, it's a bit confusing.  In this particular case, there are a couple other spots in the PRM which talk about this:<br><br>   /* From the Sky Lake PRM Vol. 7, "Render Target Resolve":<br>    *<br>    *    "When performing a render target resolve, PIPE_CONTROL with end of<br>    *    pipe sync must be delivered."<br><br>   /* From the Sky Lake PRM Vol. 7, "Render Target Fast Clear":<br>    *<br>    *    "After Render target fast clear, pipe-control with color cache<br>    *    write-flush must be issued before sending any DRAW commands on<br>    *    that render target."<br><br></div><div>So, yeah, it's a bit mirky...  In practice, at least in the Vulkan driver, the flush and CS stall appear to be sufficient.<br><br>Reading the post-sync docs in the BSPEC, I think your patch is correct according to the docs.  I think I would rather add an "post_sync" boolean parameter emit_pipe_control_flush than have a separate function for it though.  That said, I still don't understand why the writes are required over and above a CS stall.<br></div></div><br></div></div>