[Mesa-dev] [v2 11/17] i965: Track fast color clear state in level/layer granularity
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Thu Nov 24 07:34:37 UTC 2016
On Wed, Nov 23, 2016 at 01:05:31PM -0800, Jason Ekstrand wrote:
> On Wed, Nov 23, 2016 at 1:16 AM, Topi Pohjolainen
> <[1]topi.pohjolainen at gmail.com> wrote:
>
> v2: Added intel_resolve_map_clear() into intel_miptree_release()
> Signed-off-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
> Reviewed-by: Jason Ekstrand <[3]jason at jlekstrand.net> (v1)
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 64
> +++++++++++++++++++--------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 14 +++---
> 2 files changed, 53 insertions(+), 25 deletions(-)
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 3895b2e..a4a7ee0 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -397,11 +397,11 @@ intel_miptree_create_layout(struct brw_context
> *brw,
> mt->logical_width0 = width0;
> mt->logical_height0 = height0;
> mt->logical_depth0 = depth0;
> - mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;
> mt->disable_aux_buffers = (layout_flags &
> MIPTREE_LAYOUT_DISABLE_AUX) != 0;
> mt->no_ccs = true;
> mt->is_scanout = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) !=
> 0;
> exec_list_make_empty(&mt->hiz_map);
> + exec_list_make_empty(&mt->color_resolve_map);
> mt->cpp = _mesa_get_format_bytes(format);
> mt->num_samples = num_samples;
> mt->compressed = _mesa_is_format_compressed(format);
> @@ -933,7 +933,7 @@ intel_update_winsys_renderbuffer_miptree(struct
> brw_context *intel,
> */
> if (intel_tiling_supports_non_msrt_mcs(intel,
> singlesample_mt->tiling) &&
> intel_miptree_supports_non_msrt_fast_clear(intel,
> singlesample_mt)) {
> - singlesample_mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_
> RESOLVED;
> + singlesample_mt->no_ccs = false;
> }
> if (num_samples == 0) {
> @@ -1048,6 +1048,7 @@ intel_miptree_release(struct intel_mipmap_tree
> **mt)
> free((*mt)->mcs_buf);
> }
> intel_resolve_map_clear(&(*mt)->hiz_map);
> + intel_resolve_map_clear(&(*mt)->color_resolve_map);
> intel_miptree_release(&(*mt)->plane[0]);
> intel_miptree_release(&(*mt)->plane[1]);
> @@ -1633,7 +1634,12 @@ intel_miptree_alloc_mcs(struct brw_context
> *brw,
> return false;
> intel_miptree_init_mcs(brw, mt, 0xFF);
> - mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_CLEAR;
> +
> + /* Multisampled miptrees are only supported for single level. */
> + assert(mt->first_level == 0);
> + intel_miptree_set_fast_clear_state(mt, mt->first_level, 0,
> + mt->logical_depth0,
> +
> INTEL_FAST_CLEAR_STATE_CLEAR);
> return true;
> }
> @@ -1713,7 +1719,6 @@ intel_miptree_alloc_non_msrt_mcs(struct
> brw_context *brw,
> * Software needs to initialize MCS with zeros."
> */
> intel_miptree_init_mcs(brw, mt, 0);
> - mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;
> mt->msaa_layout = INTEL_MSAA_LAYOUT_CMS;
> }
> @@ -2209,7 +2214,15 @@ enum intel_fast_clear_state
> intel_miptree_get_fast_clear_state(const struct intel_mipmap_tree
> *mt,
> unsigned level, unsigned layer)
> {
> - return mt->fast_clear_state;
> + intel_miptree_check_level_layer(mt, level, layer);
> +
> + const struct intel_resolve_map *item =
> + intel_resolve_map_const_get(&mt->color_resolve_map, level,
> layer);
> +
> + if (!item)
> + return INTEL_FAST_CLEAR_STATE_RESOLVED;
> +
> + return item->fast_clear_state;
> }
> static void
> @@ -2244,7 +2257,9 @@ intel_miptree_set_fast_clear_state(struct
> intel_mipmap_tree *mt,
> assert(first_layer + num_layers <= mt->physical_depth0);
> - mt->fast_clear_state = new_state;
> + while (num_layers--)
>
> nitpick: I think I'd rather a simple "for (unsigned i = 0; i <
> num_layers; i++)" It's a bit more obvious
Agreed.
>
> + intel_resolve_map_set(&mt->color_resolve_map, level,
> + first_layer++, new_state);
>
> We probably want to assert here that state != RESOLVED (see below). If
> that's not valid, we would have to switch on whether or not it's
> RESOLVED and if it is RESOLVED, walk the list and delete things.
Color resolver needs to be able to set the state as well. So we really need
to start deleting items from the list. This worked before just because I had
logic elsewhere checking also for the RESOLVED. I'm going to drop those and
turn them into asserts as you proposed.
>
> }
> bool
> @@ -2252,7 +2267,9 @@ intel_miptree_has_color_unresolved(const
> struct intel_mipmap_tree *mt,
> unsigned start_level, unsigned
> num_levels,
> unsigned start_layer, unsigned
> num_layers)
> {
> - return mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED;
> + return intel_resolve_map_find_any(&mt->color_resolve_map,
> + start_level, num_levels,
> + start_layer, num_layers) !=
> NULL;
>
> This assumes that there are no items in the resolve map with a state of
> RESOLVED. That's a fine assumption, but we need to make sure that it's
> valid. I've made a bunch of comments all of this patch about that.
And you really found a bug here. I guess this wan't visible before because
I only used this for sanity check (which was just too loose). Now that I
fixed one of the patches to use this more widely things would have broken
down.
>
> }
> void
> @@ -2316,17 +2333,18 @@ intel_miptree_resolve_color(struct
> brw_context *brw,
> if (!intel_miptree_needs_color_resolve(brw, mt, flags))
> return false;
> - switch (mt->fast_clear_state) {
> - case INTEL_FAST_CLEAR_STATE_RESOLVED:
> - /* No resolve needed */
> + intel_miptree_check_level_layer(mt, level, layer);
> +
> + struct intel_resolve_map *item =
> + intel_resolve_map_get(&mt->color_resolve_map, level, layer);
> +
> + if (!item || item->fast_clear_state == INTEL_FAST_CLEAR_STATE_
> RESOLVED)
>
> This seems to imply that it's not. Maybe just do
> if (!item)
> return false;
> assert(item->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED)
>
> return false;
> - case INTEL_FAST_CLEAR_STATE_UNRESOLVED:
> - case INTEL_FAST_CLEAR_STATE_CLEAR:
> - brw_blorp_resolve_color(brw, mt, level, layer);
> - return true;
> - default:
> - unreachable("Invalid fast clear state");
> - }
> +
> + brw_blorp_resolve_color(brw, mt, level, layer);
> + intel_resolve_map_remove(item);
> +
> + return true;
> }
> void
> @@ -2334,7 +2352,17 @@ intel_miptree_all_slices_resolve_color(struct
> brw_context *brw,
> struct intel_mipmap_tree
> *mt,
> int flags)
> {
> - intel_miptree_resolve_color(brw, mt, 0, 0, flags);
> + if (!intel_miptree_needs_color_resolve(brw, mt, flags))
> + return;
> +
> + foreach_list_typed_safe(struct intel_resolve_map, map, link,
> + &mt->color_resolve_map) {
> + if (map->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED)
>
> Again, this should be an assert.
>
> + continue;
> +
> + brw_blorp_resolve_color(brw, mt, map->level, map->layer);
> + intel_resolve_map_remove(map);
> + }
> }
> /**
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 1f4ae6c..51ab664 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -554,15 +554,20 @@ struct intel_mipmap_tree
> struct intel_miptree_hiz_buffer *hiz_buf;
> /**
> - * \brief Map of miptree slices to needed resolves.
> + * \brief Maps of miptree slices to needed resolves.
> *
> - * This is used only when the miptree has a child HiZ miptree.
> + * hiz_map is used only when the miptree has a child HiZ
> miptree.
> *
> * Let \c mt be a depth miptree with HiZ enabled. Then the
> resolve map is
> * \c mt->hiz_map. The resolve map of the child HiZ miptree, \c
> * mt->hiz_mt->hiz_map, is unused.
> + *
> + *
> + * color_resolve_map is used only when the miptree uses fast
> clear (Gen7+)
> + * lossless compression (Gen9+).
> */
> struct exec_list hiz_map; /* List of intel_resolve_map. */
> + struct exec_list color_resolve_map; /* List of
> intel_resolve_map. */
> /**
> * \brief Stencil miptree for depthstencil textures.
> @@ -606,11 +611,6 @@ struct intel_mipmap_tree
> struct intel_mipmap_tree *plane[2];
> /**
> - * Fast clear state for this buffer.
> - */
> - enum intel_fast_clear_state fast_clear_state;
> -
> - /**
> * The SURFACE_STATE bits associated with the last fast color
> clear to this
> * color mipmap tree, if any.
> *
> --
> 2.5.5
> _______________________________________________
> mesa-dev mailing list
> [4]mesa-dev at lists.freedesktop.org
> [5]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> References
>
> 1. mailto:topi.pohjolainen at gmail.com
> 2. mailto:topi.pohjolainen at intel.com
> 3. mailto:jason at jlekstrand.net
> 4. mailto:mesa-dev at lists.freedesktop.org
> 5. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list