<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Jun 29, 2016 at 3:03 PM, Chad Versace <<a href="mailto:chad.versace@intel.com" target="_blank">chad.versace@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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, > + </div></div>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. </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > +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 </blockquote></div> </div><div class="gmail_extra">I'll just use that and we'll fix it when we get around to actually doing CCS. </div></div>