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

[Chad Versace]

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.

> +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