On Wed, Jun 10, 2015 at 05:01:44PM -0700, Nanley Chery wrote:
> From: Nanley Chery <nanley.g.ch...@intel.com>
>
> On Gen9+, vertical and horizontal alignment values for compressed textures are
> equal to the pre-Gen9 value squared. Each miplevel must be aligned to this
> value.
>
> Signed-off-by: Nanley Chery <nanley.g.ch...@intel.com>
While not a requirement, for future reference you should add Cc on patches which
are touching something someone recently changed/and or was working on. In this
case Ccing Neil would have been great.
> ---
>
> This fixes an FXT1 Piglit test regression and shows no failures on Jenkins.
You ran full piglit on SKL? Jenkins won't and it's pretty important for this
patch.
>
> src/mesa/drivers/dri/i965/brw_tex_layout.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 312a887..dffc699 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -149,15 +149,8 @@ intel_horizontal_texture_alignment_unit(struct
> brw_context *brw,
> unsigned int i, j;
> _mesa_get_format_block_size(mt->format, &i, &j);
>
> - /* On Gen9+ we can pick our own alignment for compressed textures but
> it
> - * has to be a multiple of the block size. The minimum alignment we can
> - * pick is 4 so we effectively have to align to 4 times the block
> - * size
> - */
> - if (brw->gen >= 9)
> - return i * 4;
> - else
> - return i;
> + /* On Gen9+ the alignment value is squared. */
> + return brw->gen >= 9 ? i * i : i;
I don't think this is right. Isn't this going to push non compressed textures to
an invalid HALIGN when we divide later ie. don't you get 1?
> }
>
> if (mt->format == MESA_FORMAT_S_UINT8)
> @@ -269,9 +262,12 @@ intel_vertical_texture_alignment_unit(struct brw_context
> *brw,
> * Where "*" means either VALIGN_2 or VALIGN_4 depending on the setting of
> * the SURFACE_STATE "Surface Vertical Alignment" field.
> */
> - if (_mesa_is_format_compressed(mt->format))
> + if (_mesa_is_format_compressed(mt->format)) {
> + unsigned int i, j;
> + _mesa_get_format_block_size(mt->format, &i, &j);
> /* See comment above for the horizontal alignment */
I think you need to kill this comment now. Doesn't it refer to what you killed
above?
> - return brw->gen >= 9 ? 16 : 4;
> + return brw->gen >= 9 ? j * j : j;
> + }
It kind of looks like this is just working around the way Neil implemented the
divide later on. There's probably a nicer way to do the right thing, but I
haven't tried it myself. Also, I believe this doesn't actually fix anything, it
just uses slightly more optimal aligns - ie. maybe do a separate patch. No big
deal though.
>
> if (mt->format == MESA_FORMAT_S_UINT8)
> return brw->gen >= 7 ? 8 : 4;
> @@ -379,7 +375,7 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
>
> if (mt->compressed) {
> mip1_width = ALIGN(minify(mt->physical_width0, 1), mt->align_w) +
> - ALIGN(minify(mt->physical_width0, 2), bw);
> + ALIGN(minify(mt->physical_width0, 2), mt->align_w);
> } else {
> mip1_width = ALIGN(minify(mt->physical_width0, 1), mt->align_w) +
> minify(mt->physical_width0, 2);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
