[Mesa-dev] [PATCH 1/2] i965: Add and use a helper to update the indirect miptree color
Rafael Antognolli
rafael.antognolli at intel.com
Fri Apr 20 22:12:32 UTC 2018
On Wed, Apr 11, 2018 at 01:56:16PM -0700, Nanley Chery wrote:
> Split out this functionality to enable a fast-clear optimization for
> color miptrees in the next commit.
> ---
> src/mesa/drivers/dri/i965/brw_clear.c | 54 ++++-----------------------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 22 +++++++++++
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 7 ++++
> 3 files changed, 36 insertions(+), 47 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c
> index 3d540d6d905..1cdc2241eac 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -108,7 +108,6 @@ brw_fast_clear_depth(struct gl_context *ctx)
> struct intel_mipmap_tree *mt = depth_irb->mt;
> struct gl_renderbuffer_attachment *depth_att = &fb->Attachment[BUFFER_DEPTH];
> const struct gen_device_info *devinfo = &brw->screen->devinfo;
> - bool same_clear_value = true;
>
> if (devinfo->gen < 6)
> return false;
> @@ -176,7 +175,8 @@ brw_fast_clear_depth(struct gl_context *ctx)
> /* If we're clearing to a new clear value, then we need to resolve any clear
> * flags out of the HiZ buffer into the real depth buffer.
> */
> - if (mt->fast_clear_color.f32[0] != clear_value) {
> + const bool same_clear_value = mt->fast_clear_color.f32[0] == clear_value;
> + if (!same_clear_value) {
> for (uint32_t level = mt->first_level; level <= mt->last_level; level++) {
> if (!intel_miptree_level_has_hiz(mt, level))
> continue;
> @@ -214,7 +214,6 @@ brw_fast_clear_depth(struct gl_context *ctx)
> }
>
> intel_miptree_set_depth_clear_value(brw, mt, clear_value);
> - same_clear_value = false;
> }
>
> bool need_clear = false;
> @@ -225,56 +224,17 @@ brw_fast_clear_depth(struct gl_context *ctx)
>
> if (aux_state != ISL_AUX_STATE_CLEAR) {
> need_clear = true;
> - break;
> - }
> - }
> -
> - if (!need_clear) {
> - /* If all of the layers we intend to clear are already in the clear
> - * state then simply updating the miptree fast clear value is sufficient
> - * to change their clear value.
> - */
> - if (devinfo->gen >= 10 && !same_clear_value) {
> - /* Before gen10, it was enough to just update the clear value in the
> - * miptree. But on gen10+, we let blorp update the clear value state
> - * buffer when doing a fast clear. Since we are skipping the fast
> - * clear here, we need to update the clear color ourselves.
> - */
> - uint32_t clear_offset = mt->aux_buf->clear_color_offset;
> - union isl_color_value clear_color = { .f32 = { clear_value, } };
> -
> - /* We can't update the clear color while the hardware is still using
> - * the previous one for a resolve or sampling from it. So make sure
> - * that there's no pending commands at this point.
> - */
> - brw_emit_pipe_control_flush(brw, PIPE_CONTROL_CS_STALL);
> - for (int i = 0; i < 4; i++) {
> - brw_store_data_imm32(brw, mt->aux_buf->clear_color_bo,
> - clear_offset + i * 4, clear_color.u32[i]);
> - }
> - brw_emit_pipe_control_flush(brw, PIPE_CONTROL_STATE_CACHE_INVALIDATE);
> - }
> - return true;
> - }
> -
> - for (unsigned a = 0; a < num_layers; a++) {
> - enum isl_aux_state aux_state =
> - intel_miptree_get_aux_state(mt, depth_irb->mt_level,
> - depth_irb->mt_layer + a);
> -
> - if (aux_state != ISL_AUX_STATE_CLEAR) {
> intel_hiz_exec(brw, mt, depth_irb->mt_level,
> depth_irb->mt_layer + a, 1,
> ISL_AUX_OP_FAST_CLEAR);
> + intel_miptree_set_aux_state(brw, mt, depth_irb->mt_level,
> + depth_irb->mt_layer + a, 1,
> + ISL_AUX_STATE_CLEAR);
> }
> }
>
> - /* Now, the HiZ buffer contains data that needs to be resolved to the depth
> - * buffer.
> - */
> - intel_miptree_set_aux_state(brw, mt, depth_irb->mt_level,
> - depth_irb->mt_layer, num_layers,
> - ISL_AUX_STATE_CLEAR);
> + if (!need_clear && !same_clear_value)
> + intel_miptree_update_indirect_color(brw, mt);
>
> return true;
> }
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 0b6a821d08c..23e73c5419c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -3831,3 +3831,25 @@ intel_miptree_get_clear_color(const struct gen_device_info *devinfo,
> return mt->fast_clear_color;
> }
> }
> +
> +void
> +intel_miptree_update_indirect_color(struct brw_context *brw,
> + struct intel_mipmap_tree *mt)
> +{
> + assert(mt->aux_buf);
> +
> + if (mt->aux_buf->clear_color_bo == NULL)
> + return;
> +
> + /* We can't update the clear color while the hardware is still using the
> + * previous one for a resolve or sampling from it. Make sure that there are
> + * no pending commands at this point.
> + */
> + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_CS_STALL);
> + for (int i = 0; i < 4; i++) {
> + brw_store_data_imm32(brw, mt->aux_buf->clear_color_bo,
> + mt->aux_buf->clear_color_offset + i * 4,
> + mt->fast_clear_color.u32[i]);
> + }
> + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_STATE_CACHE_INVALIDATE);
> +}
Just trying to bring attention to this piece of code. Jason suggested
that these PIPE_CONTROL's might not be sufficient, and that we need
tests that clear and texture from the surface repeatedly (I didn't look
at that yet).
And I think Ken suggested that this was such an edge case that maybe the
optimization wasn't worth it. I'm adding them both in copy in case they
want to add something.
I agree that if we are already doing it for depth surfaces, and the code
can easily be shared, let's just do it. So the two patches in this
series are
Reviewed-by: Rafael Antognolli <rafael.antognolli at intel.com>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index bb059cf4e8f..c306a9048f3 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -748,6 +748,13 @@ intel_miptree_set_depth_clear_value(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> float clear_value);
>
> +/* If this miptree has an indirect clear color, update it with the value stored
> + * in the miptree object.
> + */
> +void
> +intel_miptree_update_indirect_color(struct brw_context *brw,
> + struct intel_mipmap_tree *mt);
> +
> #ifdef __cplusplus
> }
> #endif
> --
> 2.16.2
>
> _______________________________________________
> 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