[v2,4/4] radv: add support for VK_EXT_conditional_rendering

Submitted by Samuel Pitoiset on July 9, 2018, 12:57 p.m.

Details

Message ID 20180709125709.6358-1-samuel.pitoiset@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset July 9, 2018, 12:57 p.m.
Inherited commands buffers are not supported.

v2: - disable predication for blit and copy commands

Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_cmd_buffer.c  | 29 +++++++++++++++++++++++++++++
 src/amd/vulkan/radv_device.c      |  7 +++++++
 src/amd/vulkan/radv_extensions.py |  1 +
 src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
 src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
 src/amd/vulkan/radv_meta_copy.c   | 30 ++++++++++++++++++++++++++++++
 6 files changed, 87 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 29199f2b3d..3fd8ebe2d3 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -4376,3 +4376,32 @@  void radv_CmdSetDeviceMask(VkCommandBuffer commandBuffer,
 {
    /* No-op */
 }
+
+/* VK_EXT_conditional_rendering */
+void vkCmdBeginConditionalRenderingEXT(
+	VkCommandBuffer                             commandBuffer,
+	const VkConditionalRenderingBeginInfoEXT*   pConditionalRenderingBegin)
+{
+	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
+	RADV_FROM_HANDLE(radv_buffer, buffer, pConditionalRenderingBegin->buffer);
+	bool inverted;
+	uint64_t va;
+
+	va = radv_buffer_get_va(buffer->bo) + pConditionalRenderingBegin->offset;
+
+	inverted = pConditionalRenderingBegin->flags & VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
+
+	/* Enable predication for this command buffer. */
+	si_emit_set_predication_state(cmd_buffer, inverted, va);
+	cmd_buffer->state.predicating = true;
+}
+
+void vkCmdEndConditionalRenderingEXT(
+	VkCommandBuffer                             commandBuffer)
+{
+	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
+
+	/* Disable predication for this command buffer. */
+	si_emit_set_predication_state(cmd_buffer, false, 0);
+	cmd_buffer->state.predicating = false;
+}
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index ad3465f594..06d70d305a 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -806,6 +806,13 @@  void radv_GetPhysicalDeviceFeatures2(
 			features->runtimeDescriptorArray = true;
 			break;
 		}
+		case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
+			VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
+				(VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
+			features->conditionalRendering = true;
+			features->inheritedConditionalRendering = false;
+			break;
+		}
 		default:
 			break;
 		}
diff --git a/src/amd/vulkan/radv_extensions.py b/src/amd/vulkan/radv_extensions.py
index a0f1038110..6ddbabf26e 100644
--- a/src/amd/vulkan/radv_extensions.py
+++ b/src/amd/vulkan/radv_extensions.py
@@ -89,6 +89,7 @@  EXTENSIONS = [
     Extension('VK_KHR_display',                          23, 'VK_USE_PLATFORM_DISPLAY_KHR'),
     Extension('VK_EXT_direct_mode_display',               1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
     Extension('VK_EXT_acquire_xlib_display',              1, 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
+    Extension('VK_EXT_conditional_rendering',             1, True),
     Extension('VK_EXT_display_surface_counter',           1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
     Extension('VK_EXT_display_control',                   1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
     Extension('VK_EXT_debug_report',                      9, True),
diff --git a/src/amd/vulkan/radv_meta_blit.c b/src/amd/vulkan/radv_meta_blit.c
index a6ee0cb7e9..67c26aabdb 100644
--- a/src/amd/vulkan/radv_meta_blit.c
+++ b/src/amd/vulkan/radv_meta_blit.c
@@ -520,6 +520,7 @@  void radv_CmdBlitImage(
 	RADV_FROM_HANDLE(radv_image, src_image, srcImage);
 	RADV_FROM_HANDLE(radv_image, dest_image, destImage);
 	struct radv_meta_saved_state saved_state;
+	bool old_predicating;
 
 	/* From the Vulkan 1.0 spec:
 	 *
@@ -534,6 +535,12 @@  void radv_CmdBlitImage(
 		       RADV_META_SAVE_CONSTANTS |
 		       RADV_META_SAVE_DESCRIPTORS);
 
+	/* VK_EXT_conditional_rendering says that blit commands should not be
+	 * affected by conditional rendering.
+	 */
+	old_predicating = cmd_buffer->state.predicating;
+	cmd_buffer->state.predicating = false;
+
 	for (unsigned r = 0; r < regionCount; r++) {
 		const VkImageSubresourceLayers *src_res = &pRegions[r].srcSubresource;
 		const VkImageSubresourceLayers *dst_res = &pRegions[r].dstSubresource;
@@ -648,6 +655,9 @@  void radv_CmdBlitImage(
 		}
 	}
 
+	/* Restore conditional rendering. */
+	cmd_buffer->state.predicating = old_predicating;
+
 	radv_meta_restore(&saved_state, cmd_buffer);
 }
 
diff --git a/src/amd/vulkan/radv_meta_buffer.c b/src/amd/vulkan/radv_meta_buffer.c
index 2e1ba2c7b2..a45a6d5aac 100644
--- a/src/amd/vulkan/radv_meta_buffer.c
+++ b/src/amd/vulkan/radv_meta_buffer.c
@@ -472,6 +472,13 @@  void radv_CmdCopyBuffer(
 	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
 	RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
 	RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
+	bool old_predicating;
+
+	/* VK_EXT_conditional_rendering says that copy commands should not be
+	 * affected by conditional rendering.
+	 */
+	old_predicating = cmd_buffer->state.predicating;
+	cmd_buffer->state.predicating = false;
 
 	for (unsigned r = 0; r < regionCount; r++) {
 		uint64_t src_offset = src_buffer->offset + pRegions[r].srcOffset;
@@ -481,6 +488,9 @@  void radv_CmdCopyBuffer(
 		radv_copy_buffer(cmd_buffer, src_buffer->bo, dest_buffer->bo,
 				 src_offset, dest_offset, copy_size);
 	}
+
+	/* Restore conditional rendering. */
+	cmd_buffer->state.predicating = old_predicating;
 }
 
 void radv_CmdUpdateBuffer(
diff --git a/src/amd/vulkan/radv_meta_copy.c b/src/amd/vulkan/radv_meta_copy.c
index 3442b49fb9..f4de5528ed 100644
--- a/src/amd/vulkan/radv_meta_copy.c
+++ b/src/amd/vulkan/radv_meta_copy.c
@@ -117,6 +117,7 @@  meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
 {
 	bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
 	struct radv_meta_saved_state saved_state;
+	bool old_predicating;
 
 	/* The Vulkan 1.0 spec says "dstImage must have a sample count equal to
 	 * VK_SAMPLE_COUNT_1_BIT."
@@ -129,6 +130,12 @@  meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
 		       RADV_META_SAVE_CONSTANTS |
 		       RADV_META_SAVE_DESCRIPTORS);
 
+	/* VK_EXT_conditional_rendering says that copy commands should not be
+	 * affected by conditional rendering.
+	 */
+	old_predicating = cmd_buffer->state.predicating;
+	cmd_buffer->state.predicating = false;
+
 	for (unsigned r = 0; r < regionCount; r++) {
 
 		/**
@@ -208,6 +215,9 @@  meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
 		}
 	}
 
+	/* Restore conditional rendering. */
+	cmd_buffer->state.predicating = old_predicating;
+
 	radv_meta_restore(&saved_state, cmd_buffer);
 }
 
@@ -236,12 +246,19 @@  meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
                           const VkBufferImageCopy* pRegions)
 {
 	struct radv_meta_saved_state saved_state;
+	bool old_predicating;
 
 	radv_meta_save(&saved_state, cmd_buffer,
 		       RADV_META_SAVE_COMPUTE_PIPELINE |
 		       RADV_META_SAVE_CONSTANTS |
 		       RADV_META_SAVE_DESCRIPTORS);
 
+	/* VK_EXT_conditional_rendering says that copy commands should not be
+	 * affected by conditional rendering.
+	 */
+	old_predicating = cmd_buffer->state.predicating;
+	cmd_buffer->state.predicating = false;
+
 	for (unsigned r = 0; r < regionCount; r++) {
 
 		/**
@@ -313,6 +330,9 @@  meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
 		}
 	}
 
+	/* Restore conditional rendering. */
+	cmd_buffer->state.predicating = old_predicating;
+
 	radv_meta_restore(&saved_state, cmd_buffer);
 }
 
@@ -344,6 +364,7 @@  meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
 {
 	bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
 	struct radv_meta_saved_state saved_state;
+	bool old_predicating;
 
 	/* From the Vulkan 1.0 spec:
 	 *
@@ -358,6 +379,12 @@  meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
 		       RADV_META_SAVE_CONSTANTS |
 		       RADV_META_SAVE_DESCRIPTORS);
 
+	/* VK_EXT_conditional_rendering says that copy commands should not be
+	 * affected by conditional rendering.
+	 */
+	old_predicating = cmd_buffer->state.predicating;
+	cmd_buffer->state.predicating = false;
+
 	for (unsigned r = 0; r < regionCount; r++) {
 		assert(pRegions[r].srcSubresource.aspectMask ==
 		       pRegions[r].dstSubresource.aspectMask);
@@ -465,6 +492,9 @@  meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
 		}
 	}
 
+	/* Restore conditional rendering. */
+	cmd_buffer->state.predicating = old_predicating;
+
 	radv_meta_restore(&saved_state, cmd_buffer);
 }
 

Comments

Are there any tests for this anywhere?

On Mon, Jul 9, 2018 at 5:56 AM Samuel Pitoiset <samuel.pitoiset@gmail.com>
wrote:

> Inherited commands buffers are not supported.
>
> v2: - disable predication for blit and copy commands
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c  | 29 +++++++++++++++++++++++++++++
>  src/amd/vulkan/radv_device.c      |  7 +++++++
>  src/amd/vulkan/radv_extensions.py |  1 +
>  src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
>  src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
>  src/amd/vulkan/radv_meta_copy.c   | 30 ++++++++++++++++++++++++++++++
>  6 files changed, 87 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c
> b/src/amd/vulkan/radv_cmd_buffer.c
> index 29199f2b3d..3fd8ebe2d3 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -4376,3 +4376,32 @@ void radv_CmdSetDeviceMask(VkCommandBuffer
> commandBuffer,
>  {
>     /* No-op */
>  }
> +
> +/* VK_EXT_conditional_rendering */
> +void vkCmdBeginConditionalRenderingEXT(
> +       VkCommandBuffer                             commandBuffer,
> +       const VkConditionalRenderingBeginInfoEXT*
>  pConditionalRenderingBegin)
> +{
> +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +       RADV_FROM_HANDLE(radv_buffer, buffer,
> pConditionalRenderingBegin->buffer);
> +       bool inverted;
> +       uint64_t va;
> +
> +       va = radv_buffer_get_va(buffer->bo) +
> pConditionalRenderingBegin->offset;
> +
> +       inverted = pConditionalRenderingBegin->flags &
> VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
> +
> +       /* Enable predication for this command buffer. */
> +       si_emit_set_predication_state(cmd_buffer, inverted, va);
> +       cmd_buffer->state.predicating = true;
> +}
> +
> +void vkCmdEndConditionalRenderingEXT(
> +       VkCommandBuffer                             commandBuffer)
> +{
> +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +
> +       /* Disable predication for this command buffer. */
> +       si_emit_set_predication_state(cmd_buffer, false, 0);
> +       cmd_buffer->state.predicating = false;
> +}
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index ad3465f594..06d70d305a 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -806,6 +806,13 @@ void radv_GetPhysicalDeviceFeatures2(
>                         features->runtimeDescriptorArray = true;
>                         break;
>                 }
> +               case
> VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
> +                       VkPhysicalDeviceConditionalRenderingFeaturesEXT
> *features =
> +
>  (VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
> +                       features->conditionalRendering = true;
> +                       features->inheritedConditionalRendering = false;
> +                       break;
> +               }
>                 default:
>                         break;
>                 }
> diff --git a/src/amd/vulkan/radv_extensions.py
> b/src/amd/vulkan/radv_extensions.py
> index a0f1038110..6ddbabf26e 100644
> --- a/src/amd/vulkan/radv_extensions.py
> +++ b/src/amd/vulkan/radv_extensions.py
> @@ -89,6 +89,7 @@ EXTENSIONS = [
>      Extension('VK_KHR_display',                          23,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_direct_mode_display',               1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_acquire_xlib_display',              1,
> 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
> +    Extension('VK_EXT_conditional_rendering',             1, True),
>      Extension('VK_EXT_display_surface_counter',           1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_display_control',                   1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_debug_report',                      9, True),
> diff --git a/src/amd/vulkan/radv_meta_blit.c
> b/src/amd/vulkan/radv_meta_blit.c
> index a6ee0cb7e9..67c26aabdb 100644
> --- a/src/amd/vulkan/radv_meta_blit.c
> +++ b/src/amd/vulkan/radv_meta_blit.c
> @@ -520,6 +520,7 @@ void radv_CmdBlitImage(
>         RADV_FROM_HANDLE(radv_image, src_image, srcImage);
>         RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         /* From the Vulkan 1.0 spec:
>          *
> @@ -534,6 +535,12 @@ void radv_CmdBlitImage(
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that blit commands should not
> be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>                 const VkImageSubresourceLayers *src_res =
> &pRegions[r].srcSubresource;
>                 const VkImageSubresourceLayers *dst_res =
> &pRegions[r].dstSubresource;
> @@ -648,6 +655,9 @@ void radv_CmdBlitImage(
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> diff --git a/src/amd/vulkan/radv_meta_buffer.c
> b/src/amd/vulkan/radv_meta_buffer.c
> index 2e1ba2c7b2..a45a6d5aac 100644
> --- a/src/amd/vulkan/radv_meta_buffer.c
> +++ b/src/amd/vulkan/radv_meta_buffer.c
> @@ -472,6 +472,13 @@ void radv_CmdCopyBuffer(
>         RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>         RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
>         RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
> +       bool old_predicating;
> +
> +       /* VK_EXT_conditional_rendering says that copy commands should not
> be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
>
>         for (unsigned r = 0; r < regionCount; r++) {
>                 uint64_t src_offset = src_buffer->offset +
> pRegions[r].srcOffset;
> @@ -481,6 +488,9 @@ void radv_CmdCopyBuffer(
>                 radv_copy_buffer(cmd_buffer, src_buffer->bo,
> dest_buffer->bo,
>                                  src_offset, dest_offset, copy_size);
>         }
> +
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
>  }
>
>  void radv_CmdUpdateBuffer(
> diff --git a/src/amd/vulkan/radv_meta_copy.c
> b/src/amd/vulkan/radv_meta_copy.c
> index 3442b49fb9..f4de5528ed 100644
> --- a/src/amd/vulkan/radv_meta_copy.c
> +++ b/src/amd/vulkan/radv_meta_copy.c
> @@ -117,6 +117,7 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
> *cmd_buffer,
>  {
>         bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         /* The Vulkan 1.0 spec says "dstImage must have a sample count
> equal to
>          * VK_SAMPLE_COUNT_1_BIT."
> @@ -129,6 +130,12 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
> *cmd_buffer,
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that copy commands should not
> be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>
>                 /**
> @@ -208,6 +215,9 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
> *cmd_buffer,
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> @@ -236,12 +246,19 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer
> *cmd_buffer,
>                            const VkBufferImageCopy* pRegions)
>  {
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         radv_meta_save(&saved_state, cmd_buffer,
>                        RADV_META_SAVE_COMPUTE_PIPELINE |
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that copy commands should not
> be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>
>                 /**
> @@ -313,6 +330,9 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer
> *cmd_buffer,
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> @@ -344,6 +364,7 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>  {
>         bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         /* From the Vulkan 1.0 spec:
>          *
> @@ -358,6 +379,12 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that copy commands should not
> be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>                 assert(pRegions[r].srcSubresource.aspectMask ==
>                        pRegions[r].dstSubresource.aspectMask);
> @@ -465,6 +492,9 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
On 07/09/2018 05:28 PM, Jason Ekstrand wrote:
> Are there any tests for this anywhere?

Unfortunately, not yet.

> 
> On Mon, Jul 9, 2018 at 5:56 AM Samuel Pitoiset 
> <samuel.pitoiset@gmail.com <mailto:samuel.pitoiset@gmail.com>> wrote:
> 
>     Inherited commands buffers are not supported.
> 
>     v2: - disable predication for blit and copy commands
> 
>     Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com
>     <mailto:samuel.pitoiset@gmail.com>>
>     ---
>       src/amd/vulkan/radv_cmd_buffer.c  | 29 +++++++++++++++++++++++++++++
>       src/amd/vulkan/radv_device.c      |  7 +++++++
>       src/amd/vulkan/radv_extensions.py |  1 +
>       src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
>       src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
>       src/amd/vulkan/radv_meta_copy.c   | 30 ++++++++++++++++++++++++++++++
>       6 files changed, 87 insertions(+)
> 
>     diff --git a/src/amd/vulkan/radv_cmd_buffer.c
>     b/src/amd/vulkan/radv_cmd_buffer.c
>     index 29199f2b3d..3fd8ebe2d3 100644
>     --- a/src/amd/vulkan/radv_cmd_buffer.c
>     +++ b/src/amd/vulkan/radv_cmd_buffer.c
>     @@ -4376,3 +4376,32 @@ void radv_CmdSetDeviceMask(VkCommandBuffer
>     commandBuffer,
>       {
>          /* No-op */
>       }
>     +
>     +/* VK_EXT_conditional_rendering */
>     +void vkCmdBeginConditionalRenderingEXT(
>     +       VkCommandBuffer                             commandBuffer,
>     +       const VkConditionalRenderingBeginInfoEXT* 
>       pConditionalRenderingBegin)
>     +{
>     +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>     +       RADV_FROM_HANDLE(radv_buffer, buffer,
>     pConditionalRenderingBegin->buffer);
>     +       bool inverted;
>     +       uint64_t va;
>     +
>     +       va = radv_buffer_get_va(buffer->bo) +
>     pConditionalRenderingBegin->offset;
>     +
>     +       inverted = pConditionalRenderingBegin->flags &
>     VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
>     +
>     +       /* Enable predication for this command buffer. */
>     +       si_emit_set_predication_state(cmd_buffer, inverted, va);
>     +       cmd_buffer->state.predicating = true;
>     +}
>     +
>     +void vkCmdEndConditionalRenderingEXT(
>     +       VkCommandBuffer                             commandBuffer)
>     +{
>     +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>     +
>     +       /* Disable predication for this command buffer. */
>     +       si_emit_set_predication_state(cmd_buffer, false, 0);
>     +       cmd_buffer->state.predicating = false;
>     +}
>     diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
>     index ad3465f594..06d70d305a 100644
>     --- a/src/amd/vulkan/radv_device.c
>     +++ b/src/amd/vulkan/radv_device.c
>     @@ -806,6 +806,13 @@ void radv_GetPhysicalDeviceFeatures2(
>                              features->runtimeDescriptorArray = true;
>                              break;
>                      }
>     +               case
>     VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
>     +                     
>       VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
>     +                             
>       (VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
>     +                       features->conditionalRendering = true;
>     +                       features->inheritedConditionalRendering = false;
>     +                       break;
>     +               }
>                      default:
>                              break;
>                      }
>     diff --git a/src/amd/vulkan/radv_extensions.py
>     b/src/amd/vulkan/radv_extensions.py
>     index a0f1038110..6ddbabf26e 100644
>     --- a/src/amd/vulkan/radv_extensions.py
>     +++ b/src/amd/vulkan/radv_extensions.py
>     @@ -89,6 +89,7 @@ EXTENSIONS = [
>           Extension('VK_KHR_display',                          23,
>     'VK_USE_PLATFORM_DISPLAY_KHR'),
>           Extension('VK_EXT_direct_mode_display',               1,
>     'VK_USE_PLATFORM_DISPLAY_KHR'),
>           Extension('VK_EXT_acquire_xlib_display',              1,
>     'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
>     +    Extension('VK_EXT_conditional_rendering',             1, True),
>           Extension('VK_EXT_display_surface_counter',           1,
>     'VK_USE_PLATFORM_DISPLAY_KHR'),
>           Extension('VK_EXT_display_control',                   1,
>     'VK_USE_PLATFORM_DISPLAY_KHR'),
>           Extension('VK_EXT_debug_report',                      9, True),
>     diff --git a/src/amd/vulkan/radv_meta_blit.c
>     b/src/amd/vulkan/radv_meta_blit.c
>     index a6ee0cb7e9..67c26aabdb 100644
>     --- a/src/amd/vulkan/radv_meta_blit.c
>     +++ b/src/amd/vulkan/radv_meta_blit.c
>     @@ -520,6 +520,7 @@ void radv_CmdBlitImage(
>              RADV_FROM_HANDLE(radv_image, src_image, srcImage);
>              RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>              struct radv_meta_saved_state saved_state;
>     +       bool old_predicating;
> 
>              /* From the Vulkan 1.0 spec:
>               *
>     @@ -534,6 +535,12 @@ void radv_CmdBlitImage(
>                             RADV_META_SAVE_CONSTANTS |
>                             RADV_META_SAVE_DESCRIPTORS);
> 
>     +       /* VK_EXT_conditional_rendering says that blit commands
>     should not be
>     +        * affected by conditional rendering.
>     +        */
>     +       old_predicating = cmd_buffer->state.predicating;
>     +       cmd_buffer->state.predicating = false;
>     +
>              for (unsigned r = 0; r < regionCount; r++) {
>                      const VkImageSubresourceLayers *src_res =
>     &pRegions[r].srcSubresource;
>                      const VkImageSubresourceLayers *dst_res =
>     &pRegions[r].dstSubresource;
>     @@ -648,6 +655,9 @@ void radv_CmdBlitImage(
>                      }
>              }
> 
>     +       /* Restore conditional rendering. */
>     +       cmd_buffer->state.predicating = old_predicating;
>     +
>              radv_meta_restore(&saved_state, cmd_buffer);
>       }
> 
>     diff --git a/src/amd/vulkan/radv_meta_buffer.c
>     b/src/amd/vulkan/radv_meta_buffer.c
>     index 2e1ba2c7b2..a45a6d5aac 100644
>     --- a/src/amd/vulkan/radv_meta_buffer.c
>     +++ b/src/amd/vulkan/radv_meta_buffer.c
>     @@ -472,6 +472,13 @@ void radv_CmdCopyBuffer(
>              RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>              RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
>              RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
>     +       bool old_predicating;
>     +
>     +       /* VK_EXT_conditional_rendering says that copy commands
>     should not be
>     +        * affected by conditional rendering.
>     +        */
>     +       old_predicating = cmd_buffer->state.predicating;
>     +       cmd_buffer->state.predicating = false;
> 
>              for (unsigned r = 0; r < regionCount; r++) {
>                      uint64_t src_offset = src_buffer->offset +
>     pRegions[r].srcOffset;
>     @@ -481,6 +488,9 @@ void radv_CmdCopyBuffer(
>                      radv_copy_buffer(cmd_buffer, src_buffer->bo,
>     dest_buffer->bo,
>                                       src_offset, dest_offset, copy_size);
>              }
>     +
>     +       /* Restore conditional rendering. */
>     +       cmd_buffer->state.predicating = old_predicating;
>       }
> 
>       void radv_CmdUpdateBuffer(
>     diff --git a/src/amd/vulkan/radv_meta_copy.c
>     b/src/amd/vulkan/radv_meta_copy.c
>     index 3442b49fb9..f4de5528ed 100644
>     --- a/src/amd/vulkan/radv_meta_copy.c
>     +++ b/src/amd/vulkan/radv_meta_copy.c
>     @@ -117,6 +117,7 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
>     *cmd_buffer,
>       {
>              bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>              struct radv_meta_saved_state saved_state;
>     +       bool old_predicating;
> 
>              /* The Vulkan 1.0 spec says "dstImage must have a sample
>     count equal to
>               * VK_SAMPLE_COUNT_1_BIT."
>     @@ -129,6 +130,12 @@ meta_copy_buffer_to_image(struct
>     radv_cmd_buffer *cmd_buffer,
>                             RADV_META_SAVE_CONSTANTS |
>                             RADV_META_SAVE_DESCRIPTORS);
> 
>     +       /* VK_EXT_conditional_rendering says that copy commands
>     should not be
>     +        * affected by conditional rendering.
>     +        */
>     +       old_predicating = cmd_buffer->state.predicating;
>     +       cmd_buffer->state.predicating = false;
>     +
>              for (unsigned r = 0; r < regionCount; r++) {
> 
>                      /**
>     @@ -208,6 +215,9 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
>     *cmd_buffer,
>                      }
>              }
> 
>     +       /* Restore conditional rendering. */
>     +       cmd_buffer->state.predicating = old_predicating;
>     +
>              radv_meta_restore(&saved_state, cmd_buffer);
>       }
> 
>     @@ -236,12 +246,19 @@ meta_copy_image_to_buffer(struct
>     radv_cmd_buffer *cmd_buffer,
>                                 const VkBufferImageCopy* pRegions)
>       {
>              struct radv_meta_saved_state saved_state;
>     +       bool old_predicating;
> 
>              radv_meta_save(&saved_state, cmd_buffer,
>                             RADV_META_SAVE_COMPUTE_PIPELINE |
>                             RADV_META_SAVE_CONSTANTS |
>                             RADV_META_SAVE_DESCRIPTORS);
> 
>     +       /* VK_EXT_conditional_rendering says that copy commands
>     should not be
>     +        * affected by conditional rendering.
>     +        */
>     +       old_predicating = cmd_buffer->state.predicating;
>     +       cmd_buffer->state.predicating = false;
>     +
>              for (unsigned r = 0; r < regionCount; r++) {
> 
>                      /**
>     @@ -313,6 +330,9 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer
>     *cmd_buffer,
>                      }
>              }
> 
>     +       /* Restore conditional rendering. */
>     +       cmd_buffer->state.predicating = old_predicating;
>     +
>              radv_meta_restore(&saved_state, cmd_buffer);
>       }
> 
>     @@ -344,6 +364,7 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>       {
>              bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>              struct radv_meta_saved_state saved_state;
>     +       bool old_predicating;
> 
>              /* From the Vulkan 1.0 spec:
>               *
>     @@ -358,6 +379,12 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                             RADV_META_SAVE_CONSTANTS |
>                             RADV_META_SAVE_DESCRIPTORS);
> 
>     +       /* VK_EXT_conditional_rendering says that copy commands
>     should not be
>     +        * affected by conditional rendering.
>     +        */
>     +       old_predicating = cmd_buffer->state.predicating;
>     +       cmd_buffer->state.predicating = false;
>     +
>              for (unsigned r = 0; r < regionCount; r++) {
>                      assert(pRegions[r].srcSubresource.aspectMask ==
>                             pRegions[r].dstSubresource.aspectMask);
>     @@ -465,6 +492,9 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                      }
>              }
> 
>     +       /* Restore conditional rendering. */
>     +       cmd_buffer->state.predicating = old_predicating;
>     +
>              radv_meta_restore(&saved_state, cmd_buffer);
>       }
> 
>     -- 
>     2.18.0
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
On Mon, Jul 9, 2018 at 8:29 AM Samuel Pitoiset <samuel.pitoiset@gmail.com>
wrote:

>
> On 07/09/2018 05:28 PM, Jason Ekstrand wrote:
> > Are there any tests for this anywhere?
>
> Unfortunately, not yet.
>

Someone should write some then. :-)  I recently wrote tests for
VK_EXT_vertex_attribute_divisor and we discovered that basically no one
(except RADV interestingly) implements the behavior as-written in the spec
and Intel HW can't implemnet it as-written at all.  It's good to have
tests. :)

--Jason


> >
> > On Mon, Jul 9, 2018 at 5:56 AM Samuel Pitoiset
> > <samuel.pitoiset@gmail.com <mailto:samuel.pitoiset@gmail.com>> wrote:
> >
> >     Inherited commands buffers are not supported.
> >
> >     v2: - disable predication for blit and copy commands
> >
> >     Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com
> >     <mailto:samuel.pitoiset@gmail.com>>
> >     ---
> >       src/amd/vulkan/radv_cmd_buffer.c  | 29
> +++++++++++++++++++++++++++++
> >       src/amd/vulkan/radv_device.c      |  7 +++++++
> >       src/amd/vulkan/radv_extensions.py |  1 +
> >       src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
> >       src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
> >       src/amd/vulkan/radv_meta_copy.c   | 30
> ++++++++++++++++++++++++++++++
> >       6 files changed, 87 insertions(+)
> >
> >     diff --git a/src/amd/vulkan/radv_cmd_buffer.c
> >     b/src/amd/vulkan/radv_cmd_buffer.c
> >     index 29199f2b3d..3fd8ebe2d3 100644
> >     --- a/src/amd/vulkan/radv_cmd_buffer.c
> >     +++ b/src/amd/vulkan/radv_cmd_buffer.c
> >     @@ -4376,3 +4376,32 @@ void radv_CmdSetDeviceMask(VkCommandBuffer
> >     commandBuffer,
> >       {
> >          /* No-op */
> >       }
> >     +
> >     +/* VK_EXT_conditional_rendering */
> >     +void vkCmdBeginConditionalRenderingEXT(
> >     +       VkCommandBuffer                             commandBuffer,
> >     +       const VkConditionalRenderingBeginInfoEXT*
> >       pConditionalRenderingBegin)
> >     +{
> >     +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> >     +       RADV_FROM_HANDLE(radv_buffer, buffer,
> >     pConditionalRenderingBegin->buffer);
> >     +       bool inverted;
> >     +       uint64_t va;
> >     +
> >     +       va = radv_buffer_get_va(buffer->bo) +
> >     pConditionalRenderingBegin->offset;
> >     +
> >     +       inverted = pConditionalRenderingBegin->flags &
> >     VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
> >     +
> >     +       /* Enable predication for this command buffer. */
> >     +       si_emit_set_predication_state(cmd_buffer, inverted, va);
> >     +       cmd_buffer->state.predicating = true;
> >     +}
> >     +
> >     +void vkCmdEndConditionalRenderingEXT(
> >     +       VkCommandBuffer                             commandBuffer)
> >     +{
> >     +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> >     +
> >     +       /* Disable predication for this command buffer. */
> >     +       si_emit_set_predication_state(cmd_buffer, false, 0);
> >     +       cmd_buffer->state.predicating = false;
> >     +}
> >     diff --git a/src/amd/vulkan/radv_device.c
> b/src/amd/vulkan/radv_device.c
> >     index ad3465f594..06d70d305a 100644
> >     --- a/src/amd/vulkan/radv_device.c
> >     +++ b/src/amd/vulkan/radv_device.c
> >     @@ -806,6 +806,13 @@ void radv_GetPhysicalDeviceFeatures2(
> >                              features->runtimeDescriptorArray = true;
> >                              break;
> >                      }
> >     +               case
> >
>  VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
> >     +
> >       VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
> >     +
> >       (VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
> >     +                       features->conditionalRendering = true;
> >     +                       features->inheritedConditionalRendering =
> false;
> >     +                       break;
> >     +               }
> >                      default:
> >                              break;
> >                      }
> >     diff --git a/src/amd/vulkan/radv_extensions.py
> >     b/src/amd/vulkan/radv_extensions.py
> >     index a0f1038110..6ddbabf26e 100644
> >     --- a/src/amd/vulkan/radv_extensions.py
> >     +++ b/src/amd/vulkan/radv_extensions.py
> >     @@ -89,6 +89,7 @@ EXTENSIONS = [
> >           Extension('VK_KHR_display',                          23,
> >     'VK_USE_PLATFORM_DISPLAY_KHR'),
> >           Extension('VK_EXT_direct_mode_display',               1,
> >     'VK_USE_PLATFORM_DISPLAY_KHR'),
> >           Extension('VK_EXT_acquire_xlib_display',              1,
> >     'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
> >     +    Extension('VK_EXT_conditional_rendering',             1, True),
> >           Extension('VK_EXT_display_surface_counter',           1,
> >     'VK_USE_PLATFORM_DISPLAY_KHR'),
> >           Extension('VK_EXT_display_control',                   1,
> >     'VK_USE_PLATFORM_DISPLAY_KHR'),
> >           Extension('VK_EXT_debug_report',                      9, True),
> >     diff --git a/src/amd/vulkan/radv_meta_blit.c
> >     b/src/amd/vulkan/radv_meta_blit.c
> >     index a6ee0cb7e9..67c26aabdb 100644
> >     --- a/src/amd/vulkan/radv_meta_blit.c
> >     +++ b/src/amd/vulkan/radv_meta_blit.c
> >     @@ -520,6 +520,7 @@ void radv_CmdBlitImage(
> >              RADV_FROM_HANDLE(radv_image, src_image, srcImage);
> >              RADV_FROM_HANDLE(radv_image, dest_image, destImage);
> >              struct radv_meta_saved_state saved_state;
> >     +       bool old_predicating;
> >
> >              /* From the Vulkan 1.0 spec:
> >               *
> >     @@ -534,6 +535,12 @@ void radv_CmdBlitImage(
> >                             RADV_META_SAVE_CONSTANTS |
> >                             RADV_META_SAVE_DESCRIPTORS);
> >
> >     +       /* VK_EXT_conditional_rendering says that blit commands
> >     should not be
> >     +        * affected by conditional rendering.
> >     +        */
> >     +       old_predicating = cmd_buffer->state.predicating;
> >     +       cmd_buffer->state.predicating = false;
> >     +
> >              for (unsigned r = 0; r < regionCount; r++) {
> >                      const VkImageSubresourceLayers *src_res =
> >     &pRegions[r].srcSubresource;
> >                      const VkImageSubresourceLayers *dst_res =
> >     &pRegions[r].dstSubresource;
> >     @@ -648,6 +655,9 @@ void radv_CmdBlitImage(
> >                      }
> >              }
> >
> >     +       /* Restore conditional rendering. */
> >     +       cmd_buffer->state.predicating = old_predicating;
> >     +
> >              radv_meta_restore(&saved_state, cmd_buffer);
> >       }
> >
> >     diff --git a/src/amd/vulkan/radv_meta_buffer.c
> >     b/src/amd/vulkan/radv_meta_buffer.c
> >     index 2e1ba2c7b2..a45a6d5aac 100644
> >     --- a/src/amd/vulkan/radv_meta_buffer.c
> >     +++ b/src/amd/vulkan/radv_meta_buffer.c
> >     @@ -472,6 +472,13 @@ void radv_CmdCopyBuffer(
> >              RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer,
> commandBuffer);
> >              RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
> >              RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
> >     +       bool old_predicating;
> >     +
> >     +       /* VK_EXT_conditional_rendering says that copy commands
> >     should not be
> >     +        * affected by conditional rendering.
> >     +        */
> >     +       old_predicating = cmd_buffer->state.predicating;
> >     +       cmd_buffer->state.predicating = false;
> >
> >              for (unsigned r = 0; r < regionCount; r++) {
> >                      uint64_t src_offset = src_buffer->offset +
> >     pRegions[r].srcOffset;
> >     @@ -481,6 +488,9 @@ void radv_CmdCopyBuffer(
> >                      radv_copy_buffer(cmd_buffer, src_buffer->bo,
> >     dest_buffer->bo,
> >                                       src_offset, dest_offset,
> copy_size);
> >              }
> >     +
> >     +       /* Restore conditional rendering. */
> >     +       cmd_buffer->state.predicating = old_predicating;
> >       }
> >
> >       void radv_CmdUpdateBuffer(
> >     diff --git a/src/amd/vulkan/radv_meta_copy.c
> >     b/src/amd/vulkan/radv_meta_copy.c
> >     index 3442b49fb9..f4de5528ed 100644
> >     --- a/src/amd/vulkan/radv_meta_copy.c
> >     +++ b/src/amd/vulkan/radv_meta_copy.c
> >     @@ -117,6 +117,7 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
> >     *cmd_buffer,
> >       {
> >              bool cs = cmd_buffer->queue_family_index ==
> RADV_QUEUE_COMPUTE;
> >              struct radv_meta_saved_state saved_state;
> >     +       bool old_predicating;
> >
> >              /* The Vulkan 1.0 spec says "dstImage must have a sample
> >     count equal to
> >               * VK_SAMPLE_COUNT_1_BIT."
> >     @@ -129,6 +130,12 @@ meta_copy_buffer_to_image(struct
> >     radv_cmd_buffer *cmd_buffer,
> >                             RADV_META_SAVE_CONSTANTS |
> >                             RADV_META_SAVE_DESCRIPTORS);
> >
> >     +       /* VK_EXT_conditional_rendering says that copy commands
> >     should not be
> >     +        * affected by conditional rendering.
> >     +        */
> >     +       old_predicating = cmd_buffer->state.predicating;
> >     +       cmd_buffer->state.predicating = false;
> >     +
> >              for (unsigned r = 0; r < regionCount; r++) {
> >
> >                      /**
> >     @@ -208,6 +215,9 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer
> >     *cmd_buffer,
> >                      }
> >              }
> >
> >     +       /* Restore conditional rendering. */
> >     +       cmd_buffer->state.predicating = old_predicating;
> >     +
> >              radv_meta_restore(&saved_state, cmd_buffer);
> >       }
> >
> >     @@ -236,12 +246,19 @@ meta_copy_image_to_buffer(struct
> >     radv_cmd_buffer *cmd_buffer,
> >                                 const VkBufferImageCopy* pRegions)
> >       {
> >              struct radv_meta_saved_state saved_state;
> >     +       bool old_predicating;
> >
> >              radv_meta_save(&saved_state, cmd_buffer,
> >                             RADV_META_SAVE_COMPUTE_PIPELINE |
> >                             RADV_META_SAVE_CONSTANTS |
> >                             RADV_META_SAVE_DESCRIPTORS);
> >
> >     +       /* VK_EXT_conditional_rendering says that copy commands
> >     should not be
> >     +        * affected by conditional rendering.
> >     +        */
> >     +       old_predicating = cmd_buffer->state.predicating;
> >     +       cmd_buffer->state.predicating = false;
> >     +
> >              for (unsigned r = 0; r < regionCount; r++) {
> >
> >                      /**
> >     @@ -313,6 +330,9 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer
> >     *cmd_buffer,
> >                      }
> >              }
> >
> >     +       /* Restore conditional rendering. */
> >     +       cmd_buffer->state.predicating = old_predicating;
> >     +
> >              radv_meta_restore(&saved_state, cmd_buffer);
> >       }
> >
> >     @@ -344,6 +364,7 @@ meta_copy_image(struct radv_cmd_buffer
> *cmd_buffer,
> >       {
> >              bool cs = cmd_buffer->queue_family_index ==
> RADV_QUEUE_COMPUTE;
> >              struct radv_meta_saved_state saved_state;
> >     +       bool old_predicating;
> >
> >              /* From the Vulkan 1.0 spec:
> >               *
> >     @@ -358,6 +379,12 @@ meta_copy_image(struct radv_cmd_buffer
> *cmd_buffer,
> >                             RADV_META_SAVE_CONSTANTS |
> >                             RADV_META_SAVE_DESCRIPTORS);
> >
> >     +       /* VK_EXT_conditional_rendering says that copy commands
> >     should not be
> >     +        * affected by conditional rendering.
> >     +        */
> >     +       old_predicating = cmd_buffer->state.predicating;
> >     +       cmd_buffer->state.predicating = false;
> >     +
> >              for (unsigned r = 0; r < regionCount; r++) {
> >                      assert(pRegions[r].srcSubresource.aspectMask ==
> >                             pRegions[r].dstSubresource.aspectMask);
> >     @@ -465,6 +492,9 @@ meta_copy_image(struct radv_cmd_buffer
> *cmd_buffer,
> >                      }
> >              }
> >
> >     +       /* Restore conditional rendering. */
> >     +       cmd_buffer->state.predicating = old_predicating;
> >     +
> >              radv_meta_restore(&saved_state, cmd_buffer);
> >       }
> >
> >     --
> >     2.18.0
> >
> >     _______________________________________________
> >     mesa-dev mailing list
> >     mesa-dev@lists.freedesktop.org <mailto:
> mesa-dev@lists.freedesktop.org>
> >     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
On 07/09/2018 06:10 PM, Jason Ekstrand wrote:
> On Mon, Jul 9, 2018 at 8:29 AM Samuel Pitoiset 
> <samuel.pitoiset@gmail.com <mailto:samuel.pitoiset@gmail.com>> wrote:
> 
> 
>     On 07/09/2018 05:28 PM, Jason Ekstrand wrote:
>      > Are there any tests for this anywhere?
> 
>     Unfortunately, not yet.
> 
> 
> Someone should write some then. :-)  I recently wrote tests for 
> VK_EXT_vertex_attribute_divisor and we discovered that basically no one 
> (except RADV interestingly) implements the behavior as-written in the 
> spec and Intel HW can't implemnet it as-written at all.  It's good to 
> have tests. :)

I can't say the opposite of course. :)

IIRC, Bas wrote a crucidible test when he implemented 
VK_EXT_vertex_attribute_divisor.

> 
> --Jason
> 
>      >
>      > On Mon, Jul 9, 2018 at 5:56 AM Samuel Pitoiset
>      > <samuel.pitoiset@gmail.com <mailto:samuel.pitoiset@gmail.com>
>     <mailto:samuel.pitoiset@gmail.com
>     <mailto:samuel.pitoiset@gmail.com>>> wrote:
>      >
>      >     Inherited commands buffers are not supported.
>      >
>      >     v2: - disable predication for blit and copy commands
>      >
>      >     Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com
>     <mailto:samuel.pitoiset@gmail.com>
>      >     <mailto:samuel.pitoiset@gmail.com
>     <mailto:samuel.pitoiset@gmail.com>>>
>      >     ---
>      >       src/amd/vulkan/radv_cmd_buffer.c  | 29
>     +++++++++++++++++++++++++++++
>      >       src/amd/vulkan/radv_device.c      |  7 +++++++
>      >       src/amd/vulkan/radv_extensions.py |  1 +
>      >       src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
>      >       src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
>      >       src/amd/vulkan/radv_meta_copy.c   | 30
>     ++++++++++++++++++++++++++++++
>      >       6 files changed, 87 insertions(+)
>      >
>      >     diff --git a/src/amd/vulkan/radv_cmd_buffer.c
>      >     b/src/amd/vulkan/radv_cmd_buffer.c
>      >     index 29199f2b3d..3fd8ebe2d3 100644
>      >     --- a/src/amd/vulkan/radv_cmd_buffer.c
>      >     +++ b/src/amd/vulkan/radv_cmd_buffer.c
>      >     @@ -4376,3 +4376,32 @@ void radv_CmdSetDeviceMask(VkCommandBuffer
>      >     commandBuffer,
>      >       {
>      >          /* No-op */
>      >       }
>      >     +
>      >     +/* VK_EXT_conditional_rendering */
>      >     +void vkCmdBeginConditionalRenderingEXT(
>      >     +       VkCommandBuffer                           
>       commandBuffer,
>      >     +       const VkConditionalRenderingBeginInfoEXT*
>      >       pConditionalRenderingBegin)
>      >     +{
>      >     +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer,
>     commandBuffer);
>      >     +       RADV_FROM_HANDLE(radv_buffer, buffer,
>      >     pConditionalRenderingBegin->buffer);
>      >     +       bool inverted;
>      >     +       uint64_t va;
>      >     +
>      >     +       va = radv_buffer_get_va(buffer->bo) +
>      >     pConditionalRenderingBegin->offset;
>      >     +
>      >     +       inverted = pConditionalRenderingBegin->flags &
>      >     VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
>      >     +
>      >     +       /* Enable predication for this command buffer. */
>      >     +       si_emit_set_predication_state(cmd_buffer, inverted, va);
>      >     +       cmd_buffer->state.predicating = true;
>      >     +}
>      >     +
>      >     +void vkCmdEndConditionalRenderingEXT(
>      >     +       VkCommandBuffer                           
>       commandBuffer)
>      >     +{
>      >     +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer,
>     commandBuffer);
>      >     +
>      >     +       /* Disable predication for this command buffer. */
>      >     +       si_emit_set_predication_state(cmd_buffer, false, 0);
>      >     +       cmd_buffer->state.predicating = false;
>      >     +}
>      >     diff --git a/src/amd/vulkan/radv_device.c
>     b/src/amd/vulkan/radv_device.c
>      >     index ad3465f594..06d70d305a 100644
>      >     --- a/src/amd/vulkan/radv_device.c
>      >     +++ b/src/amd/vulkan/radv_device.c
>      >     @@ -806,6 +806,13 @@ void radv_GetPhysicalDeviceFeatures2(
>      >                              features->runtimeDescriptorArray = true;
>      >                              break;
>      >                      }
>      >     +               case
>      >   
>       VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
>      >     +
>      >       VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
>      >     +
>      >       (VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
>      >     +                       features->conditionalRendering = true;
>      >     +                     
>       features->inheritedConditionalRendering = false;
>      >     +                       break;
>      >     +               }
>      >                      default:
>      >                              break;
>      >                      }
>      >     diff --git a/src/amd/vulkan/radv_extensions.py
>      >     b/src/amd/vulkan/radv_extensions.py
>      >     index a0f1038110..6ddbabf26e 100644
>      >     --- a/src/amd/vulkan/radv_extensions.py
>      >     +++ b/src/amd/vulkan/radv_extensions.py
>      >     @@ -89,6 +89,7 @@ EXTENSIONS = [
>      >           Extension('VK_KHR_display',                          23,
>      >     'VK_USE_PLATFORM_DISPLAY_KHR'),
>      >           Extension('VK_EXT_direct_mode_display',               1,
>      >     'VK_USE_PLATFORM_DISPLAY_KHR'),
>      >           Extension('VK_EXT_acquire_xlib_display',              1,
>      >     'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
>      >     +    Extension('VK_EXT_conditional_rendering',             1,
>     True),
>      >           Extension('VK_EXT_display_surface_counter',           1,
>      >     'VK_USE_PLATFORM_DISPLAY_KHR'),
>      >           Extension('VK_EXT_display_control',                   1,
>      >     'VK_USE_PLATFORM_DISPLAY_KHR'),
>      >           Extension('VK_EXT_debug_report',                     
>     9, True),
>      >     diff --git a/src/amd/vulkan/radv_meta_blit.c
>      >     b/src/amd/vulkan/radv_meta_blit.c
>      >     index a6ee0cb7e9..67c26aabdb 100644
>      >     --- a/src/amd/vulkan/radv_meta_blit.c
>      >     +++ b/src/amd/vulkan/radv_meta_blit.c
>      >     @@ -520,6 +520,7 @@ void radv_CmdBlitImage(
>      >              RADV_FROM_HANDLE(radv_image, src_image, srcImage);
>      >              RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>      >              struct radv_meta_saved_state saved_state;
>      >     +       bool old_predicating;
>      >
>      >              /* From the Vulkan 1.0 spec:
>      >               *
>      >     @@ -534,6 +535,12 @@ void radv_CmdBlitImage(
>      >                             RADV_META_SAVE_CONSTANTS |
>      >                             RADV_META_SAVE_DESCRIPTORS);
>      >
>      >     +       /* VK_EXT_conditional_rendering says that blit commands
>      >     should not be
>      >     +        * affected by conditional rendering.
>      >     +        */
>      >     +       old_predicating = cmd_buffer->state.predicating;
>      >     +       cmd_buffer->state.predicating = false;
>      >     +
>      >              for (unsigned r = 0; r < regionCount; r++) {
>      >                      const VkImageSubresourceLayers *src_res =
>      >     &pRegions[r].srcSubresource;
>      >                      const VkImageSubresourceLayers *dst_res =
>      >     &pRegions[r].dstSubresource;
>      >     @@ -648,6 +655,9 @@ void radv_CmdBlitImage(
>      >                      }
>      >              }
>      >
>      >     +       /* Restore conditional rendering. */
>      >     +       cmd_buffer->state.predicating = old_predicating;
>      >     +
>      >              radv_meta_restore(&saved_state, cmd_buffer);
>      >       }
>      >
>      >     diff --git a/src/amd/vulkan/radv_meta_buffer.c
>      >     b/src/amd/vulkan/radv_meta_buffer.c
>      >     index 2e1ba2c7b2..a45a6d5aac 100644
>      >     --- a/src/amd/vulkan/radv_meta_buffer.c
>      >     +++ b/src/amd/vulkan/radv_meta_buffer.c
>      >     @@ -472,6 +472,13 @@ void radv_CmdCopyBuffer(
>      >              RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer,
>     commandBuffer);
>      >              RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
>      >              RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
>      >     +       bool old_predicating;
>      >     +
>      >     +       /* VK_EXT_conditional_rendering says that copy commands
>      >     should not be
>      >     +        * affected by conditional rendering.
>      >     +        */
>      >     +       old_predicating = cmd_buffer->state.predicating;
>      >     +       cmd_buffer->state.predicating = false;
>      >
>      >              for (unsigned r = 0; r < regionCount; r++) {
>      >                      uint64_t src_offset = src_buffer->offset +
>      >     pRegions[r].srcOffset;
>      >     @@ -481,6 +488,9 @@ void radv_CmdCopyBuffer(
>      >                      radv_copy_buffer(cmd_buffer, src_buffer->bo,
>      >     dest_buffer->bo,
>      >                                       src_offset, dest_offset,
>     copy_size);
>      >              }
>      >     +
>      >     +       /* Restore conditional rendering. */
>      >     +       cmd_buffer->state.predicating = old_predicating;
>      >       }
>      >
>      >       void radv_CmdUpdateBuffer(
>      >     diff --git a/src/amd/vulkan/radv_meta_copy.c
>      >     b/src/amd/vulkan/radv_meta_copy.c
>      >     index 3442b49fb9..f4de5528ed 100644
>      >     --- a/src/amd/vulkan/radv_meta_copy.c
>      >     +++ b/src/amd/vulkan/radv_meta_copy.c
>      >     @@ -117,6 +117,7 @@ meta_copy_buffer_to_image(struct
>     radv_cmd_buffer
>      >     *cmd_buffer,
>      >       {
>      >              bool cs = cmd_buffer->queue_family_index ==
>     RADV_QUEUE_COMPUTE;
>      >              struct radv_meta_saved_state saved_state;
>      >     +       bool old_predicating;
>      >
>      >              /* The Vulkan 1.0 spec says "dstImage must have a sample
>      >     count equal to
>      >               * VK_SAMPLE_COUNT_1_BIT."
>      >     @@ -129,6 +130,12 @@ meta_copy_buffer_to_image(struct
>      >     radv_cmd_buffer *cmd_buffer,
>      >                             RADV_META_SAVE_CONSTANTS |
>      >                             RADV_META_SAVE_DESCRIPTORS);
>      >
>      >     +       /* VK_EXT_conditional_rendering says that copy commands
>      >     should not be
>      >     +        * affected by conditional rendering.
>      >     +        */
>      >     +       old_predicating = cmd_buffer->state.predicating;
>      >     +       cmd_buffer->state.predicating = false;
>      >     +
>      >              for (unsigned r = 0; r < regionCount; r++) {
>      >
>      >                      /**
>      >     @@ -208,6 +215,9 @@ meta_copy_buffer_to_image(struct
>     radv_cmd_buffer
>      >     *cmd_buffer,
>      >                      }
>      >              }
>      >
>      >     +       /* Restore conditional rendering. */
>      >     +       cmd_buffer->state.predicating = old_predicating;
>      >     +
>      >              radv_meta_restore(&saved_state, cmd_buffer);
>      >       }
>      >
>      >     @@ -236,12 +246,19 @@ meta_copy_image_to_buffer(struct
>      >     radv_cmd_buffer *cmd_buffer,
>      >                                 const VkBufferImageCopy* pRegions)
>      >       {
>      >              struct radv_meta_saved_state saved_state;
>      >     +       bool old_predicating;
>      >
>      >              radv_meta_save(&saved_state, cmd_buffer,
>      >                             RADV_META_SAVE_COMPUTE_PIPELINE |
>      >                             RADV_META_SAVE_CONSTANTS |
>      >                             RADV_META_SAVE_DESCRIPTORS);
>      >
>      >     +       /* VK_EXT_conditional_rendering says that copy commands
>      >     should not be
>      >     +        * affected by conditional rendering.
>      >     +        */
>      >     +       old_predicating = cmd_buffer->state.predicating;
>      >     +       cmd_buffer->state.predicating = false;
>      >     +
>      >              for (unsigned r = 0; r < regionCount; r++) {
>      >
>      >                      /**
>      >     @@ -313,6 +330,9 @@ meta_copy_image_to_buffer(struct
>     radv_cmd_buffer
>      >     *cmd_buffer,
>      >                      }
>      >              }
>      >
>      >     +       /* Restore conditional rendering. */
>      >     +       cmd_buffer->state.predicating = old_predicating;
>      >     +
>      >              radv_meta_restore(&saved_state, cmd_buffer);
>      >       }
>      >
>      >     @@ -344,6 +364,7 @@ meta_copy_image(struct radv_cmd_buffer
>     *cmd_buffer,
>      >       {
>      >              bool cs = cmd_buffer->queue_family_index ==
>     RADV_QUEUE_COMPUTE;
>      >              struct radv_meta_saved_state saved_state;
>      >     +       bool old_predicating;
>      >
>      >              /* From the Vulkan 1.0 spec:
>      >               *
>      >     @@ -358,6 +379,12 @@ meta_copy_image(struct radv_cmd_buffer
>     *cmd_buffer,
>      >                             RADV_META_SAVE_CONSTANTS |
>      >                             RADV_META_SAVE_DESCRIPTORS);
>      >
>      >     +       /* VK_EXT_conditional_rendering says that copy commands
>      >     should not be
>      >     +        * affected by conditional rendering.
>      >     +        */
>      >     +       old_predicating = cmd_buffer->state.predicating;
>      >     +       cmd_buffer->state.predicating = false;
>      >     +
>      >              for (unsigned r = 0; r < regionCount; r++) {
>      >                      assert(pRegions[r].srcSubresource.aspectMask ==
>      >                             pRegions[r].dstSubresource.aspectMask);
>      >     @@ -465,6 +492,9 @@ meta_copy_image(struct radv_cmd_buffer
>     *cmd_buffer,
>      >                      }
>      >              }
>      >
>      >     +       /* Restore conditional rendering. */
>      >     +       cmd_buffer->state.predicating = old_predicating;
>      >     +
>      >              radv_meta_restore(&saved_state, cmd_buffer);
>      >       }
>      >
>      >     --
>      >     2.18.0
>      >
>      >     _______________________________________________
>      >     mesa-dev mailing list
>      > mesa-dev@lists.freedesktop.org
>     <mailto:mesa-dev@lists.freedesktop.org>
>     <mailto:mesa-dev@lists.freedesktop.org
>     <mailto:mesa-dev@lists.freedesktop.org>>
>      > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>      >
>
Don't we need to disable predication too for the PipelineBarriers when
a layout change happens?

Also in cases the barrier or a blit/copy does different predication,
do we not need to do si_emit_set_predication_state again as the state
was overridden?

On Mon, Jul 9, 2018 at 2:57 PM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
> Inherited commands buffers are not supported.
>
> v2: - disable predication for blit and copy commands
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c  | 29 +++++++++++++++++++++++++++++
>  src/amd/vulkan/radv_device.c      |  7 +++++++
>  src/amd/vulkan/radv_extensions.py |  1 +
>  src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
>  src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
>  src/amd/vulkan/radv_meta_copy.c   | 30 ++++++++++++++++++++++++++++++
>  6 files changed, 87 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 29199f2b3d..3fd8ebe2d3 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -4376,3 +4376,32 @@ void radv_CmdSetDeviceMask(VkCommandBuffer commandBuffer,
>  {
>     /* No-op */
>  }
> +
> +/* VK_EXT_conditional_rendering */
> +void vkCmdBeginConditionalRenderingEXT(
> +       VkCommandBuffer                             commandBuffer,
> +       const VkConditionalRenderingBeginInfoEXT*   pConditionalRenderingBegin)
> +{
> +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +       RADV_FROM_HANDLE(radv_buffer, buffer, pConditionalRenderingBegin->buffer);
> +       bool inverted;
> +       uint64_t va;
> +
> +       va = radv_buffer_get_va(buffer->bo) + pConditionalRenderingBegin->offset;
> +
> +       inverted = pConditionalRenderingBegin->flags & VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
> +
> +       /* Enable predication for this command buffer. */
> +       si_emit_set_predication_state(cmd_buffer, inverted, va);
> +       cmd_buffer->state.predicating = true;
> +}
> +
> +void vkCmdEndConditionalRenderingEXT(
> +       VkCommandBuffer                             commandBuffer)
> +{
> +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +
> +       /* Disable predication for this command buffer. */
> +       si_emit_set_predication_state(cmd_buffer, false, 0);
> +       cmd_buffer->state.predicating = false;
> +}
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index ad3465f594..06d70d305a 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -806,6 +806,13 @@ void radv_GetPhysicalDeviceFeatures2(
>                         features->runtimeDescriptorArray = true;
>                         break;
>                 }
> +               case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
> +                       VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
> +                               (VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
> +                       features->conditionalRendering = true;
> +                       features->inheritedConditionalRendering = false;
> +                       break;
> +               }
>                 default:
>                         break;
>                 }
> diff --git a/src/amd/vulkan/radv_extensions.py b/src/amd/vulkan/radv_extensions.py
> index a0f1038110..6ddbabf26e 100644
> --- a/src/amd/vulkan/radv_extensions.py
> +++ b/src/amd/vulkan/radv_extensions.py
> @@ -89,6 +89,7 @@ EXTENSIONS = [
>      Extension('VK_KHR_display',                          23, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_direct_mode_display',               1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_acquire_xlib_display',              1, 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
> +    Extension('VK_EXT_conditional_rendering',             1, True),
>      Extension('VK_EXT_display_surface_counter',           1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_display_control',                   1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_debug_report',                      9, True),
> diff --git a/src/amd/vulkan/radv_meta_blit.c b/src/amd/vulkan/radv_meta_blit.c
> index a6ee0cb7e9..67c26aabdb 100644
> --- a/src/amd/vulkan/radv_meta_blit.c
> +++ b/src/amd/vulkan/radv_meta_blit.c
> @@ -520,6 +520,7 @@ void radv_CmdBlitImage(
>         RADV_FROM_HANDLE(radv_image, src_image, srcImage);
>         RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         /* From the Vulkan 1.0 spec:
>          *
> @@ -534,6 +535,12 @@ void radv_CmdBlitImage(
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that blit commands should not be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>                 const VkImageSubresourceLayers *src_res = &pRegions[r].srcSubresource;
>                 const VkImageSubresourceLayers *dst_res = &pRegions[r].dstSubresource;
> @@ -648,6 +655,9 @@ void radv_CmdBlitImage(
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> diff --git a/src/amd/vulkan/radv_meta_buffer.c b/src/amd/vulkan/radv_meta_buffer.c
> index 2e1ba2c7b2..a45a6d5aac 100644
> --- a/src/amd/vulkan/radv_meta_buffer.c
> +++ b/src/amd/vulkan/radv_meta_buffer.c
> @@ -472,6 +472,13 @@ void radv_CmdCopyBuffer(
>         RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>         RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
>         RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
> +       bool old_predicating;
> +
> +       /* VK_EXT_conditional_rendering says that copy commands should not be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
>
>         for (unsigned r = 0; r < regionCount; r++) {
>                 uint64_t src_offset = src_buffer->offset + pRegions[r].srcOffset;
> @@ -481,6 +488,9 @@ void radv_CmdCopyBuffer(
>                 radv_copy_buffer(cmd_buffer, src_buffer->bo, dest_buffer->bo,
>                                  src_offset, dest_offset, copy_size);
>         }
> +
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
>  }
>
>  void radv_CmdUpdateBuffer(
> diff --git a/src/amd/vulkan/radv_meta_copy.c b/src/amd/vulkan/radv_meta_copy.c
> index 3442b49fb9..f4de5528ed 100644
> --- a/src/amd/vulkan/radv_meta_copy.c
> +++ b/src/amd/vulkan/radv_meta_copy.c
> @@ -117,6 +117,7 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>  {
>         bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         /* The Vulkan 1.0 spec says "dstImage must have a sample count equal to
>          * VK_SAMPLE_COUNT_1_BIT."
> @@ -129,6 +130,12 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that copy commands should not be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>
>                 /**
> @@ -208,6 +215,9 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> @@ -236,12 +246,19 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
>                            const VkBufferImageCopy* pRegions)
>  {
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         radv_meta_save(&saved_state, cmd_buffer,
>                        RADV_META_SAVE_COMPUTE_PIPELINE |
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that copy commands should not be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>
>                 /**
> @@ -313,6 +330,9 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> @@ -344,6 +364,7 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>  {
>         bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>         struct radv_meta_saved_state saved_state;
> +       bool old_predicating;
>
>         /* From the Vulkan 1.0 spec:
>          *
> @@ -358,6 +379,12 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                        RADV_META_SAVE_CONSTANTS |
>                        RADV_META_SAVE_DESCRIPTORS);
>
> +       /* VK_EXT_conditional_rendering says that copy commands should not be
> +        * affected by conditional rendering.
> +        */
> +       old_predicating = cmd_buffer->state.predicating;
> +       cmd_buffer->state.predicating = false;
> +
>         for (unsigned r = 0; r < regionCount; r++) {
>                 assert(pRegions[r].srcSubresource.aspectMask ==
>                        pRegions[r].dstSubresource.aspectMask);
> @@ -465,6 +492,9 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                 }
>         }
>
> +       /* Restore conditional rendering. */
> +       cmd_buffer->state.predicating = old_predicating;
> +
>         radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 07/11/2018 10:53 AM, Bas Nieuwenhuizen wrote:
> Don't we need to disable predication too for the PipelineBarriers when
> a layout change happens?

I don't think. Shouldn't the app handle this correctly? If a 
CmdPipelineBarrier() is between Begin()/End() it will be skipped, that's it?

> 
> Also in cases the barrier or a blit/copy does different predication,
> do we not need to do si_emit_set_predication_state again as the state
> was overridden?

Hmm, I think we need, at least, to restore the state after a 
decompression pass in case the predicate is different. What do you think?

> 
> On Mon, Jul 9, 2018 at 2:57 PM, Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>> Inherited commands buffers are not supported.
>>
>> v2: - disable predication for blit and copy commands
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c  | 29 +++++++++++++++++++++++++++++
>>   src/amd/vulkan/radv_device.c      |  7 +++++++
>>   src/amd/vulkan/radv_extensions.py |  1 +
>>   src/amd/vulkan/radv_meta_blit.c   | 10 ++++++++++
>>   src/amd/vulkan/radv_meta_buffer.c | 10 ++++++++++
>>   src/amd/vulkan/radv_meta_copy.c   | 30 ++++++++++++++++++++++++++++++
>>   6 files changed, 87 insertions(+)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index 29199f2b3d..3fd8ebe2d3 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -4376,3 +4376,32 @@ void radv_CmdSetDeviceMask(VkCommandBuffer commandBuffer,
>>   {
>>      /* No-op */
>>   }
>> +
>> +/* VK_EXT_conditional_rendering */
>> +void vkCmdBeginConditionalRenderingEXT(
>> +       VkCommandBuffer                             commandBuffer,
>> +       const VkConditionalRenderingBeginInfoEXT*   pConditionalRenderingBegin)
>> +{
>> +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>> +       RADV_FROM_HANDLE(radv_buffer, buffer, pConditionalRenderingBegin->buffer);
>> +       bool inverted;
>> +       uint64_t va;
>> +
>> +       va = radv_buffer_get_va(buffer->bo) + pConditionalRenderingBegin->offset;
>> +
>> +       inverted = pConditionalRenderingBegin->flags & VK_CONDITIONAL_RENDERING_INVERTED_BIT_EXT;
>> +
>> +       /* Enable predication for this command buffer. */
>> +       si_emit_set_predication_state(cmd_buffer, inverted, va);
>> +       cmd_buffer->state.predicating = true;
>> +}
>> +
>> +void vkCmdEndConditionalRenderingEXT(
>> +       VkCommandBuffer                             commandBuffer)
>> +{
>> +       RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>> +
>> +       /* Disable predication for this command buffer. */
>> +       si_emit_set_predication_state(cmd_buffer, false, 0);
>> +       cmd_buffer->state.predicating = false;
>> +}
>> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
>> index ad3465f594..06d70d305a 100644
>> --- a/src/amd/vulkan/radv_device.c
>> +++ b/src/amd/vulkan/radv_device.c
>> @@ -806,6 +806,13 @@ void radv_GetPhysicalDeviceFeatures2(
>>                          features->runtimeDescriptorArray = true;
>>                          break;
>>                  }
>> +               case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
>> +                       VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
>> +                               (VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
>> +                       features->conditionalRendering = true;
>> +                       features->inheritedConditionalRendering = false;
>> +                       break;
>> +               }
>>                  default:
>>                          break;
>>                  }
>> diff --git a/src/amd/vulkan/radv_extensions.py b/src/amd/vulkan/radv_extensions.py
>> index a0f1038110..6ddbabf26e 100644
>> --- a/src/amd/vulkan/radv_extensions.py
>> +++ b/src/amd/vulkan/radv_extensions.py
>> @@ -89,6 +89,7 @@ EXTENSIONS = [
>>       Extension('VK_KHR_display',                          23, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>>       Extension('VK_EXT_direct_mode_display',               1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>>       Extension('VK_EXT_acquire_xlib_display',              1, 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
>> +    Extension('VK_EXT_conditional_rendering',             1, True),
>>       Extension('VK_EXT_display_surface_counter',           1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>>       Extension('VK_EXT_display_control',                   1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
>>       Extension('VK_EXT_debug_report',                      9, True),
>> diff --git a/src/amd/vulkan/radv_meta_blit.c b/src/amd/vulkan/radv_meta_blit.c
>> index a6ee0cb7e9..67c26aabdb 100644
>> --- a/src/amd/vulkan/radv_meta_blit.c
>> +++ b/src/amd/vulkan/radv_meta_blit.c
>> @@ -520,6 +520,7 @@ void radv_CmdBlitImage(
>>          RADV_FROM_HANDLE(radv_image, src_image, srcImage);
>>          RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>>          struct radv_meta_saved_state saved_state;
>> +       bool old_predicating;
>>
>>          /* From the Vulkan 1.0 spec:
>>           *
>> @@ -534,6 +535,12 @@ void radv_CmdBlitImage(
>>                         RADV_META_SAVE_CONSTANTS |
>>                         RADV_META_SAVE_DESCRIPTORS);
>>
>> +       /* VK_EXT_conditional_rendering says that blit commands should not be
>> +        * affected by conditional rendering.
>> +        */
>> +       old_predicating = cmd_buffer->state.predicating;
>> +       cmd_buffer->state.predicating = false;
>> +
>>          for (unsigned r = 0; r < regionCount; r++) {
>>                  const VkImageSubresourceLayers *src_res = &pRegions[r].srcSubresource;
>>                  const VkImageSubresourceLayers *dst_res = &pRegions[r].dstSubresource;
>> @@ -648,6 +655,9 @@ void radv_CmdBlitImage(
>>                  }
>>          }
>>
>> +       /* Restore conditional rendering. */
>> +       cmd_buffer->state.predicating = old_predicating;
>> +
>>          radv_meta_restore(&saved_state, cmd_buffer);
>>   }
>>
>> diff --git a/src/amd/vulkan/radv_meta_buffer.c b/src/amd/vulkan/radv_meta_buffer.c
>> index 2e1ba2c7b2..a45a6d5aac 100644
>> --- a/src/amd/vulkan/radv_meta_buffer.c
>> +++ b/src/amd/vulkan/radv_meta_buffer.c
>> @@ -472,6 +472,13 @@ void radv_CmdCopyBuffer(
>>          RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>>          RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
>>          RADV_FROM_HANDLE(radv_buffer, dest_buffer, destBuffer);
>> +       bool old_predicating;
>> +
>> +       /* VK_EXT_conditional_rendering says that copy commands should not be
>> +        * affected by conditional rendering.
>> +        */
>> +       old_predicating = cmd_buffer->state.predicating;
>> +       cmd_buffer->state.predicating = false;
>>
>>          for (unsigned r = 0; r < regionCount; r++) {
>>                  uint64_t src_offset = src_buffer->offset + pRegions[r].srcOffset;
>> @@ -481,6 +488,9 @@ void radv_CmdCopyBuffer(
>>                  radv_copy_buffer(cmd_buffer, src_buffer->bo, dest_buffer->bo,
>>                                   src_offset, dest_offset, copy_size);
>>          }
>> +
>> +       /* Restore conditional rendering. */
>> +       cmd_buffer->state.predicating = old_predicating;
>>   }
>>
>>   void radv_CmdUpdateBuffer(
>> diff --git a/src/amd/vulkan/radv_meta_copy.c b/src/amd/vulkan/radv_meta_copy.c
>> index 3442b49fb9..f4de5528ed 100644
>> --- a/src/amd/vulkan/radv_meta_copy.c
>> +++ b/src/amd/vulkan/radv_meta_copy.c
>> @@ -117,6 +117,7 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>>   {
>>          bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>>          struct radv_meta_saved_state saved_state;
>> +       bool old_predicating;
>>
>>          /* The Vulkan 1.0 spec says "dstImage must have a sample count equal to
>>           * VK_SAMPLE_COUNT_1_BIT."
>> @@ -129,6 +130,12 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>>                         RADV_META_SAVE_CONSTANTS |
>>                         RADV_META_SAVE_DESCRIPTORS);
>>
>> +       /* VK_EXT_conditional_rendering says that copy commands should not be
>> +        * affected by conditional rendering.
>> +        */
>> +       old_predicating = cmd_buffer->state.predicating;
>> +       cmd_buffer->state.predicating = false;
>> +
>>          for (unsigned r = 0; r < regionCount; r++) {
>>
>>                  /**
>> @@ -208,6 +215,9 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>>                  }
>>          }
>>
>> +       /* Restore conditional rendering. */
>> +       cmd_buffer->state.predicating = old_predicating;
>> +
>>          radv_meta_restore(&saved_state, cmd_buffer);
>>   }
>>
>> @@ -236,12 +246,19 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
>>                             const VkBufferImageCopy* pRegions)
>>   {
>>          struct radv_meta_saved_state saved_state;
>> +       bool old_predicating;
>>
>>          radv_meta_save(&saved_state, cmd_buffer,
>>                         RADV_META_SAVE_COMPUTE_PIPELINE |
>>                         RADV_META_SAVE_CONSTANTS |
>>                         RADV_META_SAVE_DESCRIPTORS);
>>
>> +       /* VK_EXT_conditional_rendering says that copy commands should not be
>> +        * affected by conditional rendering.
>> +        */
>> +       old_predicating = cmd_buffer->state.predicating;
>> +       cmd_buffer->state.predicating = false;
>> +
>>          for (unsigned r = 0; r < regionCount; r++) {
>>
>>                  /**
>> @@ -313,6 +330,9 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
>>                  }
>>          }
>>
>> +       /* Restore conditional rendering. */
>> +       cmd_buffer->state.predicating = old_predicating;
>> +
>>          radv_meta_restore(&saved_state, cmd_buffer);
>>   }
>>
>> @@ -344,6 +364,7 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>>   {
>>          bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>>          struct radv_meta_saved_state saved_state;
>> +       bool old_predicating;
>>
>>          /* From the Vulkan 1.0 spec:
>>           *
>> @@ -358,6 +379,12 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>>                         RADV_META_SAVE_CONSTANTS |
>>                         RADV_META_SAVE_DESCRIPTORS);
>>
>> +       /* VK_EXT_conditional_rendering says that copy commands should not be
>> +        * affected by conditional rendering.
>> +        */
>> +       old_predicating = cmd_buffer->state.predicating;
>> +       cmd_buffer->state.predicating = false;
>> +
>>          for (unsigned r = 0; r < regionCount; r++) {
>>                  assert(pRegions[r].srcSubresource.aspectMask ==
>>                         pRegions[r].dstSubresource.aspectMask);
>> @@ -465,6 +492,9 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>>                  }
>>          }
>>
>> +       /* Restore conditional rendering. */
>> +       cmd_buffer->state.predicating = old_predicating;
>> +
>>          radv_meta_restore(&saved_state, cmd_buffer);
>>   }
>>
>> --
>> 2.18.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev