[Mesa-dev] [PATCH] gallium: Add PIPE_BARRIER_UPDATE_BUFFER and UPDATE_TEXTURE bits.

[Ilia Mirkin]

On Wed, Mar 6, 2019 at 3:32 AM Kenneth Graunke <kenneth at whitecape.org> 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.

I didn't go back to check the code earlier, but just saw the pushed
patch, forgot this comment, and decided to check. Looks like

https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nouveau/nvc0/nvc0_context.c#n90

would get executed, no?

  -ilia