<p dir="ltr">On Nov 24, 2016 6:06 AM, "Topi Pohjolainen" <<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote:<br>
><br>
> Note that RESOLVED is not tracked in the map explicitly. Absence<br>
> of item implicitly means RESOLVED state.<br>
><br>
> v2: Added intel_resolve_map_clear() into intel_miptree_release()<br>
> v3 (Jason): Properly handle the assumption of resolve map not<br>
>             containing any items with state RESOLVED. Removed<br>
>             unnecessary intel_miptree_set_fast_clear_state() call<br>
>             in brw_blorp_resolve_color() preventing<br>
>             intel_miptree_set_fast_clear_state() from asserting<br>
>             against RESOLVED.<br>
><br>
> Signed-off-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>><br>
> Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> (v1)<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_blorp.c         |  3 -<br>
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 79 ++++++++++++++++++++-------<br>
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 16 +++---<br>
>  3 files changed, 68 insertions(+), 30 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c<br>
> index 67f4cce..d900e15 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c<br>
> @@ -969,9 +969,6 @@ brw_blorp_resolve_color(struct brw_context *brw, struct intel_mipmap_tree *mt,<br>
>                       brw_blorp_to_isl_format(brw, format, true),<br>
>                       resolve_op);<br>
>     blorp_batch_finish(&batch);<br>
> -<br>
> -   intel_miptree_set_fast_clear_state(mt, level, layer, 1,<br>
> -                                      INTEL_FAST_CLEAR_STATE_RESOLVED);<br>
>  }<br>
><br>
>  static void<br>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c<br>
> index a5913bc..d629901 100644<br>
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c<br>
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c<br>
> @@ -397,11 +397,11 @@ intel_miptree_create_layout(struct brw_context *brw,<br>
>     mt->logical_width0 = width0;<br>
>     mt->logical_height0 = height0;<br>
>     mt->logical_depth0 = depth0;<br>
> -   mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;<br>
>     mt->disable_aux_buffers = (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) != 0;<br>
>     mt->no_ccs = true;<br>
>     mt->is_scanout = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) != 0;<br>
>     exec_list_make_empty(&mt->hiz_map);<br>
> +   exec_list_make_empty(&mt->color_resolve_map);<br>
>     mt->cpp = _mesa_get_format_bytes(format);<br>
>     mt->num_samples = num_samples;<br>
>     mt->compressed = _mesa_is_format_compressed(format);<br>
> @@ -933,7 +933,7 @@ intel_update_winsys_renderbuffer_miptree(struct brw_context *intel,<br>
>      */<br>
>     if (intel_tiling_supports_non_msrt_mcs(intel, singlesample_mt->tiling) &&<br>
>         intel_miptree_supports_non_msrt_fast_clear(intel, singlesample_mt)) {<br>
> -      singlesample_mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;<br>
> +      singlesample_mt->no_ccs = false;<br>
>     }<br>
><br>
>     if (num_samples == 0) {<br>
> @@ -1048,6 +1048,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)<br>
>           free((*mt)->mcs_buf);<br>
>        }<br>
>        intel_resolve_map_clear(&(*mt)->hiz_map);<br>
> +      intel_resolve_map_clear(&(*mt)->color_resolve_map);<br>
><br>
>        intel_miptree_release(&(*mt)->plane[0]);<br>
>        intel_miptree_release(&(*mt)->plane[1]);<br>
> @@ -1633,7 +1634,12 @@ intel_miptree_alloc_mcs(struct brw_context *brw,<br>
>        return false;<br>
><br>
>     intel_miptree_init_mcs(brw, mt, 0xFF);<br>
> -   mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_CLEAR;<br>
> +<br>
> +   /* Multisampled miptrees are only supported for single level. */<br>
> +   assert(mt->first_level == 0);<br>
> +   intel_miptree_set_fast_clear_state(mt, mt->first_level, 0,<br>
> +                                      mt->logical_depth0,<br>
> +                                      INTEL_FAST_CLEAR_STATE_CLEAR);<br>
><br>
>     return true;<br>
>  }<br>
> @@ -1713,7 +1719,6 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,<br>
>         *    Software needs to initialize MCS with zeros."<br>
>         */<br>
>        intel_miptree_init_mcs(brw, mt, 0);<br>
> -      mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;<br>
>        mt->msaa_layout = INTEL_MSAA_LAYOUT_CMS;<br>
>     }<br>
><br>
> @@ -2209,7 +2214,15 @@ enum intel_fast_clear_state<br>
>  intel_miptree_get_fast_clear_state(const struct intel_mipmap_tree *mt,<br>
>                                     unsigned level, unsigned layer)<br>
>  {<br>
> -   return mt->fast_clear_state;<br>
> +   intel_miptree_check_level_layer(mt, level, layer);<br>
> +<br>
> +   const struct intel_resolve_map *item =<br>
> +      intel_resolve_map_const_get(&mt->color_resolve_map, level, layer);<br>
> +<br>
> +   if (!item)<br>
> +      return INTEL_FAST_CLEAR_STATE_RESOLVED;<br>
> +<br>
> +   return item->fast_clear_state;<br>
>  }<br>
><br>
>  static void<br>
> @@ -2240,11 +2253,18 @@ intel_miptree_set_fast_clear_state(struct intel_mipmap_tree *mt,<br>
>                                     unsigned num_layers,<br>
>                                     enum intel_fast_clear_state new_state)<br>
>  {<br>
> +   /* Setting the state to resolved means removing the item from the list<br>
> +    * altogether.<br>
> +    */<br>
> +   assert(new_state != INTEL_FAST_CLEAR_STATE_RESOLVED);</p>
<p dir="ltr">I'm honestly a bit surprised that nothing calls this to set RESOLVED but if it passes Jenkins, I'll believe it.</p>
<p dir="ltr">Series is</p>
<p dir="ltr">Reviewed-by:  Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></p>
<p dir="ltr">> +<br>
>     intel_miptree_check_color_resolve(mt, level, first_layer);<br>
><br>
>     assert(first_layer + num_layers <= mt->physical_depth0);<br>
><br>
> -   mt->fast_clear_state = new_state;<br>
> +   for (unsigned i = 0; i < num_layers; i++)<br>
> +      intel_resolve_map_set(&mt->color_resolve_map, level,<br>
> +                            first_layer + i, new_state);<br>
>  }<br>
><br>
>  bool<br>
> @@ -2252,7 +2272,9 @@ intel_miptree_has_color_unresolved(const struct intel_mipmap_tree *mt,<br>
>                                     unsigned start_level, unsigned num_levels,<br>
>                                     unsigned start_layer, unsigned num_layers)<br>
>  {<br>
> -   return mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED;<br>
> +   return intel_resolve_map_find_any(&mt->color_resolve_map,<br>
> +                                     start_level, num_levels,<br>
> +                                     start_layer, num_layers) != NULL;<br>
>  }<br>
><br>
>  void<br>
> @@ -2316,19 +2338,27 @@ intel_miptree_resolve_color(struct brw_context *brw,<br>
>     if (!intel_miptree_needs_color_resolve(brw, mt, flags))<br>
>        return false;<br>
><br>
> -   switch (mt->fast_clear_state) {<br>
> -   case INTEL_FAST_CLEAR_STATE_RESOLVED:<br>
> -      /* No resolve needed */<br>
> -      return false;<br>
> -   case INTEL_FAST_CLEAR_STATE_UNRESOLVED:<br>
> -   case INTEL_FAST_CLEAR_STATE_CLEAR:<br>
> -      /* For now arrayed fast clear is not supported. */<br>
> -      assert(num_layers == 1);<br>
> -      brw_blorp_resolve_color(brw, mt, level, start_layer);<br>
> -      return true;<br>
> -   default:<br>
> -      unreachable("Invalid fast clear state");<br>
> +   /* For now arrayed fast clear is not supported. */<br>
> +   assert(num_layers == 1);<br>
> +<br>
> +   bool resolved = false;<br>
> +   for (unsigned i = 0; i < num_layers; ++i) {<br>
> +      intel_miptree_check_level_layer(mt, level, start_layer + i);<br>
> +<br>
> +      struct intel_resolve_map *item =<br>
> +         intel_resolve_map_get(&mt->color_resolve_map, level,<br>
> +                               start_layer + i);<br>
> +<br>
> +      if (item) {<br>
> +         assert(item->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED);<br>
> +<br>
> +         brw_blorp_resolve_color(brw, mt, level, start_layer);<br>
> +         intel_resolve_map_remove(item);<br>
> +         resolved = true;<br>
> +      }<br>
>     }<br>
> +<br>
> +   return resolved;<br>
>  }<br>
><br>
>  void<br>
> @@ -2336,7 +2366,16 @@ intel_miptree_all_slices_resolve_color(struct brw_context *brw,<br>
>                                         struct intel_mipmap_tree *mt,<br>
>                                         int flags)<br>
>  {<br>
> -   intel_miptree_resolve_color(brw, mt, 0, 0, 1, flags);<br>
> +   if (!intel_miptree_needs_color_resolve(brw, mt, flags))<br>
> +      return;<br>
> +<br>
> +   foreach_list_typed_safe(struct intel_resolve_map, map, link,<br>
> +                           &mt->color_resolve_map) {<br>
> +      assert(map->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED);<br>
> +<br>
> +      brw_blorp_resolve_color(brw, mt, map->level, map->layer);<br>
> +      intel_resolve_map_remove(map);<br>
> +   }<br>
>  }<br>
><br>
>  /**<br>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h<br>
> index 02f131c..29d0e2f 100644<br>
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h<br>
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h<br>
> @@ -554,15 +554,22 @@ struct intel_mipmap_tree<br>
>     struct intel_miptree_hiz_buffer *hiz_buf;<br>
><br>
>     /**<br>
> -    * \brief Map of miptree slices to needed resolves.<br>
> +    * \brief Maps of miptree slices to needed resolves.<br>
>      *<br>
> -    * This is used only when the miptree has a child HiZ miptree.<br>
> +    * hiz_map is used only when the miptree has a child HiZ miptree.<br>
>      *<br>
>      * Let \c mt be a depth miptree with HiZ enabled. Then the resolve map is<br>
>      * \c mt->hiz_map. The resolve map of the child HiZ miptree, \c<br>
>      * mt->hiz_mt->hiz_map, is unused.<br>
> +    *<br>
> +    *<br>
> +    * color_resolve_map is used only when the miptree uses fast clear (Gen7+)<br>
> +    * lossless compression (Gen9+). It should be noted that absence in the<br>
> +    * map means implicitly RESOLVED state. If item is found it always<br>
> +    * indicates state other than RESOLVED.<br>
>      */<br>
>     struct exec_list hiz_map; /* List of intel_resolve_map. */<br>
> +   struct exec_list color_resolve_map; /* List of intel_resolve_map. */<br>
><br>
>     /**<br>
>      * \brief Stencil miptree for depthstencil textures.<br>
> @@ -606,11 +613,6 @@ struct intel_mipmap_tree<br>
>     struct intel_mipmap_tree *plane[2];<br>
><br>
>     /**<br>
> -    * Fast clear state for this buffer.<br>
> -    */<br>
> -   enum intel_fast_clear_state fast_clear_state;<br>
> -<br>
> -   /**<br>
>      * The SURFACE_STATE bits associated with the last fast color clear to this<br>
>      * color mipmap tree, if any.<br>
>      *<br>
> --<br>
> 2.5.5<br>
><br>
> _______________________________________________<br>
> mesa-dev mailing list<br>
><a href="mailto:mesa-dev@lists.freedesktop.org"> mesa-dev@lists.freedesktop.org</a><br>
><a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev"> https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</p>