[Mesa-dev] [PATCH 14/14] isl/state: Add support for handling color control surfaces

[Jason Ekstrand]

On Wed, Jul 13, 2016 at 10:36 AM, Chad Versace <chad.versace at intel.com>
wrote:

> On Sat 09 Jul 2016, Jason Ekstrand wrote:
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  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
>
> 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 <chad.versace at intel.com>
>
> As a tangent, isl should probably acquire a function for filling
> 3DSTATE_DEPTH_BUFFER, analagous to isl_genX_surf_fill_state().
>

And STENCIL_BUFFER and maybe HIER_DEPTH as well.  It's all on my TODO list.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160713/dbdced20/attachment.html>