[Mesa-dev] [PATCH v3 04/19] i965/gen6: Adjust render height in errata case for MSAA

[Kenneth Graunke]

On Friday, August 01, 2014 12:53:34 AM Jordan Justen wrote:
> In the gen6 PRM Volume 1 Part 1: Graphics Core, Section
> 7.18.3.7.1 (Surface Arrays For all surfaces other than separate
> stencil buffer):
> 
>  "[DevSNB] Errata: Sampler MSAA Qpitch will be 4 greater than the
>   value calculated in the equation above , for every other odd Surface
>   Height starting from 1 i.e. 1,5,9,13"
> 
> Since this Qpitch errata only impacts the sampler, we have to adjust
> the input for the rendering surface to achieve the same qpitch. For
> the affected heights, we increment the height by 1 for the rendering
> surface.
> 
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/gen6_surface_state.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen6_surface_state.c b/src/mesa/drivers/dri/i965/gen6_surface_state.c
> index db58de9..141ca6f 100644
> --- a/src/mesa/drivers/dri/i965/gen6_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_surface_state.c
> @@ -96,8 +96,24 @@ gen6_update_renderbuffer_surface(struct brw_context *brw,
>     /* reloc */
>     surf[1] = mt->bo->offset64;
>  
> +   /* In the gen6 PRM Volume 1 Part 1: Graphics Core, Section 7.18.3.7.1
> +    * (Surface Arrays For all surfaces other than separate stencil buffer):
> +    *
> +    * "[DevSNB] Errata: Sampler MSAA Qpitch will be 4 greater than the value
> +    *  calculated in the equation above , for every other odd Surface Height
> +    *  starting from 1 i.e. 1,5,9,13"
> +    *
> +    * Since this Qpitch errata only impacts the sampler, we have to adjust the
> +    * input for the rendering surface to achieve the same qpitch. For the
> +    * affected heights, we increment the height by 1 for the rendering
> +    * surface.
> +    */
> +   int height0 = irb->mt->logical_height0;
> +   if (brw->gen == 6 && irb->mt->num_samples > 1 && (height0 % 4) == 1)
> +      height0++;
> +
>     surf[2] = SET_FIELD(mt->logical_width0 - 1, BRW_SURFACE_WIDTH) |
> -             SET_FIELD(mt->logical_height0 - 1, BRW_SURFACE_HEIGHT) |
> +             SET_FIELD(height0 - 1, BRW_SURFACE_HEIGHT) |
>               SET_FIELD(irb->mt_level - irb->mt->first_level, BRW_SURFACE_LOD);
>  
>     surf[3] = brw_get_surface_tiling_bits(mt->tiling) |
> 

FWIW, I believe this code is correct after all.  I worked through a lot of math to show the effects this has on QPitch, and it originally didn't work out.  It turns out our QPitch computation in brw_tex_layout.c is wrong; once I did the correct QPitch computation, it worked out.

I'm going to write patches to fix that.

So this gets a:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

But please wait on the series for a bit - I'd like to look over the rest, and see how my qpitch fixes affect things.  I'll also post my math demonstrating why this does the right thing.  Sorry for the hold up...

Really nice work tracking this down.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140806/3d1e4d98/attachment.sig>