On Mon, May 18, 2015 at 10:39:58AM -0700, Ben Widawsky wrote:
> On Mon, May 18, 2015 at 11:02:32AM +0300, Pohjolainen, Topi wrote:
> > On Fri, May 15, 2015 at 10:06:22PM -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.
> > >
> > > v2: Redo the string for the aux buffer type to address compressed
> > > variants.
> > >
> > > Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> > > Cc: Topi Pohjolainen <topi.pohjolai...@intel.com>
> > > ---
> > > src/mesa/drivers/dri/i965/brw_defines.h | 2 ++
> > > src/mesa/drivers/dri/i965/brw_state_dump.c | 44
> > > ++++++++++++++++++++----------
> > > 2 files changed, 31 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> > > b/src/mesa/drivers/dri/i965/brw_defines.h
> > > index 8fd5a49..3a5cb81 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > > @@ -608,6 +608,8 @@
> > > #define GEN8_SURFACE_AUX_MODE_HIZ 3
> > >
> > > /* Surface state DW7 */
> > > +#define GEN9_SURFACE_RT_COMPRESSION_SHIFT 30
> > > +#define GEN9_SURFACE_RT_COMPRESSION_MASK INTEL_MASK(31, 31)
> >
> > Your shift is 30 and mask is 31:31. Bit 31 is memory compression mode while
> > bit 30 is memory compression enable - I guess you meant to use the latter,
> > so the shift is correct while the mask is not?
> >
>
> Oops, thanks.
>
> > > #define GEN7_SURFACE_CLEAR_COLOR_SHIFT 28
> > > #define GEN7_SURFACE_SCS_R_SHIFT 25
> > > #define GEN7_SURFACE_SCS_R_MASK INTEL_MASK(27, 25)
> > > diff --git a/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > b/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > index e0245b0..232d0c1 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_state_dump.c
> > > @@ -66,15 +66,6 @@ static const char *surface_tiling[] = {
> > > "Y-tiled"
> > > };
> > >
> > > -static const char *surface_aux_mode[] = {
> > > - "AUX_NONE",
> > > - "AUX_MCS",
> > > - "AUX_APPEND",
> > > - "AUX_HIZ",
> > > - "RSVD",
> > > - "RSVD"
> > > -};
> > > -
> > > static void
> > > batch_out(struct brw_context *brw, const char *name, uint32_t offset,
> > > int index, char *fmt, ...) PRINTFLIKE(5, 6);
> > > @@ -272,6 +263,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[6] & INTEL_MASK(2, 0);
> > > + const char *aux_str;
> > > +
> > > + if (brw->gen >= 9 && (aux_mode == 1 || aux_mode == 5)) {
> > > + bool msrt = GET_BITS(surf[4], 5, 3) > 0;
> > > + bool compression = GET_FIELD(surf[7], GEN9_SURFACE_RT_COMPRESSION)
> > > == 1;
> > > + aux_str = ralloc_asprintf(NULL, "AUX_CCS_%c (%s,
> > > MULTISAMPLE_COUNT%c1)",
> > > + (aux_mode == 1) ? 'D' : 'E',
> > > + compression ? "Compressed RT" :
> > > "Uncompressed",
> > > + msrt ? '>' : '=');
> >
> > If you shift one (1 << msrt) you get the actual number of samples. Should we
> > print that instead? And may be s/MULTISAMPLE_COUNT/SAMPLES/ to shrink the
> > output a little.
> >
> > Otherwise this looks good to me:
> >
>
> I wanted to match the docs as much as possible in this case, since as you
> mentioned in the other email, the docs are somewhat vague where needed. Are
> you
> okay with me leaving it as is (the multisample info is elsewhere)?
Quite okay :)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
