On Wed, May 10, 2017 at 02:30:30PM -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.
> ---
> src/intel/isl/isl_gen7.c | 161
> +++++++++++++++++++++++------------------------
> 1 file changed, 78 insertions(+), 83 deletions(-)
>
> diff --git a/src/intel/isl/isl_gen7.c b/src/intel/isl/isl_gen7.c
> index 8e6b441..4c8f530 100644
> --- a/src/intel/isl/isl_gen7.c
> +++ b/src/intel/isl/isl_gen7.c
> @@ -289,105 +289,100 @@ 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) {
> + *image_align_el = isl_extent3d(8, 4, 1);
> + return;
> + } else {
> + *image_align_el = isl_extent3d(4, 4, 1);
> + return;
> + }
> + } else if (isl_surf_usage_is_stencil(info->usage)) {
> + *image_align_el = isl_extent3d(8, 8, 1);
> + return;
> + } else if (isl_format_is_compressed(info->format)) {
> + /* Compressed formats all have alignment equal to block size. */
> + *image_align_el = isl_extent3d(1, 1, 1);
> + return;
> + }
>
> -/**
> - * 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]. valign=8 is outside the set of valid values of
> - * RENDER_SURFACE_STATE.SurfaceVerticalAlignment, but that's ok because
> - * a stencil buffer will never be used directly for texturing or
> - * rendering on gen7.
> - */
> - return 8;
> - }
> -
> - 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);
> + if (tiling == ISL_TILING_Y0 &&
> + (info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT))
> + require_valign4 = true;
>
> - /* Handled by isl_choose_image_alignment_el */
> - assert(info->format != ISL_FORMAT_HIZ);
> + assert(!(require_valign4 && gen7_format_needs_valign2(dev,
> info->format)));
>
> - /* IVB+ does not support combined depthstencil. */
> - assert(!isl_surf_usage_is_depth_and_stencil(info->usage));
> + /* We default to HALIGN_2 because it uses the least memory. */
VALIGN_2
Otherwise looks good to me:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> + const uint32_t valign = require_valign4 ? 4 : 2;
>
> - *image_align_el = (struct isl_extent3d) {
> - .w = gen7_choose_halign_el(dev, info),
> - .h = gen7_choose_valign_el(dev, info, tiling),
> - .d = 1,
> - };
> + *image_align_el = isl_extent3d(halign, valign, 1);
> }
> --
> 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
