[Mesa-dev] [PATCH 3/3] r600g: Don't set the dest cache bits on surface sync for R600_CONTEXT_FLUSH_AND_INV

[Marek Olšák]

This is a funny subject. Originally, we only used SURFACE_SYNC on
Evergreen, which is what the hw guys recommend using, but then Jerome
came and rewrote it with no reasonable argument to back it up (what he
was trying to fix by his cache-flush rework is not fixed to this day),
such that we now flush and invalidate more caches than we need.

FLUSH_AND_INV isn't recommended, because it should be slower in theory
when streamout is used. Frequent changes of streamout buffers would
also flush and invalidate the framebuffer cache, which is undesirable.
Unfortunately, I don't know of any apps using streamout.

This patch looks good. However, once we start seeing apps taking full
advantage of GL3 and GL4, we will have to switch back to SURFACE_SYNC
at least for graphics.

Marek

On Fri, Apr 26, 2013 at 7:21 PM, Tom Stellard <tom at stellard.net> wrote:
> From: Tom Stellard <thomas.stellard at amd.com>
>
> We are already emitting a EVENT_TYPE_CACHE_FLUSH_AND_INV_EVENT packet
> when this flush flag is set, so flushing the dest caches with a
> SURFACE_SYNC should not be necessary.
>
> The motivation for this change is that emitting a SURFACE_SYNC packet with
> the CB bits set was causing compute shaders to hang on Cayman.
> ---
>  src/gallium/drivers/r600/r600_hw_context.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_hw_context.c b/src/gallium/drivers/r600/r600_hw_context.c
> index b4fb3bf..8aebd25 100644
> --- a/src/gallium/drivers/r600/r600_hw_context.c
> +++ b/src/gallium/drivers/r600/r600_hw_context.c
> @@ -231,21 +231,19 @@ void r600_flush_emit(struct r600_context *rctx)
>                 cs->buf[cs->cdw++] = PKT3(PKT3_EVENT_WRITE, 0, 0);
>                 cs->buf[cs->cdw++] = EVENT_TYPE(EVENT_TYPE_CACHE_FLUSH_AND_INV_EVENT) | EVENT_INDEX(0);
>                 if (rctx->chip_class >= EVERGREEN) {
> -                       cp_coher_cntl = S_0085F0_CB0_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB1_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB2_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB3_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB4_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB5_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB6_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB7_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB8_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB9_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB10_DEST_BASE_ENA(1) |
> -                                       S_0085F0_CB11_DEST_BASE_ENA(1) |
> -                                       S_0085F0_DB_DEST_BASE_ENA(1) |
> -                                       S_0085F0_TC_ACTION_ENA(1) |
> -                                       S_0085F0_CB_ACTION_ENA(1) |
> +                       /* We were previously setting the CB and DB bits on
> +                        * cp_coher_cntl, but this is unnecessary since
> +                        * we are emitting the
> +                        * EVENT_TYPE_CACHE_FLUSH_AND_INV_EVENT packet.
> +                        * Setting the CB bits was causing lockups when using
> +                        * compute on cayman.
> +                        *
> +                        * XXX: Do even need to emit a surface sync packet here?
> +                        * Prior to e5e4c07e7964a3258ed02b530bcdc24c0650204b
> +                        * surface sync was not being emitted with the
> +                        * R600_CONTEXT_FLUSH_AND_INV flag.
> +                        */
> +                       cp_coher_cntl = S_0085F0_TC_ACTION_ENA(1) |
>                                         S_0085F0_DB_ACTION_ENA(1) |
>                                         S_0085F0_SH_ACTION_ENA(1) |
>                                         S_0085F0_SMX_ACTION_ENA(1) |
> --
> 1.8.1.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev