[Mesa-dev] [PATCH 5/7] i965: Add Gen9 surface state decoding

[Ben Widawsky]

On Fri, May 15, 2015 at 08:22:29PM -0700, Ben Widawsky wrote:
> On Fri, Apr 24, 2015 at 09:05:44PM +0300, Pohjolainen, Topi wrote:
> > On Thu, Apr 23, 2015 at 04:50:02PM -0700, Ben Widawsky wrote:
> > > Gen9 surface state is very similar to the previous generation. The important
> > > changes here are aux mode, and the way clear colors work.
> > > 
> > > NOTE: There are some things intentionally left out of this decoding.
> > > 
> > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_state_dump.c | 36 ++++++++++++++++++++++++------
> > >  1 file changed, 29 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_state_dump.c b/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > index 642bdc8..60e6b05 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > @@ -491,6 +491,22 @@ static void dump_gen8_surface_state(struct brw_context *brw, uint32_t offset)
> > >  {
> > >     const char *name = "SURF";
> > >     uint32_t *surf = brw->batch.bo->virtual + offset;
> > > +   int aux_mode = surf[7] & INTEL_MASK(2, 0);
> > 
> > Same question as in the previous patch, surf[6] ?
> > 
> > > +   const char *aux_str;
> > > +
> > > +   if (brw->gen >= 9) {
> > > +      bool msrt = GET_BITS(surf[4], 5, 3) > 0;
> > > +      switch (aux_mode) {
> > > +      case 5:
> > > +         aux_str = msrt ? "AUX_CCS_E [MCS]" : "AUX_CCS_D [MCS]";
> > 
> > The way I read the spec, I would have written this as:
> > 
> >             aux_str = msrt ? "AUX_CCS_E [MCS]" : "AUX_CCS_E [CCS]";
> > 
> > Ande the one below:
> > 
> >             aux_str = msrt ? "AUX_CCS_D [MCS]" : "AUX_CCS_D [CCS]";
> > 
> > But maybe I'm misreading.
> > 
> 
> You are right, sorta... First, this was definitely a copy and paste error. I
> believe there are actually 3 things to program here:
> 
> if (render_compression || something else)?
> 	CCS
> else if (msrt)
> 	MCS/MSAA
> else
> 	MCS/1x (fast clear)
> 

Okay, I suck today with quickly changing my mind after sending a mail... So, I
was regurgitating what Chad told me earlier today, but I now think he misspoke.
There is no more MCS/1x surface, it's just the CCS with compression disabled. So
the 3 different options remain, but the wording is a bit different, maybe this:

if (render_compression || something else)?
	CCS
else if (msrt)
 	MCS (MSAA)
 else
 	CCS (fast clear)

> What do you think?
> 
> > > +         break;
> > > +      case 1:
> > > +         aux_str = msrt ? "AUX_CCS_E [CCS]" : "AUX_CCS_D [MCS]";
> > 
> > Missing break?
> > 
> 
> Yep. You found both bugs I found... should have read your mail first :D
> 
> > > +      default:
> > > +         aux_str = surface_aux_mode[aux_mode];
> > > +      }
> > > +   } else
> > > +      aux_str = surface_aux_mode[aux_mode];
> > >  
> > >     batch_out(brw, "SURF'", offset, 0, "%s %s %s VALIGN%d HALIGN%d %s\n",
> > >               get_965_surfacetype(GET_FIELD(surf[0], BRW_SURFACE_TYPE)),
> > > @@ -509,7 +525,7 @@ static void dump_gen8_surface_state(struct brw_context *brw, uint32_t offset)
> > >     batch_out(brw, name, offset, 2, "%dx%d [%s]\n",
> > >               GET_FIELD(surf[2], GEN7_SURFACE_WIDTH) + 1,
> > >               GET_FIELD(surf[2], GEN7_SURFACE_HEIGHT) + 1,
> > > -             surface_aux_mode[surf[7] & INTEL_MASK(2, 0)]
> > > +             aux_str
> > >              );
> > >     batch_out(brw, name, offset, 3, "%d slices (depth), pitch: %d\n",
> > >               GET_FIELD(surf[3], BRW_SURFACE_DEPTH) + 1,
> > > @@ -529,12 +545,18 @@ static void dump_gen8_surface_state(struct brw_context *brw, uint32_t offset)
> > >               GET_FIELD(surf[6], GEN8_SURFACE_AUX_QPITCH) << 2,
> > >               GET_FIELD(surf[6], GEN8_SURFACE_AUX_PITCH) << 2
> > >              );
> > > -   batch_out(brw, name, offset, 7, "Clear color: %c%c%c%c\n",
> > > -             GET_BITS(surf[7], 31, 31) ? 'R' : '-',
> > > -             GET_BITS(surf[7], 30, 30) ? 'G' : '-',
> > > -             GET_BITS(surf[7], 29, 29) ? 'B' : '-',
> > > -             GET_BITS(surf[7], 28, 28) ? 'A' : '-'
> > > -            );
> > > +   if (brw->gen >= 9) {
> > > +      batch_out(brw, name, offset, 7, "Clear color: R(%x)G(%x)B(%x)A(%x)\n",
> > > +                surf[12], surf[13], surf[14], surf[15]
> > > +               );
> > > +   } else {
> > > +      batch_out(brw, name, offset, 7, "Clear color: %c%c%c%c\n",
> > > +                GET_BITS(surf[7], 31, 31) ? 'R' : '-',
> > > +                GET_BITS(surf[7], 30, 30) ? 'G' : '-',
> > > +                GET_BITS(surf[7], 29, 29) ? 'B' : '-',
> > > +                GET_BITS(surf[7], 28, 28) ? 'A' : '-'
> > > +               );
> > > +   }
> > >  }
> > >  
> > >  static void
> > > -- 
> > > 2.3.6
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev