[v5,13/19] i965/blorp: Update the fast clear value buffer.

Submitted by Rafael Antognolli on March 29, 2018, 5:58 p.m.

Details

Message ID 20180329175853.23728-14-rafael.antognolli@intel.com
State New
Headers show
Series "Use clear color address in surface state." ( rev: 7 6 5 ) in Mesa

Not browsing as part of any series.

Commit Message

Rafael Antognolli March 29, 2018, 5:58 p.m.
On Gen10, whenever we do a fast clear, blorp will update the clear color
state buffer for us, as long as we set the clear color address
correctly.

However, on a hiz clear, if the surface is already on the fast clear
state we skip the actual fast clear operation and, before gen10, only
updated the miptree. On gen10+ we need to update the clear value state
buffer too, since blorp will not be doing a fast clear and updating it
for us.

v4:
 - do not use clear_value_size in the for loop
 - Get the address of the clear color from the aux buffer or the
 clear_color_bo, depending on which one is available.
 - let core blorp update the clear color, but also update it when we
 skip a fast clear depth.

v5: Better subject (Jordan).

Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 src/mesa/drivers/dri/i965/brw_blorp.c | 11 +++++++++++
 src/mesa/drivers/dri/i965/brw_clear.c | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
index a0977598309..e2287cbad3b 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -185,6 +185,17 @@  blorp_surf_for_miptree(struct brw_context *brw,
 
       surf->aux_addr.buffer = aux_buf->bo;
       surf->aux_addr.offset = aux_buf->offset;
+
+      if (devinfo->gen >= 10) {
+         /* If we have a CCS surface and clear_color_bo set, use that bo as
+          * storage for the indirect clear color. Otherwise, use the extra
+          * space at the end of the aux_buffer.
+          */
+         surf->clear_color_addr = (struct blorp_address) {
+            .buffer = aux_buf->clear_color_bo,
+            .offset = aux_buf->clear_color_offset,
+         };
+      }
    } else {
       surf->aux_addr = (struct blorp_address) {
          .buffer = NULL,
diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c
index 8aa83722ee9..63c0b241898 100644
--- a/src/mesa/drivers/dri/i965/brw_clear.c
+++ b/src/mesa/drivers/dri/i965/brw_clear.c
@@ -108,6 +108,7 @@  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;
@@ -213,6 +214,7 @@  brw_fast_clear_depth(struct gl_context *ctx)
       }
 
       intel_miptree_set_depth_clear_value(ctx, mt, clear_value);
+      same_clear_value = false;
    }
 
    bool need_clear = false;
@@ -232,6 +234,26 @@  brw_fast_clear_depth(struct gl_context *ctx)
        * 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->hiz_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->hiz_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;
    }
 

Comments

On Thu, Mar 29, 2018 at 10:58 AM, Rafael Antognolli <
rafael.antognolli@intel.com> wrote:

> On Gen10, whenever we do a fast clear, blorp will update the clear color
> state buffer for us, as long as we set the clear color address
> correctly.
>
> However, on a hiz clear, if the surface is already on the fast clear
> state we skip the actual fast clear operation and, before gen10, only
> updated the miptree. On gen10+ we need to update the clear value state
> buffer too, since blorp will not be doing a fast clear and updating it
> for us.
>
> v4:
>  - do not use clear_value_size in the for loop
>  - Get the address of the clear color from the aux buffer or the
>  clear_color_bo, depending on which one is available.
>  - let core blorp update the clear color, but also update it when we
>  skip a fast clear depth.
>
> v5: Better subject (Jordan).
>
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c | 11 +++++++++++
>  src/mesa/drivers/dri/i965/brw_clear.c | 22 ++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index a0977598309..e2287cbad3b 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -185,6 +185,17 @@ blorp_surf_for_miptree(struct brw_context *brw,
>
>        surf->aux_addr.buffer = aux_buf->bo;
>        surf->aux_addr.offset = aux_buf->offset;
> +
> +      if (devinfo->gen >= 10) {
> +         /* If we have a CCS surface and clear_color_bo set, use that bo
> as
> +          * storage for the indirect clear color. Otherwise, use the extra
> +          * space at the end of the aux_buffer.
> +          */
>

This comment is out of date now that we have a clear_color_bo/offset in
aux_buf.  Let's just drop the comment.


> +         surf->clear_color_addr = (struct blorp_address) {
> +            .buffer = aux_buf->clear_color_bo,
> +            .offset = aux_buf->clear_color_offset,
> +         };
> +      }
>     } else {
>        surf->aux_addr = (struct blorp_address) {
>           .buffer = NULL,
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> b/src/mesa/drivers/dri/i965/brw_clear.c
> index 8aa83722ee9..63c0b241898 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -108,6 +108,7 @@ 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;
> @@ -213,6 +214,7 @@ brw_fast_clear_depth(struct gl_context *ctx)
>        }
>
>        intel_miptree_set_depth_clear_value(ctx, mt, clear_value);
> +      same_clear_value = false;
>     }
>
>     bool need_clear = false;
> @@ -232,6 +234,26 @@ brw_fast_clear_depth(struct gl_context *ctx)
>         * 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->hiz_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->hiz_buf->clear_color_bo,
> +                                 clear_offset + i * 4,
> clear_color.u32[i]);
> +         }
> +         brw_emit_pipe_control_flush(brw, PIPE_CONTROL_STATE_CACHE_
> INVALIDATE);
>

I'm still not convinced by those pipe controls but I'm also not too worried
about this edge case.

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>


> +      }
>        return true;
>     }
>
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>