[Mesa-dev] [PATCH 10/40] i965/blorp: Prepare render target write for gen8

[Pohjolainen, Topi]

On Mon, Apr 18, 2016 at 10:59:25AM +0300, Pohjolainen, Topi wrote:
> On Mon, Apr 18, 2016 at 12:25:19AM -0700, Kenneth Graunke wrote:
> > On Saturday, April 16, 2016 4:42:38 PM PDT Topi Pohjolainen wrote:
> > > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp    |  2 +-
> > >  src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp |  3 ++-
> > >  src/mesa/drivers/dri/i965/brw_blorp_blit_eu.h   |  2 +-
> > >  src/mesa/drivers/dri/i965/brw_fs.h              |  2 +-
> > >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp  | 11 +++++++----
> > >  5 files changed, 12 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/
> > drivers/dri/i965/brw_blorp_blit.cpp
> > > index 5fd25f1..444ba26 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > > @@ -1786,7 +1786,7 @@ brw_blorp_blit_program::render_target_write()
> > >     /* Now write to the render target and terminate the thread */
> > >     emit_render_target_write(
> > >        mrf_rt_write,
> > > -      base_mrf,
> > > +      brw->gen < 8 ? base_mrf : -1,
> > >        mrf_offset /* msg_length.  TODO: Should be smaller for non-RGBA 
> > formats. */,
> > >        use_header);
> > >  }
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp b/src/mesa/
> > drivers/dri/i965/brw_blorp_blit_eu.cpp
> > > index fd23e23..3a5297c 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> > > @@ -96,13 +96,14 @@ brw_blorp_eu_emitter::emit_texture_lookup(const struct 
> > brw_reg &dst,
> > >  
> > >  void
> > >  brw_blorp_eu_emitter::emit_render_target_write(const struct brw_reg &src0,
> > > -                                               unsigned msg_reg_nr,
> > > +                                               int msg_reg_nr,
> > >                                                 unsigned msg_length,
> > >                                                 bool use_header)
> > >  {
> > >     fs_inst *inst = new (mem_ctx) fs_inst(FS_OPCODE_BLORP_FB_WRITE, 16);
> > >  
> > >     inst->src[0] = src0;
> > > +   inst->sources = 1;
> > >     inst->base_mrf = msg_reg_nr;
> > >     inst->mlen = msg_length;
> > >     inst->header_size = use_header ? 2 : 0;
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.h b/src/mesa/
> > drivers/dri/i965/brw_blorp_blit_eu.h
> > > index 14a8e50..8ed6d6b 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.h
> > > @@ -47,7 +47,7 @@ protected:
> > >                              unsigned msg_length);
> > >  
> > >     void emit_render_target_write(const struct brw_reg &src0,
> > > -                                 unsigned msg_reg_nr,
> > > +                                 int msg_reg_nr,
> > >                                   unsigned msg_length,
> > >                                   bool use_header);
> > >  
> > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/
> > brw_fs.h
> > > index 6afb9b6..775f665 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_fs.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> > > @@ -448,7 +448,7 @@ private:
> > >     void generate_stencil_ref_packing(fs_inst *inst, struct brw_reg dst,
> > >                                       struct brw_reg src);
> > >     void generate_barrier(fs_inst *inst, struct brw_reg src);
> > > -   void generate_blorp_fb_write(fs_inst *inst);
> > > +   void generate_blorp_fb_write(fs_inst *inst, struct brw_reg payload);
> > >     void generate_linterp(fs_inst *inst, struct brw_reg dst,
> > >  			 struct brw_reg *src);
> > >     void generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src,
> > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/
> > drivers/dri/i965/brw_fs_generator.cpp
> > > index 851cccf..0d3c942 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > > @@ -552,12 +552,15 @@ fs_generator::generate_barrier(fs_inst *inst, struct 
> > brw_reg src)
> > >  }
> > >  
> > >  void
> > > -fs_generator::generate_blorp_fb_write(fs_inst *inst)
> > > +fs_generator::generate_blorp_fb_write(fs_inst *inst, struct brw_reg 
> > payload)
> > >  {
> > >     brw_fb_WRITE(p,
> > >                  16 /* dispatch_width */,
> > > -                brw_message_reg(inst->base_mrf),
> > > -                brw_reg_from_fs_reg(inst, &inst->src[0], devinfo->gen),
> > > +                inst->base_mrf >= 0 ?
> > > +                   brw_message_reg(inst->base_mrf) : vec8(payload),
> > 
> > Is there some reason we need vec8() here?
> > 
> > > +                inst->header_size ?
> > > +                   brw_reg_from_fs_reg(inst, &inst->src[0], devinfo->gen) :
> > 
> > The existing code looks a bit fishy...this corresponds to brw_fb_WRITE's
> > implied_header parameter, which is always ignored on Gen6+.  BLORP only
> > works on Gen6+.  So this can't be right...is this code just pointless?
> 
> We have:
> 
> void
> brw_blorp_blit_program::render_target_write()
> {
>    struct brw_reg mrf_rt_write =
>       retype(vec16(brw_message_reg(base_mrf)), key->texture_data_type);
>    int mrf_offset = 0;
> 
>    /* If we may have killed pixels, then we need to send R0 and R1 in a header
>     * so that the render target knows which pixels we killed.
>     */
>    bool use_header = key->use_kill;

If I force "header = false", we get, for example:

/home/tpohjola/work/piglit/bin/ext_framebuffer_multisample-unaligned-blit 4 depth upsample -auto -fbo
Probe color at (49,0)
  Left: 0.000000 1.000000 1.000000 1.000000
  Right: 0.000000 0.000000 0.000000 1.000000
PIGLIT: {"result": "fail" }