[Mesa-dev] [PATCH] radv: sync before resetting a pool if there is active pending queries

[Bas Nieuwenhuizen]

r-b

On Tue, May 28, 2019 at 11:05 AM Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
> Make sure to sync all previous work if the given command buffer
> has pending active queries. Otherwise the GPU might write queries
> data after the reset operation.
>
> This fixes a bunch of new dEQP-VK.query_pool.* CTS failures.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c |  6 ++++++
>  src/amd/vulkan/radv_private.h    |  5 +++++
>  src/amd/vulkan/radv_query.c      | 11 +++++++++++
>  src/amd/vulkan/si_cmd_buffer.c   |  5 +++++
>  4 files changed, 27 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 43730f0568c..3c7d3de69e8 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -2950,6 +2950,12 @@ VkResult radv_EndCommandBuffer(
>         if (cmd_buffer->queue_family_index != RADV_QUEUE_TRANSFER) {
>                 if (cmd_buffer->device->physical_device->rad_info.chip_class == GFX6)
>                         cmd_buffer->state.flush_bits |= RADV_CMD_FLAG_CS_PARTIAL_FLUSH | RADV_CMD_FLAG_PS_PARTIAL_FLUSH | RADV_CMD_FLAG_WRITEBACK_GLOBAL_L2;
> +
> +               /* Make sure to sync all pending active queries at the end of
> +                * command buffer.
> +                */
> +               cmd_buffer->state.flush_bits |= cmd_buffer->active_query_flush_bits;
> +
>                 si_emit_cache_flush(cmd_buffer);
>         }
>
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 7834a505562..08b2621685d 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1142,6 +1142,11 @@ struct radv_cmd_buffer {
>          * Whether a query pool has been resetted and we have to flush caches.
>          */
>         bool pending_reset_query;
> +
> +       /**
> +        * Bitmask of pending active query flushes.
> +        */
> +       enum radv_cmd_flush_bits active_query_flush_bits;
>  };
>
>  struct radv_image;
> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
> index 014f5cede6a..ff9ceceb3ed 100644
> --- a/src/amd/vulkan/radv_query.c
> +++ b/src/amd/vulkan/radv_query.c
> @@ -1444,6 +1444,12 @@ void radv_CmdResetQueryPool(
>                          ? TIMESTAMP_NOT_READY : 0;
>         uint32_t flush_bits = 0;
>
> +       /* Make sure to sync all previous work if the given command buffer has
> +        * pending active queries. Otherwise the GPU might write queries data
> +        * after the reset operation.
> +        */
> +       cmd_buffer->state.flush_bits |= cmd_buffer->active_query_flush_bits;
> +
>         flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
>                                        firstQuery * pool->stride,
>                                        queryCount * pool->stride, value);
> @@ -1624,6 +1630,11 @@ static void emit_end_query(struct radv_cmd_buffer *cmd_buffer,
>         default:
>                 unreachable("ending unhandled query type");
>         }
> +
> +       cmd_buffer->active_query_flush_bits |= RADV_CMD_FLAG_PS_PARTIAL_FLUSH |
> +                                              RADV_CMD_FLAG_CS_PARTIAL_FLUSH |
> +                                              RADV_CMD_FLAG_INV_GLOBAL_L2 |
> +                                              RADV_CMD_FLAG_INV_VMEM_L1;
>  }
>
>  void radv_CmdBeginQueryIndexedEXT(
> diff --git a/src/amd/vulkan/si_cmd_buffer.c b/src/amd/vulkan/si_cmd_buffer.c
> index c73c6ecd65c..aae8d578c10 100644
> --- a/src/amd/vulkan/si_cmd_buffer.c
> +++ b/src/amd/vulkan/si_cmd_buffer.c
> @@ -989,6 +989,11 @@ si_emit_cache_flush(struct radv_cmd_buffer *cmd_buffer)
>         if (unlikely(cmd_buffer->device->trace_bo))
>                 radv_cmd_buffer_trace_emit(cmd_buffer);
>
> +       /* Clear the caches that have been flushed to avoid syncing too much
> +        * when there is some pending active queries.
> +        */
> +       cmd_buffer->active_query_flush_bits &= ~cmd_buffer->state.flush_bits;
> +
>         cmd_buffer->state.flush_bits = 0;
>
>         /* If the driver used a compute shader for resetting a query pool, it
> --
> 2.21.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev