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

[Anuj Phogat]

On Mon, Aug 10, 2015 at 4:15 PM, Ben Widawsky <benjamin.widawsky at intel.com>
wrote:

> NOTE: The commit message is retained for posterity, however there were some
> changes in the code since the patch was originally written that may make
> the old
> commit message false. Starting with v4 is actually much simpler than the
> original change.
>
> This patch is needed to help keep some of the churn down later in the
> series
> (IIRC)
>
> v3: Rebased + redone on top of the layout flags + tiling patch.
>
> Original commit message:
>
> 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()
>
> v3: Removed the check against X-tiling since its removal in
> 9f78e27fc60b3473b708ab4ca04e4ebd6be6cf4e
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index e85c3f0..e0a7f11 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -651,10 +651,7 @@ intel_miptree_create(struct brw_context *brw,
>        total_height = ALIGN(total_height, 64);
>     }
>
> -   bool y_or_x = false;
> -
>     if (mt->tiling == (I915_TILING_Y | I915_TILING_X)) {
> -      y_or_x = true;
>        mt->tiling = I915_TILING_Y;
>     }
>
> @@ -684,7 +681,9 @@ 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) {
>
This change might force x tiling on miptrees which specifically asked for Y
tiling
using layout flag MIPTREE_LAYOUT_TILING_Y in brw_miptree_choose_tiling().
This flag is used while allocating mcs buffer which I think is not relevant
for
< gen6? It'll be useful if you can add a comment here explaining how we're
never gonna hit a case of layout flag MIPTREE_LAYOUT_TILING_Y for < gen6.

       perf_debug("%dx%d miptree larger than aperture; falling back to
> X-tiled\n",
>                   mt->total_width, mt->total_height);
>
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150810/6858dbc2/attachment.html>