[v2] radv: update the ZRANGE_PRECISION value for the TC-compat bug

Submitted by Samuel Pitoiset on June 13, 2018, 10:04 a.m.

Details

Message ID 20180613100445.7333-1-samuel.pitoiset@gmail.com
State New
Headers show
Series "radv: update the ZRANGE_PRECISION value for the TC-compat bug" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset June 13, 2018, 10:04 a.m.
On GFX8+, there is a bug that affects TC-compatible depth surfaces
when the ZRange is not reset after LateZ kills pixels.

The workaround is to always set DB_Z_INFO.ZRANGE_PRECISION to match
the last fast clear value. Because the value is set to 1 by default,
we only need to update it when clearing Z to 0.0.

Original patch from James Legg.

v2: - only update ZRANGE_PRECISION for depth aspects
    - adjust base address in presence of stencil

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
CC: <mesa-stable@lists.freedesktop.org>
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_cmd_buffer.c | 94 ++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 043b4a2f44a..b8724d6b937 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -1044,6 +1044,68 @@  radv_emit_fb_color_state(struct radv_cmd_buffer *cmd_buffer,
 	}
 }
 
+static void
+radv_update_zrange_precision(struct radv_cmd_buffer *cmd_buffer,
+			     struct radv_ds_buffer_info *ds,
+			     struct radv_image *image, VkImageLayout layout,
+			     bool requires_cond_write)
+{
+	uint32_t db_z_info = ds->db_z_info;
+	uint32_t db_z_info_reg;
+
+	if (!radv_image_is_tc_compat_htile(image))
+		return;
+
+	if (!radv_layout_has_htile(image, layout,
+	                           radv_image_queue_family_mask(image,
+	                                                        cmd_buffer->queue_family_index,
+	                                                        cmd_buffer->queue_family_index))) {
+		db_z_info &= C_028040_TILE_SURFACE_ENABLE;
+	}
+
+	db_z_info &= C_028040_ZRANGE_PRECISION;
+
+	if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) {
+		db_z_info_reg = R_028038_DB_Z_INFO;
+	} else {
+		db_z_info_reg = R_028040_DB_Z_INFO;
+	}
+
+	/* When we don't know the last fast clear value we need to emit a
+	 * conditional packet, otherwise we can update DB_Z_INFO directly.
+	 */
+	if (requires_cond_write) {
+		radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 7, 0));
+
+		const uint32_t write_space = 0 << 8;	/* register */
+		const uint32_t poll_space = 1 << 4;	/* memory */
+		const uint32_t function = 3 << 0;	/* equal to the reference */
+		const uint32_t options = write_space | poll_space | function;
+		radeon_emit(cmd_buffer->cs, options);
+
+		/* poll address - location of the depth clear value */
+		uint64_t va = radv_buffer_get_va(image->bo);
+		va += image->offset + image->clear_value_offset;
+
+		/* In presence of stencil format, we have to adjust the base
+		 * address because the first value is the stencil clear value.
+		 */
+		if (vk_format_is_stencil(image->vk_format))
+			va += 4;
+
+		radeon_emit(cmd_buffer->cs, va);
+		radeon_emit(cmd_buffer->cs, va >> 32);
+
+		radeon_emit(cmd_buffer->cs, fui(0.0f));		 /* reference value */
+		radeon_emit(cmd_buffer->cs, (uint32_t)-1);	 /* comparison mask */
+		radeon_emit(cmd_buffer->cs, db_z_info_reg >> 2); /* write address low */
+		radeon_emit(cmd_buffer->cs, 0u);		 /* write address high */
+		radeon_emit(cmd_buffer->cs, db_z_info);
+	} else {
+		radeon_set_context_reg(cmd_buffer->cs, db_z_info_reg, db_z_info);
+	}
+}
+
 static void
 radv_emit_fb_ds_state(struct radv_cmd_buffer *cmd_buffer,
 		      struct radv_ds_buffer_info *ds,
@@ -1102,6 +1164,9 @@  radv_emit_fb_ds_state(struct radv_cmd_buffer *cmd_buffer,
 
 	}
 
+	/* Update the ZRANGE_PRECISION value for the TC-compat bug. */
+	radv_update_zrange_precision(cmd_buffer, ds, image, layout, true);
+
 	radeon_set_context_reg(cmd_buffer->cs, R_028B78_PA_SU_POLY_OFFSET_DB_FMT_CNTL,
 			       ds->pa_su_poly_offset_db_fmt_cntl);
 }
@@ -1143,6 +1208,35 @@  radv_set_depth_clear_regs(struct radv_cmd_buffer *cmd_buffer,
 		radeon_emit(cmd_buffer->cs, ds_clear_value.stencil); /* R_028028_DB_STENCIL_CLEAR */
 	if (aspects & VK_IMAGE_ASPECT_DEPTH_BIT)
 		radeon_emit(cmd_buffer->cs, fui(ds_clear_value.depth)); /* R_02802C_DB_DEPTH_CLEAR */
+
+	/* Update the ZRANGE_PRECISION value for the TC-compat bug. This is
+	 * only needed when clearing Z to 0.0.
+	 */
+	if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
+	    ds_clear_value.depth == 0.0) {
+		struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
+		const struct radv_subpass *subpass = cmd_buffer->state.subpass;
+
+		if (!framebuffer)
+			return;
+
+		if (subpass->depth_stencil_attachment.attachment == VK_ATTACHMENT_UNUSED)
+			return;
+
+		int idx = subpass->depth_stencil_attachment.attachment;
+		VkImageLayout layout = subpass->depth_stencil_attachment.layout;
+		struct radv_attachment_info *att = &framebuffer->attachments[idx];
+		struct radv_image *image = att->attachment->image;
+
+		/* Only needed if the image is currently bound as the depth
+		 * surface.
+		 */
+		if (att->attachment->image != image)
+			return;
+
+		radv_update_zrange_precision(cmd_buffer, &att->ds, image,
+					     layout, false);
+	}
 }
 
 static void

Comments

Thanks for figuring out the remaning issues,

Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>

On Wed, Jun 13, 2018 at 12:04 PM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
> On GFX8+, there is a bug that affects TC-compatible depth surfaces
> when the ZRange is not reset after LateZ kills pixels.
>
> The workaround is to always set DB_Z_INFO.ZRANGE_PRECISION to match
> the last fast clear value. Because the value is set to 1 by default,
> we only need to update it when clearing Z to 0.0.
>
> Original patch from James Legg.
>
> v2: - only update ZRANGE_PRECISION for depth aspects
>     - adjust base address in presence of stencil
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
> CC: <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 94 ++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 043b4a2f44a..b8724d6b937 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -1044,6 +1044,68 @@ radv_emit_fb_color_state(struct radv_cmd_buffer *cmd_buffer,
>         }
>  }
>
> +static void
> +radv_update_zrange_precision(struct radv_cmd_buffer *cmd_buffer,
> +                            struct radv_ds_buffer_info *ds,
> +                            struct radv_image *image, VkImageLayout layout,
> +                            bool requires_cond_write)
> +{
> +       uint32_t db_z_info = ds->db_z_info;
> +       uint32_t db_z_info_reg;
> +
> +       if (!radv_image_is_tc_compat_htile(image))
> +               return;
> +
> +       if (!radv_layout_has_htile(image, layout,
> +                                  radv_image_queue_family_mask(image,
> +                                                               cmd_buffer->queue_family_index,
> +                                                               cmd_buffer->queue_family_index))) {
> +               db_z_info &= C_028040_TILE_SURFACE_ENABLE;
> +       }
> +
> +       db_z_info &= C_028040_ZRANGE_PRECISION;
> +
> +       if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) {
> +               db_z_info_reg = R_028038_DB_Z_INFO;
> +       } else {
> +               db_z_info_reg = R_028040_DB_Z_INFO;
> +       }
> +
> +       /* When we don't know the last fast clear value we need to emit a
> +        * conditional packet, otherwise we can update DB_Z_INFO directly.
> +        */
> +       if (requires_cond_write) {
> +               radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 7, 0));
> +
> +               const uint32_t write_space = 0 << 8;    /* register */
> +               const uint32_t poll_space = 1 << 4;     /* memory */
> +               const uint32_t function = 3 << 0;       /* equal to the reference */
> +               const uint32_t options = write_space | poll_space | function;
> +               radeon_emit(cmd_buffer->cs, options);
> +
> +               /* poll address - location of the depth clear value */
> +               uint64_t va = radv_buffer_get_va(image->bo);
> +               va += image->offset + image->clear_value_offset;
> +
> +               /* In presence of stencil format, we have to adjust the base
> +                * address because the first value is the stencil clear value.
> +                */
> +               if (vk_format_is_stencil(image->vk_format))
> +                       va += 4;
> +
> +               radeon_emit(cmd_buffer->cs, va);
> +               radeon_emit(cmd_buffer->cs, va >> 32);
> +
> +               radeon_emit(cmd_buffer->cs, fui(0.0f));          /* reference value */
> +               radeon_emit(cmd_buffer->cs, (uint32_t)-1);       /* comparison mask */
> +               radeon_emit(cmd_buffer->cs, db_z_info_reg >> 2); /* write address low */
> +               radeon_emit(cmd_buffer->cs, 0u);                 /* write address high */
> +               radeon_emit(cmd_buffer->cs, db_z_info);
> +       } else {
> +               radeon_set_context_reg(cmd_buffer->cs, db_z_info_reg, db_z_info);
> +       }
> +}
> +
>  static void
>  radv_emit_fb_ds_state(struct radv_cmd_buffer *cmd_buffer,
>                       struct radv_ds_buffer_info *ds,
> @@ -1102,6 +1164,9 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer *cmd_buffer,
>
>         }
>
> +       /* Update the ZRANGE_PRECISION value for the TC-compat bug. */
> +       radv_update_zrange_precision(cmd_buffer, ds, image, layout, true);
> +
>         radeon_set_context_reg(cmd_buffer->cs, R_028B78_PA_SU_POLY_OFFSET_DB_FMT_CNTL,
>                                ds->pa_su_poly_offset_db_fmt_cntl);
>  }
> @@ -1143,6 +1208,35 @@ radv_set_depth_clear_regs(struct radv_cmd_buffer *cmd_buffer,
>                 radeon_emit(cmd_buffer->cs, ds_clear_value.stencil); /* R_028028_DB_STENCIL_CLEAR */
>         if (aspects & VK_IMAGE_ASPECT_DEPTH_BIT)
>                 radeon_emit(cmd_buffer->cs, fui(ds_clear_value.depth)); /* R_02802C_DB_DEPTH_CLEAR */
> +
> +       /* Update the ZRANGE_PRECISION value for the TC-compat bug. This is
> +        * only needed when clearing Z to 0.0.
> +        */
> +       if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> +           ds_clear_value.depth == 0.0) {
> +               struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> +               const struct radv_subpass *subpass = cmd_buffer->state.subpass;
> +
> +               if (!framebuffer)
> +                       return;
> +
> +               if (subpass->depth_stencil_attachment.attachment == VK_ATTACHMENT_UNUSED)
> +                       return;
> +
> +               int idx = subpass->depth_stencil_attachment.attachment;
> +               VkImageLayout layout = subpass->depth_stencil_attachment.layout;
> +               struct radv_attachment_info *att = &framebuffer->attachments[idx];
> +               struct radv_image *image = att->attachment->image;
> +
> +               /* Only needed if the image is currently bound as the depth
> +                * surface.
> +                */
> +               if (att->attachment->image != image)
> +                       return;
> +
> +               radv_update_zrange_precision(cmd_buffer, &att->ds, image,
> +                                            layout, false);
> +       }
>  }
>
>  static void
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev