Re: [Mesa-dev] [PATCH] anv/cmd_buffer: Remove the 1-D case from the HiZ QPitch calculation
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
> Cc: "13.0"
> ---
> 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@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
[Mesa-dev] [PATCH] anv/cmd_buffer: Remove the 1-D case from the HiZ QPitch calculation
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
Cc: "13.0"
---
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.
*/
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/cmd_buffer: Remove the 1-D case from the HiZ QPitch calculation
drp... This should have been two patches... I'll resend
On Fri, Nov 25, 2016 at 10:13 PM, 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
> Cc: "13.0"
> ---
> src/intel/isl/isl.h| 15 ++-
> src/intel/isl/isl_surface_state.c | 12
> src/intel/vulkan/genX_cmd_buffer.c | 9 ++---
> 3 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index 07368f9..427724a 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -1336,7 +1336,7 @@ isl_surf_get_image_alignment_sa(const struct
> isl_surf *surf)
> * Pitch between vertically adjacent surface elements, in bytes.
> */
> static inline uint32_t
> -isl_surf_get_row_pitch(const struct isl_surf *surf)
> +isl_surf_get_row_pitch_B(const struct isl_surf *surf)
> {
> return surf->row_pitch;
> }
> @@ -1354,6 +1354,19 @@ isl_surf_get_row_pitch_el(const struct isl_surf
> *surf)
> }
>
> /**
> + * Pitch between vertically adjacent tiles, in units of tiles.
> + */
> +static inline uint32_t
> +isl_surf_get_row_pitch_tl(const struct isl_device *dev,
> + const struct isl_surf *surf)
> +{
> + assert(surf->tiling != ISL_TILING_LINEAR);
> + struct isl_tile_info tile_info;
> + isl_surf_get_tile_info(dev, surf, &tile_info);
> + return surf->row_pitch / tile_info.phys_extent_B.width;
> +}
> +
> +/**
> * Pitch between physical array slices, in rows of surface elements.
> */
> static inline uint32_t
> diff --git a/src/intel/isl/isl_surface_state.c
> b/src/intel/isl/isl_surface_state.c
> index 3bb0abd..27468b3 100644
> --- a/src/intel/isl/isl_surface_state.c
> +++ b/src/intel/isl/isl_surface_state.c
> @@ -405,7 +405,7 @@ isl_genX(surf_fill_state_s)(const struct isl_device
> *dev, void *state,
>/* For gen9 1-D textures, surface pitch is ignored */
>s.SurfacePitch = 0;
> } else {
> - s.SurfacePitch = info->surf->row_pitch - 1;
> + s.SurfacePitch = isl_surf_get_row_pitch_B(info->surf) - 1;
> }
>
> #if GEN_GEN >= 8
> @@ -503,14 +503,10 @@ isl_genX(surf_fill_state_s)(const struct isl_device
> *dev, void *state,
>
> #if GEN_GEN >= 7
> if (info->aux_surf && info->aux_usage != ISL_AUX_USAGE_NONE) {
> - struct isl_tile_info tile_info;
> - isl_surf_get_tile_info(dev, info->aux_surf, &tile_info);
> - uint32_t pitch_in_tiles =
> - info->aux_surf->row_pitch / tile_info.phys_extent_B.width;
> -
> #if GEN_GEN >= 8
>assert(GEN_GEN >= 9 || info->aux_usage != ISL_AUX_USAGE_CCS_E);
> - s.AuxiliarySurfacePitch = pitch_in_tiles - 1;
> + s.AuxiliarySurfacePitch =
> + isl_surf_get_row_pitch_tl(dev, info->aux_surf) - 1;
>/* Auxiliary surfaces in ISL have compressed formats but the
> hardware
> * doesn't expect our definition of the compression, it expects
> qpitch
> * in units of samples on the main surface.
> @@ -523,7 +519,7 @@ isl_genX(surf_fill_state_s)(const struct isl_device
> *dev, void *state,
>assert(info->aux_usage == ISL_AUX_USAGE_MCS ||
> info->aux_usage == ISL_AUX_USAGE_CCS_D);
>s.MCSBaseAddress = info->aux_address,
> - s.MCSSurfacePitch = pitch_in_tiles - 1;
> + s.MCSSurfacePitch = isl_surf_get_row_pitch_tl(dev, info->aux_surf)
> - 1;
>s.MCSEnable = true;
> #endif
> }
> 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.
>*/
> 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] [PATCH] anv/cmd_buffer: Remove the 1-D case from the HiZ QPitch calculation
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
Cc: "13.0"
---
src/intel/isl/isl.h| 15 ++-
src/intel/isl/isl_surface_state.c | 12
src/intel/vulkan/genX_cmd_buffer.c | 9 ++---
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
index 07368f9..427724a 100644
--- a/src/intel/isl/isl.h
+++ b/src/intel/isl/isl.h
@@ -1336,7 +1336,7 @@ isl_surf_get_image_alignment_sa(const struct isl_surf
*surf)
* Pitch between vertically adjacent surface elements, in bytes.
*/
static inline uint32_t
-isl_surf_get_row_pitch(const struct isl_surf *surf)
+isl_surf_get_row_pitch_B(const struct isl_surf *surf)
{
return surf->row_pitch;
}
@@ -1354,6 +1354,19 @@ isl_surf_get_row_pitch_el(const struct isl_surf *surf)
}
/**
+ * Pitch between vertically adjacent tiles, in units of tiles.
+ */
+static inline uint32_t
+isl_surf_get_row_pitch_tl(const struct isl_device *dev,
+ const struct isl_surf *surf)
+{
+ assert(surf->tiling != ISL_TILING_LINEAR);
+ struct isl_tile_info tile_info;
+ isl_surf_get_tile_info(dev, surf, &tile_info);
+ return surf->row_pitch / tile_info.phys_extent_B.width;
+}
+
+/**
* Pitch between physical array slices, in rows of surface elements.
*/
static inline uint32_t
diff --git a/src/intel/isl/isl_surface_state.c
b/src/intel/isl/isl_surface_state.c
index 3bb0abd..27468b3 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -405,7 +405,7 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev,
void *state,
/* For gen9 1-D textures, surface pitch is ignored */
s.SurfacePitch = 0;
} else {
- s.SurfacePitch = info->surf->row_pitch - 1;
+ s.SurfacePitch = isl_surf_get_row_pitch_B(info->surf) - 1;
}
#if GEN_GEN >= 8
@@ -503,14 +503,10 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev,
void *state,
#if GEN_GEN >= 7
if (info->aux_surf && info->aux_usage != ISL_AUX_USAGE_NONE) {
- struct isl_tile_info tile_info;
- isl_surf_get_tile_info(dev, info->aux_surf, &tile_info);
- uint32_t pitch_in_tiles =
- info->aux_surf->row_pitch / tile_info.phys_extent_B.width;
-
#if GEN_GEN >= 8
assert(GEN_GEN >= 9 || info->aux_usage != ISL_AUX_USAGE_CCS_E);
- s.AuxiliarySurfacePitch = pitch_in_tiles - 1;
+ s.AuxiliarySurfacePitch =
+ isl_surf_get_row_pitch_tl(dev, info->aux_surf) - 1;
/* Auxiliary surfaces in ISL have compressed formats but the hardware
* doesn't expect our definition of the compression, it expects qpitch
* in units of samples on the main surface.
@@ -523,7 +519,7 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev,
void *state,
assert(info->aux_usage == ISL_AUX_USAGE_MCS ||
info->aux_usage == ISL_AUX_USAGE_CCS_D);
s.MCSBaseAddress = info->aux_address,
- s.MCSSurfacePitch = pitch_in_tiles - 1;
+ s.MCSSurfacePitch = isl_surf_get_row_pitch_tl(dev, info->aux_surf) - 1;
s.MCSEnable = true;
#endif
}
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.
*/
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
