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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
