[Mesa-dev] [PATCH] radv: allow secondary command buffers to inherit unknown framebuffers
Bas Nieuwenhuizen
bas at basnieuwenhuizen.nl
Sat Jan 12 21:51:45 UTC 2019
On Thu, Dec 20, 2018 at 8:05 PM Rhys Perry <pendingchaos02 at gmail.com> wrote:
>
> Fixes: f4e499ec79 ('radv: add initial non-conformant radv vulkan driver')
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107986
> Signed-off-by: Rhys Perry <pendingchaos02 at gmail.com>
> ---
> src/amd/vulkan/radv_cmd_buffer.c | 59 ++++++++++++++++++++++----------
> src/amd/vulkan/radv_meta_clear.c | 8 +++++
> src/amd/vulkan/radv_private.h | 2 ++
> 3 files changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index c61310f3fc9..96fe5acb3bf 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -730,6 +730,9 @@ radv_emit_rbplus_state(struct radv_cmd_buffer *cmd_buffer)
> struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> const struct radv_subpass *subpass = cmd_buffer->state.subpass;
>
> + /* FIXME: handle when the framebuffer is unknown in secondary framebuffers */
> + assert(!cmd_buffer->inherit_unknown_fb);
> +
> unsigned sx_ps_downconvert = 0;
> unsigned sx_blend_opt_epsilon = 0;
> unsigned sx_blend_opt_control = 0;
> @@ -1189,19 +1192,22 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer *cmd_buffer,
> struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> const struct radv_subpass *subpass = cmd_buffer->state.subpass;
> struct radeon_cmdbuf *cs = cmd_buffer->cs;
> - struct radv_attachment_info *att;
> - uint32_t att_idx;
> + struct radv_attachment_info *att = NULL;
>
> - if (!framebuffer || !subpass)
> + if (!subpass)
> return;
> -
> - att_idx = subpass->depth_stencil_attachment.attachment;
> - if (att_idx == VK_ATTACHMENT_UNUSED)
> + if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
> return;
>
> - att = &framebuffer->attachments[att_idx];
> - if (att->attachment->image != image)
> - return;
> + if (framebuffer) {
> + uint32_t att_idx = subpass->depth_stencil_attachment.attachment;
> + if (att_idx == VK_ATTACHMENT_UNUSED)
> + return;
> +
> + att = &framebuffer->attachments[att_idx];
> + if (att->attachment->image != image)
> + return;
> + }
>
> radeon_set_context_reg_seq(cs, R_028028_DB_STENCIL_CLEAR, 2);
> radeon_emit(cs, ds_clear_value.stencil);
> @@ -1212,6 +1218,8 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer *cmd_buffer,
> */
> if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> ds_clear_value.depth == 0.0) {
> + assert(att);
> +
> VkImageLayout layout = subpass->depth_stencil_attachment.layout;
>
> radv_update_zrange_precision(cmd_buffer, &att->ds, image,
> @@ -1426,19 +1434,22 @@ radv_update_bound_fast_clear_color(struct radv_cmd_buffer *cmd_buffer,
> struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> const struct radv_subpass *subpass = cmd_buffer->state.subpass;
> struct radeon_cmdbuf *cs = cmd_buffer->cs;
> - struct radv_attachment_info *att;
> - uint32_t att_idx;
>
> - if (!framebuffer || !subpass)
> + if (!subpass)
> return;
> -
> - att_idx = subpass->color_attachments[cb_idx].attachment;
> - if (att_idx == VK_ATTACHMENT_UNUSED)
> + if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
> return;
>
> - att = &framebuffer->attachments[att_idx];
> - if (att->attachment->image != image)
> - return;
> + if (framebuffer) {
> + struct radv_attachment_info *att;
> + uint32_t att_idx = subpass->color_attachments[cb_idx].attachment;
> + if (att_idx == VK_ATTACHMENT_UNUSED)
> + return;
> +
> + att = &framebuffer->attachments[att_idx];
> + if (att->attachment->image != image)
> + return;
> + }
If we don't do fast clears I don't think this can be called?
>
> radeon_set_context_reg_seq(cs, R_028C8C_CB_COLOR0_CLEAR_WORD0 + cb_idx * 0x3c, 2);
> radeon_emit(cs, color_values[0]);
> @@ -2528,6 +2539,7 @@ VkResult radv_BeginCommandBuffer(
> cmd_buffer->state.last_first_instance = -1;
> cmd_buffer->state.predication_type = -1;
> cmd_buffer->usage_flags = pBeginInfo->flags;
> + cmd_buffer->inherit_unknown_fb = false;
>
> if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY &&
> (pBeginInfo->flags & VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)) {
> @@ -2535,6 +2547,9 @@ VkResult radv_BeginCommandBuffer(
> cmd_buffer->state.framebuffer = radv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
> cmd_buffer->state.pass = radv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
>
> + if (cmd_buffer->state.pass && !pBeginInfo->pInheritanceInfo->framebuffer)
> + cmd_buffer->inherit_unknown_fb = true;
> +
> struct radv_subpass *subpass =
> &cmd_buffer->state.pass->subpasses[pBeginInfo->pInheritanceInfo->subpass];
>
> @@ -2542,7 +2557,9 @@ VkResult radv_BeginCommandBuffer(
> if (result != VK_SUCCESS)
> return result;
>
> - radv_cmd_buffer_set_subpass(cmd_buffer, subpass, false);
> + cmd_buffer->state.subpass = subpass;
> + if (!cmd_buffer->inherit_unknown_fb && cmd_buffer->state.framebuffer)
> + cmd_buffer->state.dirty |= RADV_CMD_DIRTY_FRAMEBUFFER;
> }
>
> if (unlikely(cmd_buffer->device->trace_bo)) {
> @@ -3118,6 +3135,10 @@ void radv_CmdExecuteCommands(
> if (secondary->sample_positions_needed)
> primary->sample_positions_needed = true;
>
> + if (secondary->inherit_unknown_fb &&
> + primary->state.dirty & RADV_CMD_DIRTY_FRAMEBUFFER)
> + radv_emit_framebuffer_state(primary);
> +
> primary->device->ws->cs_execute_secondary(primary->cs, secondary->cs);
>
>
> diff --git a/src/amd/vulkan/radv_meta_clear.c b/src/amd/vulkan/radv_meta_clear.c
> index 7a364ec684a..d6a64906196 100644
> --- a/src/amd/vulkan/radv_meta_clear.c
> +++ b/src/amd/vulkan/radv_meta_clear.c
> @@ -950,6 +950,14 @@ radv_can_fast_clear_depth(struct radv_cmd_buffer *cmd_buffer,
> const VkClearDepthStencilValue clear_value,
> uint32_t view_mask)
> {
> + /* Framebuffer information is needed for the TC-compat bug workaround
> + * in radv_update_bound_fast_clear_ds(). This isn't available when
> + * cmd_buffer->inherit_unknown_fb is true.
> + */
> + if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> + cmd_buffer->inherit_unknown_fb && clear_value.depth == 0.0)
> + return false;
> +
I think the big problem here is that slow clears and depend on being
able to rebind the current pass. (And color fast clears depend on
knowing the framebuffer to find the DCC/CMASK)
> if (!radv_image_view_can_fast_clear(cmd_buffer->device, iview))
> return false;
>
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 9a9cd5ff935..56dfd9882d3 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1118,6 +1118,8 @@ struct radv_cmd_buffer {
> * Whether a query pool has been resetted and we have to flush caches.
> */
> bool pending_reset_query;
> +
> + bool inherit_unknown_fb;
> };
>
> struct radv_image;
> --
> 2.19.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list