On Mon, Aug 17, 2015 at 12:24 PM, Ben Widawsky <b...@bwidawsk.net> wrote:
> 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 <b...@bwidawsk.net>
>> Cc: <mesa-sta...@lists.freedesktop.org>
>> Signed-off-by: Anuj Phogat <anuj.pho...@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).
>
I can drop the Cc stable because we had enough checks earlier in the
code to make sure we get the tile aligned pitch.
>> ---
>> 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);
>
drm_intel_gem_bo_tile_pitch() aligns the pitch to 64 in case of no tiling.
That makes this assert to fail. So, we need to exclude the no tiling case here.
> (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 <b...@bwidawsk.net>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
