Re: [Mesa-dev] [PATCH v13 27/36] i965: Change resolve flags to enum

[Jason Ekstrand]

So, I think I'm going to end up doing a fairly significant rework of
resolves over the course of the next couple of weeks.  Carry on with the
branch as is and I'll figure out how to rebase it on top of whatever
changes I do later.  But it'll probably be significant.  Just a heads up.

--Jason

On Fri, May 19, 2017 at 2:38 AM, Daniel Stone  wrote:

> From: Ben Widawsky 
>
> 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)
>
> Signed-off-by: Ben Widawsky 
> Acked-by: Daniel Stone 
> Reviewed-by: Topi Pohjolainen 
> Signed-off-by: Daniel Stone 
> ---
>  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 b69cb4fc7b..938600875e 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -210,12 +210,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 f0e08b9874..5e446abb1f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -259,9 +259,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 :
> + 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 ||
> @@ -313,9 +314,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 b33afdeb3f..8a33010543 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2234,7 +2234,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;
> @@ -2246,7 +2246,7 @@ intel_miptree_needs_color_resolve(const struct
> brw_context *brw,
>  * surfaces called "lossless compressed". These don't need to be always
>  * resolved.
>  */
> -   

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

[Daniel Stone]

From: Ben Widawsky 

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)

Signed-off-by: Ben Widawsky 
Acked-by: Daniel Stone 
Reviewed-by: Topi Pohjolainen 
Signed-off-by: Daniel Stone 
---
 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 b69cb4fc7b..938600875e 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -210,12 +210,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 f0e08b9874..5e446abb1f 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -259,9 +259,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 :
+ 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 ||
@@ -313,9 +314,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 b33afdeb3f..8a33010543 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2234,7 +2234,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;
@@ -2246,7 +2246,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)
   return false;
 
/* Fast color clear resolves only make sense for non-MSAA buffers. */
@@ -2260,11 +2260,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