On Mon, Mar 9, 2015 at 9:43 PM, Ben Widawsky
<benjamin.widaw...@intel.com> wrote:
> IMHO, intel_miptree_choose_tiling() is an unfortunate incarnation because it
> conflates what is permitted vs. what is desirable. This makes doing any sort
> of
> "fallback" operations after the fact somewhat kludgey.
>
> The original code basically says:
> "if we requested x XOR y-tiled, and the region > aperture; then try to
> x-tiled"
>
> Better would be:
> "if we *received* x OR y-tiled, and the region > aperture; then try to
> x-tiled"
>
> Optimally it is:
> "if we can use either x OR y-tiled and got y-tiled, and the region >
> aperture; then try
> x tiled"
>
> This patch actually addresses two potential issues:
> 1. As far as I can tell, drm_intel_gem_bo_alloc_tiled() which is invariably
> called, can potentially change the tiling type. Therefore, we shouldn't be
> checking the requested tiling type, but rather the granted tiling
> 2. The existing code can fall back to X-tiled even if choose_tiling said
> X-tiling was not okay.
>
> Neither of these are probably actually an issue, but this simply makes the
> code
> correct.
>
> The changes behavior originally introduced here:
> commit cbe24ff7c8d69a6d378d9c2cce2bc117517f4302
> Author: Kenneth Graunke <kenn...@whitecape.org>
> Date: Wed Apr 10 13:49:16 2013 -0700
>
> intel: Fall back to X-tiling when larger than estimated aperture size.
>
> v2: This was rebased on a recent commit than Anuj pushed since I originally
> authored the patch.
> commit 524a729f68c15da3fc6c42b3158a13e0b84c2728
> Author: Anuj Phogat <anuj.pho...@gmail.com>
> Date: Tue Feb 17 10:40:58 2015 -0800
>
> i965: Fix condition to use Y tiling in blitter in intel_miptree_create()
>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> Cc: Chad Versace <chad.vers...@linux.intel.com>
> Cc: Anuj Phogat <anuj.pho...@gmail.com>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 36c3b26..cc422d3 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -630,10 +630,8 @@ intel_miptree_create(struct brw_context *brw,
> uint32_t tiling = intel_miptree_choose_tiling(brw, format, width0,
> num_samples,
> requested_tiling,
> mt);
> - bool y_or_x = false;
>
> if (tiling == (I915_TILING_Y | I915_TILING_X)) {
> - y_or_x = true;
> mt->tiling = I915_TILING_Y;
> } else {
> mt->tiling = tiling;
> @@ -652,7 +650,10 @@ intel_miptree_create(struct brw_context *brw,
> * BLT engine to support it. Prior to Sandybridge, the BLT paths can't
> * handle Y-tiling, so we need to fall back to X.
> */
> - if (brw->gen < 6 && y_or_x && mt->bo->size >=
> brw->max_gtt_map_object_size) {
> + if (brw->gen < 6 &&
> + mt->bo->size >= brw->max_gtt_map_object_size &&
> + mt->tiling == I915_TILING_Y &&
> + requested_tiling == INTEL_MIPTREE_TILING_ANY) {
> perf_debug("%dx%d miptree larger than aperture; falling back to
> X-tiled\n",
> mt->total_width, mt->total_height);
>
> --
> 2.3.1
>
With the suggested changes, patches 1-5 are:
Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
