On 03/01/2018 10:21 AM, Alex Smith wrote:
Hi Samuel,

On 28 February 2018 at 20:47, Samuel Pitoiset <[email protected] <mailto:[email protected]>> wrote:

    If the query pool has been previously resetted using the compute
    shader path.

    v3: set pending_reset_query only for the compute shader path
    v2: handle multiple commands buffers with same pool

    Fixes: a41e2e9cf5 ("radv: allow to use a compute shader for
    resetting the query pool")
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105292
    <https://bugs.freedesktop.org/show_bug.cgi?id=105292>
    Cc: "18.0" <[email protected]
    <mailto:[email protected]>>
    Signed-off-by: Samuel Pitoiset <[email protected]
    <mailto:[email protected]>>
    ---
      src/amd/vulkan/radv_cmd_buffer.c |  7 +++++++
      src/amd/vulkan/radv_private.h    |  5 +++++
      src/amd/vulkan/radv_query.c      | 28 +++++++++++++++++++++-------
      3 files changed, 33 insertions(+), 7 deletions(-)

    diff --git a/src/amd/vulkan/radv_cmd_buffer.c
    b/src/amd/vulkan/radv_cmd_buffer.c
    index 2b41baea3d..cfdc531acd 100644
    --- a/src/amd/vulkan/radv_cmd_buffer.c
    +++ b/src/amd/vulkan/radv_cmd_buffer.c
    @@ -1930,6 +1930,13 @@ VkResult radv_BeginCommandBuffer(

             cmd_buffer->status = RADV_CMD_BUFFER_STATUS_RECORDING;

    +       /* Force cache flushes before starting a new query in case the
    +        * corresponding pool has been resetted from a different command
    +        * buffer. This is because we have to flush caches between
    reset and
    +        * begin if the compute shader path has been used.
    +        */
    +       cmd_buffer->pending_reset_query = true;


Since this just ends up calling si_emit_cache_flush, doesn't flush_bits need to be set accordingly for it to actually do anything? If the reset is done in a previous command buffer, I think the flush would already have been done in EndCommandBuffer on that?

You are right, this is just useless because we always flushes caches in EndCommandBuffer(). Thanks for pointing this out. Will remove that hunk.


Thanks,
Alex

    +
             return result;
      }

    diff --git a/src/amd/vulkan/radv_private.h
    b/src/amd/vulkan/radv_private.h
    index c72df5a737..b76d2eb5cb 100644
    --- a/src/amd/vulkan/radv_private.h
    +++ b/src/amd/vulkan/radv_private.h
    @@ -1003,6 +1003,11 @@ struct radv_cmd_buffer {
             uint32_t gfx9_fence_offset;
             struct radeon_winsys_bo *gfx9_fence_bo;
             uint32_t gfx9_fence_idx;
    +
    +       /**
    +        * Whether a query pool has been resetted and we have to
    flush caches.
    +        */
    +       bool pending_reset_query;
      };

      struct radv_image;
    diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
    index ace745e4e6..b1393a2ec7 100644
    --- a/src/amd/vulkan/radv_query.c
    +++ b/src/amd/vulkan/radv_query.c
    @@ -1058,17 +1058,23 @@ void radv_CmdResetQueryPool(
      {
             RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
             RADV_FROM_HANDLE(radv_query_pool, pool, queryPool);
    -       struct radv_cmd_state *state = &cmd_buffer->state;
    +       uint32_t flush_bits = 0;

    -       state->flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
    -                                             firstQuery * pool->stride,
    -                                             queryCount *
    pool->stride, 0);
    +       flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
    +                                      firstQuery * pool->stride,
    +                                      queryCount * pool->stride, 0);

             if (pool->type == VK_QUERY_TYPE_TIMESTAMP ||
                 pool->type == VK_QUERY_TYPE_PIPELINE_STATISTICS) {
    -               state->flush_bits |= radv_fill_buffer(cmd_buffer,
    pool->bo,
-  pool->availability_offset + firstQuery * 4,
    -                                                     queryCount *
    4, 0);
    +               flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
+ pool->availability_offset + firstQuery * 4,
    +                                              queryCount * 4, 0);
    +       }
    +
    +       if (flush_bits) {
    +               /* Only need to flush caches for the compute shader
    path. */
    +               cmd_buffer->pending_reset_query = true;
    +               cmd_buffer->state.flush_bits |= flush_bits;
             }
      }

    @@ -1086,6 +1092,14 @@ void radv_CmdBeginQuery(

             radv_cs_add_buffer(cmd_buffer->device->ws, cs, pool->bo, 8);

    +       if (cmd_buffer->pending_reset_query) {
    +               /* Make sure to flush caches if the query pool has been
    +                * previously resetted using the compute shader path.
    +                */
    +               si_emit_cache_flush(cmd_buffer);
    +               cmd_buffer->pending_reset_query = false;
    +       }
    +
             switch (pool->type) {
             case VK_QUERY_TYPE_OCCLUSION:
                     radeon_check_space(cmd_buffer->device->ws, cs, 7);
    --
    2.16.2

    _______________________________________________
    mesa-dev mailing list
    [email protected] <mailto:[email protected]>
    https://lists.freedesktop.org/mailman/listinfo/mesa-dev
    <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>


_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to