[Mesa-dev] [PATCH 4/4] i965/blorp: Make post draw flush more explicit
Jason Ekstrand
jason at jlekstrand.net
Thu Jan 19 19:16:39 UTC 2017
On Wed, Jan 18, 2017 at 11:25 PM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:
> On Wed, Jan 18, 2017 at 01:54:43PM -0800, Jason Ekstrand wrote:
> > On Jan 18, 2017 1:47 PM, "Francisco Jerez" <[1]currojerez at riseup.net>
> > wrote:
> >
> > Topi Pohjolainen <[2]topi.pohjolainen at 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?
>
Sorry. I should have been more specific. We need special flushes for fast
clears but we don't need them for regular clears. Regular clears will get
handled by the standard render target cache stuff.
I also discovered, while getting CCS up-and-going in Vulkan, that it
appears we need a RT flush and CS stall *before* we do a color resolve.
Otherwise, you may end up reading stale data during the resolve. I've got
a comment about it in anv_blorp.c:1493.
> 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.pohjolainen at 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 at lists.freedesktop.org
> > > [5]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > _______________________________________________
> > mesa-dev mailing list
> > [6]mesa-dev at lists.freedesktop.org
> > [7]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > References
> >
> > 1. mailto:currojerez at riseup.net
> > 2. mailto:topi.pohjolainen at gmail.com
> > 3. mailto:topi.pohjolainen at intel.com
> > 4. mailto:mesa-dev at lists.freedesktop.org
> > 5. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 6. mailto:mesa-dev at lists.freedesktop.org
> > 7. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170119/3e32fd0c/attachment-0001.html>
More information about the mesa-dev
mailing list