[Mesa-dev] [PATCH] radv: move save/restore operations close to the slow clears
Samuel Pitoiset
samuel.pitoiset at gmail.com
Tue Apr 10 14:03:57 UTC 2018
On 04/10/2018 11:20 AM, Bas Nieuwenhuizen wrote:
> Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>
> I'm wondering though, doesn't this result in more saves/restores, as
> we now do it for each part of a subpass clear separately?
Yes, possibly. I'm actually not sure myself if it's the right thing to
do. I will postpone this change for later.
>
> On Mon, Apr 9, 2018 at 11:10 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> This removes the emission of unnecessary states, for example
>> when performing a fast depth stencil clear (ie. clearing htile),
>> we don't need to save/restore the graphics pipeline.
>>
>> For GFX9 chips that have the scissor bug workaround, that
>> should also reduce the number of partial flushes.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>> src/amd/vulkan/radv_meta_bufimage.c | 8 +++++
>> src/amd/vulkan/radv_meta_clear.c | 47 +++++++++--------------------
>> 2 files changed, 22 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_meta_bufimage.c b/src/amd/vulkan/radv_meta_bufimage.c
>> index 69e15d3213..5018ce1f2e 100644
>> --- a/src/amd/vulkan/radv_meta_bufimage.c
>> +++ b/src/amd/vulkan/radv_meta_bufimage.c
>> @@ -1242,8 +1242,14 @@ radv_meta_clear_image_cs(struct radv_cmd_buffer *cmd_buffer,
>> {
>> VkPipeline pipeline = cmd_buffer->device->meta_state.cleari.pipeline;
>> struct radv_device *device = cmd_buffer->device;
>> + struct radv_meta_saved_state saved_state;
>> struct radv_image_view dst_iview;
>>
>> + radv_meta_save(&saved_state, cmd_buffer,
>> + RADV_META_SAVE_COMPUTE_PIPELINE |
>> + RADV_META_SAVE_CONSTANTS |
>> + RADV_META_SAVE_DESCRIPTORS);
>> +
>> create_iview(cmd_buffer, dst, &dst_iview);
>> cleari_bind_descriptors(cmd_buffer, &dst_iview);
>>
>> @@ -1268,4 +1274,6 @@ radv_meta_clear_image_cs(struct radv_cmd_buffer *cmd_buffer,
>> push_constants);
>>
>> radv_unaligned_dispatch(cmd_buffer, dst->image->info.width, dst->image->info.height, 1);
>> +
>> + radv_meta_restore(&saved_state, cmd_buffer);
>> }
>> diff --git a/src/amd/vulkan/radv_meta_clear.c b/src/amd/vulkan/radv_meta_clear.c
>> index 016c1ee296..833e3cebab 100644
>> --- a/src/amd/vulkan/radv_meta_clear.c
>> +++ b/src/amd/vulkan/radv_meta_clear.c
>> @@ -342,6 +342,7 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>> unsigned fs_key = radv_format_meta_fs_key(iview->vk_format);
>> VkClearColorValue clear_value = clear_att->clearValue.color;
>> VkCommandBuffer cmd_buffer_h = radv_cmd_buffer_to_handle(cmd_buffer);
>> + struct radv_meta_saved_state saved_state;
>> VkPipeline pipeline;
>>
>> if (fs_key == -1) {
>> @@ -359,6 +360,10 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>> assert(clear_att->aspectMask == VK_IMAGE_ASPECT_COLOR_BIT);
>> assert(clear_att->colorAttachment < subpass->color_count);
>>
>> + radv_meta_save(&saved_state, cmd_buffer,
>> + RADV_META_SAVE_GRAPHICS_PIPELINE |
>> + RADV_META_SAVE_CONSTANTS);
>> +
>> radv_CmdPushConstants(radv_cmd_buffer_to_handle(cmd_buffer),
>> device->meta_state.clear_color_p_layout,
>> VK_SHADER_STAGE_FRAGMENT_BIT, 0, 16,
>> @@ -397,6 +402,8 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>> }
>>
>> radv_cmd_buffer_set_subpass(cmd_buffer, subpass, false);
>> +
>> + radv_meta_restore(&saved_state, cmd_buffer);
>> }
>>
>>
>> @@ -613,12 +620,17 @@ emit_depthstencil_clear(struct radv_cmd_buffer *cmd_buffer,
>> const uint32_t samples = iview->image->info.samples;
>> const uint32_t samples_log2 = ffs(samples) - 1;
>> VkCommandBuffer cmd_buffer_h = radv_cmd_buffer_to_handle(cmd_buffer);
>> + struct radv_meta_saved_state saved_state;
>>
>> assert(pass_att != VK_ATTACHMENT_UNUSED);
>>
>> if (!(aspects & VK_IMAGE_ASPECT_DEPTH_BIT))
>> clear_value.depth = 1.0f;
>>
>> + radv_meta_save(&saved_state, cmd_buffer,
>> + RADV_META_SAVE_GRAPHICS_PIPELINE |
>> + RADV_META_SAVE_CONSTANTS);
>> +
>> radv_CmdPushConstants(radv_cmd_buffer_to_handle(cmd_buffer),
>> device->meta_state.clear_depth_p_layout,
>> VK_SHADER_STAGE_VERTEX_BIT, 0, 4,
>> @@ -664,6 +676,8 @@ emit_depthstencil_clear(struct radv_cmd_buffer *cmd_buffer,
>> radv_CmdSetStencilReference(cmd_buffer_h, VK_STENCIL_FACE_FRONT_BIT,
>> prev_reference);
>> }
>> +
>> + radv_meta_restore(&saved_state, cmd_buffer);
>> }
>>
>> static bool
>> @@ -1165,17 +1179,12 @@ void
>> radv_cmd_buffer_clear_subpass(struct radv_cmd_buffer *cmd_buffer)
>> {
>> struct radv_cmd_state *cmd_state = &cmd_buffer->state;
>> - struct radv_meta_saved_state saved_state;
>> enum radv_cmd_flush_bits pre_flush = 0;
>> enum radv_cmd_flush_bits post_flush = 0;
>>
>> if (!radv_subpass_needs_clear(cmd_buffer))
>> return;
>>
>> - radv_meta_save(&saved_state, cmd_buffer,
>> - RADV_META_SAVE_GRAPHICS_PIPELINE |
>> - RADV_META_SAVE_CONSTANTS);
>> -
>> for (uint32_t i = 0; i < cmd_state->subpass->color_count; ++i) {
>> uint32_t a = cmd_state->subpass->color_attachments[i].attachment;
>>
>> @@ -1210,7 +1219,6 @@ radv_cmd_buffer_clear_subpass(struct radv_cmd_buffer *cmd_buffer)
>> &post_flush);
>> }
>>
>> - radv_meta_restore(&saved_state, cmd_buffer);
>> cmd_buffer->state.flush_bits |= post_flush;
>> }
>>
>> @@ -1405,25 +1413,11 @@ void radv_CmdClearColorImage(
>> {
>> RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>> RADV_FROM_HANDLE(radv_image, image, image_h);
>> - struct radv_meta_saved_state saved_state;
>> bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>>
>> - if (cs) {
>> - radv_meta_save(&saved_state, cmd_buffer,
>> - RADV_META_SAVE_COMPUTE_PIPELINE |
>> - RADV_META_SAVE_CONSTANTS |
>> - RADV_META_SAVE_DESCRIPTORS);
>> - } else {
>> - radv_meta_save(&saved_state, cmd_buffer,
>> - RADV_META_SAVE_GRAPHICS_PIPELINE |
>> - RADV_META_SAVE_CONSTANTS);
>> - }
>> -
>> radv_cmd_clear_image(cmd_buffer, image, imageLayout,
>> (const VkClearValue *) pColor,
>> rangeCount, pRanges, cs);
>> -
>> - radv_meta_restore(&saved_state, cmd_buffer);
>> }
>>
>> void radv_CmdClearDepthStencilImage(
>> @@ -1436,17 +1430,10 @@ void radv_CmdClearDepthStencilImage(
>> {
>> RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>> RADV_FROM_HANDLE(radv_image, image, image_h);
>> - struct radv_meta_saved_state saved_state;
>> -
>> - radv_meta_save(&saved_state, cmd_buffer,
>> - RADV_META_SAVE_GRAPHICS_PIPELINE |
>> - RADV_META_SAVE_CONSTANTS);
>>
>> radv_cmd_clear_image(cmd_buffer, image, imageLayout,
>> (const VkClearValue *) pDepthStencil,
>> rangeCount, pRanges, false);
>> -
>> - radv_meta_restore(&saved_state, cmd_buffer);
>> }
>>
>> void radv_CmdClearAttachments(
>> @@ -1457,17 +1444,12 @@ void radv_CmdClearAttachments(
>> const VkClearRect* pRects)
>> {
>> RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>> - struct radv_meta_saved_state saved_state;
>> enum radv_cmd_flush_bits pre_flush = 0;
>> enum radv_cmd_flush_bits post_flush = 0;
>>
>> if (!cmd_buffer->state.subpass)
>> return;
>>
>> - radv_meta_save(&saved_state, cmd_buffer,
>> - RADV_META_SAVE_GRAPHICS_PIPELINE |
>> - RADV_META_SAVE_CONSTANTS);
>> -
>> /* FINISHME: We can do better than this dumb loop. It thrashes too much
>> * state.
>> */
>> @@ -1478,6 +1460,5 @@ void radv_CmdClearAttachments(
>> }
>> }
>>
>> - radv_meta_restore(&saved_state, cmd_buffer);
>> cmd_buffer->state.flush_bits |= post_flush;
>> }
>> --
>> 2.17.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