Re: [Mesa-dev] [PATCH] radv: implement a workaround for VK_EXT_conditional_rendering
On 4/26/19 1:58 PM, Samuel Pitoiset wrote:
Per the Vulkan spec 1.1.107, the predicate is a 32-bit value. Though
the AMD hardware treats it as a 64-bit value which means it might
fail to discard.
I don't know why this extension has been drafted like that but this
definitely not fit with AMD. The hardware doesn't seem to support
a 32-bit value for the predicate, so we need to implement a workaround.
This fixes an issue when DXVK enables conditional rendering with RADV.
This also fixes the Sasha conditionalrender demo.
Fixes: e45ba51ea45 ("radv: add support for VK_EXT_conditional_rendering")
Reported-by: Philip Rebohle
Signed-off-by: Samuel Pitoiset
Reviewed-by: Bas Nieuwenhuizen
---
src/amd/vulkan/radv_cmd_buffer.c | 46 +---
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 7ee5a5ca7dc..9d9f2577f75 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -4932,8 +4932,11 @@ void radv_CmdBeginConditionalRenderingEXT(
{
RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
RADV_FROM_HANDLE(radv_buffer, buffer,
pConditionalRenderingBegin->buffer);
+ struct radeon_cmdbuf *cs = cmd_buffer->cs;
bool draw_visible = true;
- uint64_t va;
+ uint64_t pred_value = 0;
+ uint64_t va, new_va;
+ unsigned pred_offset;
va = radv_buffer_get_va(buffer->bo) + pConditionalRenderingBegin->offset;
@@ -4949,13 +4952,50 @@ void radv_CmdBeginConditionalRenderingEXT(
si_emit_cache_flush(cmd_buffer);
+ /* From the Vulkan spec 1.1.107:
+*
+* "If the 32-bit value at offset in buffer memory is zero, then the
+* rendering commands are discarded, otherwise they are executed as
+* normal. If the value of the predicate in buffer memory changes while
+* conditional rendering is active, the rendering commands may be
+* discarded in an implementation-dependent way. Some implementations
+* may latch the value of the predicate upon beginning conditional
+* rendering while others may read it before every rendering command."
+*
+* But, the AMD hardware treats the predicate as a 64-bit value which
+* means we need a workaround in the driver. Luckily, it's not required
+* to support if the value changes when predication is active.
+*
+* The workaround is as follows:
+* 1) allocate a 64-value in the upload BO and initialize it to 0
+* 2) copy the 32-bit predicate value to the upload BO
+* 3) use the new allocated VA address for predication
+*
+* Based on the conditionalrender demo, it's faster to do the COPY_DATA
+* in ME (+ sync PFP) instead of PFP.
+*/
+ radv_cmd_buffer_upload_data(cmd_buffer, 8, 16, _value,
_offset);
+
+ new_va = radv_buffer_get_va(cmd_buffer->upload.upload_bo) + pred_offset;
+
+ radeon_emit(cs, PKT3(PKT3_COPY_DATA, 4, 0));
+ radeon_emit(cs, COPY_DATA_SRC_SEL(COPY_DATA_SRC_MEM) |
+ COPY_DATA_DST_SEL(COPY_DATA_DST_MEM));
+ radeon_emit(cs, va);
+ radeon_emit(cs, va >> 32);
+ radeon_emit(cs, new_va);
+ radeon_emit(cs, new_va >> 32);
+
+ radeon_emit(cs, PKT3(PKT3_PFP_SYNC_ME, 0, 0));
+ radeon_emit(cs, 0);
+
/* Enable predication for this command buffer. */
- si_emit_set_predication_state(cmd_buffer, draw_visible, va);
+ si_emit_set_predication_state(cmd_buffer, draw_visible, new_va);
cmd_buffer->state.predicating = true;
/* Store conditional rendering user info. */
cmd_buffer->state.predication_type = draw_visible;
- cmd_buffer->state.predication_va = va;
+ cmd_buffer->state.predication_va = new_va;
}
void radv_CmdEndConditionalRenderingEXT(
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radv: implement a workaround for VK_EXT_conditional_rendering
Per the Vulkan spec 1.1.107, the predicate is a 32-bit value. Though
the AMD hardware treats it as a 64-bit value which means it might
fail to discard.
I don't know why this extension has been drafted like that but this
definitely not fit with AMD. The hardware doesn't seem to support
a 32-bit value for the predicate, so we need to implement a workaround.
This fixes an issue when DXVK enables conditional rendering with RADV.
Fixes: e45ba51ea45 ("radv: add support for VK_EXT_conditional_rendering")
Reported-by: Philip Rebohle
Signed-off-by: Samuel Pitoiset
Reviewed-by: Bas Nieuwenhuizen
---
src/amd/vulkan/radv_cmd_buffer.c | 46 +---
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 7ee5a5ca7dc..9d9f2577f75 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -4932,8 +4932,11 @@ void radv_CmdBeginConditionalRenderingEXT(
{
RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
RADV_FROM_HANDLE(radv_buffer, buffer,
pConditionalRenderingBegin->buffer);
+ struct radeon_cmdbuf *cs = cmd_buffer->cs;
bool draw_visible = true;
- uint64_t va;
+ uint64_t pred_value = 0;
+ uint64_t va, new_va;
+ unsigned pred_offset;
va = radv_buffer_get_va(buffer->bo) +
pConditionalRenderingBegin->offset;
@@ -4949,13 +4952,50 @@ void radv_CmdBeginConditionalRenderingEXT(
si_emit_cache_flush(cmd_buffer);
+ /* From the Vulkan spec 1.1.107:
+*
+* "If the 32-bit value at offset in buffer memory is zero, then the
+* rendering commands are discarded, otherwise they are executed as
+* normal. If the value of the predicate in buffer memory changes while
+* conditional rendering is active, the rendering commands may be
+* discarded in an implementation-dependent way. Some implementations
+* may latch the value of the predicate upon beginning conditional
+* rendering while others may read it before every rendering command."
+*
+* But, the AMD hardware treats the predicate as a 64-bit value which
+* means we need a workaround in the driver. Luckily, it's not required
+* to support if the value changes when predication is active.
+*
+* The workaround is as follows:
+* 1) allocate a 64-value in the upload BO and initialize it to 0
+* 2) copy the 32-bit predicate value to the upload BO
+* 3) use the new allocated VA address for predication
+*
+* Based on the conditionalrender demo, it's faster to do the COPY_DATA
+* in ME (+ sync PFP) instead of PFP.
+*/
+ radv_cmd_buffer_upload_data(cmd_buffer, 8, 16, _value,
_offset);
+
+ new_va = radv_buffer_get_va(cmd_buffer->upload.upload_bo) + pred_offset;
+
+ radeon_emit(cs, PKT3(PKT3_COPY_DATA, 4, 0));
+ radeon_emit(cs, COPY_DATA_SRC_SEL(COPY_DATA_SRC_MEM) |
+ COPY_DATA_DST_SEL(COPY_DATA_DST_MEM));
+ radeon_emit(cs, va);
+ radeon_emit(cs, va >> 32);
+ radeon_emit(cs, new_va);
+ radeon_emit(cs, new_va >> 32);
+
+ radeon_emit(cs, PKT3(PKT3_PFP_SYNC_ME, 0, 0));
+ radeon_emit(cs, 0);
+
/* Enable predication for this command buffer. */
- si_emit_set_predication_state(cmd_buffer, draw_visible, va);
+ si_emit_set_predication_state(cmd_buffer, draw_visible, new_va);
cmd_buffer->state.predicating = true;
/* Store conditional rendering user info. */
cmd_buffer->state.predication_type = draw_visible;
- cmd_buffer->state.predication_va = va;
+ cmd_buffer->state.predication_va = new_va;
}
void radv_CmdEndConditionalRenderingEXT(
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
