[Mesa-dev] [PATCH] i965/blorp: Do and end-of-pipe sync on both sides of fast-clear ops

[Jason Ekstrand]

On Mon, May 15, 2017 at 8:12 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Mon, May 15, 2017 at 07:55:46AM -0700, Jason Ekstrand wrote:
> > We've discovered in the Vulkan driver that simply doing the end-of-pipe
> > sync afterwards is insufficient.  The specific requirement stated in the
> > PRM is that you have to do one every time you transition between the
> > tree modes of "clear", "render", and "resolve".  This is GL, so we could
> > track it but any attempt to do so would most likely get it wrong.  For
> > now, it's easier to just assume that every fast-clear op is an island
> > and do the sync both before and after.
> >
> > This also removes the unneeded flush and stall after slow-clear
> > operations.
> >
> > Cc: "17.0 17.1" <mesa-stable at lists.freedesktop.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c | 56
> ++++++++++++++++++++++++-----------
> >  1 file changed, 38 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index b69cb4f..ebc4612 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -876,6 +876,22 @@ do_single_blorp_clear(struct brw_context *brw,
> struct gl_framebuffer *fb,
> >        DBG("%s (fast) to mt %p level %d layers %d+%d\n", __FUNCTION__,
> >            irb->mt, irb->mt_level, irb->mt_layer, num_layers);
> >
> > +      /* Ivybrigde PRM Vol 2, Part 1, "11.7 MCS Buffer for Render
> Target(s)":
> > +       *
> > +       *    "Any transition from any value in {Clear, Render, Resolve}
> to a
> > +       *    different value in {Clear, Render, Resolve} requires end of
> pipe
> > +       *    synchronization."
>
> I've been meaning to ask should we add some clarification when we refer to
> end-of-pipe sync but don't actually do exactly what the spec says.
> According
> to spec there should be also a write just after the flush. I have quite a
> bit
> of details in:
>

Yeah, it's a bit confusing.  In this particular case, there are a couple
other spots in the PRM which talk about this:

   /* From the Sky Lake PRM Vol. 7, "Render Target Resolve":
    *
    *    "When performing a render target resolve, PIPE_CONTROL with end of
    *    pipe sync must be delivered."

   /* From the Sky Lake PRM Vol. 7, "Render Target Fast Clear":
    *
    *    "After Render target fast clear, pipe-control with color cache
    *    write-flush must be issued before sending any DRAW commands on
    *    that render target."

So, yeah, it's a bit mirky...  In practice, at least in the Vulkan driver,
the flush and CS stall appear to be sufficient.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170515/529e20c2/attachment-0001.html>