[Mesa-dev] [PATCH] anv/cmd_buffer: Remove the 1-D case from the HiZ QPitch calculation

[Nanley Chery]

On Fri, Nov 25, 2016 at 10:17:54PM -0800, Jason Ekstrand wrote:
> The 1-D special case doesn't actually apply to depth or HiZ.  I discovered
> this while converting BLORP over to genxml and ISL.  The reason is that the
> 1-D special case only applies to the new Sky Lake 1-D layout which is only
> used for LINEAR 1-D images.  For tiled 1-D images, such as depth buffers,
> the old gen4 2-D layout is used and the QPitch should be in rows.
> 
> Cc: Nanley Chery <nanley.g.chery at intel.com>
> Cc: "13.0" <mesa-stable at lists.freedesktop.org>
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
> index a965cd6..2ef1745 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -2130,11 +2130,14 @@ cmd_buffer_emit_depth_stencil(struct anv_cmd_buffer *cmd_buffer)
>            *    - SURFTYPE_1D: distance in pixels between array slices
>            *    - SURFTYPE_2D/CUBE: distance in rows between array slices
>            *    - SURFTYPE_3D: distance in rows between R - slices
> +          *
> +          * Unfortunately, the docs aren't 100% accurate here.  They fail to
> +          * mention that the 1-D rule only applies to linear 1-D images.
> +          * Since depth and HiZ buffers are always tiled, we use the 2-D rule
> +          * in the 1-D case.

Could you update your commit message and the above comment to include
(and/or refer to) PRM citations to clarify the situation? I originally
didn't take into account the following when writing this code:

>From 3DSTATE_DEPTH_BUFFER in the SKL PRM:

	Programming Notes
	The Surface Type of the depth buffer must be the same as the
	Surface Type of the render target(s) (defined in SURFACE_STATE),
	unless either the depth buffer or render targets are
	SURFTYPE_NULL (see exception below for SKL). 1D surface type
	not allowed for depth surface and stencil surface.

	Workaround 
	If depth/stencil is enabled with 1D render target, depth/stencil
	surface type needs to be set to 2D surface type and height set
	to 1. Depth will use (legacy) TileY and stencil will use TileW.
	For this case only, the Surface Type of the depth buffer can be
	2D while the Surface Type of the render target(s) are 1D,
	representing an exception to a programming note above.


Thanks for finding this error.

-Nanley

>            */
>           hdb.SurfaceQPitch =
> -            image->aux_surface.isl.dim == ISL_SURF_DIM_1D ?
> -               isl_surf_get_array_pitch_el(&image->aux_surface.isl) >> 2 :
> -               isl_surf_get_array_pitch_el_rows(&image->aux_surface.isl) >> 2;
> +            isl_surf_get_array_pitch_el_rows(&image->aux_surface.isl) >> 2;
>  #endif
>        }
>     } else {
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev