[Mesa-dev] [PATCH] i965: Program the dynamic state heap size to MAX_STATE_SIZE.

[Jordan Justen]

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On 2017-11-29 01:08:23, Kenneth Graunke wrote:
> STATE_BASE_ADDRESS specifies a maximum size of the dynamic state
> section, beyond which data supposedly reads back as 0.  On Gen8+,
> we were programming it to the size of the buffer.  This worked fine
> until we started growing the state buffer in commit 2dfc119f22f25708.
> When the state buffer grows, the value in STATE_BASE_ADDRESS becomes
> too small, and our state beyond STATE_SZ bytes would read back as 0.
> 
> To avoid having to update the value, we program it to MAX_STATE_SIZE.
> We used to program the upper bound to the maximum on older hardware
> anyway, so programming it too large isn't a big deal.
> 
> Bogus SURFACE_STATE can easily lead to GPU hangs and misrendering.
> DiRT Rally was hitting the statebuffer growth path, and suffered from
> bad texture corruption and GPU hangs (usually around the same time).
> 
> This patch fixes both issues.
> 
> Fixes: 2dfc119f22f257082ab0 "i965: Grow the batch/state buffers if we need space and can't flush."
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103101
> Tested-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_misc_state.c    | 2 +-
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 9 ---------
>  src/mesa/drivers/dri/i965/intel_batchbuffer.h | 9 +++++++++
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index 94d5c9783db..2d7471d40c9 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -641,7 +641,7 @@ brw_upload_state_base_address(struct brw_context *brw)
>        /* General state buffer size */
>        OUT_BATCH(0xfffff001);
>        /* Dynamic state buffer size */
> -      OUT_BATCH(ALIGN(brw->batch.state_bo->size, 4096) | 1);
> +      OUT_BATCH(ALIGN(MAX_STATE_SIZE, 4096) | 1);
>        /* Indirect object upper bound */
>        OUT_BATCH(0xfffff001);
>        /* Instruction access upper bound */
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 216073129ba..10e33bb8c7b 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -52,15 +52,6 @@
>  #define BATCH_SZ (20 * 1024)
>  #define STATE_SZ (16 * 1024)
>  
> -/* The kernel assumes batchbuffers are smaller than 256kB. */
> -#define MAX_BATCH_SIZE (256 * 1024)
> -
> -/* 3DSTATE_BINDING_TABLE_POINTERS has a U16 offset from Surface State Base
> - * Address, which means that we can't put binding tables beyond 64kB.  This
> - * effectively limits the maximum statebuffer size to 64kB.
> - */
> -#define MAX_STATE_SIZE (64 * 1024)
> -
>  static void
>  intel_batchbuffer_reset(struct brw_context *brw);
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> index 5a2649f06f3..e186e29ec67 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> @@ -10,6 +10,15 @@
>  extern "C" {
>  #endif
>  
> +/* The kernel assumes batchbuffers are smaller than 256kB. */
> +#define MAX_BATCH_SIZE (256 * 1024)
> +
> +/* 3DSTATE_BINDING_TABLE_POINTERS has a U16 offset from Surface State Base
> + * Address, which means that we can't put binding tables beyond 64kB.  This
> + * effectively limits the maximum statebuffer size to 64kB.
> + */
> +#define MAX_STATE_SIZE (64 * 1024)
> +
>  struct intel_batchbuffer;
>  
>  void intel_batchbuffer_init(struct brw_context *brw);
> -- 
> 2.15.0
>