[Mesa-dev] [PATCH v4 11/34] i965/miptree: Add a helper for getting the ISL clear color from a miptree

[Chad Versace]

On Wed 13 Jul 2016, Jason Ekstrand wrote:
> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 24 ++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  4 ++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 8519f46..7d3cec2 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -3170,3 +3170,27 @@ intel_miptree_get_isl_surf(struct brw_context *brw,
>  
>     surf->usage = 0; /* TODO */
>  }
> +
> +union isl_color_value
> +intel_miptree_get_isl_clear_color(struct brw_context *brw,
> +                                  const struct intel_mipmap_tree *mt)
> +{
> +   union isl_color_value clear_color;
> +   if (brw->gen >= 9) {
> +      clear_color.i32[0] = mt->gen9_fast_clear_color.i[0];
> +      clear_color.i32[1] = mt->gen9_fast_clear_color.i[1];
> +      clear_color.i32[2] = mt->gen9_fast_clear_color.i[2];
> +      clear_color.i32[3] = mt->gen9_fast_clear_color.i[3];
> +   } else if (_mesa_is_format_integer(mt->format)) {
> +      clear_color.i32[0] = (mt->fast_clear_color_value & (1u << 31)) != 0;
> +      clear_color.i32[1] = (mt->fast_clear_color_value & (1u << 30)) != 0;
> +      clear_color.i32[2] = (mt->fast_clear_color_value & (1u << 29)) != 0;
> +      clear_color.i32[3] = (mt->fast_clear_color_value & (1u << 28)) != 0;
> +   } else {
> +      clear_color.f32[0] = (mt->fast_clear_color_value & (1u << 31)) != 0;
> +      clear_color.f32[1] = (mt->fast_clear_color_value & (1u << 30)) != 0;
> +      clear_color.f32[2] = (mt->fast_clear_color_value & (1u << 29)) != 0;
> +      clear_color.f32[3] = (mt->fast_clear_color_value & (1u << 28)) != 0;
> +   }
> +   return clear_color;
> +}

Small nit. The function looks weird compared to other i965 code.
I believe two conventions in most of i965 is:

    - When a function begins with a block of declarations, a newline
      follows the block.
    - When a closing brace precedes a terminal 'return' statement,
      a newline separates the brace and 'return'.

> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index cf5d1a6..a50f181 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -802,6 +802,10 @@ intel_miptree_get_isl_surf(struct brw_context *brw,
>                             const struct intel_mipmap_tree *mt,
>                             struct isl_surf *surf);
>  
> +union isl_color_value
> +intel_miptree_get_isl_clear_color(struct brw_context *brw,
> +                                  const struct intel_mipmap_tree *mt);

Another nit: You can declare 'brw' as const.

Nit or not, this patch is
Reviewed-by: Chad Versace <chad.versace at intel.com>