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

[Jason Ekstrand]

On Thu, Jul 14, 2016 at 3:41 PM, Chad Versace <chad.versace at intel.com>
wrote:

> 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.
>

That *is* in another patch :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160714/c6333f84/attachment-0001.html>