[Mesa-dev] [PATCH v3] i965: intel_texture_barrier reimplemented

[Alejandro Piñeiro]

On 30/06/16 23:16, Francisco Jerez wrote:
> Alejandro Piñeiro <apinheiro at igalia.com> writes:
>
>> Fixes:
>> GL44-CTS.texture_barrier_ARB.same-texel-rw-multipass
>>
>> On Haswell, Broadwell and Skylake (note that in order to execute that
>> test, it is needed to override GL and GLSL versions).
>>
>> On gen6 this test was already working without this change. It keeps
>> working after it.
>>
>> This commit replaces the call to brw_emit_mi_flush for gen6+ with two
>> calls to brw_emit_pipe_control_flush:
>>
>>  * The first one with RENDER_TARGET_FLUSH and CS_STALL set to initiate
>>    a render cache flush after any concurrent rendering completes and
>>    cause the CS to stop parsing commands until the render cache
>>    becomes coherent with memory.
>>
>>  * The second one have TEXTURE_CACHE_INVALIDATE set (and no CS stall)
>>    to clean up any stale data from the sampler caches before rendering
>>    continues.
>>
>> Didn't touch gen4-5, basically because I don't have a way to test
>> them.
>>
>> More info on commits:
>> 0aa4f99f562a05880a779707cbcd46be459863bf
>> 72473658c51d5e074ce219c1e6385a4cce29f467
>>
>> Thanks to Curro to help to tracking this down, as the root case was a
>> hw race condition.
>>
>> v2: use two calls to pipe_control_flush instead of a combination of
>>     gen7_emit_cs_stall_flush and brw_emit_mi_flush calls (Curro)
>> v3: no need to const cache invalidation (Curro)
>> ---
>>
>> FWIW: checked with the CTS tests, and the piglit series, and confirmed
>> that the const cache invalidation is not needed.
>>
>>  src/mesa/drivers/dri/i965/intel_tex.c | 21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c
>> index cac33ac..a802d5a 100644
>> --- a/src/mesa/drivers/dri/i965/intel_tex.c
>> +++ b/src/mesa/drivers/dri/i965/intel_tex.c
>> @@ -9,6 +9,7 @@
>>  #include "intel_mipmap_tree.h"
>>  #include "intel_tex.h"
>>  #include "intel_fbo.h"
>> +#include "intel_reg.h"
>>  
>>  #define FILE_DEBUG_FLAG DEBUG_TEXTURE
>>  
>> @@ -362,7 +363,25 @@ intel_texture_barrier(struct gl_context *ctx)
>>  {
>>     struct brw_context *brw = brw_context(ctx);
>>  
>> -   brw_emit_mi_flush(brw);
>> +   if (brw->gen >= 6) {
>> +      if (brw->gen == 6) {
>> +         /* [Dev-SNB{W/A}]: Before a PIPE_CONTROL with Write Cache
>> +          * Flush Enable = 1, a PIPE_CONTROL with any non-zero
>> +          * post-sync-op is required.
>> +          */
>> +         brw_emit_post_sync_nonzero_flush(brw);
>> +      }
>> +
>> +      brw_emit_pipe_control_flush(brw,
>> +                                  PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>> +                                  PIPE_CONTROL_RENDER_TARGET_FLUSH |
>> +                                  PIPE_CONTROL_CS_STALL);
>> +
>> +      brw_emit_pipe_control_flush(brw,
>> +                                  PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE);
>> +   } else {
>> +      brw_emit_mi_flush(brw);
>> +   }
> Looks reasonable to me, let's get this bug fixed on master, things can
> be refactored later on:
>
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>

Ok, just pushed. Thanks for all the review and comments.


BR

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160701/431725f8/attachment-0001.sig>