[Mesa-dev] [PATCH v2 07/34] isl: Add surface formats for color compression surfaces

[Jason Ekstrand]

On Wed, Jun 29, 2016 at 3:03 PM, Chad Versace <chad.versace at intel.com>
wrote:

> On Thu 23 Jun 2016, Jason Ekstrand wrote:
> > ---
> >  src/intel/isl/isl.h                 | 21 +++++++++++++++++++++
> >  src/intel/isl/isl_format_layout.csv | 14 ++++++++++++++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > index f2128d8..4aedb11 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -349,6 +349,27 @@ enum isl_format {
> >     ISL_FORMAT_ASTC_LDR_2D_12X10_FLT16 =                        638,
> >     ISL_FORMAT_ASTC_LDR_2D_12X12_FLT16 =                        639,
> >
> > +   /* The formats that follow are internal to ISL and as such don't
> have an
> > +    * explicit number.  We'll just let the C compiler assign it for
> us.  Any
> > +    * actual hardware formats *must* come before these in the list.
> > +    */
> > +
> > +   /* Formats for color compression surfaces */
> > +   ISL_FORMAT_HIZ,
> > +   ISL_FORMAT_MCS_2X,
> > +   ISL_FORMAT_MCS_4X,
> > +   ISL_FORMAT_MCS_8X,
> > +   ISL_FORMAT_MCS_16X,
> > +   ISL_FORMAT_GEN7_CCS_32BPP_X,
> > +   ISL_FORMAT_GEN7_CCS_64BPP_X,
> > +   ISL_FORMAT_GEN7_CCS_128BPP_X,
> > +   ISL_FORMAT_GEN7_CCS_32BPP_Y,
> > +   ISL_FORMAT_GEN7_CCS_64BPP_Y,
> > +   ISL_FORMAT_GEN7_CCS_128BPP_Y,
> > +   ISL_FORMAT_GEN9_CCS_32BPP,
> > +   ISL_FORMAT_GEN9_CCS_64BPP,
> > +   ISL_FORMAT_GEN9_CCS_128BPP,
> > +
>
> The names look good to me.
>
> >     /* Hardware doesn't understand this out-of-band value */
> >     ISL_FORMAT_UNSUPPORTED =                             UINT16_MAX,
> >  };
> > diff --git a/src/intel/isl/isl_format_layout.csv
> b/src/intel/isl/isl_format_layout.csv
> > index f90fbe0..bf86b05 100644
> > --- a/src/intel/isl/isl_format_layout.csv
> > +++ b/src/intel/isl/isl_format_layout.csv
> > @@ -314,3 +314,17 @@ ASTC_LDR_2D_10X8_FLT16      , 128, 10,  8,  1,
> sf16, sf16, sf16, sf16,     ,
> >  ASTC_LDR_2D_10X10_FLT16     , 128, 10, 10,  1, sf16, sf16, sf16, sf16,
>    ,     ,    , linear,  astc
> >  ASTC_LDR_2D_12X10_FLT16     , 128, 12, 10,  1, sf16, sf16, sf16, sf16,
>    ,     ,    , linear,  astc
> >  ASTC_LDR_2D_12X12_FLT16     , 128, 12, 12,  1, sf16, sf16, sf16, sf16,
>    ,     ,    , linear,  astc
> > +HIZ                         , 128,  8,  4,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
>
> HIZ looks correct. Though, as we discussed, maybe 16x8 is the better
> choice. But 8x4 is good for now.
>

As mentioned on IRC, I'm pretty sure we need 8x4 with ISL_TILING_HIZ.
Also, Nanley pointed out that we need different formats for multisampling
on gen8 and prior.  MSAA on gen9 should just be ISL_FORMAT_HIZ.  We can
talk about it more when you're actually in the office if that works better.


> > +MCS_2X                      ,   8,  1,  1,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +MCS_4X                      ,   8,  1,  1,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +MCS_8X                      ,  32,  1,  1,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +MCS_16X                     ,  64,  1,  1,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
>
> MCS looks correct.
>
> > +GEN7_CCS_32BPP_X            ,   8,  8,  4,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN7_CCS_64BPP_X            ,   8,  4,  4,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN7_CCS_128BPP_X           ,   8,  2,  4,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN7_CCS_32BPP_Y            ,   8, 16,  2,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN7_CCS_64BPP_Y            ,   8,  8,  2,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN7_CCS_128BPP_Y           ,   8,  4,  2,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN9_CCS_32BPP              ,  16, 16,  2,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN9_CCS_64BPP              ,  16,  8,  2,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
> > +GEN9_CCS_128BPP             ,  16,  4,  2,  1,     ,     ,     ,     ,
>    ,     ,    ,       ,
>
> The gen9 css layouts look very wrong.
>
> First, the compression ratio looks wrong. The Skylake PRM, Volume 12
> "Display", p159 says:
>
>   The Color Control Surface (CCS) contains the compression status of the
>   cache-line pairs. The compression state of the cache-line pair is
>   specified by 2 bits in the CCS.  Each CCS cache-line represents an
>   area on the main surface of 16 x16 sets of 128 byte Y-tiled
>   cache-line-pairs. CCS is always Y tiled.
>
> So, the memory compression ratio is actually
>
>   Main / CCS = 2cl / 2b = 128B / 2b = 1024b / 2b = 512
>
> but the patch claims
>
>   Main / CCS = 1024b / 16b = 64
>
> Second, how did you determine that the block layout was Nx2? I highly
> expected that the CCS compression block would comprise an entire
> CCS cacheline, and therefore the the block layout would be larger.
>
> I expected the CSV to look like below. PLEASE take my expectation with
> a hunk of salt. I'm not confident that the CCS block is a full CCS
> cacheline. However, the below table does preserve the memory ratio
> Main/CCS = 512.
>
>                         bpb,  bw,  bh,  bd
>         GEN9_CCS_32BPP  512, 128,  64,   1
>         GEN9_CCS_64BPP  512,  64,  64,   1
>         GEN9_CCS_128BPP 512,  32,  64,   1
>

I'll just use that and we'll fix it when we get around to actually doing
CCS.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160629/162e7711/attachment.html>