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

[Chris Wilson]

Quoting Kenneth Graunke (2017-11-29 09:08:23)
> 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.

Oh crumbs.
 
> 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);

Ok, this is the only brw->batch.state_bo->size in use.
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

(Sorry for not spotting this earlier.)
-Chris