[Mesa-dev] [PATCH v2 03/24] anv/blorp: return early if we failed to create the shader binary

[Iago Toral]

On Tue, 2017-03-14 at 10:12 +0200, Pohjolainen, Topi wrote:
> On Fri, Mar 10, 2017 at 01:38:16PM +0100, Iago Toral Quiroga wrote:
> > 
> > ---
> >  src/intel/vulkan/anv_blorp.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/intel/vulkan/anv_blorp.c
> > b/src/intel/vulkan/anv_blorp.c
> > index d79c5e0..c871f03 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -72,6 +72,9 @@ upload_blorp_shader(struct blorp_context *blorp,
> >                                         key, key_size, kernel,
> > kernel_size,
> >                                         prog_data, prog_data_size,
> > &bind_map);
> >  
> > +   if (!bin)
> > +      return;
> > +
> This is called by brw_blorp_get_blit_kernel() which in turn is called
> from
> try_blorp_blit(). The latter will happily submit batch to the kernel
> where
> there is now no kernel to execute - that will surely lead to gpu
> hang.
> 
> It looks to me we would need to mark down the failure here for anv so
> that
> it knows the blit failed. In addition, brw_blorp_get_blit_kernel()
> should
> tell to try_blorp_blit() that shader upload failed so that
> try_blorp_blit()
> won't send the invalid batch to the gpu.

The problem here is that as far as I understand, all this happens in
code that doesn't know if it is being run inside anv or i965 and that
does not have access to an anv batch object.

One solution would be to make all the functions in between these and
the entry points (blorp_copy and blorp_blit as far as I can tell)
return a bool to indicate failure so that when we call this from anv we
can register the error in the corresponding batch.

> Jason: any thoughts?
> 
> > 
> >     /* The cache already has a reference and it's not going
> > anywhere so there
> >      * is no need to hold a second reference.
> >      */