[Mesa-dev] [PATCH] i965: Do Sandybridge workaround flushes before each primitive.

[Chad Versace]

On 01/09/2015 11:07 PM, Kenneth Graunke wrote:
> Sandybridge requires the post-sync non-zero workaround in a ton of
> places, and if you ever miss one, the GPU usually hangs.
> 
> Currently, we try to track exactly when a workaround flush is
> necessary (via the brw->batch.need_workaround_flush flag).  This is
> tricky to get right, and we've botched it several times in the past.
> 
> This patch unconditionally performs the post-sync non-zero flush at the
> start of each primitive's state upload (including BLORP).  We drop the
> needs_workaround_flush flag, and drop all the other callers, as the
> flush has already been performed.
> 
> We have no data to indicate that simply flushing all the time will
> hurt performance, and it has the potential to help stability.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>

Ken, what prompted you to write this patch? Did a SNB hang bite you
recently?

I know how painful it is to solve i-forgot-fush-hangs on SNB, so
I'm in favor of this patch if it doesn't hurt performance?

Before I give it a rb, I'd like to know how you confirmed that it
didn't hurt perf.

> @@ -883,10 +863,6 @@ brw_upload_invariant_state(struct brw_context *brw)
>  {
>     const bool is_965 = brw->gen == 4 && !brw->is_g4x;
>  
> -   /* 3DSTATE_SIP, 3DSTATE_MULTISAMPLE, etc. are nonpipelined. */
> -   if (brw->gen == 6)
> -      intel_emit_post_sync_nonzero_flush(brw);
> -

With this hunk, no workaround flush happens before uploading the invariant
state if hw ctx is enabled (which it isn't on Chrome OS btw).
Pre-patch, the flush did happen because intel_batchbuffer_init()
set need_workaround_flush = true. This worries me. Should I be worried?

Since this patch's goal is to be overly cautious, I would like to be
overlay cautious here too and continue to do the flush.