[Mesa-dev] [PATCH 1/2] radv: add radv_vertex_elements data structure

Samuel Pitoiset samuel.pitoiset at gmail.com
Fri Sep 8 13:33:53 UTC 2017



On 09/08/2017 03:31 PM, Bas Nieuwenhuizen wrote:
> On Fri, Sep 8, 2017 at 3:14 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> In my opinion, this improves code readability.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c | 17 +++++++++--------
>>   src/amd/vulkan/radv_pipeline.c   | 12 +++++++-----
>>   src/amd/vulkan/radv_private.h    | 15 ++++++++++-----
>>   3 files changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index 4de24c96e6..a79b513ffc 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -1584,37 +1584,38 @@ radv_cmd_buffer_update_vertex_descriptors(struct radv_cmd_buffer *cmd_buffer)
>>          struct radv_device *device = cmd_buffer->device;
>>
>>          if ((cmd_buffer->state.pipeline != cmd_buffer->state.emitted_pipeline || cmd_buffer->state.vb_dirty) &&
>> -           cmd_buffer->state.pipeline->num_vertex_attribs &&
>> +           cmd_buffer->state.pipeline->vertex_elements.count &&
>>              cmd_buffer->state.pipeline->shaders[MESA_SHADER_VERTEX]->info.info.vs.has_vertex_buffers) {
>> +               struct radv_vertex_elements *velems = &cmd_buffer->state.pipeline->vertex_elements;
>>                  unsigned vb_offset;
>>                  void *vb_ptr;
>>                  uint32_t i = 0;
>> -               uint32_t num_attribs = cmd_buffer->state.pipeline->num_vertex_attribs;
>> +               uint32_t count = velems->count;
>>                  uint64_t va;
>>
>>                  /* allocate some descriptor state for vertex buffers */
>> -               radv_cmd_buffer_upload_alloc(cmd_buffer, num_attribs * 16, 256,
>> +               radv_cmd_buffer_upload_alloc(cmd_buffer, count * 16, 256,
>>                                               &vb_offset, &vb_ptr);
>>
>> -               for (i = 0; i < num_attribs; i++) {
>> +               for (i = 0; i < count; i++) {
>>                          uint32_t *desc = &((uint32_t *)vb_ptr)[i * 4];
>>                          uint32_t offset;
>> -                       int vb = cmd_buffer->state.pipeline->va_binding[i];
>> +                       int vb = velems->binding[i];
>>                          struct radv_buffer *buffer = cmd_buffer->state.vertex_bindings[vb].buffer;
>>                          uint32_t stride = cmd_buffer->state.pipeline->binding_stride[vb];
>>
>>                          device->ws->cs_add_buffer(cmd_buffer->cs, buffer->bo, 8);
>>                          va = device->ws->buffer_get_va(buffer->bo);
>>
>> -                       offset = cmd_buffer->state.vertex_bindings[vb].offset + cmd_buffer->state.pipeline->va_offset[i];
>> +                       offset = cmd_buffer->state.vertex_bindings[vb].offset + velems->offset[i];
>>                          va += offset + buffer->offset;
>>                          desc[0] = va;
>>                          desc[1] = S_008F04_BASE_ADDRESS_HI(va >> 32) | S_008F04_STRIDE(stride);
>>                          if (cmd_buffer->device->physical_device->rad_info.chip_class <= CIK && stride)
>> -                               desc[2] = (buffer->size - offset - cmd_buffer->state.pipeline->va_format_size[i]) / stride + 1;
>> +                               desc[2] = (buffer->size - offset - velems->format_size[i]) / stride + 1;
>>                          else
>>                                  desc[2] = buffer->size - offset;
>> -                       desc[3] = cmd_buffer->state.pipeline->va_rsrc_word3[i];
>> +                       desc[3] = velems->rsrc_word3[i];
>>                  }
>>
>>                  va = device->ws->buffer_get_va(cmd_buffer->upload.upload_bo);
>> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
>> index c090b8e2f0..32e8102502 100644
>> --- a/src/amd/vulkan/radv_pipeline.c
>> +++ b/src/amd/vulkan/radv_pipeline.c
>> @@ -2408,6 +2408,8 @@ radv_pipeline_init(struct radv_pipeline *pipeline,
>>
>>          const VkPipelineVertexInputStateCreateInfo *vi_info =
>>                  pCreateInfo->pVertexInputState;
>> +       struct radv_vertex_elements *velems = &pipeline->vertex_elements;
>> +
>>          for (uint32_t i = 0; i < vi_info->vertexAttributeDescriptionCount; i++) {
>>                  const VkVertexInputAttributeDescription *desc =
>>                          &vi_info->pVertexAttributeDescriptions[i];
>> @@ -2421,16 +2423,16 @@ radv_pipeline_init(struct radv_pipeline *pipeline,
>>                  num_format = radv_translate_buffer_numformat(format_desc, first_non_void);
>>                  data_format = radv_translate_buffer_dataformat(format_desc, first_non_void);
>>
>> -               pipeline->va_rsrc_word3[loc] = S_008F0C_DST_SEL_X(si_map_swizzle(format_desc->swizzle[0])) |
>> +               velems->rsrc_word3[loc] = S_008F0C_DST_SEL_X(si_map_swizzle(format_desc->swizzle[0])) |
>>                          S_008F0C_DST_SEL_Y(si_map_swizzle(format_desc->swizzle[1])) |
>>                          S_008F0C_DST_SEL_Z(si_map_swizzle(format_desc->swizzle[2])) |
>>                          S_008F0C_DST_SEL_W(si_map_swizzle(format_desc->swizzle[3])) |
>>                          S_008F0C_NUM_FORMAT(num_format) |
>>                          S_008F0C_DATA_FORMAT(data_format);
>> -               pipeline->va_format_size[loc] = format_desc->block.bits / 8;
>> -               pipeline->va_offset[loc] = desc->offset;
>> -               pipeline->va_binding[loc] = desc->binding;
>> -               pipeline->num_vertex_attribs = MAX2(pipeline->num_vertex_attribs, loc + 1);
>> +               velems->format_size[loc] = format_desc->block.bits / 8;
>> +               velems->offset[loc] = desc->offset;
>> +               velems->binding[loc] = desc->binding;
>> +               velems->count = MAX2(velems->count, loc + 1);
>>          }
>>
>>          for (uint32_t i = 0; i < vi_info->vertexBindingDescriptionCount; i++) {
>> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
>> index e32fa576fb..e3771ae747 100644
>> --- a/src/amd/vulkan/radv_private.h
>> +++ b/src/amd/vulkan/radv_private.h
>> @@ -1076,6 +1076,14 @@ struct radv_tessellation_state {
>>          uint32_t tf_param;
>>   };
>>
>> +struct radv_vertex_elements {
> 
> Can we add an _info suffix or such, to clarify that this is not the
> verex elements themselves but info about number and type etc. ?

Sure, will do.

> 
> Otherwise r-b.
>> +       uint32_t rsrc_word3[MAX_VERTEX_ATTRIBS];
>> +       uint32_t format_size[MAX_VERTEX_ATTRIBS];
>> +       uint32_t binding[MAX_VERTEX_ATTRIBS];
>> +       uint32_t offset[MAX_VERTEX_ATTRIBS];
>> +       uint32_t count;
>> +};
>> +
>>   struct radv_pipeline {
>>          struct radv_device *                          device;
>>          uint32_t                                     dynamic_state_mask;
>> @@ -1089,11 +1097,8 @@ struct radv_pipeline {
>>          struct radv_shader_variant *gs_copy_shader;
>>          VkShaderStageFlags                           active_stages;
>>
>> -       uint32_t va_rsrc_word3[MAX_VERTEX_ATTRIBS];
>> -       uint32_t va_format_size[MAX_VERTEX_ATTRIBS];
>> -       uint32_t va_binding[MAX_VERTEX_ATTRIBS];
>> -       uint32_t va_offset[MAX_VERTEX_ATTRIBS];
>> -       uint32_t num_vertex_attribs;
>> +       struct radv_vertex_elements                  vertex_elements;
>> +
>>          uint32_t                                     binding_stride[MAX_VBS];
>>
>>          union {
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list