[Mesa-dev] [PATCH v2 22/24] anv: handle errors while allocating new binding table blocks

[Pohjolainen, Topi]

On Wed, Mar 15, 2017 at 09:48:32AM +0100, Iago Toral wrote:
> On Wed, 2017-03-15 at 10:39 +0200, Pohjolainen, Topi wrote:
> > On Fri, Mar 10, 2017 at 01:38:35PM +0100, Iago Toral Quiroga wrote:
> > > 
> > > Also, we had a couple of instances in flush_descriptor_sets() were
> > > we were returning a VkResult directly upon error, but the return
> > > value of this function is not a VkResult but a uint32_t dirty mask,
> > > so simply return 0 in these cases which reduces the amount of
> > > work the driver will do after the error has been raised.
> > > ---
> > >  src/intel/vulkan/anv_batch_chain.c |  8 ++++++--
> > >  src/intel/vulkan/genX_cmd_buffer.c | 11 +++++++----
> > >  2 files changed, 13 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > b/src/intel/vulkan/anv_batch_chain.c
> > > index 0dc781c..d01d9d7 100644
> > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > @@ -661,8 +661,10 @@ anv_cmd_buffer_new_binding_table_block(struct
> > > anv_cmd_buffer *cmd_buffer)
> > >         &cmd_buffer->device->surface_state_block_pool;
> > >  
> > >     int32_t *offset = u_vector_add(&cmd_buffer->bt_blocks);
> > > -   if (offset == NULL)
> > > +   if (offset == NULL) {
> > > +      anv_batch_set_error(&cmd_buffer->batch,
> > > VK_ERROR_OUT_OF_HOST_MEMORY);
> > >        return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> > > +   }
> > >  
> > >     *offset = anv_block_pool_alloc_back(block_pool);
> > >     cmd_buffer->bt_next = 0;
> > > @@ -715,7 +717,9 @@ anv_cmd_buffer_init_batch_bo_chain(struct
> > > anv_cmd_buffer *cmd_buffer)
> > >        goto fail_bt_blocks;
> > >     cmd_buffer->last_ss_pool_center = 0;
> > >  
> > > -   anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > > +   result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > > +   if (result != VK_SUCCESS)
> > > +      goto fail_bt_blocks;
> > >  
> > >     return VK_SUCCESS;
> > >  
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index 510355e..3069297 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -1348,7 +1348,8 @@ flush_descriptor_sets(struct anv_cmd_buffer
> > > *cmd_buffer)
> > >        assert(result == VK_ERROR_OUT_OF_DEVICE_MEMORY);
> > >  
> > >        result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > > -      assert(result == VK_SUCCESS);
> > > +      if (result != VK_SUCCESS)
> > > +         return 0;
> > >  
> > >        /* Re-emit state base addresses so we get the new surface
> > > state base
> > >         * address before we start emitting binding tables etc.
> > > @@ -1360,11 +1361,11 @@ flush_descriptor_sets(struct anv_cmd_buffer
> > > *cmd_buffer)
> > >        anv_foreach_stage(s, dirty) {
> > >           result = emit_samplers(cmd_buffer, s, &cmd_buffer-
> > > >state.samplers[s]);
> > >           if (result != VK_SUCCESS)
> > > -            return result;
> > > +            return 0;
> > >           result = emit_binding_table(cmd_buffer, s,
> > >                                       &cmd_buffer-
> > > >state.binding_tables[s]);
> > >           if (result != VK_SUCCESS)
> > > -            return result;
> > > +            return 0;
> > >        }
> > >     }
> > >  
> > > @@ -1819,8 +1820,10 @@ flush_compute_descriptor_set(struct
> > > anv_cmd_buffer *cmd_buffer)
> > >     result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE,
> > > &surfaces);
> > >     if (result != VK_SUCCESS) {
> > >        assert(result == VK_ERROR_OUT_OF_DEVICE_MEMORY);
> > > +
> > Intentional or just left over?
> 
> Intentional. We also have an empty line after that same assert in
> flush_descriptor_sets() and I liked that better but I can remove it if
> you think it doesn't belong here.

Fine by me, just checking.