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