<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Jul 14, 2016 at 3:41 PM, Chad Versace <<a href="mailto:chad.versace@intel.com" target="_blank">chad.versace@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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 <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > > > --- > > > 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." </div></div>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. </blockquote></div> </div><div class="gmail_extra">That *is* in another patch :) </div></div>