[Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 9:43 AM Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:

> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Suggested-by: Jason Ekstrand <jason at jlekstrand.net>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/intel/vulkan/genX_query.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
> index ce8757f2643..edf9962179a 100644
> --- a/src/intel/vulkan/genX_query.c
> +++ b/src/intel/vulkan/genX_query.c
> @@ -730,10 +730,9 @@ void genX(CmdCopyQueryPoolResults)(
>     ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
>
>     if (flags & VK_QUERY_RESULT_WAIT_BIT) {
> -      anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) {
> -         pc.CommandStreamerStallEnable = true;
> -         pc.StallAtPixelScoreboard     = true;
> -      }
> +      if (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)
> +         cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> +      genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
>

Unfortunately, I think we need to do it somewhat unconditionally.  More
specifically....

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_CS_STALL_BIT;
   genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
}

Otherwise, we still have a write-after-write hazard if the client does a
vkCmdFillBuffer to zero out the destination prior to copying query pool
results into it.

--Jason


>     }
>
>     struct anv_address dest_addr = anv_address_add(buffer->address,
> destOffset);
> --
> 2.20.0.rc1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181129/194016b2/attachment.html>