[Mesa-dev] [PATCH 1/5] i965: Push miptree tiling request into flags

[Chad Versace]

On Tue 14 Jul 2015, Ben Widawsky wrote:
> With the last few patches a way was provided to influence lower layer miptree
> layout and allocation decisions via flags (replacing bools). For simplicity, I
> chose not to touch the tiling requests because the change was slightly less
> mechanical than replacing the bools.
> 
> The goal is to organize the code so we can continue to add new parameters and
> tiling types while minimizing risk to the existing code, and not having to
> constantly add new function parameters.
> 
> v2: Rebased on Anuj's recent Yf/Ys changes
> Fix non-msrt MCS allocation (was only happening in gen8 case before)
> 
> Cc: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: Chad Versace <chad.versace at intel.com>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>

I have one nitpick...

> -   /* 'requested' parameter of intel_miptree_create_layout() is relevant
> -    * only for non bo miptree. Tiling for bo is already computed above.
> -    * So, the tiling requested (INTEL_MIPTREE_TILING_ANY) below is
> -    * just a place holder and will not make any change to the miptree
> -    * tiling format.
> +   /* The BO already has a tiling format and we shouldn't confuse the lower
> +    * layers by making it try to find a tiling format again.
>      */
> +   assert((layout_flags &
> +          (MIPTREE_LAYOUT_ALLOC_ANY_TILED | MIPTREE_LAYOUT_ALLOC_LINEAR)) == 0);
>     layout_flags |= MIPTREE_LAYOUT_FOR_BO;

I think the assert would be more readable if split as below. Also, the
split version eliminates uncertainty when interpreting the assertion
failure: it will tell exactly which offending flag is present.

    assert(layout_flags & MIPTREE_LAYOUT_ALLOC_ANY_TILED == 0);
    assert(layout_flags & MIPTREE_LAYOUT_ALLOC_LINEAR == 0);

With or without that change, patch 1 is
Reviewed-by: Chad Versace <chad.versace at intel.com>