[Mesa-dev] [PATCH] anv/query: flush render target before copying results
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Tue Dec 4 10:37:30 UTC 2018
On 03/12/2018 22:08, Jason Ekstrand wrote:
> On Mon, Dec 3, 2018 at 11:26 AM Lionel Landwerlin
> <lionel.g.landwerlin at intel.com <mailto:lionel.g.landwerlin at intel.com>>
> wrote:
>
> This change tracks render target writes in the pipeline and applies a
> render target flush before copying the query results to make sure the
> preceding operations have landed in memory before the command streamer
> initiates the copy.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com
> <mailto:lionel.g.landwerlin at intel.com>>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108909
> Fixes: 37f9788e9a8e44 ("anv: flush pipeline before query result
> copies")
> Cc: mesa-stable at lists.freedesktop.org
> <mailto:mesa-stable at lists.freedesktop.org>
> ---
> src/intel/vulkan/anv_private.h | 7 +++++++
> src/intel/vulkan/genX_blorp_exec.c | 1 +
> src/intel/vulkan/genX_cmd_buffer.c | 14 ++++++++++++++
> src/intel/vulkan/genX_gpu_memcpy.c | 1 +
> src/intel/vulkan/genX_query.c | 10 +++++++++-
> 5 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index 62c563294f5..aff076a55d9 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1747,6 +1747,13 @@ enum anv_pipe_bits {
> * we would have to CS stall on every flush which could be bad.
> */
> ANV_PIPE_NEEDS_CS_STALL_BIT = (1 << 21),
> +
> + /* This bit does not exist directly in PIPE_CONTROL. It means
> that render
> + * target operations are ongoing. Some operations like copies
> on the
> + * command streamer might need to be aware of this to trigger the
> + * appropriate stall before they can proceed with the copy.
> + */
> + ANV_PIPE_RENDER_TARGET_WRITES = (1 << 22),
> };
>
> #define ANV_PIPE_FLUSH_BITS ( \
> diff --git a/src/intel/vulkan/genX_blorp_exec.c
> b/src/intel/vulkan/genX_blorp_exec.c
> index 2035017ce0e..c573e890946 100644
> --- a/src/intel/vulkan/genX_blorp_exec.c
> +++ b/src/intel/vulkan/genX_blorp_exec.c
> @@ -263,4 +263,5 @@ genX(blorp_exec)(struct blorp_batch *batch,
> cmd_buffer->state.gfx.vb_dirty = ~0;
> cmd_buffer->state.gfx.dirty = ~0;
> cmd_buffer->state.push_constants_dirty = ~0;
> + cmd_buffer->state.pending_pipe_bits |=
> ANV_PIPE_RENDER_TARGET_WRITES;
> }
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index c7e5ef9596e..fb70cd2e386 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -1766,6 +1766,12 @@ genX(cmd_buffer_apply_pipe_flushes)(struct
> anv_cmd_buffer *cmd_buffer)
> pipe.StallAtPixelScoreboard = true;
> }
>
> + /* If a render target flush was emitted, then we can toggle
> off the bit
> + * saying that render target writes are ongoing.
> + */
> + if (bits & ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT)
> + bits &= ~(ANV_PIPE_RENDER_TARGET_WRITES);
> +
> bits &= ~(ANV_PIPE_FLUSH_BITS | ANV_PIPE_CS_STALL_BIT);
> }
>
> @@ -2777,6 +2783,8 @@ void genX(CmdDraw)(
> prim.StartInstanceLocation = firstInstance;
> prim.BaseVertexLocation = 0;
> }
> +
> + cmd_buffer->state.pending_pipe_bits |=
> ANV_PIPE_RENDER_TARGET_WRITES;
> }
>
> void genX(CmdDrawIndexed)(
> @@ -2816,6 +2824,8 @@ void genX(CmdDrawIndexed)(
> prim.StartInstanceLocation = firstInstance;
> prim.BaseVertexLocation = vertexOffset;
> }
> +
> + cmd_buffer->state.pending_pipe_bits |=
> ANV_PIPE_RENDER_TARGET_WRITES;
> }
>
> /* Auto-Draw / Indirect Registers */
> @@ -2949,6 +2959,8 @@ void genX(CmdDrawIndirect)(
>
> offset += stride;
> }
> +
> + cmd_buffer->state.pending_pipe_bits |=
> ANV_PIPE_RENDER_TARGET_WRITES;
> }
>
> void genX(CmdDrawIndexedIndirect)(
> @@ -2988,6 +3000,8 @@ void genX(CmdDrawIndexedIndirect)(
>
> offset += stride;
> }
> +
> + cmd_buffer->state.pending_pipe_bits |=
> ANV_PIPE_RENDER_TARGET_WRITES;
> }
>
> static VkResult
> diff --git a/src/intel/vulkan/genX_gpu_memcpy.c
> b/src/intel/vulkan/genX_gpu_memcpy.c
> index 81522986550..1bee1c6dc17 100644
> --- a/src/intel/vulkan/genX_gpu_memcpy.c
> +++ b/src/intel/vulkan/genX_gpu_memcpy.c
> @@ -302,4 +302,5 @@ genX(cmd_buffer_so_memcpy)(struct
> anv_cmd_buffer *cmd_buffer,
> }
>
> cmd_buffer->state.gfx.dirty |= ANV_CMD_DIRTY_PIPELINE;
> + cmd_buffer->state.pending_pipe_bits |=
> ANV_PIPE_RENDER_TARGET_WRITES;
>
>
> This is a bit odd since there are no RT writes going on.... Still,
> probably accurate enough so meh.
I could add another bit like "side buffers writes" which would call for
a pixel scoreboard stall only.
> }
> diff --git a/src/intel/vulkan/genX_query.c
> b/src/intel/vulkan/genX_query.c
> index 4831c4ea334..130c9661f15 100644
> --- a/src/intel/vulkan/genX_query.c
> +++ b/src/intel/vulkan/genX_query.c
> @@ -730,7 +730,15 @@ void genX(CmdCopyQueryPoolResults)(
> ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
>
> if ((flags & VK_QUERY_RESULT_WAIT_BIT) ||
> - (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {
> + (cmd_buffer->state.pending_pipe_bits &
> + (ANV_PIPE_FLUSH_BITS | ANV_PIPE_RENDER_TARGET_WRITES))) {
> + /* If render target writes are ongoing, request a render
> target cache
> + * flush.
> + */
> + if (cmd_buffer->state.pending_pipe_bits &
> ANV_PIPE_RENDER_TARGET_WRITES) {
> + cmd_buffer->state.pending_pipe_bits |=
> + ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
> + }
>
>
> You could also just put the above bit above the whole if and you
> wouldn't need to explicitly check ANV_PIPE_RENDER_TARGET_WRITE there.
> I'm not sure that's actually better; it just looks like less code.
> Either way,
Will do.
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net
> <mailto:jason at jlekstrand.net>>
>
> Thanks for sorting this out!
>
> --Jason
>
> cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
> }
> --
> 2.20.0.rc1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181204/7a256ab6/attachment-0001.html>
More information about the mesa-dev
mailing list