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

[Matt Turner]

On Thu, Jul 16, 2015 at 11:49 AM, Chad Versace <chad.versace at intel.com> wrote:
> 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);

Note that you need parentheses around the & expression. Otherwise you
actually get a & (b == c).