[Mesa-dev] [PATCH v2 04/24] anv/cmd_buffer: report errors in vkBeginCommandBuffer()

[Pohjolainen, Topi]

On Mon, Mar 13, 2017 at 11:02:30AM +0100, Iago Toral wrote:
> On Mon, 2017-03-13 at 11:59 +0200, Pohjolainen, Topi wrote:
> > On Fri, Mar 10, 2017 at 01:38:17PM +0100, Iago Toral Quiroga wrote:
> > > 
> > > ---
> > >  src/intel/vulkan/genX_cmd_buffer.c | 15 +++++++++------
> > >  1 file changed, 9 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index 9fdc08b..f74109e 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -381,7 +381,7 @@ transition_depth_buffer(struct anv_cmd_buffer
> > > *cmd_buffer,
> > >  /**
> > >   * Setup anv_cmd_state::attachments for vkCmdBeginRenderPass.
> > >   */
> > > -static void
> > > +static VkResult
> > >  genX(cmd_buffer_setup_attachments)(struct anv_cmd_buffer
> > > *cmd_buffer,
> > >                                     struct anv_render_pass *pass,
> > >                                     const VkRenderPassBeginInfo
> > > *begin)
> > > @@ -393,7 +393,7 @@ genX(cmd_buffer_setup_attachments)(struct
> > > anv_cmd_buffer *cmd_buffer,
> > >  
> > >     if (pass->attachment_count == 0) {
> > >        state->attachments = NULL;
> > > -      return;
> > > +      return VK_SUCCESS;
> > >     }
> > >  
> > >     state->attachments = vk_alloc(&cmd_buffer->pool->alloc,
> > > @@ -402,7 +402,7 @@ genX(cmd_buffer_setup_attachments)(struct
> > > anv_cmd_buffer *cmd_buffer,
> > >                                   8,
> > > VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> > >     if (state->attachments == NULL) {
> > >        /* FIXME: Propagate VK_ERROR_OUT_OF_HOST_MEMORY to
> > > vkEndCommandBuffer */
> > > -      abort();
> > > +      return VK_ERROR_OUT_OF_HOST_MEMORY;
> > Locally this looks fine. I just don't know what are the requirements
> > for the
> > caller. Should we somehow mark the error down in the driver side so
> > that any
> > following calls are rejected/ignored? The FIXME above talks about
> > vkEndCommandBuffer().
> 
> This is addressed in a later patch (#14) that removes the FIXME. 

Right. I suppose it would be cleaner to squash this with patch 14. Is there
something here that is needed before 14?

> 
> > > 
> > >     }
> > >  
> > >     bool need_null_state = false;
> > > @@ -557,6 +557,8 @@ genX(cmd_buffer_setup_attachments)(struct
> > > anv_cmd_buffer *cmd_buffer,
> > >  
> > >        anv_state_flush(cmd_buffer->device, state-
> > > >render_pass_states);
> > >     }
> > > +
> > > +   return VK_SUCCESS;
> > >  }
> > >  
> > >  VkResult
> > > @@ -589,6 +591,7 @@ genX(BeginCommandBuffer)(
> > >  
> > >     genX(cmd_buffer_emit_state_base_address)(cmd_buffer);
> > >  
> > > +   VkResult result = VK_SUCCESS;
> > >     if (cmd_buffer->usage_flags &
> > >         VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT) {
> > >        cmd_buffer->state.pass =
> > > @@ -597,13 +600,13 @@ genX(BeginCommandBuffer)(
> > >           &cmd_buffer->state.pass->subpasses[pBeginInfo-
> > > >pInheritanceInfo->subpass];
> > >        cmd_buffer->state.framebuffer = NULL;
> > >  
> > > -      genX(cmd_buffer_setup_attachments)(cmd_buffer, cmd_buffer-
> > > >state.pass,
> > > -                                         NULL);
> > > +      result = genX(cmd_buffer_setup_attachments)(cmd_buffer,
> > > +                                                  cmd_buffer-
> > > >state.pass, NULL);
> > >  
> > >        cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
> > >     }
> > >  
> > > -   return VK_SUCCESS;
> > > +   return result;
> > >  }
> > >  
> > >  VkResult