[Mesa-dev] [PATCH 2/2] radv: do not update vertex descriptors if the allocation failed

[Samuel Pitoiset]

On 09/08/2017 03:33 PM, Bas Nieuwenhuizen wrote:
> While this is a fix, I'm not sure if it is a fix we want without more
> fixes. If we end up not finishing the flush_state, we could end up
> emitting a draw packet without having emitted the shader which sounds
> susceptible to hangs to me.

That's true. How about skipping the draw call by returning a boolean in 
radv_cmd_buffer_flush_state() ? That way we don't emit any packets if 
the allocation fails.

> 
> On Fri, Sep 8, 2017 at 3:15 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> A return code error is stored in the command buffer and should
>> be returned to the user via EndCommandBuffer().
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index a79b513ffc..07d9be9bf4 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -1578,7 +1578,7 @@ static void radv_emit_primitive_reset_state(struct radv_cmd_buffer *cmd_buffer,
>>          }
>>   }
>>
>> -static void
>> +static bool
>>   radv_cmd_buffer_update_vertex_descriptors(struct radv_cmd_buffer *cmd_buffer)
>>   {
>>          struct radv_device *device = cmd_buffer->device;
>> @@ -1594,8 +1594,9 @@ radv_cmd_buffer_update_vertex_descriptors(struct radv_cmd_buffer *cmd_buffer)
>>                  uint64_t va;
>>
>>                  /* allocate some descriptor state for vertex buffers */
>> -               radv_cmd_buffer_upload_alloc(cmd_buffer, count * 16, 256,
>> -                                            &vb_offset, &vb_ptr);
>> +               if (!radv_cmd_buffer_upload_alloc(cmd_buffer, count * 16, 256,
>> +                                                 &vb_offset, &vb_ptr))
>> +                       return false;
>>
>>                  for (i = 0; i < count; i++) {
>>                          uint32_t *desc = &((uint32_t *)vb_ptr)[i * 4];
>> @@ -1625,6 +1626,8 @@ radv_cmd_buffer_update_vertex_descriptors(struct radv_cmd_buffer *cmd_buffer)
>>                                             AC_UD_VS_VERTEX_BUFFERS, va);
>>          }
>>          cmd_buffer->state.vb_dirty = false;
>> +
>> +       return true;
>>   }
>>
>>   static void
>> @@ -1639,7 +1642,8 @@ radv_cmd_buffer_flush_state(struct radv_cmd_buffer *cmd_buffer,
>>          MAYBE_UNUSED unsigned cdw_max = radeon_check_space(cmd_buffer->device->ws,
>>                                                             cmd_buffer->cs, 4096);
>>
>> -       radv_cmd_buffer_update_vertex_descriptors(cmd_buffer);
>> +       if (!radv_cmd_buffer_update_vertex_descriptors(cmd_buffer))
>> +               return;
>>
>>          if (cmd_buffer->state.dirty & RADV_CMD_DIRTY_PIPELINE)
>>                  radv_emit_graphics_pipeline(cmd_buffer, pipeline);
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev