[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 9:41 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> 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?

Yup, rest looked good to me.
>
>
>>
>> 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