[Mesa-dev] [PATCH 1/5] i965: Push miptree tiling request into flags
chad.versace at intel.com
Thu Jul 16 11:49:14 PDT 2015
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>
More information about the mesa-dev