[Mesa-dev] [PATCH 6/8] [v2] i965: Add Gen9 surface state decoding
Ben Widawsky
ben at bwidawsk.net
Mon May 18 10:39:58 PDT 2015
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 <ben at bwidawsk.net>
> > Cc: Topi Pohjolainen <topi.pohjolainen at 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)?
> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>
> > + } else {
> > + static const char *surface_aux_mode[] = { "AUX_NONE", "AUX_MCS",
> > + "AUX_APPEND", "AUX_HIZ",
> > + "RSVD", "RSVD"};
> > + aux_str = ralloc_asprintf(NULL, "%s", 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)),
> > @@ -288,7 +295,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[6] & 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,
> > (surf[3] & INTEL_MASK(17, 0)) + 1);
> > @@ -303,14 +310,21 @@ static void dump_gen8_surface_state(struct brw_context *brw, uint32_t offset)
> > batch_out(brw, name, offset, 6, "AUX pitch: %d qpitch: %d\n",
> > 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' : '-');
> > + }
> >
> > for (int i = 8; i < 12; i++)
> > batch_out(brw, name, offset, i, "0x%08x\n", surf[i]);
> > +
> > + ralloc_free((void *)aux_str);
> > }
> >
> > static void
> > --
> > 2.4.1
> >
More information about the mesa-dev
mailing list