On Thu, 2018-08-09 at 15:33 +0200, Bas Nieuwenhuizen wrote: > On Thu, Aug 9, 2018 at 2:56 PM, Andres Gomez <ago...@igalia.com> wrote: > > Bas, James, did you eventually come with a resolution for this? Can I > > just ignore this nominated patch in the -stable ML? > > The fix in this patch landed as part of
Great! Thanks ☺ > > commit 68dead112e710b261ad33604175d635dec6afd34 > Author: Samuel Pitoiset <samuel.pitoi...@gmail.com> > Date: Wed Jun 13 14:27:40 2018 +0200 > > radv: update the ZRANGE_PRECISION value for the TC-compat bug > > 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. > > We also need to set the depth clear regs and to update > ZRANGE_PRECISION when initializing a TC-compat depth image to 0. > > Original patch from James Legg. > > This fixes random CTS fails with > dEQP-VK.renderpass.suballocation.formats.d32_sfloat_s8_uint.input.* > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396 > CC: <mesa-sta...@lists.freedesktop.org> > Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com> > Reviewed-by: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl> > > > > > > On Wed, 2018-03-28 at 15:28 +0200, Bas Nieuwenhuizen wrote: > > > No final resolution yet. > > > > > > I was trying to fix my minor comment, but looks like I have a bunch of > > > CTS regressions here with the original patch, so still working on it. > > > > > > On Tue, Mar 27, 2018 at 2:03 PM, Juan A. Suarez Romero > > > <jasua...@igalia.com> wrote: > > > > On Thu, 2018-03-22 at 12:31 +0000, James Legg wrote: > > > > > On Thu, 2018-03-22 at 02:36 +0100, Bas Nieuwenhuizen wrote: > > > > > > On Thu, Mar 8, 2018 at 12:59 PM, James Legg > > > > > > <jl...@feralinteractive.com> wrote: > > > > > > > This avoids bug 105396 somehow. I suspect it is a VI and GFX9 > > > > > > > hardware > > > > > > > bug which PAL calls WaTcCompatZRange, but I don't know for sure. > > > > > > > > > > > > > > In the VK_FORMAT_D32_SFLOAT case, TILE_STENCIL_DISABLE is not set > > > > > > > for > > > > > > > tc compatible image formats regardless of not having a stencil > > > > > > > aspect. > > > > > > > If TILE_STENCIL_DISABLE was set, ZRANGE_PRECISION would have no > > > > > > > effect > > > > > > > and the bug would occur again. > > > > > > > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396 > > > > > > > CC: <mesa-sta...@lists.freedesktop.org> > > > > > > > CC: Dave Airlie <airl...@redhat.com> > > > > > > > CC: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl> > > > > > > > CC: Samuel Pitoiset <samuel.pitoi...@gmail.com> > > > > > > > --- > > > > > > > src/amd/vulkan/radv_cmd_buffer.c | 52 > > > > > > > +++++++++++++++++++++++++++++++++++++--- > > > > > > > 1 file changed, 49 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > diff --git a/src/amd/vulkan/radv_cmd_buffer.c > > > > > > > b/src/amd/vulkan/radv_cmd_buffer.c > > > > > > > index 3e0ed0e9a9..89e31a0347 100644 > > > > > > > --- a/src/amd/vulkan/radv_cmd_buffer.c > > > > > > > +++ b/src/amd/vulkan/radv_cmd_buffer.c > > > > > > > @@ -915,6 +915,37 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer > > > > > > > *cmd_buffer, > > > > > > > > > > > > > > } > > > > > > > > > > > > > > + if (image->surface.htile_size) > > > > > > > + { > > > > > > > + /* If the last depth clear value was 0.0f, set > > > > > > > ZRANGE_PRECISION > > > > > > > + * to 0 in dp_z_info for more accuracy with > > > > > > > reverse depth; and > > > > > > > + * to avoid > > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=105396. > > > > > > > + * Otherwise, we leave it set to 1. > > > > > > > + */ > > > > > > > + 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; > > > > > > > + 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, R_028040_DB_Z_INFO >> > > > > > > > 2); /* write address low */ > > > > > > > + radeon_emit(cmd_buffer->cs, 0u); > > > > > > > /* write address high */ > > > > > > > + > > > > > > > + /* The value to write data when the condition > > > > > > > passes */ > > > > > > > + uint32_t db_z_info_clear_zero = db_z_info & > > > > > > > C_028040_ZRANGE_PRECISION; > > > > > > > + radeon_emit(cmd_buffer->cs, db_z_info_clear_zero); > > > > > > > + } > > > > > > > + > > > > > > > radeon_set_context_reg(cmd_buffer->cs, > > > > > > > R_028B78_PA_SU_POLY_OFFSET_DB_FMT_CNTL, > > > > > > > ds->pa_su_poly_offset_db_fmt_cntl); > > > > > > > } > > > > > > > @@ -3479,7 +3510,8 @@ void radv_CmdEndRenderPass( > > > > > > > > > > > > > > /* > > > > > > > * For HTILE we have the following interesting clear words: > > > > > > > - * 0xfffff30f: Uncompressed, full depth range, for > > > > > > > depth+stencil HTILE > > > > > > > + * 0xfffff30f: Uncompressed, full depth range, for > > > > > > > depth+stencil HTILE when ZRANGE_PRECISION is 1 > > > > > > > + * 0x0003f30f: Uncompressed, full depth range, for > > > > > > > depth+stencil HTILE when ZRANGE_PRECISION is 0 > > > > > > > * 0xfffc000f: Uncompressed, full depth range, for depth only > > > > > > > HTILE. > > > > > > > * 0xfffffff0: Clear depth to 1.0 > > > > > > > * 0x00000000: Clear depth to 0.0 > > > > > > > @@ -3528,8 +3560,22 @@ static void > > > > > > > radv_handle_depth_image_transition(struct radv_cmd_buffer > > > > > > > *cmd_buffe > > > > > > > radv_initialize_htile(cmd_buffer, image, range, > > > > > > > 0); > > > > > > > } else if (!radv_layout_is_htile_compressed(image, > > > > > > > src_layout, src_queue_mask) && > > > > > > > radv_layout_is_htile_compressed(image, > > > > > > > dst_layout, dst_queue_mask)) { > > > > > > > - uint32_t clear_value = > > > > > > > vk_format_is_stencil(image->vk_format) ? 0xfffff30f : 0xfffc000f; > > > > > > > - radv_initialize_htile(cmd_buffer, image, range, > > > > > > > clear_value); > > > > > > > + if (vk_format_is_stencil(image->vk_format)) { > > > > > > > + /* The appropriate clear value depends on > > > > > > > DB_Z_INFO's > > > > > > > + * ZRANGE_PRECISION, which can vary > > > > > > > depending on the > > > > > > > + * last used clear value, which could be > > > > > > > from another > > > > > > > + * command buffer. Instead of picking the > > > > > > > appropriate > > > > > > > + * clear value on the GPU, resummarize > > > > > > > accurately. > > > > > > > + */ > > > > > > > + VkImageSubresourceRange local_range = > > > > > > > *range; > > > > > > > + local_range.aspectMask = > > > > > > > VK_IMAGE_ASPECT_DEPTH_BIT; > > > > > > > + local_range.baseMipLevel = 0; > > > > > > > + local_range.levelCount = 1; > > > > > > > + > > > > > > > + > > > > > > > radv_resummarize_depth_image_inplace(cmd_buffer, image, > > > > > > > &local_range); > > > > > > > > > > > > Can we instead of resummarizing, just do the reset + write the > > > > > > clear words? > > > > > > > > > > That would be fine. I used a resummarize as it was already implemented > > > > > and I wasn't confident I would get the clearing implementation right. > > > > > > > > > > > Otherwise looks good to me, please tell if I should do it. I'd do it > > > > > > in a follow up patch, except this is nominated to stable. > > > > > > > > Hi! What was the final resolution for this patch? It was nominated for > > > > stable, > > > > but didn't land in master, nor I've seen it merged in a follow up patch. > > > > > > > > > > > > J.A. > > > > > > > > > If you wouldn't mind, that would be great. > > > > > > > > > > > Apologies for the delay. > > > > > > > > > > No problem. > > > > > > > > > > > > > > > > > > + } else { > > > > > > > + radv_initialize_htile(cmd_buffer, image, > > > > > > > range, 0xfffc000f); > > > > > > > + } > > > > > > > } else if (radv_layout_is_htile_compressed(image, > > > > > > > src_layout, src_queue_mask) && > > > > > > > !radv_layout_is_htile_compressed(image, > > > > > > > dst_layout, dst_queue_mask)) { > > > > > > > VkImageSubresourceRange local_range = *range; > > > > > > > -- > > > > > > > 2.14.3 > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > mesa-dev mailing list > > > > > mesa-dev@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > > _______________________________________________ > > > mesa-stable mailing list > > > mesa-sta...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-stable > > > > -- > > Br, > > > > Andres > > -- Br, Andres _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
