On 01/05/2014 02:28 PM, Eric Anholt wrote: > Anuj Phogat <anuj.pho...@gmail.com> writes: > >> On Mon, Dec 23, 2013 at 4:08 PM, Eric Anholt <e...@anholt.net> wrote: >>> Noticed by tex3d-maxsize on my next commit to check that our addresses >>> don't overflow. >>> --- >>> src/mesa/drivers/dri/i965/intel_blit.c | 20 ++++++++++++++++++++ >>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 23 ++++++++++++++++++++--- >>> 2 files changed, 40 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c >>> b/src/mesa/drivers/dri/i965/intel_blit.c >>> index 7bc289f..13cc777 100644 >>> --- a/src/mesa/drivers/dri/i965/intel_blit.c >>> +++ b/src/mesa/drivers/dri/i965/intel_blit.c >>> @@ -229,12 +229,32 @@ intel_miptree_blit(struct brw_context *brw, >>> src_x += src_image_x; >>> src_y += src_image_y; >>> >>> + /* The blitter interprets the 16-bit src x/y as a signed 16-bit value, >>> + * where negative values are invalid. The values we're working with are >>> + * unsigned, so make sure we don't overflow. >>> + */ >>> + if (src_x >= 32768 || src_y >= 32768) { >>> + perf_debug("Falling back due to >=32k src offset (%d, %d)\n", >>> + src_x, src_y); >>> + return false; >>> + } >>> + >>> uint32_t dst_image_x, dst_image_y; >>> intel_miptree_get_image_offset(dst_mt, dst_level, dst_slice, >>> &dst_image_x, &dst_image_y); >>> dst_x += dst_image_x; >>> dst_y += dst_image_y; >>> >>> + /* The blitter interprets the 16-bit destination x/y as a signed 16-bit >>> + * value. The values we're working with are unsigned, so make sure we >>> + * don't overflow. >>> + */ >>> + if (dst_x >= 32768 || dst_y >= 32768) { >>> + perf_debug("Falling back due to >=32k dst offset (%d, %d)\n", >>> + dst_x, dst_y); >>> + return false; >>> + } >>> + >>> if (!intelEmitCopyBlit(brw, >>> src_mt->cpp, >>> src_pitch, >>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c >>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c >>> index de47143..0818226 100644 >>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c >>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c >>> @@ -443,7 +443,8 @@ intel_miptree_choose_tiling(struct brw_context *brw, >>> if (minimum_pitch < 64) >>> return I915_TILING_NONE; >>> >>> - if (ALIGN(minimum_pitch, 512) >= 32768) { >>> + if (ALIGN(minimum_pitch, 512) >= 32768 || >>> + mt->total_width >= 32768 || mt->total_height >= 32768) { >>> perf_debug("%dx%d miptree too large to blit, falling back to >>> untiled", >>> mt->total_width, mt->total_height); >>> return I915_TILING_NONE; >>> @@ -2233,6 +2234,22 @@ intel_miptree_release_map(struct intel_mipmap_tree >>> *mt, >>> *map = NULL; >>> } >>> >>> +static bool >>> +can_blit_slice(struct intel_mipmap_tree *mt, >>> + unsigned int level, unsigned int slice) >>> +{ >>> + uint32_t image_x; >>> + uint32_t image_y; >>> + intel_miptree_get_image_offset(mt, level, slice, &image_x, &image_y); >>> + if (image_x >= 32768 || image_y >= 32768) >>> + return false; >>> + >>> + if (mt->region->pitch >= 32768) >>> + return false; >>> + >>> + return true; >>> +} >>> + >>> static void >>> intel_miptree_map_singlesample(struct brw_context *brw, >>> struct intel_mipmap_tree *mt, >>> @@ -2276,11 +2293,11 @@ intel_miptree_map_singlesample(struct brw_context >>> *brw, >>> !mt->compressed && >>> (mt->region->tiling == I915_TILING_X || >>> (brw->gen >= 6 && mt->region->tiling == I915_TILING_Y)) && >>> - mt->region->pitch < 32768) { >>> + can_blit_slice(mt, level, slice)) { >>> intel_miptree_map_blit(brw, mt, map, level, slice); >>> } else if (mt->region->tiling != I915_TILING_NONE && >>> mt->region->bo->size >= brw->max_gtt_map_object_size) { >>> - assert(mt->region->pitch < 32768); >>> + assert(can_blit_slice(mt, level, slice)); >> I think the right thing to do here is: >> - mt->region->bo->size >= brw->max_gtt_map_object_size) { >> - assert(mt->region->pitch < 32768); >> + mt->region->bo->size >= brw->max_gtt_map_object_size && >> + can_blit_slice(mt, level, slice)) { >> >> This allow us falling back to intel_miptree_map_gtt(). I suggested this >> change >> as a part of '[PATCH] i965: Fix the region's pitch condition to use blitter'. > > If region->bo->size >= max_gtt_map_object_size, you can't fall back to > GTT mapping, though. Since you can't do that, previous code needs to > have made sure you can't reach this point.
I agree with Eric - we can't fall through to the next cases, since they'll fail too. Our only hope is to ensure giant objects are forced untiled (which I'm pretty sure we do these days). --Ken _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev