On 2018-03-08 08:49:10, Rafael Antognolli wrote: > Instead of updating the clear color in anv before a resolve, just let > blorp handle that for us during fast clears. > > Signed-off-by: Rafael Antognolli <rafael.antogno...@intel.com> > --- > src/intel/vulkan/anv_blorp.c | 69 > +++++++++++++++++++++++++++----------- > src/intel/vulkan/anv_private.h | 6 ++-- > src/intel/vulkan/genX_cmd_buffer.c | 48 +++++++------------------- > 3 files changed, 66 insertions(+), 57 deletions(-) > > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c > index 8f29bc8398f..269be2b73ad 100644 > --- a/src/intel/vulkan/anv_blorp.c > +++ b/src/intel/vulkan/anv_blorp.c > @@ -222,6 +222,28 @@ get_blorp_surf_for_anv_image(const struct anv_device > *device, > .mocs = device->default_mocs, > }; > blorp_surf->aux_usage = aux_usage; > + > + /* If we're doing a partial resolve, then we need the indirect clear > + * color. If we are doing a fast clear and want to store/update the > + * clear color, we also pass the address to blorp, otherwise it will > only > + * stomp the CCS to a particular value and won't care about format or > + * clear value > + */ > + if (aspect & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) { > + const struct anv_address clear_color_addr = > + anv_image_get_clear_color_addr(device, image, aspect); > + blorp_surf->clear_color_addr = > anv_to_blorp_address(clear_color_addr); > + } else if (aspect & VK_IMAGE_ASPECT_DEPTH_BIT > + && device->info.gen >= 10) { > + /* On gen9 and older, we don't care about hiz color because we only > + * clear to 1.0f. On gen10 and newer, we get the clear value from > the > + * hiz_clear_bo. > + */
I'm not sure about this comment. I'm not sure a comment is needed, but what about this? /* Vulkan always clears to 1.0. On gen < 10, we set that directly in * the state packet. For gen >= 10, must provide the clear value in a * buffer. We have a single global buffer that stores the 1.0 value. */ Patches 15 - 18: Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> > + const struct anv_address clear_color_addr = (struct anv_address) { > + .bo = (struct anv_bo *)&device->hiz_clear_bo > + }; > + blorp_surf->clear_color_addr = > anv_to_blorp_address(clear_color_addr); > + } > } > } > > @@ -1594,7 +1616,8 @@ anv_image_mcs_op(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > VkImageAspectFlagBits aspect, > uint32_t base_layer, uint32_t layer_count, > - enum isl_aux_op mcs_op, bool predicate) > + enum isl_aux_op mcs_op, union isl_color_value *clear_value, > + bool predicate) > { > assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT); > assert(image->samples > 1); > @@ -1612,14 +1635,18 @@ anv_image_mcs_op(struct anv_cmd_buffer *cmd_buffer, > ANV_IMAGE_LAYOUT_EXPLICIT_AUX, > ISL_AUX_USAGE_MCS, &surf); > > - if (mcs_op == ISL_AUX_OP_PARTIAL_RESOLVE) { > - /* If we're doing a partial resolve, then we need the indirect clear > - * color. The clear operation just stomps the CCS to a particular > value > - * and don't care about format or clear value. > - */ > - const struct anv_address clear_color_addr = > - anv_image_get_clear_color_addr(cmd_buffer->device, image, aspect); > - surf.clear_color_addr = anv_to_blorp_address(clear_color_addr); > + /* Blorp will store the clear color for us if we provide the clear color > + * address and we are doing a fast clear. So we save the clear value into > + * the blorp surface. However, in some situations we want to do a fast > clear > + * without changing the clear value stored in the state buffer. For those > + * cases, we set the clear color address pointer to NULL, so blorp will > not > + * try to store a garbage color. > + */ > + if (mcs_op == ISL_AUX_OP_FAST_CLEAR) { > + if (clear_value) > + surf.clear_color = *clear_value; > + else > + surf.clear_color_addr.buffer = NULL; > } > > /* From the Sky Lake PRM Vol. 7, "Render Target Fast Clear": > @@ -1667,7 +1694,8 @@ anv_image_ccs_op(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > VkImageAspectFlagBits aspect, uint32_t level, > uint32_t base_layer, uint32_t layer_count, > - enum isl_aux_op ccs_op, bool predicate) > + enum isl_aux_op ccs_op, union isl_color_value *clear_value, > + bool predicate) > { > assert(image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV); > assert(image->samples == 1); > @@ -1693,15 +1721,18 @@ anv_image_ccs_op(struct anv_cmd_buffer *cmd_buffer, > fast_clear_aux_usage(image, aspect), > &surf); > > - if (ccs_op == ISL_AUX_OP_FULL_RESOLVE || > - ccs_op == ISL_AUX_OP_PARTIAL_RESOLVE) { > - /* If we're doing a resolve operation, then we need the indirect clear > - * color. The clear and ambiguate operations just stomp the CCS to a > - * particular value and don't care about format or clear value. > - */ > - const struct anv_address clear_color_addr = > - anv_image_get_clear_color_addr(cmd_buffer->device, image, aspect); > - surf.clear_color_addr = anv_to_blorp_address(clear_color_addr); > + /* Blorp will store the clear color for us if we provide the clear color > + * address and we are doing a fast clear. So we save the clear value into > + * the blorp surface. However, in some situations we want to do a fast > clear > + * without changing the clear value stored in the state buffer. For those > + * cases, we set the clear color address pointer to NULL, so blorp will > not > + * try to store a garbage color. > + */ > + if (ccs_op == ISL_AUX_OP_FAST_CLEAR) { > + if (clear_value) > + surf.clear_color = *clear_value; > + else > + surf.clear_color_addr.buffer = NULL; > } > > /* From the Sky Lake PRM Vol. 7, "Render Target Fast Clear": > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h > index b7de7621250..32f32c9c9e4 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -2704,13 +2704,15 @@ anv_image_mcs_op(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > VkImageAspectFlagBits aspect, > uint32_t base_layer, uint32_t layer_count, > - enum isl_aux_op mcs_op, bool predicate); > + enum isl_aux_op mcs_op, union isl_color_value *clear_value, > + bool predicate); > void > anv_image_ccs_op(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > VkImageAspectFlagBits aspect, uint32_t level, > uint32_t base_layer, uint32_t layer_count, > - enum isl_aux_op ccs_op, bool predicate); > + enum isl_aux_op ccs_op, union isl_color_value *clear_value, > + bool predicate); > > void > anv_image_copy_to_shadow(struct anv_cmd_buffer *cmd_buffer, > diff --git a/src/intel/vulkan/genX_cmd_buffer.c > b/src/intel/vulkan/genX_cmd_buffer.c > index ae4616eb451..b1b60bd1267 100644 > --- a/src/intel/vulkan/genX_cmd_buffer.c > +++ b/src/intel/vulkan/genX_cmd_buffer.c > @@ -750,7 +750,7 @@ anv_cmd_predicated_ccs_resolve(struct anv_cmd_buffer > *cmd_buffer, > resolve_op = ISL_AUX_OP_FULL_RESOLVE; > > anv_image_ccs_op(cmd_buffer, image, aspect, level, > - array_layer, 1, resolve_op, true); > + array_layer, 1, resolve_op, NULL, true); > } > > static void > @@ -770,7 +770,7 @@ anv_cmd_predicated_mcs_resolve(struct anv_cmd_buffer > *cmd_buffer, > resolve_op, fast_clear_supported); > > anv_image_mcs_op(cmd_buffer, image, aspect, > - array_layer, 1, resolve_op, true); > + array_layer, 1, resolve_op, NULL, true); > #else > unreachable("MCS resolves are unsupported on Ivybridge and Bay Trail"); > #endif > @@ -1025,7 +1025,7 @@ transition_color_buffer(struct anv_cmd_buffer > *cmd_buffer, > > anv_image_ccs_op(cmd_buffer, image, aspect, level, > base_layer, level_layer_count, > - ISL_AUX_OP_AMBIGUATE, false); > + ISL_AUX_OP_AMBIGUATE, NULL, false); > > if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E) { > set_image_compressed_bit(cmd_buffer, image, aspect, > @@ -1043,7 +1043,7 @@ transition_color_buffer(struct anv_cmd_buffer > *cmd_buffer, > assert(base_level == 0 && level_count == 1); > anv_image_mcs_op(cmd_buffer, image, aspect, > base_layer, layer_count, > - ISL_AUX_OP_FAST_CLEAR, false); > + ISL_AUX_OP_FAST_CLEAR, NULL, false); > } > return; > } > @@ -1134,34 +1134,6 @@ transition_color_buffer(struct anv_cmd_buffer > *cmd_buffer, > ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT; > } > > -static void > -update_fast_clear_color(struct anv_cmd_buffer *cmd_buffer, > - const struct anv_attachment_state *att_state, > - const struct anv_image_view *iview) > -{ > - assert(GEN_GEN >= 10); > - assert(iview->image->aspects == VK_IMAGE_ASPECT_COLOR_BIT); > - > - struct anv_address clear_address = > - anv_image_get_clear_color_addr(cmd_buffer->device, iview->image, > - VK_IMAGE_ASPECT_COLOR_BIT); > - union isl_color_value clear_color; > - anv_clear_color_from_att_state(&clear_color, att_state, iview); > - > - /* Clear values are stored at the same bo as the aux surface, right > - * after the surface. > - */ > - for (int i = 0; i < 4; i++) { > - anv_batch_emit(&cmd_buffer->batch, GENX(MI_STORE_DATA_IMM), sdi) { > - sdi.Address = (struct anv_address) { > - .bo = clear_address.bo, > - .offset = clear_address.offset + i * 4, > - }; > - sdi.ImmediateData = clear_color.u32[i]; > - } > - } > -} > - > /** > * Setup anv_cmd_state::attachments for vkCmdBeginRenderPass. > */ > @@ -3551,12 +3523,18 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer > *cmd_buffer, > assert(iview->planes[0].isl.base_level == 0); > assert(iview->planes[0].isl.base_array_layer == 0); > > + union isl_color_value clear_color = {}; > + anv_clear_color_from_att_state(&clear_color, att_state, iview); > if (iview->image->samples == 1) { > anv_image_ccs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT, > - 0, 0, 1, ISL_AUX_OP_FAST_CLEAR, false); > + 0, 0, 1, ISL_AUX_OP_FAST_CLEAR, > + &clear_color, > + false); > } else { > anv_image_mcs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT, > - 0, 1, ISL_AUX_OP_FAST_CLEAR, false); > + 0, 1, ISL_AUX_OP_FAST_CLEAR, > + &clear_color, > + false); > } > base_clear_layer++; > clear_layer_count--; > @@ -3565,8 +3543,6 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer > *cmd_buffer, > genX(copy_fast_clear_dwords)(cmd_buffer, > att_state->color.state, > image, VK_IMAGE_ASPECT_COLOR_BIT, > true /* copy from ss */); > - } else { > - update_fast_clear_color(cmd_buffer, att_state, iview); > } > > if (att_state->clear_color_is_zero) { > -- > 2.14.3 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev