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

[Alex Deucher]

On Fri, May 3, 2013 at 9:30 AM, Vadim Girlin <vadimgirlin at gmail.com> wrote:
> This patch results in lockups with Heaven on juniper for me.

Does dropping the surface_sync packet completely help?  We shouldn't
need a surface_sync packet after a CACHE_FLUSH_AND_INV_EVENT packet
and prior to e5e4c07e7964a3258ed02b530bcdc24c0650204b we never emitted
it.

Alex

diff --git a/src/gallium/drivers/r600/r600_hw_context.c
b/src/gallium/drivers/r600/r600_hw_context.c
index 6d8b2cf..944b666 100644
--- a/src/gallium/drivers/r600/r600_hw_context.c
+++ b/src/gallium/drivers/r600/r600_hw_context.c
@@ -226,32 +226,6 @@ void r600_flush_emit(struct r600_context *rctx)
        if (rctx->flags & R600_CONTEXT_FLUSH_AND_INV) {
                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) {
-                       /* 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) |
-                                       S_0085F0_FULL_CACHE_ENA(1);
-               } else {
-                       cp_coher_cntl = S_0085F0_SMX_ACTION_ENA(1) |
-                                       S_0085F0_SH_ACTION_ENA(1) |
-                                       S_0085F0_VC_ACTION_ENA(1) |
-                                       S_0085F0_TC_ACTION_ENA(1) |
-                                       S_0085F0_FULL_CACHE_ENA(1);
-               }
-               emit_flush = 1;
        }

        if (rctx->flags & R600_CONTEXT_INVAL_READ_CACHES) {


>
> Vadim
>
>
>
> On 04/26/2013 09:21 PM, Tom Stellard 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) |
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev