[Mesa-dev] [PATCH 27/32] i965: Change resolve flags to enum

Pohjolainen, Topi topi.pohjolainen at gmail.com
Wed Jan 4 08:00:59 UTC 2017


On Mon, Jan 02, 2017 at 06:37:18PM -0800, Ben Widawsky wrote:
> In the foreseeable future it doesn't seem to make sense to have multiple
> resolve flags. What does make sense is to have the caller give an
> indication to the lower layers what it things should be done for
> resolve. The enum change distinguishes this binary selection.
> 
> v2: Make setting the hint more concise (Topi)
> 
> Cc: Topi Pohjolainen <topi.pohjolainen at gmail.com>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Acked-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c         |  8 ++++----
>  src/mesa/drivers/dri/i965/brw_context.c       | 13 +++++++------
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 12 ++++++------
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 13 ++++++++-----
>  4 files changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
> index 8d58616f59..d02920be57 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -209,12 +209,12 @@ blorp_surf_for_miptree(struct brw_context *brw,
>              surf->aux_usage = ISL_AUX_USAGE_NONE;
>           }
>        } else if (!(safe_aux_usage & (1 << surf->aux_usage))) {
> -         uint32_t flags = 0;
> -         if (safe_aux_usage & (1 << ISL_AUX_USAGE_CCS_E))
> -            flags |= INTEL_MIPTREE_IGNORE_CCS_E;
> +         const enum intel_resolve_hint hint =
> +            safe_aux_usage & (1 << ISL_AUX_USAGE_CCS_E) ?
> +            INTEL_RESOLVE_HINT_IGNORE_CCS_E : 0;
>  
>           intel_miptree_resolve_color(brw, mt,
> -                                     *level, start_layer, num_layers, flags);
> +                                     *level, start_layer, num_layers, hint);
>  
>           assert(!intel_miptree_has_color_unresolved(mt, *level, 1,
>                                                      start_layer, num_layers));
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index ce01605826..ffcefe799d 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -261,9 +261,10 @@ intel_update_state(struct gl_context * ctx, GLuint new_state)
>        /* Sampling engine understands lossless compression and resolving
>         * those surfaces should be skipped for performance reasons.
>         */
> -      const int flags = intel_texture_view_requires_resolve(brw, tex_obj) ?
> -                           0 : INTEL_MIPTREE_IGNORE_CCS_E;
> -      intel_miptree_all_slices_resolve_color(brw, tex_obj->mt, flags);
> +      const enum intel_resolve_hint hint =
> +         intel_texture_view_requires_resolve(brw, tex_obj) ?  0 :
                                                                ^
extra space. You could also put it to the line below with the else-branch...

> +         INTEL_RESOLVE_HINT_IGNORE_CCS_E;
> +      intel_miptree_all_slices_resolve_color(brw, tex_obj->mt, hint);
>        brw_render_cache_set_check_flush(brw, tex_obj->mt->bo);
>  
>        if (tex_obj->base.StencilSampling ||
> @@ -316,9 +317,9 @@ intel_update_state(struct gl_context * ctx, GLuint new_state)
>              intel_renderbuffer(fb->_ColorDrawBuffers[i]);
>  
>           if (irb &&
> -             intel_miptree_resolve_color(
> -                brw, irb->mt, irb->mt_level, irb->mt_layer, irb->layer_count,
> -                INTEL_MIPTREE_IGNORE_CCS_E))
> +             intel_miptree_resolve_color(brw, irb->mt, irb->mt_level,
> +                                         irb->mt_layer, irb->layer_count,
> +                                         INTEL_RESOLVE_HINT_IGNORE_CCS_E))
>              brw_render_cache_set_check_flush(brw, irb->mt->bo);
>        }
>     }
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 5057d517a6..26029df09a 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2321,7 +2321,7 @@ intel_miptree_used_for_rendering(const struct brw_context *brw,
>  static bool
>  intel_miptree_needs_color_resolve(const struct brw_context *brw,
>                                    const struct intel_mipmap_tree *mt,
> -                                  int flags)
> +                                  enum intel_resolve_hint hint)
>  {
>     if (mt->aux_disable & INTEL_AUX_DISABLE_CCS)
>        return false;
> @@ -2333,7 +2333,7 @@ intel_miptree_needs_color_resolve(const struct brw_context *brw,
>      * surfaces called "lossless compressed". These don't need to be always
>      * resolved.
>      */
> -   if ((flags & INTEL_MIPTREE_IGNORE_CCS_E) && is_lossless_compressed)
> +   if ((hint == INTEL_RESOLVE_HINT_IGNORE_CCS_E) && is_lossless_compressed)

Should we keep on "and-ing"? If we add any parallel hints in the future this
won't work anymore. Otherwise:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

>        return false;
>  
>     /* Fast color clear resolves only make sense for non-MSAA buffers. */
> @@ -2347,11 +2347,11 @@ bool
>  intel_miptree_resolve_color(struct brw_context *brw,
>                              struct intel_mipmap_tree *mt, unsigned level,
>                              unsigned start_layer, unsigned num_layers,
> -                            int flags)
> +                            enum intel_resolve_hint hint)
>  {
>     intel_miptree_check_color_resolve(brw, mt, level, start_layer);
>  
> -   if (!intel_miptree_needs_color_resolve(brw, mt, flags))
> +   if (!intel_miptree_needs_color_resolve(brw, mt, hint))
>        return false;
>  
>     /* Arrayed fast clear is only supported for gen8+. */
> @@ -2380,9 +2380,9 @@ intel_miptree_resolve_color(struct brw_context *brw,
>  void
>  intel_miptree_all_slices_resolve_color(struct brw_context *brw,
>                                         struct intel_mipmap_tree *mt,
> -                                       int flags)
> +                                       enum intel_resolve_hint hint)
>  {
> -   if (!intel_miptree_needs_color_resolve(brw, mt, flags))
> +   if (!intel_miptree_needs_color_resolve(brw, mt, hint))
>        return;
>        
>     foreach_list_typed_safe(struct intel_resolve_map, map, link,
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 1f1166a12b..d5f2689edd 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -946,21 +946,24 @@ intel_miptree_used_for_rendering(const struct brw_context *brw,
>   * Flag values telling color resolve pass which special types of buffers
>   * can be ignored.
>   *
> - * INTEL_MIPTREE_IGNORE_CCS_E:   Lossless compressed (single-sample
> - *                               compression scheme since gen9)
> + * INTEL_RESOLVE_HINT_IGNORE_CCS_E:   Lossless compressed (single-sample
> + *                                    compression scheme since gen9)
>   */
> -#define INTEL_MIPTREE_IGNORE_CCS_E (1 << 0)
> +enum intel_resolve_hint {
> +   INTEL_RESOLVE_HINT_NO_HINT = 0,
> +   INTEL_RESOLVE_HINT_IGNORE_CCS_E
> +};
>  
>  bool
>  intel_miptree_resolve_color(struct brw_context *brw,
>                              struct intel_mipmap_tree *mt, unsigned level,
>                              unsigned start_layer, unsigned num_layers,
> -                            int flags);
> +                            enum intel_resolve_hint);
>  
>  void
>  intel_miptree_all_slices_resolve_color(struct brw_context *brw,
>                                         struct intel_mipmap_tree *mt,
> -                                       int flags);
> +                                       enum intel_resolve_hint hint);
>  
>  void
>  intel_miptree_make_shareable(struct brw_context *brw,
> -- 
> 2.11.0
> 


More information about the mesa-dev mailing list