[Mesa-dev] [RFC] intel/isl: Rewrite gen7_choose_image_alignment_el
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Wed May 10 18:14:06 UTC 2017
On Tue, May 09, 2017 at 10:00:34AM -0700, Jason Ekstrand wrote:
> The Ivy Bridge PRM provides a nice table that handles most of the
> alignment cases in one place. For standard color buffers we have a
> little freedom of choice but for most depth, stencil and compressed it's
> hard-coded. Chad's original functions split halign and valign apart and
> implemented them almost entirely based on restrictions and not the
> table. This makes things way more confusing than they need to be. This
> commit gets rid of the split and makes us implement the exact table
> up-front. If our surface isn't one of the ones in the table we then to
> on to make real choices.
>
> The only functional change here is that we now return a vertical
> alignment of 8 for stencil because that's what the table says to do.
> That change should probably go in it's own patch.
>
> If people like this change, I'm happy to do the same refactoring for
> gen6, gen8, and gen9.
Really nice, I'm in favor.
>
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: Topi Pohjolainen <topi.pohjolainen at intel.com>
> Cc: Nanley Chery <nanley.g.chery at intel.com>
> ---
> src/intel/isl/isl_gen7.c | 178 ++++++++++++++++++++---------------------------
> 1 file changed, 76 insertions(+), 102 deletions(-)
>
> diff --git a/src/intel/isl/isl_gen7.c b/src/intel/isl/isl_gen7.c
> index 18687b5..7607fde 100644
> --- a/src/intel/isl/isl_gen7.c
> +++ b/src/intel/isl/isl_gen7.c
> @@ -289,123 +289,97 @@ isl_gen6_filter_tiling(const struct isl_device *dev,
> *flags &= ~ISL_TILING_Y0_BIT;
> }
>
> -/**
> - * Choose horizontal subimage alignment, in units of surface elements.
> - */
> -static uint32_t
> -gen7_choose_halign_el(const struct isl_device *dev,
> - const struct isl_surf_init_info *restrict info)
> +void
> +isl_gen7_choose_image_alignment_el(const struct isl_device *dev,
> + const struct isl_surf_init_info *restrict info,
> + enum isl_tiling tiling,
> + enum isl_dim_layout dim_layout,
> + enum isl_msaa_layout msaa_layout,
> + struct isl_extent3d *image_align_el)
> {
> - if (isl_format_is_compressed(info->format))
> - return 1;
> + assert(ISL_DEV_GEN(dev) == 7);
>
> - /* From the Ivybridge PRM (2012-05-31), Volume 4, Part 1, Section 2.12.1,
> - * RENDER_SURFACE_STATE Surface Hoizontal Alignment:
> + /* Handled by isl_choose_image_alignment_el */
> + assert(info->format != ISL_FORMAT_HIZ);
> +
> + /* IVB+ does not support combined depthstencil. */
> + assert(!isl_surf_usage_is_depth_and_stencil(info->usage));
> +
> + /* From the Ivy Bridge PRM, Vol. 2, Part 2, Section 6.18.4.4,
> + * "Alignment unit size", the alignment parameters are summarized in the
> + * following table:
> *
> - * - This field is intended to be set to HALIGN_8 only if the surface
> - * was rendered as a depth buffer with Z16 format or a stencil buffer,
> - * since these surfaces support only alignment of 8. Use of HALIGN_8
> - * for other surfaces is supported, but uses more memory.
> + * Surface Defined By | Surface Format | Align Width | Align Height
> + * --------------------+-----------------+-------------+--------------
> + * DEPTH_BUFFER | D16_UNORM | 8 | 4
> + * | other | 4 | 4
> + * --------------------+-----------------+-------------+--------------
> + * STENCIL_BUFFER | N/A | 8 | 8
> + * --------------------+-----------------+-------------+--------------
> + * SURFACE_STATE | BC*, ETC*, EAC* | 4 | 4
> + * | FXT1 | 8 | 4
> + * | all others | HALIGN | VALIGN
> + * -------------------------------------------------------------------
> */
> - if (isl_surf_info_is_z16(info) ||
> - isl_surf_usage_is_stencil(info->usage))
> - return 8;
> -
> - return 4;
> -}
> + if (isl_surf_usage_is_depth(info->usage)) {
> + if (info->format == ISL_FORMAT_R16_UNORM) {
> + return isl_extent3d(8, 4, 1);
> + } else {
> + return isl_extent3d(4, 4, 1);
> + }
> + } else if (isl_surf_usage_is_stencil(info->usage)) {
> + return isl_extent3d(8, 8, 1);
> + } else if (isl_format_is_compressed(info->format)) {
> + /* Compressed formats all have alignment equal to block size. */
> + return isl_extent3d(1, 1, 1);
> + }
>
> -/**
> - * Choose vertical subimage alignment, in units of surface elements.
> - */
> -static uint32_t
> -gen7_choose_valign_el(const struct isl_device *dev,
> - const struct isl_surf_init_info *restrict info,
> - enum isl_tiling tiling)
> -{
> - MAYBE_UNUSED bool require_valign2 = false;
> - bool require_valign4 = false;
> + /* Everything after this point is in the "set by Surface Horizontal or
> + * Vertical Alignment" case. Now it's just a matter of applying
> + * restrictions.
> + */
>
> - if (isl_format_is_compressed(info->format))
> - return 1;
> + /* There are no restrictions on halign beyond what's given in the table
> + * above. We set it to the minimum value of 4 because that uses the least
> + * memory.
> + */
> + const uint32_t halign = 4;
>
> - if (gen7_format_needs_valign2(dev, info->format))
> - require_valign2 = true;
> + bool require_valign4 = false;
>
> /* From the Ivybridge PRM, Volume 4, Part 1, Section 2.12.1:
> * RENDER_SURFACE_STATE Surface Vertical Alignment:
> *
> - * - This field is intended to be set to VALIGN_4 if the surface was
> - * rendered as a depth buffer, for a multisampled (4x) render target,
> - * or for a multisampled (8x) render target, since these surfaces
> - * support only alignment of 4. Use of VALIGN_4 for other surfaces is
> - * supported, but uses more memory. This field must be set to
> - * VALIGN_4 for all tiled Y Render Target surfaces.
> + * * This field is intended to be set to VALIGN_4 if the surface was
> + * rendered as a depth buffer,
> + *
> + * * for a multisampled (4x) render target, or for a multisampled (8x)
> + * render target, since these surfaces support only alignment of 4.
> + *
> + * * This field must be set to VALIGN_4 for all tiled Y Render Target
> + * surfaces
> + *
> + * * Value of 1 is not supported for format YCRCB_NORMAL (0x182),
> + * YCRCB_SWAPUVY (0x183), YCRCB_SWAPUV (0x18f), YCRCB_SWAPY (0x190)
> + *
> + * * If Number of Multisamples is not MULTISAMPLECOUNT_1, this field
> + * must be set to VALIGN_4."
> *
> + * The first restriction is already handled by the table above and the
> + * second restriction is redundant with the fifth.
> */
> - if (isl_surf_usage_is_depth(info->usage) ||
> - info->samples > 1 ||
> - ((info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) &&
> - tiling == ISL_TILING_Y0)) {
> + if (info->samples > 1)
> require_valign4 = true;
> - }
>
> - if (isl_surf_usage_is_stencil(info->usage)) {
> - /* The Ivybridge PRM states that the stencil buffer's vertical alignment
> - * is 8 [Ivybridge PRM, Volume 1, Part 1, Section 6.18.4.4 Alignment
> - * Unit Size]. However, valign=8 is outside the set of valid values of
> - * RENDER_SURFACE_STATE.SurfaceVerticalAlignment, which is VALIGN_2
> - * (0x0) and VALIGN_4 (0x1).
> - *
> - * The PRM is generally confused about the width, height, and alignment
> - * of the stencil buffer; and this confusion appears elsewhere. For
> - * example, the following PRM text effectively converts the stencil
> - * buffer's 8-pixel alignment to a 4-pixel alignment [Ivybridge PRM,
> - * Volume 1, Part 1, Section
> - * 6.18.4.2 Base Address and LOD Calculation]:
> - *
> - * For separate stencil buffer, the width must be mutiplied by 2 and
> - * height divided by 2 as follows:
> - *
> - * w_L = 2*i*ceil(W_L/i)
> - * h_L = 1/2*j*ceil(H_L/j)
> - *
> - * The root of the confusion is that, in W tiling, each pair of rows is
> - * interleaved into one.
> - *
> - * FINISHME(chadv): Decide to set valign=4 or valign=8 after isl's API
> - * is more polished.
> - */
> + if (tiling == ISL_TILING_Y0 &&
> + (info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT))
> require_valign4 = true;
> - }
> -
> - assert(!require_valign2 || !require_valign4);
> -
> - if (require_valign4)
> - return 4;
> -
> - /* Prefer VALIGN_2 because it conserves memory. */
> - return 2;
> -}
>
> -void
> -isl_gen7_choose_image_alignment_el(const struct isl_device *dev,
> - const struct isl_surf_init_info *restrict info,
> - enum isl_tiling tiling,
> - enum isl_dim_layout dim_layout,
> - enum isl_msaa_layout msaa_layout,
> - struct isl_extent3d *image_align_el)
> -{
> - assert(ISL_DEV_GEN(dev) == 7);
> -
> - /* Handled by isl_choose_image_alignment_el */
> - assert(info->format != ISL_FORMAT_HIZ);
> -
> - /* IVB+ does not support combined depthstencil. */
> - assert(!isl_surf_usage_is_depth_and_stencil(info->usage));
> -
> - *image_align_el = (struct isl_extent3d) {
> - .w = gen7_choose_halign_el(dev, info),
> - .h = gen7_choose_valign_el(dev, info, tiling),
> - .d = 1,
> - };
> + if (require_valign4) {
> + assert(!gen7_format_needs_valign2(dev, info->format));
> + return isl_extent3d(halign, 4, 1);
> + } else {
> + /* We default to HALIGN_2 because it uses the least memory. */
> + return isl_extent3d(halign, 2, 1);
> + }
> }
> --
> 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