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

[Iago Toral]

On Mon, 2017-03-13 at 15:16 +0200, Pohjolainen, Topi wrote:
> 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?

At this point in the series we don't have the error tracking in
anv_batch yet, which patch 14 needs. That support is added in patches 5
and 6, so we can probably move this right after those and squash patch
14 with it if you like that better. I kept both patches separate
because the patches fix different issues in different API calls, one
only needs the return value and can report the error immediately, while
the other one needs the error handler so it can report the error at a
later time.

Iago

> > 
> > 
> > > 
> > > > 
> > > > 
> > > >     }
> > > >  
> > > >     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