[Mesa-stable] [PATCH 7/8] i965: Fix {src, dst}_pitch alignment check for XY_SRC_COPY_BLT

[Ben Widawsky]

On Fri, Aug 14, 2015 at 04:51:58PM -0700, Anuj Phogat wrote:
> Current code checks the alignment restrictions only for Y tiling.
> From Broadwell PRM vol 10:
> 
>  "pitch is of 512Byte granularity for Tile-X: This means the tiled-x
>   surface pitch can be (512, 1024, 1536, 2048...)/4 (in Dwords)."
> 
> This patch adds the restriction for X tiling as well.
> 
> Cc: Ben Widawsky <ben at bwidawsk.net>
> Cc: <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>

I don't think you need to Cc stable because the other tiling modes aren't used,
right? (also, the backport you've left is kinda sucky, you should probably
manually send the backport to stable if you feel it's applicabale).

> ---
>  src/mesa/drivers/dri/i965/intel_blit.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c
> index d15a64d..729ffb0 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -531,6 +531,8 @@ intelEmitCopyBlit(struct brw_context *brw,
>     bool dst_y_tiled = dst_tiling == I915_TILING_Y;
>     bool src_y_tiled = src_tiling == I915_TILING_Y;
>     bool use_fast_copy_blit = false;
> +   uint32_t src_tile_w, src_tile_h;
> +   uint32_t dst_tile_w, dst_tile_h;
>  
>     if ((dst_y_tiled || src_y_tiled) && brw->gen < 6)
>        return false;
> @@ -559,6 +561,11 @@ intelEmitCopyBlit(struct brw_context *brw,
>         src_buffer, src_pitch, src_offset, src_x, src_y,
>         dst_buffer, dst_pitch, dst_offset, dst_x, dst_y, w, h);
>  
> +   intel_miptree_get_tile_dimensions(src_tiling, src_tr_mode, cpp,
> +                                     &src_tile_w, &src_tile_h);
> +   intel_miptree_get_tile_dimensions(dst_tiling, dst_tr_mode, cpp,
> +                                     &dst_tile_w, &dst_tile_h);
> +
>     use_fast_copy_blit = can_fast_copy_blit(brw,
>                                             src_buffer,
>                                             src_x, src_y,
> @@ -597,8 +604,8 @@ intelEmitCopyBlit(struct brw_context *brw,
>                          cpp, use_fast_copy_blit);
>  
>     } else {
> -      assert(!dst_y_tiled || (dst_pitch % 128) == 0);
> -      assert(!src_y_tiled || (src_pitch % 128) == 0);
> +      assert(src_tiling == I915_TILING_NONE || (src_pitch % src_tile_w) == 0);
> +      assert(dst_tiling == I915_TILING_NONE || (dst_pitch % dst_tile_w) == 0);
>  
>        /* For big formats (such as floating point), do the copy using 16 or
>         * 32bpp and multiply the coordinates.


I like the generalization again (like the last patch). Do you actually need the
check for TILING_NONE after all the plumbing you've done? I believe it's just:
assert(src_pitch % src_tile_w == 0);
assert(dst_pitch % dst_tile_w == 0);

(since I don't think pitch can be anything but a multiple of cpp).

Kill the cc stable unless you prove it's needed and it's:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>