[Mesa-dev] [PATCH 4/4] intel/isl: Refactor gen8_choose_image_alignment_el

[Chad Versace]

On Wed 10 May 2017, Jason Ekstrand wrote:
> ---
>  src/intel/isl/isl_gen8.c | 193 +++++++++++++----------------------------------
>  1 file changed, 53 insertions(+), 140 deletions(-)


> +   /* For all other formats, the alignment is determined by the horizontal and
> +    * vertical alignment fields of RENDER_SURFACE_STATE.  There are a few
> +    * restrictions, but we generally have a choice.
> +    */
>  
[...]
> +   /* Vertical alignment is unrestricted so we choose the smallest allowed
> +    * alignment because that will use the least memory
>      */
> +   const uint32_t valign = 4;
>  
> +   bool needs_halign16 = false;
>     if (!(info->usage & ISL_SURF_USAGE_DISABLE_AUX_BIT)) {
[...]
> +      needs_halign16 = true;
>     }

> +   const uint32_t halign = needs_halign16 ? 16 : 4;
[...]
> +   *image_align_el = isl_extent3d(halign, valign, 1);
>  }

Small suggestion. The code would be more straightforward without the
temporary valign, needs_halign16 vars. Something like this:

        ...

        if (!(info->usage & ISL_SURF_USAGE_DISABLE_AUX_BIT)) {
            *image_align_el = isl_extent3d(16, 4, 1);
            return;
        }

        *image_align_el = isl_extent3d(4, 4, 1);
    }

With or without that change, this patch is a good cleanup.
Reviewed-by: Chad Versace <chadversary at chromium.org>