On Thu, Jan 19, 2017 at 09:25:19AM +0200, Pohjolainen, Topi wrote:
> 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.
Dropping the flush only in case of clears but leaving it in place for resolves
gives me 15k regressions.
>
> >
> > > 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
