[Mesa-dev] [PATCH] gallium: Add PIPE_BARRIER_UPDATE_BUFFER and UPDATE_TEXTURE bits.
Kenneth Graunke
kenneth at whitecape.org
Fri Mar 8 04:22:48 UTC 2019
Forgot Marek is on vacation. Nicolai, do you have an opinion?
It looks like you added the original comments that it isn't
necessary to handle these cases, too...
--Ken
On Thursday, March 7, 2019 3:48:04 PM PST Kenneth Graunke wrote:
> Hey Ilia, Marek,
>
> Do you have an opinion about this? I've got a R-b from Eric Anholt
> and what sounds like an Ack from Roland, but I wanted to make sure
> everyone was OK with this before landing it.
>
> --Ken
>
> On Wednesday, March 6, 2019 12:32:23 AM PST Kenneth Graunke wrote:
> > The glMemoryBarrier() function makes shader memory stores ordered with
> > respect to things specified by the given bits. Until now, st/mesa has
> > ignored GL_TEXTURE_UPDATE_BARRIER_BIT and GL_BUFFER_UPDATE_BARRIER_BIT,
> > saying that drivers should implicitly perform the needed flushing.
> >
> > This seems like a pretty big assumption to make. Instead, this commit
> > opts to translate them to new PIPE_BARRIER bits, and adjusts existing
> > drivers to continue ignoring them (preserving the current behavior).
> >
> > The i965 driver performs actions on these memory barriers. Shader
> > memory stores go through a "data cache" which is separate from the
> > render cache and other read caches (like the texture cache). All
> > memory barriers need to flush the data cache (to ensure shader memory
> > stores are visible), and possibly invalidate read caches (to ensure
> > stale data is no longer visible). The driver implicitly flushes for
> > most caches, but not for data cache, since ARB_shader_image_load_store
> > introduced MemoryBarrier() precisely to order these explicitly.
> >
> > I would like to follow i965's approach in iris, flushing the data cache
> > on any MemoryBarrier() call, so I need st/mesa to actually call the
> > pipe->memory_barrier() callback.
> > ---
> > .../drivers/freedreno/freedreno_context.c | 3 ++
> > src/gallium/drivers/r600/r600_state_common.c | 4 +++
> > src/gallium/drivers/radeonsi/si_state.c | 3 ++
> > src/gallium/drivers/softpipe/sp_flush.c | 3 ++
> > src/gallium/drivers/tegra/tegra_context.c | 3 ++
> > src/gallium/drivers/v3d/v3d_context.c | 3 ++
> > src/gallium/include/pipe/p_defines.h | 7 +++-
> > src/mesa/state_tracker/st_cb_texturebarrier.c | 34 +++++++++++--------
> > 8 files changed, 44 insertions(+), 16 deletions(-)
> >
> > I am curious to hear people's thoughts on this. It seems useful for the
> > driver to receive a memory_barrier() call...and adding a few bits seemed
> > to be the cleanest way to make that happen. But I'm open to ideas.
> >
> > There are no nouveau changes in this patch, but that's only because none
> > appeared to be necessary. Most drivers performed some kind of flush on
> > any memory_barrier() call, regardless of the bits - but nouveau's hooks
> > don't. So the newly added case would already be a no-op.
> >
> > diff --git a/src/gallium/drivers/freedreno/freedreno_context.c b/src/gallium/drivers/freedreno/freedreno_context.c
> > index 6c01c15bb32..4e86d099974 100644
> > --- a/src/gallium/drivers/freedreno/freedreno_context.c
> > +++ b/src/gallium/drivers/freedreno/freedreno_context.c
> > @@ -99,6 +99,9 @@ fd_texture_barrier(struct pipe_context *pctx, unsigned flags)
> > static void
> > fd_memory_barrier(struct pipe_context *pctx, unsigned flags)
> > {
> > + if (!(flags & ~PIPE_BARRIER_UPDATE))
> > + return;
> > +
> > fd_context_flush(pctx, NULL, 0);
> > /* TODO do we need to check for persistently mapped buffers and fd_bo_cpu_prep()?? */
> > }
> > diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> > index f886a27170d..c7c606f131b 100644
> > --- a/src/gallium/drivers/r600/r600_state_common.c
> > +++ b/src/gallium/drivers/r600/r600_state_common.c
> > @@ -94,6 +94,10 @@ void r600_emit_alphatest_state(struct r600_context *rctx, struct r600_atom *atom
> > static void r600_memory_barrier(struct pipe_context *ctx, unsigned flags)
> > {
> > struct r600_context *rctx = (struct r600_context *)ctx;
> > +
> > + if (!(flags & ~PIPE_BARRIER_UPDATE))
> > + return;
> > +
> > if (flags & PIPE_BARRIER_CONSTANT_BUFFER)
> > rctx->b.flags |= R600_CONTEXT_INV_CONST_CACHE;
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
> > index 458b108a7e3..3c29b4c92ed 100644
> > --- a/src/gallium/drivers/radeonsi/si_state.c
> > +++ b/src/gallium/drivers/radeonsi/si_state.c
> > @@ -4710,6 +4710,9 @@ void si_memory_barrier(struct pipe_context *ctx, unsigned flags)
> > {
> > struct si_context *sctx = (struct si_context *)ctx;
> >
> > + if (!(flags & ~PIPE_BARRIER_UPDATE))
> > + return;
> > +
> > /* Subsequent commands must wait for all shader invocations to
> > * complete. */
> > sctx->flags |= SI_CONTEXT_PS_PARTIAL_FLUSH |
> > diff --git a/src/gallium/drivers/softpipe/sp_flush.c b/src/gallium/drivers/softpipe/sp_flush.c
> > index 3bf8c499218..5eccbe34d46 100644
> > --- a/src/gallium/drivers/softpipe/sp_flush.c
> > +++ b/src/gallium/drivers/softpipe/sp_flush.c
> > @@ -192,5 +192,8 @@ void softpipe_texture_barrier(struct pipe_context *pipe, unsigned flags)
> >
> > void softpipe_memory_barrier(struct pipe_context *pipe, unsigned flags)
> > {
> > + if (!(flags & ~PIPE_BARRIER_UPDATE))
> > + return;
> > +
> > softpipe_texture_barrier(pipe, 0);
> > }
> > diff --git a/src/gallium/drivers/tegra/tegra_context.c b/src/gallium/drivers/tegra/tegra_context.c
> > index bbc03628336..e9e51656921 100644
> > --- a/src/gallium/drivers/tegra/tegra_context.c
> > +++ b/src/gallium/drivers/tegra/tegra_context.c
> > @@ -974,6 +974,9 @@ tegra_memory_barrier(struct pipe_context *pcontext, unsigned int flags)
> > {
> > struct tegra_context *context = to_tegra_context(pcontext);
> >
> > + if (!(flags & ~PIPE_BARRIER_UPDATE))
> > + return;
> > +
> > context->gpu->memory_barrier(context->gpu, flags);
> > }
> >
> > diff --git a/src/gallium/drivers/v3d/v3d_context.c b/src/gallium/drivers/v3d/v3d_context.c
> > index d07ad403590..fcd2d5ec69b 100644
> > --- a/src/gallium/drivers/v3d/v3d_context.c
> > +++ b/src/gallium/drivers/v3d/v3d_context.c
> > @@ -71,6 +71,9 @@ v3d_memory_barrier(struct pipe_context *pctx, unsigned int flags)
> > {
> > struct v3d_context *v3d = v3d_context(pctx);
> >
> > + if (!(flags & ~PIPE_BARRIER_UPDATE))
> > + return;
> > +
> > /* We only need to flush jobs writing to SSBOs/images. */
> > perf_debug("Flushing all jobs for glMemoryBarrier(), could do better");
> > v3d_flush(pctx);
> > diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> > index e2b0104ce43..2970c47c3c7 100644
> > --- a/src/gallium/include/pipe/p_defines.h
> > +++ b/src/gallium/include/pipe/p_defines.h
> > @@ -425,7 +425,12 @@ enum pipe_flush_flags
> > #define PIPE_BARRIER_FRAMEBUFFER (1 << 9)
> > #define PIPE_BARRIER_STREAMOUT_BUFFER (1 << 10)
> > #define PIPE_BARRIER_GLOBAL_BUFFER (1 << 11)
> > -#define PIPE_BARRIER_ALL ((1 << 12) - 1)
> > +#define PIPE_BARRIER_UPDATE_BUFFER (1 << 12)
> > +#define PIPE_BARRIER_UPDATE_TEXTURE (1 << 13)
> > +#define PIPE_BARRIER_ALL ((1 << 14) - 1)
> > +
> > +#define PIPE_BARRIER_UPDATE \
> > + (PIPE_BARRIER_UPDATE_BUFFER | PIPE_BARRIER_UPDATE_TEXTURE)
> >
> > /**
> > * Flags for pipe_context::texture_barrier.
> > diff --git a/src/mesa/state_tracker/st_cb_texturebarrier.c b/src/mesa/state_tracker/st_cb_texturebarrier.c
> > index 2bff03b484a..4a9c72f2c62 100644
> > --- a/src/mesa/state_tracker/st_cb_texturebarrier.c
> > +++ b/src/mesa/state_tracker/st_cb_texturebarrier.c
> > @@ -95,21 +95,25 @@ st_MemoryBarrier(struct gl_context *ctx, GLbitfield barriers)
> > */
> > flags |= PIPE_BARRIER_TEXTURE;
> > }
> > - /* GL_TEXTURE_UPDATE_BARRIER_BIT:
> > - * Texture updates translate to:
> > - * (1) texture transfers to/from the CPU,
> > - * (2) texture as blit destination, or
> > - * (3) texture as framebuffer.
> > - * In all cases, we assume the driver does the required flushing
> > - * automatically.
> > - */
> > - /* GL_BUFFER_UPDATE_BARRIER_BIT:
> > - * Buffer updates translate to
> > - * (1) buffer transfers to/from the CPU,
> > - * (2) resource copies and clears.
> > - * In all cases, we assume the driver does the required flushing
> > - * automatically.
> > - */
> > + if (barriers & GL_TEXTURE_UPDATE_BARRIER_BIT) {
> > + /* GL_TEXTURE_UPDATE_BARRIER_BIT:
> > + * Texture updates translate to:
> > + * (1) texture transfers to/from the CPU,
> > + * (2) texture as blit destination, or
> > + * (3) texture as framebuffer.
> > + * Some drivers may handle these automatically, and can ignore the bit.
> > + */
> > + flags |= PIPE_BARRIER_UPDATE_TEXTURE;
> > + }
> > + if (barriers & GL_BUFFER_UPDATE_BARRIER_BIT) {
> > + /* GL_BUFFER_UPDATE_BARRIER_BIT:
> > + * Buffer updates translate to
> > + * (1) buffer transfers to/from the CPU,
> > + * (2) resource copies and clears.
> > + * Some drivers may handle these automatically, and can ignore the bit.
> > + */
> > + flags |= PIPE_BARRIER_UPDATE_BUFFER;
> > + }
> > if (barriers & GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT)
> > flags |= PIPE_BARRIER_MAPPED_BUFFER;
> > if (barriers & GL_QUERY_BUFFER_BARRIER_BIT)
> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190307/ac152883/attachment.sig>
More information about the mesa-dev
mailing list