On Tue, Mar 14, 2017 at 12:06:16PM +0100, Iago Toral wrote: > On Tue, 2017-03-14 at 12:01 +0100, Iago Toral wrote: > > On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote: > > > > > > On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga wrote: > > > > > > > > > > > > Growing the reloc list happens through calling > > > > anv_reloc_list_add() > > > > or > > > > anv_reloc_list_append(). Make sure that we call these through > > > > helpers > > > > that check the result and set the batch error status if needed. > > > > > > > > v2: > > > > - Handling the crashes is not good enough, we need to keep > > > > track > > > > of > > > > the error, for that, keep track of the errors in the batch > > > > instead (Jason). > > > > - Make reloc list growth go through helpers so we can have a > > > > central > > > > place where we can do error tracking (Jason). > > > > --- > > > > src/intel/vulkan/anv_batch_chain.c | 29 ++++++++++++++++++++---- > > > > ----- > > > > src/intel/vulkan/genX_blorp_exec.c | 7 +++++-- > > > > src/intel/vulkan/genX_cmd_buffer.c | 25 ++++++++++++++-------- > > > > --- > > > > 3 files changed, 39 insertions(+), 22 deletions(-) > > > > > > > > diff --git a/src/intel/vulkan/anv_batch_chain.c > > > > b/src/intel/vulkan/anv_batch_chain.c > > > > index 3774172..2add5bd 100644 > > > > --- a/src/intel/vulkan/anv_batch_chain.c > > > > +++ b/src/intel/vulkan/anv_batch_chain.c > > > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list > > > > *list, > > > > const uint32_t domain = > > > > target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0; > > > > > > > > - anv_reloc_list_grow(list, alloc, 1); > > > > - /* TODO: Handle failure */ > > > > + VkResult result = anv_reloc_list_grow(list, alloc, 1); > > > > + if (result != VK_SUCCESS) > > > > + return 0; > > > None of the callers of anv_reloc_list_add() or > > > anv_reloc_list_append() are > > > currently interested of the return value. > > and_reloc_list_append() already returns a VkResult. > > > > As for the callers of anv_reloc_list_add(), anv_batch_emit_reloc() > > needs the offset computed by anv_reloc_list_add(), but that seems to > > be > > the only one, so we could do what you suggest below. > > > > > > > > And if they were, they could > > > easily calculate it themselves: both target_bo and delta are > > > inputs. > > > So instead of relying on zero offset indicating error we could > > > simply > > > change the return type to VkResult. Or did I miss something? > > I think this should be fine. > > Well, except that anv_batch_emit_reloc needs to return that offset to > its callers, so even if receives a VkResult and computes the offset > itself, in the case of an error we still need to return an offset, so I > don't think there is much gain in doing this, right?
Oh, I missed that. Yeah, then it doesn't work. I'm just not super happy relying on offset == 0 indicating failure. Does the diff get out hand if provided offset as outgoing argument (and returning VkResult)? _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
