<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 8, 2017 at 3:09 AM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, 2017-03-07 at 08:44 -0800, Jason Ekstrand wrote:<br>
> On Mon, Mar 6, 2017 at 11:15 PM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.co">itoral@igalia.co</a><br>
> m> wrote:<br>
> > ---<br>
> >  src/intel/vulkan/anv_batch_<wbr>chain.c | 10 ++++++----<br>
> >  1 file changed, 6 insertions(+), 4 deletions(-)<br>
> ><br>
> > diff --git a/src/intel/vulkan/anv_batch_<wbr>chain.c<br>
> > b/src/intel/vulkan/anv_batch_<wbr>chain.c<br>
> > index 3f6039e..4cbfb4d 100644<br>
> > --- a/src/intel/vulkan/anv_batch_<wbr>chain.c<br>
> > +++ b/src/intel/vulkan/anv_batch_<wbr>chain.c<br>
> > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list *list,<br>
> >     const uint32_t domain =<br>
> >        target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;<br>
> ><br>
> > -   anv_reloc_list_grow(list, alloc, 1);<br>
> > -   /* TODO: Handle failure */<br>
> > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);<br>
> > +   if (result != VK_SUCCESS)<br>
> > +      return 0;<br>
> This prevents crashes, yes, but it doesn't flag the failure anywhere<br>
> so it gets lost and you now have  command buffer that references<br>
> something but doesn't have a relocaiton.<br>
><br>
> I think the right thing to do would be to add an<br>
> anv_batch_emit_surface_reloc helper and then we have a more central<br>
> place to handle it.<br>
<br>
</div></div>Actually, we already have anv_batch_emit_reloc() which simply calls<br>
anv_reloc_list_add, so I think we can use that.<span class=""><br></span></blockquote><div><br></div><div>No, that emits a relocation on the batch.  Emitting a relocation on the surface state BO is a bit different.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> Would it make sense to put the VkResult status in the anv_batch? <br>
> This can get called from either batch building or pipeline building.<br>
<br>
</span>Yes, I think this is a good idea since the batch associated with the<br>
command buffer is the only thing we have available during batch<br>
emissions.<br></blockquote><div><br></div><div>Yeah, if we did that, we could handle OOM even in things such as anv_batch_emit()<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> ><br>
> >     /* XXX: Can we use I915_EXEC_HANDLE_LUT? */<br>
> >     index = list->num_relocs++;<br>
> > @@ -174,8 +175,9 @@ anv_reloc_list_append(struct anv_reloc_list<br>
> > *list,<br>
> >                        const VkAllocationCallbacks *alloc,<br>
> >                        struct anv_reloc_list *other, uint32_t<br>
> > offset)<br>
> >  {<br>
> > -   anv_reloc_list_grow(list, alloc, other->num_relocs);<br>
> > -   /* TODO: Handle failure */<br>
> > +   VkResult result = anv_reloc_list_grow(list, alloc, other-<br>
> > >num_relocs);<br>
> > +   if (result != VK_SUCCESS)<br>
> > +      return;<br>
> Similarly, this one should handle failure.<br>
>  <br>
> ><br>
> >     memcpy(&list->relocs[list-><wbr>num_relocs], &other->relocs[0],<br>
> >            other->num_relocs * sizeof(other->relocs[0]));<br>
> > --<br>
> > 2.7.4<br>
> ><br>
> > ______________________________<wbr>_________________<br>
> > mesa-dev mailing list<br>
> > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
> ><br>
</div></div></blockquote></div><br></div></div>