[Mesa-dev] [PATCH 1/2] radv: be smarter with descriptors when emitting secondary buffers

[Samuel Pitoiset]

On 10/19/2017 09:08 PM, Bas Nieuwenhuizen wrote:
> On Wed, Oct 11, 2017 at 5:18 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> If the secondary buffers don't use any descriptors we don't
>> have to re-emit the ones from the primary command buffer.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c | 29 ++++++++++++++++++++++++++++-
>>   1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index 22637950c4..69ca16b52d 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -2615,6 +2615,29 @@ void radv_CmdSetStencilReference(
>>          cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_STENCIL_REFERENCE;
>>   }
>>
>> +static bool
>> +radv_cmd_buffer_has_descriptors(struct radv_cmd_buffer *cmd_buffer)
>> +{
>> +       struct radv_cmd_state *state = &cmd_buffer->state;
>> +       int i;
>> +
>> +       for (i = 0; i < MAX_SETS; i++) {
>> +               if (cmd_buffer->state.descriptors[i])
>> +                       return true;
>> +       }
> This does not work, because of the meta restore stuff resetting to NULL.

Right, but the meta restore operation should be smarter and not reset to 
NULL if the operation doesn't use any descriptors.

> 
>> +
>> +       if ((state->pipeline &&
>> +            state->pipeline->need_indirect_descriptor_sets) ||
>> +           (state->compute_pipeline &&
>> +            state->compute_pipeline->need_indirect_descriptor_sets))
>> +               return true;
> 
> This seems unnecessary to me here.
> 
>> +
>> +       if (cmd_buffer->push_descriptors.capacity > 0)
>> +               return true;
> 
> This as well, we have a separate descriptor set per cmd buffer, so
> what is in there does not matter.
> 
> Furthermore, if the parent command buffer and child command buffer
> have different emitted pipelines the user SGPR assignment is different
> and we need a flush too.

Yeah, well, this stuff needs more work.

Thanks for your feedback!

> 
> 
>> +
>> +       return false;
>> +}
>> +
>>   void radv_CmdExecuteCommands(
>>          VkCommandBuffer                             commandBuffer,
>>          uint32_t                                    commandBufferCount,
>> @@ -2675,6 +2698,11 @@ void radv_CmdExecuteCommands(
>>                                  secondary->state.last_primitive_reset_en;
>>                  }
>>
>> +               /* Only re-emit all descriptors when needed. */
>> +               if (radv_cmd_buffer_has_descriptors(secondary)) {
>> +                       radv_mark_descriptor_sets_dirty(primary);
>> +               }
>> +
>>                  primary->state.last_primitive_reset_index = secondary->state.last_primitive_reset_index;
>>                  primary->state.last_ia_multi_vgt_param = secondary->state.last_ia_multi_vgt_param;
>>          }
>> @@ -2684,7 +2712,6 @@ void radv_CmdExecuteCommands(
>>           */
>>          primary->state.dirty |= RADV_CMD_DIRTY_PIPELINE;
>>          primary->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_ALL;
>> -       radv_mark_descriptor_sets_dirty(primary);
>>   }
>>
>>   VkResult radv_CreateCommandPool(
>> --
>> 2.14.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev