[Mesa-dev] [PATCH 8/8] i965: Avoid flushing the batch for every blorp op.

[Eric Anholt]

Paul Berry <stereotype441 at gmail.com> writes:

> On 27 August 2013 15:21, Eric Anholt <eric at anholt.net> wrote:
>
>> This brings over the batch-wrap-prevention and aperture space checking
>> code from the normal brw_draw.c path, so that we don't need to flush the
>> batch every time.
>>
>> There's a risk here if the intel_emit_post_sync_nonzero_flush() call isn't
>> high enough up in the state emit sequences -- before, we implicitly had
>> one at the batch flush before any state was emitted, so Mesa's workaround
>> emits didn't really matter.
>>
>> Improves cairo-gl performance by 13.7733% +/- 1.74876% (n=30/32)
>> Improves minecraft apitrace performance by 1.03183% +/- 0.482297% (n=90).
>> Reduces low-resolution GLB 2.7 performance by 1.17553% +/- 0.432263% (n=88)
>> Reduces Lightsmark performance by 3.70246% +/- 0.322432% (n=126)
>> No statistically significant performance difference on unigine tropics
>> (n=10)
>> No statistically significant performance difference on openarena (n=755)
>>
>> The two apps that are hurt happen to include stalls on busy buffer
>> objects, so I think this is an effect of missing out on an opportune
>> flush.
>> ---
>>  src/mesa/drivers/dri/i965/brw_blorp.cpp  | 50
>> ++++++++++++++++++++++++++++++++
>>  src/mesa/drivers/dri/i965/brw_blorp.h    |  4 ---
>>  src/mesa/drivers/dri/i965/gen6_blorp.cpp | 12 --------
>>  src/mesa/drivers/dri/i965/gen7_blorp.cpp |  1 -
>>  4 files changed, 50 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
>> b/src/mesa/drivers/dri/i965/brw_blorp.cpp
>> index 1576ff2..c566d1d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
>> @@ -21,6 +21,7 @@
>>   * IN THE SOFTWARE.
>>   */
>>
>> +#include <errno.h>
>>  #include "intel_batchbuffer.h"
>>  #include "intel_fbo.h"
>>
>> @@ -191,6 +192,26 @@ intel_hiz_exec(struct brw_context *brw, struct
>> intel_mipmap_tree *mt,
>>  void
>>  brw_blorp_exec(struct brw_context *brw, const brw_blorp_params *params)
>>  {
>> +   struct gl_context *ctx = &brw->ctx;
>> +   uint32_t estimated_max_batch_usage = 1500;
>> +   bool check_aperture_failed_once = false;
>> +
>> +   /* Flush the sampler and render caches.  We definitely need to flush
>> the
>> +    * sampler cache so that we get updated contents from the render cache
>> for
>> +    * the glBlitFramebuffer() source.  Also, we are sometimes warned in
>> the
>> +    * docs to flush the cache between reinterpretations of the same
>> surface
>> +    * data with different formats, which blorp does for stencil and depth
>> +    * data.
>> +    */
>> +   intel_batchbuffer_emit_mi_flush(brw);
>> +
>> +retry:
>> +   intel_batchbuffer_require_space(brw, estimated_max_batch_usage, false);
>> +   intel_batchbuffer_save_state(brw);
>> +   drm_intel_bo *saved_bo = brw->batch.bo;
>> +   uint32_t saved_used = brw->batch.used;
>> +   uint32_t saved_state_batch_offset = brw->batch.state_batch_offset;
>> +
>>     switch (brw->gen) {
>>     case 6:
>>        gen6_blorp_exec(brw, params);
>> @@ -204,6 +225,35 @@ brw_blorp_exec(struct brw_context *brw, const
>> brw_blorp_params *params)
>>        break;
>>     }
>>
>>
> Would it be feasible to add an assertion here to verify that the amount of
> batch space actually used by this blorp call is less than or equal to
> estimated_max_batch_usage?  That would give me a lot of increased
> confidence that the magic number 1500 is correct.
>
> With the added assertion, the series is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>

That's this code:

+   /* Make sure we didn't wrap the batch unintentionally, and make sure we
+    * reserved enough space that a wrap will never happen.
+    */
+   assert(brw->batch.bo == saved_bo);
+   assert((brw->batch.used - saved_used) * 4 +
+          (saved_state_batch_offset - brw->batch.state_batch_offset) <
+          estimated_max_batch_usage);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130828/6720f269/attachment.pgp>