[Mesa-dev] [PATCH v2 4/4] radv: add support for VK_EXT_conditional_rendering

Samuel Pitoiset samuel.pitoiset at gmail.com
Fri Jul 13 10:26:24 UTC 2018



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 at gmail.com> wrote:
>> Inherited commands buffers are not supported.
>>
>> v2: - disable predication for blit and copy commands
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list