<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Jul 13, 2016 at 10:36 AM, 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 Sat 09 Jul 2016, Jason Ekstrand wrote: > Reviewed-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > --- > src/intel/isl/isl.h | 7 +++++++ > src/intel/isl/isl_surface_state.c | 43 +++++++++++++++++++++++++++++++++++++-- > 2 files changed, 48 insertions(+), 2 deletions(-) > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > index dfc00d5..8716cbd 100644 > --- a/src/intel/isl/isl.h > +++ b/src/intel/isl/isl.h > @@ -899,6 +899,13 @@ struct isl_surf_fill_state_info { > uint32_t mocs; > > /** > + * The auxilary surface or NULL if no auxilary surface is to be used. > + */ > + const struct isl_surf *aux_surf; > + enum isl_aux_usage aux_usage; > + uint64_t aux_address; > + > + /** > * The clear color for this surface > * > * Valid values depend on hardware generation. > diff --git a/src/intel/isl/isl_surface_state.c b/src/intel/isl/isl_surface_state.c > index c65126d..9ef0042 100644 > --- a/src/intel/isl/isl_surface_state.c > +++ b/src/intel/isl/isl_surface_state.c > @@ -84,6 +84,23 @@ static const uint32_t isl_to_gen_multisample_layout[] = { > [ISL_MSAA_LAYOUT_ARRAY] = MSFMT_MSS, > }; > > +#if GEN_GEN >= 9 > +static const uint32_t isl_to_gen_aux_mode[] = { > + [ISL_AUX_USAGE_NONE] = AUX_NONE, > + [ISL_AUX_USAGE_HIZ] = AUX_HIZ, > + [ISL_AUX_USAGE_MCS] = AUX_CCS_D, > + [ISL_AUX_USAGE_CCS_D] = AUX_CCS_D, > + [ISL_AUX_USAGE_CCS_E] = AUX_CCS_E, > +}; > +#elif GEN_GEN >= 8 > +static const uint32_t isl_to_gen_aux_mode[] = { > + [ISL_AUX_USAGE_NONE] = AUX_NONE, > + [ISL_AUX_USAGE_HIZ] = AUX_HIZ, > + [ISL_AUX_USAGE_MCS] = AUX_MCS, > + [ISL_AUX_USAGE_CCS_D] = AUX_MCS, > +}; > +#endif </div></div>Argh! This is where your "table lookup with validation" idea would be very useful. It would protect against isl_to_gen_aux_mode[ISL_AUX_USAGE_CCS_E] on gen8. Oh well. That improvement can happen later. > + > static uint8_t > get_surftype(enum isl_surf_dim dim, isl_surf_usage_flags_t usage) > { > @@ -353,10 +370,32 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, void *state, > s.SurfaceBaseAddress = info->address; > s.MOCS = info->mocs; > > +#if GEN_GEN >= 7 > + if (info->aux_surf && info->aux_usage != ISL_AUX_USAGE_NONE) { > + struct isl_tile_info tile_info; > + isl_surf_get_tile_info(dev, info->aux_surf, &tile_info); > + uint32_t pitch_in_tiles = > + info->aux_surf->row_pitch / tile_info.phys_extent_B.width; > + > #if GEN_GEN >= 8 > - s.AuxiliarySurfaceMode = AUX_NONE; > + assert(GEN_GEN >= 9 || info->aux_usage != ISL_AUX_USAGE_CCS_E); > + s.AuxiliarySurfacePitch = pitch_in_tiles - 1; > + /* Auxiliary surfaces in ISL have compressed formats but the hardware > + * doesn't expect our definition of the compression, it expects qpitch > + * in units of samples the main surface. grammar: ^^^^^^^^^^^^^^^^^ on the main surface > + */ > + s.AuxiliarySurfaceQPitch = > + isl_surf_get_array_pitch_sa_rows(info->aux_surf); > + s.AuxiliarySurfaceBaseAddress = info->aux_address; > + s.AuxiliarySurfaceMode = isl_to_gen_aux_mode[info->aux_usage]; > #else > - s.MCSEnable = false; > + assert(info->aux_usage == ISL_AUX_USAGE_MCS || > + info->aux_usage == ISL_AUX_USAGE_CCS_D); > + s.MCSBaseAddress = info->aux_address, > + s.MCSSurfacePitch = pitch_in_tiles - 1; > + s.MCSEnable = true; > +#endif > + } > #endif I really like how this patch turned out. ISL's auxiliary surface support is fairly straightforward, especially the SURFACE_STATE parts. Fix the little comment mistake, and Reviewed-by: Chad Versace <<a href="mailto:chad.versace@intel.com">chad.versace@intel.com</a>> As a tangent, isl should probably acquire a function for filling 3DSTATE_DEPTH_BUFFER, analagous to isl_genX_surf_fill_state(). </blockquote></div> </div><div class="gmail_extra">And STENCIL_BUFFER and maybe HIER_DEPTH as well. It's all on my TODO list. </div></div>