<div dir="ltr"><div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Jul 13, 2016 at 10:16 AM, Chad Versace <<a href="mailto:chad.versace@intel.com" target="_blank">chad.versace@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat 09 Jul 2016, Jason Ekstrand wrote: > --- > src/intel/isl/isl.c | 32 ++++++++++++++++++++++++++++++++ > src/intel/isl/isl.h | 14 ++++++++++++++ > src/intel/isl/isl_format_layout.csv | 9 +++++++++ > src/intel/isl/isl_gen7.c | 7 +++++++ > src/intel/isl/isl_gen8.c | 13 +++++++++++++ > src/intel/isl/isl_gen9.c | 12 ++++++++++++ > 6 files changed, 87 insertions(+) > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > index 9ccdea2..da9d3d4 100644 > --- a/src/intel/isl/isl.c > +++ b/src/intel/isl/isl.c > @@ -177,6 +177,15 @@ isl_tiling_get_info(const struct isl_device *dev, > phys_B = isl_extent2d(128, 32); > break; > > + case ISL_TILING_CCS: > + /* CCS surfaces are required to have one of the GENX_CCS_* formats which > + * have a block size of 1 or 2 bits per block. I'd like to see a comment here, similar to case ISL_TILING_HIZ, that states ISL_TILING_CCS is related to Y-tiling. Such a comment would immediately explain the magic numbers below. </blockquote><div> </div><div>How about this: /* CCS surfaces are required to have one of the GENX_CCS_* formats which * have a block size of 1 or 2 bits per block and each CCS element * corresponds to one cache-line pair in the main surface. From the Sky * Lake PRM Vol. 12 in the section on planes: * * "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 16x16 sets of 128 byte * Y-tiled cache-line-pairs. CCS is always Y tiled." * * The CCS being Y-tiled implies that it's an 8x8 grid of cache-lines. * Since each cache line corresponds to a 16x16 set of cache-line pairs, * that yields total tile area of 128x128 cache-line pairs or CCS * elements. On older hardware, each CCS element is 1 bit and the tile * is 128x256 elements. */ </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > + */ > + assert(format_bpb == 1 || format_bpb == 2); > + logical_el = isl_extent2d(128, 256 / format_bpb); > + phys_B = isl_extent2d(128, 32); > + break; > + > default: > unreachable("not reached"); > } /* end switch */ > @@ -844,6 +854,28 @@ isl_calc_array_pitch_el_rows(const struct isl_device *dev, > assert(pitch_sa_rows % fmtl->bh == 0); > uint32_t pitch_el_rows = pitch_sa_rows / fmtl->bh; > > + if (ISL_DEV_GEN(dev) >= 9 && fmtl->txc == ISL_TXC_CCS) { > + /* > + * From the Sky Lake PRM Vol 7, "MCS Buffer for Render Target(s)" (p. 632): > + * > + * "Mip-mapped and arrayed surfaces are supported with MCS buffer > + * layout with these alignments in the RT space: Horizontal > + * Alignment = 128 and Vertical Alignment = 64." > + * > + * From the Sky Lake PRM Vol. 2d, "RENDER_SURFACE_STATE" (p. 435): > + * > + * "For non-multisampled render target's CCS auxiliary surface, > + * QPitch must be computed with Horizontal Alignment = 128 and > + * Surface Vertical Alignment = 256. These alignments are only for > + * CCS buffer and not for associated render target." > + * > + * The alignment in the second PRM quotation only applies to the qpitch > + * so it needs to be applied here. > + */ I have a comment about the first restriction (HorizontalAlignment=64 and VerticalAlignment=64). The code snippet below implicitly satisfies the first restriction, and it would be good to explicitly call that out. </blockquote><div> </div><div>I'm not sure what you mean. Maybe something like this? </div><div>The first restriction is already handled by isl_choose_image_alignment_el but the second restriction, which is an extension of the first, only applies to qpitch and must be applied here. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > + assert(fmtl->bh == 4); > + pitch_el_rows = isl_align(pitch_el_rows, 256 / 4); > + } > + > if (ISL_DEV_GEN(dev) >= 9 && > info->dim == ISL_SURF_DIM_3D && > tile_info->tiling != ISL_TILING_LINEAR) { > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > index 598ed2c..0ea19d1 100644 > --- a/src/intel/isl/isl.h > +++ b/src/intel/isl/isl.h > @@ -356,6 +356,15 @@ enum isl_format { > 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 format names look good. > @@ -989,6 +1002,7 @@ isl_format_has_bc_compression(enum isl_format fmt) > > case ISL_TXC_HIZ: > case ISL_TXC_MCS: > + case ISL_TXC_CCS: > unreachable("Should not be called on an aux surface"); > } > > diff --git a/src/intel/isl/isl_format_layout.csv b/src/intel/isl/isl_format_layout.csv > index 972d50f..f0f31c7 100644 > --- a/src/intel/isl/isl_format_layout.csv > +++ b/src/intel/isl/isl_format_layout.csv > @@ -319,3 +319,12 @@ MCS_2X , 8, 1, 1, 1, , , , , , > MCS_4X , 8, 1, 1, 1, , , , , , , , , mcs > MCS_8X , 32, 1, 1, 1, , , , , , , , , mcs > MCS_16X , 64, 1, 1, 1, , , , , , , , , mcs > +GEN7_CCS_32BPP_X , 1, 16, 2, 1, , , , , , , , , ccs > +GEN7_CCS_64BPP_X , 1, 8, 2, 1, , , , , , , , , ccs > +GEN7_CCS_128BPP_X , 1, 4, 2, 1, , , , , , , , , ccs > +GEN7_CCS_32BPP_Y , 1, 8, 4, 1, , , , , , , , , ccs > +GEN7_CCS_64BPP_Y , 1, 4, 4, 1, , , , , , , , , ccs > +GEN7_CCS_128BPP_Y , 1, 2, 4, 1, , , , , , , , , ccs > +GEN9_CCS_32BPP , 2, 8, 4, 1, , , , , , , , , ccs > +GEN9_CCS_64BPP , 2, 4, 4, 1, , , , , , , , , ccs > +GEN9_CCS_128BPP , 2, 2, 4, 1, , , , , , , , , ccs The block dimensions look right to me. > diff --git a/src/intel/isl/isl_gen7.c b/src/intel/isl/isl_gen7.c > index 4b511c8..1c31d98 100644 > --- a/src/intel/isl/isl_gen7.c > +++ b/src/intel/isl/isl_gen7.c > @@ -242,6 +242,13 @@ gen7_filter_tiling(const struct isl_device *dev, > if (isl_format_get_layout(info->format)->txc == ISL_TXC_MCS) > *flags &= ISL_TILING_Y0_BIT; > > + /* The CCS formats and tiling always go together */ > + if (isl_format_get_layout(info->format)->txc == ISL_TXC_CCS) { > + *flags &= ISL_TILING_CCS_BIT; > + } else { > + *flags &= ~ISL_TILING_CCS_BIT; > + } > + > if (info->usage & (ISL_SURF_USAGE_DISPLAY_ROTATE_90_BIT | > ISL_SURF_USAGE_DISPLAY_ROTATE_180_BIT | > ISL_SURF_USAGE_DISPLAY_ROTATE_270_BIT)) { I expected to this patch to make changes to gen7_choose_image_alignment_el(). Why is that missing? Does gen7 not impose special alignment restrictions on the CCS? </blockquote><div> </div><div>gen7 doesn't allow CCS for multi-LOD or multi-slice images so it doesn't matter. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div><div class="h5"> > diff --git a/src/intel/isl/isl_gen8.c b/src/intel/isl/isl_gen8.c > index 62c331c..ec40d6d 100644 > --- a/src/intel/isl/isl_gen8.c > +++ b/src/intel/isl/isl_gen8.c > @@ -201,6 +201,19 @@ gen8_choose_image_alignment_el(const struct isl_device *dev, > { > assert(!isl_tiling_is_std_y(tiling)); > > + const struct isl_format_layout *fmtl = isl_format_get_layout(info->format); > + if (fmtl->txc == ISL_TXC_CCS) { > + /* > + * Broadwell PRM Vol 7, "MCS Buffer for Render Target(s)" (p. 676): > + * > + * "Mip-mapped and arrayed surfaces are supported with MCS buffer > + * layout with these alignments in the RT space: Horizontal > + * Alignment = 256 and Vertical Alignment = 128. > + */ > + *image_align_el = isl_extent3d(256 / fmtl->bw, 128 / fmtl->bh, 1); > + return; > + } > + > /* The below text from the Broadwell PRM provides some insight into the > * hardware's requirements for LOD alignment. From the Broadwell PRM >> > * Volume 5: Memory Views >> Surface Layout >> 2D Surfaces: > diff --git a/src/intel/isl/isl_gen9.c b/src/intel/isl/isl_gen9.c > index 39f4092..bb7f298 100644 > --- a/src/intel/isl/isl_gen9.c > +++ b/src/intel/isl/isl_gen9.c > @@ -103,6 +103,18 @@ gen9_choose_image_alignment_el(const struct isl_device *dev, > enum isl_msaa_layout msaa_layout, > struct isl_extent3d *image_align_el) > { > + const struct isl_format_layout *fmtl = isl_format_get_layout(info->format); > + if (fmtl->txc == ISL_TXC_CCS) { > + /* Sky Lake PRM Vol. 7, "MCS Buffer for Render Target(s)" (p. 632): > + * > + * "Mip-mapped and arrayed surfaces are supported with MCS buffer > + * layout with these alignments in the RT space: Horizontal > + * Alignment = 128 and Vertical Alignment = 64." > + */ > + *image_align_el = isl_extent3d(128 / fmtl->bw, 64 / fmtl->bh, 1); > + return; > + } > + > /* This BSpec text provides some insight into the hardware's alignment > * requirements [Skylake BSpec > Memory Views > Common Surface Formats > > * Surface Layout and Tiling > 2D Surfaces]: > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div></div>