[Mesa-dev] [PATCH 1/6] i965: Kill y_or_x variable in miptree tiling selection

[Anuj Phogat]

On Mon, Mar 9, 2015 at 9:43 PM, Ben Widawsky
<benjamin.widawsky at 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 <kenneth at 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.phogat at 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 <kenneth at whitecape.org>
> Cc: Chad Versace <chad.versace at linux.intel.com>
> Cc: Anuj Phogat <anuj.phogat at gmail.com>
> Signed-off-by: Ben Widawsky <ben at 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.phogat at gmail.com>