[Mesa-dev] [PATCH v4 19/34] i965/state: Add a helper for emitting a surface state using isl

[Chad Versace]

On Thu 14 Jul 2016, Chad Versace wrote:
> On Thu 14 Jul 2016, Chad Versace wrote:
> > On Wed 13 Jul 2016, Jason Ekstrand wrote:
> > > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_state.h            |  8 +++
> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 79 ++++++++++++++++++++++++
> > >  2 files changed, 87 insertions(+)
> 
> 
> 
> > > +void
> > > +brw_emit_surface_state(struct brw_context *brw,
> > > +                       struct intel_mipmap_tree *mt,
> > > +                       const struct isl_view *view,
> > > +                       uint32_t mocs, bool for_gather,
> > > +                       uint32_t *surf_offset, int surf_index,
> > > +                       unsigned read_domains, unsigned write_domains)
> > > +{
> > > +   const struct surface_state_info ss_info = surface_state_infos[brw->gen];
> > > +
> > > +   struct isl_surf surf;
> > > +   intel_miptree_get_isl_surf(brw, mt, &surf);
> > > +
> > > +   union isl_color_value clear_color = { .u32 = { 0, 0, 0, 0 } };
> > > +
> > > +   struct isl_surf *aux_surf = NULL, aux_surf_s;
> > > +   uint64_t aux_offset = 0;
> > > +   enum isl_aux_usage aux_usage = ISL_AUX_USAGE_NONE;
> > > +   if (mt->mcs_mt &&
> > > +       ((view->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
> > > +        mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED)) {
> > > +      intel_miptree_get_aux_isl_surf(brw, mt, &aux_surf_s, &aux_usage);
> > > +      aux_surf = &aux_surf_s;
> > > +      assert(mt->mcs_mt->offset == 0);
> > > +      aux_offset = mt->mcs_mt->bo->offset64;
> > > +
> > > +      /* We only really need a clear color if we also have an auxiliary
> > > +       * surfacae.  Without one, it does nothing.
> > > +       */
> > > +      clear_color = intel_miptree_get_isl_clear_color(brw, mt);
> > > +   }
> > 
> > What about the auxiliary hiz surface for depth textures? The function only handles mcs/ccs.
> 
> Wait... I predict your answer will be "Keep reading. It's in another
> patch."

I answered it myself. gen8_update_texture_surface() emits the
SURFACE_STATE aux fields *only* for intel_mipmap_tree::mcs_mt. It
ignores the auxiliary hiz surface, to my surprise.

Well, that investigation corrected my longstanding (and unfounded)
belief that i965 was sampling from depth surfaces without resolving.
That really needs to get fixed, for performance's sake.

Do you mind inserting a FINISHME comment here at the appropriate places
in the patch series, as a reminder to finish hiz sampling? It's no
problem if you don't want to.