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

[Samuel Pitoiset]

On 02/28/2018 09:06 PM, Bas Nieuwenhuizen wrote:
> 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).

Yes, good point.

I'm going to send a v3 for the backport and I will update the rest later.

Is the Rb for the whole series?

> 
> 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