[Mesa-dev] [PATCH 1/2] i965 gen4-6: Fix off-by-one errors brw_create_constant_surface()

[Kenneth Graunke]

On 01/11/2012 12:06 PM, Paul Berry wrote:
> Commit 9bdc44a52804a64219a0ca1a061b18596863e524 (i965: Replace struct
> with bit shifting for WM pull constant surfaces) accidentally
> introduced off-by-one errors into the calculation of the surface
> width, height, and depth.  This patch restores the correct
> computation.
>
> The reason this wasn't noticed by Piglit tests is that the size of our
> constant surfaces is always less than 2^20, therefore the off-by-one
> error was causing the "depth" field of the surface to be set to all
> 1's.  The hardware interpreted this as an extremely large surface, so
> overflow checking was effectively disabled.
>
> No Piglit regressions on Sandy Bridge.
>
> NOTE: This is a candidate for the 7.11 and 8.0 branches.
> ---
>   src/mesa/drivers/dri/i965/brw_wm_surface_state.c |    6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index dedf594..b40f5e1 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -698,10 +698,10 @@ brw_create_constant_surface(struct brw_context *brw,
>
>      surf[1] = bo->offset; /* reloc */
>
> -   surf[2] = (((w&  0x7f) - 1)<<  BRW_SURFACE_WIDTH_SHIFT |
> -	      (((w>>  7)&  0x1fff) - 1)<<  BRW_SURFACE_HEIGHT_SHIFT);
> +   surf[2] = ((w&  0x7f)<<  BRW_SURFACE_WIDTH_SHIFT |
> +	      ((w>>  7)&  0x1fff)<<  BRW_SURFACE_HEIGHT_SHIFT);
>
> -   surf[3] = ((((w>>  20)&  0x7f) - 1)<<  BRW_SURFACE_DEPTH_SHIFT |
> +   surf[3] = (((w>>  20)&  0x7f)<<  BRW_SURFACE_DEPTH_SHIFT |
>   	      (width * 16 - 1)<<  BRW_SURFACE_PITCH_SHIFT);
>
>      surf[4] = 0;

Yeah, from reading the simulator, these -1's are clearly wrong.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>