[Mesa-dev] [PATCH v2 12/14] isl: Add support for color control surfaces
Chad Versace
chad.versace at intel.com
Wed Jul 13 17:16:28 UTC 2016
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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list