On Wed, Jan 18, 2017 at 01:54:43PM -0800, Jason Ekstrand wrote:
>    On Jan 18, 2017 1:47 PM, "Francisco Jerez" <[1]curroje...@riseup.net>
>    wrote:
> 
>    Topi Pohjolainen <[2]topi.pohjolai...@gmail.com> writes:
>    > Blits do not need any special treatment as the target buffer
>    > object is added to render cache just as one does for normal draw.
>    > Color clears and resolves in turn require explicit "end of pipe
>    > synchronization". It is not clear what this means exactly but the
>    > assumption is that render cache flush with command stream stall
>    > should be sufficient.
>    >
> 
>      Don't the clear and resolve paths end up calling genX(blorp_exec),
>      where
>      you used the render cache mechanism to flush a superset of the
>      caches
>      flushed below?  Why do you need to synchronize and flush twice?
> 
>    Thanks.  I recall having that same thought but apparently forgot it.  I
>    think we may still bed the resolve flush (there are extra flushes
>    required around resolves) but not the color clear flush.

I was hesitant to drop them as the spec specifically chooses to point them
out (see below). Otherwise they wouldn't be any different than normal renders
which require render target flush before using the buffers again, right?


Anyway, I tried dropping both flushes, the one after clear and the one
after resolve. Jenkins gives me some 16k regressions. Jason, you say that
resolves need the extra flushes but not clears. Is there something more in the
specs than the IVB quote I have below? I'm going to check what jenkins says
about dropping the flush after clear alone.

> 
>    > Signed-off-by: Topi Pohjolainen <[3]topi.pohjolai...@intel.com>
>    > ---
>    >  src/mesa/drivers/dri/i965/brw_blorp.c       | 22
>    ++++++++++++++++++++++
>    >  src/mesa/drivers/dri/i965/genX_blorp_exec.c |  5 -----
>    >  2 files changed, 22 insertions(+), 5 deletions(-)
>    >
>    > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>    b/src/mesa/drivers/dri/i965/brw_blorp.c
>    > index 8d58616..845abe3 100644
>    > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>    > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>    > @@ -908,6 +908,17 @@ do_single_blorp_clear(struct brw_context *brw,
>    struct gl_framebuffer *fb,
>    >        blorp_batch_finish(&batch);
>    >     }
>    >
>    > +   /*
>    > +    * 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.
>    > +    */
>    > +   brw_emit_pipe_control_flush(brw,
>    > +                               PIPE_CONTROL_RENDER_TARGET_FLUSH |
>    > +                               PIPE_CONTROL_CS_STALL);
>    > +
>    >     return true;
>    >  }
>    >
>    > @@ -975,6 +986,17 @@ brw_blorp_resolve_color(struct brw_context *brw,
>    struct intel_mipmap_tree *mt,
>    >                       brw_blorp_to_isl_format(brw, format, true),
>    >                       resolve_op);
>    >     blorp_batch_finish(&batch);
>    > +
>    > +   /*
>    > +    * 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.
>    > +    */
>    > +   brw_emit_pipe_control_flush(brw,
>    > +                               PIPE_CONTROL_RENDER_TARGET_FLUSH |
>    > +                               PIPE_CONTROL_CS_STALL);
>    >  }
>    >
>    >  static void
>    > diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    > index c0cbde5..2c53444 100644
>    > --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    > +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    > @@ -260,9 +260,4 @@ retry:
>    >     brw->ib.type = -1;
>    >
>    >     brw_render_cache_set_add_bo(brw, params->dst.addr.buffer);
>    > -
>    > -   /* Flush the sampler cache so any texturing from the destination
>    is
>    > -    * coherent.
>    > -    */
>    > -   brw_emit_mi_flush(brw);
>    >  }
>    > --
>    > 2.5.5
>    >
>    > _______________________________________________
>    > mesa-dev mailing list
>    > [4]mesa-dev@lists.freedesktop.org
>    > [5]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>      _______________________________________________
>      mesa-dev mailing list
>      [6]mesa-dev@lists.freedesktop.org
>      [7]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:curroje...@riseup.net
>    2. mailto:topi.pohjolai...@gmail.com
>    3. mailto:topi.pohjolai...@intel.com
>    4. mailto:mesa-dev@lists.freedesktop.org
>    5. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>    6. mailto:mesa-dev@lists.freedesktop.org
>    7. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to