[Mesa-dev] [PATCH 3/7] [v2] i965/skl: skip fast clears for certain surface formats

[Ben Widawsky]

On Fri, Nov 13, 2015 at 12:22:47PM -0800, Chad Versace wrote:
> On Wed 11 Nov 2015, Ben Widawsky wrote:
> > Some of the information originally in this commit message is now in the patch
> > before this.
> > 
> > SKL adds compressible render targets and as a result mutates some of the
> > programming for fast clears and resolves. There is a new internal surface type
> > called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary surface is
> > a CCS (Color Control Surface) with compression disabled or an MCS with
> > compression enabled, depending on number of multisamples. MCS (Multisample
> > Control Surface) is a special type of CCS."
> > 
> > The formats which are supported are defined in the table titled "Render Target
> > Surface Types [SKL+]". There is no PRM yet to reference. The previously
> > implemented helper function already does the right thing provided the table is
> > correct.
> > 
> > v2: Use better English in commit message (Matt)
> > s/compressable/compressible/ (Matt)
> > Don't compare bools to true (Matt)
> > Use the helper function and don't increase the context size - this is mostly
> > implemented in the patch just before this (Chad, Neil)
> > Remove an "invalid" assert (Chad)
> > Fix assertion to check num_samples > 1, instead of num_samples (Chad)
> > 
> > Cc: Chad Versace <chad.versace at linux.intel.com>
> > Cc: Neil Roberts <neil at linux.intel.com>
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_surface_formats.c | 52 ++++++++++++-------------
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  7 +++-
> >  2 files changed, 31 insertions(+), 28 deletions(-)
> 
> 
> > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > index 6909858..8fe480c 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > @@ -222,6 +222,7 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> >     int surf_index = surf_offset - &brw->wm.base.surf_offset[0];
> >     unsigned tiling_mode, pitch;
> >     const unsigned tr_mode = surface_tiling_resource_mode(mt->tr_mode);
> > +   const uint32_t surf_type = translate_tex_target(target);
> >  
> >     if (mt->format == MESA_FORMAT_S_UINT8) {
> >        tiling_mode = GEN8_SURFACE_TILING_W;
> > @@ -243,11 +244,13 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> >         * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN
> >         *  16 must be used."
> >         */
> > -      if (brw->gen >= 9 || mt->num_samples == 1)
> > +      if (brw->gen >= 9 || mt->num_samples == 1) {
> >           assert(mt->halign == 16);
> > +         assert(mt->num_samples > 1 ||
> > +                brw_losslessly_compressible_format(brw, surf_type));
> > +      }
> 
> Please expand this if-then-assert block to be more straightforward. It's
> very confusing.

That's fine, can you tell me what you want, and maybe give me some intention of
adding an r-b?