[Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.

[Kenneth Graunke]

On 10/08/2013 10:36 AM, Eric Anholt wrote:
> We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd
> experimentally found to be necessary!), but while the spec still requires
> it on gen5, we'd been missing it in the array-layout cubemaps.

Ah, I see it now:

>From the Sandybridge PRM, Volume 1, Part 1, Section 7.19...OR...the
Ivybridge PRM, Volume 1, Part 1, Section 6.19 (Surface Padding
Requirements):

"For cube surfaces, an additional two rows of padding are required at
the bottom of the surface.  This must be ensured regardless of whether
the surface is stored tiled or linear.  This is due to the potential
rotation of cache line orientation from memory to cache."

So it sounds like this is indeed necessary on all platforms.  Unless
they just forgot to remove this text from the docs.  Have you observed
it to fix anything?

Even if you haven't, we should do it anyway - it won't hurt anything and
may avoid page faults which manifest as hangs.

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

> 
> Cc: "9.1 9.2" <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/drivers/dri/i965/brw_tex_layout.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index e4e66b4..e9128a3 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -197,6 +197,18 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
>  }
>  
>  static void
> +align_cube(struct intel_mipmap_tree *mt)
> +{
> +   /* The 965's sampler lays cachelines out according to how accesses
> +    * in the texture surfaces run, so they may be "vertical" through
> +    * memory.  As a result, the docs say in Surface Padding Requirements:
> +    * Sampling Engine Surfaces that two extra rows of padding are required.
> +    */
> +   if (mt->target == GL_TEXTURE_CUBE_MAP)
> +      mt->total_height += 2;
> +}
> +
> +static void
>  brw_miptree_layout_texture_array(struct brw_context *brw,
>  				 struct intel_mipmap_tree *mt)
>  {
> @@ -220,6 +232,8 @@ brw_miptree_layout_texture_array(struct brw_context *brw,
>        }
>     }
>     mt->total_height = qpitch * mt->physical_depth0;
> +
> +   align_cube(mt);
>  }
>  
>  static void
> @@ -291,13 +305,7 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
>        }
>     }
>  
> -   /* The 965's sampler lays cachelines out according to how accesses
> -    * in the texture surfaces run, so they may be "vertical" through
> -    * memory.  As a result, the docs say in Surface Padding Requirements:
> -    * Sampling Engine Surfaces that two extra rows of padding are required.
> -    */
> -   if (mt->target == GL_TEXTURE_CUBE_MAP)
> -      mt->total_height += 2;
> +   align_cube(mt);
>  }
>  
>  void
>