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

[Lionel Landwerlin]

On 29/11/2018 16:58, Jason Ekstrand wrote:
> On Thu, Nov 29, 2018 at 10:45 AM Lionel Landwerlin 
> <lionel.g.landwerlin at intel.com <mailto: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.
>
>     v2: Unconditionally flush the pipeline before copying the results
>         (Jason)
>
>     Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com
>     <mailto:lionel.g.landwerlin at intel.com>>
>     Suggested-by: Jason Ekstrand <jason at jlekstrand.net
>     <mailto:jason at jlekstrand.net>>
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
>     Cc: mesa-stable at lists.freedesktop.org
>     <mailto:mesa-stable at lists.freedesktop.org>
>     ---
>      src/intel/vulkan/genX_query.c | 9 ++++-----
>      1 file changed, 4 insertions(+), 5 deletions(-)
>
>     diff --git a/src/intel/vulkan/genX_query.c
>     b/src/intel/vulkan/genX_query.c
>     index ce8757f2643..f1472c0d570 100644
>     --- a/src/intel/vulkan/genX_query.c
>     +++ b/src/intel/vulkan/genX_query.c
>     @@ -729,11 +729,10 @@ void genX(CmdCopyQueryPoolResults)(
>         ANV_FROM_HANDLE(anv_query_pool, pool, queryPool);
>         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 (flags & VK_QUERY_RESULT_WAIT_BIT ||
>
>
> That bare & should have some parens.  With that,
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net 
> <mailto:jason at jlekstrand.net>>


Thanks, pushed with your requested change.


-

Lionel


>     +       (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);
>         }
>
>         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/019258c4/attachment.html>