[Mesa-dev] [v2 14/19] i965/gen9: Prepare surface state setup for lossless compression

[Pohjolainen, Topi]

On Mon, Feb 15, 2016 at 12:55:28PM -0800, Ben Widawsky wrote:
> On Thu, Feb 11, 2016 at 08:34:07PM +0200, Topi Pohjolainen wrote:
> > v2 (Ben): Use combination of msaa_layout and number of samples
> >           instead of introducing explicit type for lossless
> >           compression (intel_miptree_is_lossless_compressed()).
> > 
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_defines.h        | 1 +
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c | 6 ++++++
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> > index b1fa559..f903335 100644
> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > @@ -656,6 +656,7 @@
> >  #define GEN8_SURFACE_AUX_MODE_MCS               1
> >  #define GEN8_SURFACE_AUX_MODE_APPEND            2
> >  #define GEN8_SURFACE_AUX_MODE_HIZ               3
> > +#define GEN9_SURFACE_AUX_MODE_CCS_E             5
> >  
> >  /* Surface state DW7 */
> >  #define GEN9_SURFACE_RT_COMPRESSION_SHIFT       30
> > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > index 0a52815..e1a37d8 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > @@ -216,6 +216,9 @@ gen8_get_aux_mode(const struct brw_context *brw,
> >     if (brw->gen >= 9 || mt->num_samples == 1)
> >        assert(mt->halign == 16);
> >  
> > +   if (intel_miptree_is_lossless_compressed(brw, mt))
> > +      return GEN9_SURFACE_AUX_MODE_CCS_E;
> > +
> >     return GEN8_SURFACE_AUX_MODE_MCS;
> >  }
> >  
> > @@ -484,6 +487,9 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
> >     struct intel_mipmap_tree *aux_mt = mt->mcs_mt;
> >     const uint32_t aux_mode = gen8_get_aux_mode(brw, mt, surf_type);
> >  
> > +   if (aux_mode == GEN9_SURFACE_AUX_MODE_CCS_E)
> > +      mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_UNRESOLVED;
> > +
> 
> I am somewhat undecided about whether or not this should be here. On the one
> hand, this is a gen specific thing (you rendered to a losslessy compressed
> buffer, which is only a gen9+ thing). On the other hand, we handle all similar
> stuff in the common meta code, and modifying fast_clear_state here seems a bit
> unclean.
> 
> I'm not opposed to doing this as long as you've considered my potential
> objection.

I wasn't that happy putting this here either. Main render target loop in
brw_wm_surface_state.c::brw_update_renderbuffer_surfaces() is even more
gen-agnostic, and therefore I didn't want to put this there either.

Now that you brought this up, I think the correct place would be
brw_postdraw_set_buffers_need_resolve() called in the end of
brw_try_draw_prims(). What do you think?