[Mesa-dev] [PATCH 3/3] radv: only emit cache flushes when the pool size is large enough

[Bas Nieuwenhuizen]

On Wed, Feb 28, 2018 at 8:31 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> This is an optimization which reduces the number of flushes for
> small pool buffers.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/amd/vulkan/radv_meta_buffer.c |  6 ------
>  src/amd/vulkan/radv_private.h     |  6 ++++++
>  src/amd/vulkan/radv_query.c       | 12 ++++++++----
>  3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta_buffer.c b/src/amd/vulkan/radv_meta_buffer.c
> index e6ad235e93..2e1ba2c7b2 100644
> --- a/src/amd/vulkan/radv_meta_buffer.c
> +++ b/src/amd/vulkan/radv_meta_buffer.c
> @@ -4,12 +4,6 @@
>  #include "sid.h"
>  #include "radv_cs.h"
>
> -/*
> - * This is the point we switch from using CP to compute shader
> - * for certain buffer operations.
> - */
> -#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
> -
>  static nir_shader *
>  build_buffer_fill_shader(struct radv_device *dev)
>  {
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 752b6a7592..0f8ddb2e10 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -95,6 +95,12 @@ typedef uint32_t xcb_window_t;
>
>  #define NUM_DEPTH_CLEAR_PIPELINES 3
>
> +/*
> + * This is the point we switch from using CP to compute shader
> + * for certain buffer operations.
> + */
> +#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
> +
>  enum radv_mem_heap {
>         RADV_MEM_HEAP_VRAM,
>         RADV_MEM_HEAP_VRAM_CPU_ACCESS,
> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
> index 82831961ac..da2bcf5212 100644
> --- a/src/amd/vulkan/radv_query.c
> +++ b/src/amd/vulkan/radv_query.c
> @@ -1088,10 +1088,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);
> +               if (pool->size >= RADV_BUFFER_OPS_CS_THRESHOLD) {
> +                       /* Only need to flush caches if the query pool size is
> +                        * large enough to be resetted using the compute shader
> +                        * path. Small pools don't need any cache flushes
> +                        * because we use a CP dma clear.
> +                        */
> +                       si_emit_cache_flush(cmd_buffer);
> +               }
>                 cmd_buffer->pending_reset_query = false;

I think the pending query reset also need to be inside, as otherwise
we possibly forget to flush a larger query behind it?

I think you can do a similar opt by only setting the reset bit if the
fill_buffer command returns non-zero. That way you don't set it for
small pools in the first place (though it does not replace this opt).

Otherwise

Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>         }
>
> --
> 2.16.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev