[Mesa-dev] [PATCH 09/26] i965: Split per miptree and per slice/level fast clear bits
Jason Ekstrand
jason at jlekstrand.net
Sat Oct 29 07:47:02 UTC 2016
On Tue, Oct 11, 2016 at 12:26 PM, Topi Pohjolainen <
topi.pohjolainen at gmail.com> wrote:
> Currently the status bits for fast clear include the flag telling
> if non-multisampled mcs buffer should be used at all. Once the
> state tracking is changed to follow individual levels/layers one
> still needs to have the mcs enabling information in the miptree.
> Therefore simply split it out to its own boolean.
>
Thank you! The fast clear state bits have been somewhat confused for some
time. Between disable_aux_buffers, intel_fast_clear_state,
intel_msaa_layout, and the hiz resolve tracking, we have a bunch of
different fields that track memory layout, what aux is available, and the
current state of affairs and some of them are trying to track two things at
once. I hope to fix that some day but this is definitely a step in the
right direction.
> Possible follow-up work is to combine disable_aux_buffers and
> no_msrt_mcs into single enum.
>
Yeah, I think that's a good direction. I did something similar in my
Vulkan CCS series where I had an "aux usage" field in the image that
provided a sort of "global" picture of the usage.
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_blorp.c | 2 +-
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 14 +++++++++-----
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 12 ++++++------
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index c55bbc8..6332788 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -780,7 +780,7 @@ do_single_blorp_clear(struct brw_context *brw, struct
> gl_framebuffer *fb,
> if (set_write_disables(irb, ctx->Color.ColorMask[buf],
> color_write_disable))
> can_fast_clear = false;
>
> - if (irb->mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_NO_MCS ||
> + if (irb->mt->no_msrt_mcs ||
> !brw_is_color_fast_clear_compatible(brw, irb->mt,
> &ctx->Color.ClearColor))
> can_fast_clear = false;
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index ce72e2c..4fb07e9 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -374,8 +374,9 @@ 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_NO_MCS;
> + mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;
> mt->disable_aux_buffers = (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX)
> != 0;
> + mt->no_msrt_mcs = true;
> mt->is_scanout = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) != 0;
> exec_list_make_empty(&mt->hiz_map);
> mt->cpp = _mesa_get_format_bytes(format);
> @@ -787,7 +788,7 @@ intel_miptree_create(struct brw_context *brw,
> */
> if (intel_tiling_supports_non_msrt_mcs(brw, mt->tiling) &&
> intel_miptree_supports_non_msrt_fast_clear(brw, mt)) {
> - mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_RESOLVED;
> + mt->no_msrt_mcs = false;
> assert(brw->gen < 8 || mt->halign == 16 || num_samples <= 1);
>
> /* On Gen9+ clients are not currently capable of consuming
> compressed
> @@ -1582,6 +1583,7 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context
> *brw,
> {
> assert(mt->mcs_mt == NULL);
> assert(!mt->disable_aux_buffers);
> + assert(!mt->no_msrt_mcs);
>
> /* The format of the MCS buffer is opaque to the driver; all that
> matters
> * is that we get its size and pitch right. We'll pretend that the
> format
> @@ -2126,6 +2128,9 @@ intel_miptree_resolve_color(struct brw_context *brw,
> unsigned level, unsigned layer,
> int flags)
> {
> + if (mt->no_msrt_mcs)
> + return false;
> +
> intel_miptree_check_color_resolve(mt, level, layer);
>
> /* From gen9 onwards there is new compression scheme for single sampled
> @@ -2137,7 +2142,6 @@ intel_miptree_resolve_color(struct brw_context *brw,
> return false;
>
> switch (mt->fast_clear_state) {
> - case INTEL_FAST_CLEAR_STATE_NO_MCS:
> case INTEL_FAST_CLEAR_STATE_RESOLVED:
> /* No resolve needed */
> return false;
> @@ -2187,7 +2191,7 @@ intel_miptree_make_shareable(struct brw_context
> *brw,
> if (mt->mcs_mt) {
> intel_miptree_all_slices_resolve_color(brw, mt, 0);
> intel_miptree_release(&mt->mcs_mt);
> - mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_NO_MCS;
> + mt->no_msrt_mcs = true;
> }
> }
>
> @@ -3268,7 +3272,7 @@ intel_miptree_get_aux_isl_surf(struct brw_context
> *brw,
> } else if (intel_miptree_is_lossless_compressed(brw, mt)) {
> assert(brw->gen >= 9);
> *usage = ISL_AUX_USAGE_CCS_E;
> - } else if (mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_NO_MCS) {
> + } else if (!mt->no_msrt_mcs) {
> *usage = ISL_AUX_USAGE_CCS_D;
> } else {
> unreachable("Invalid MCS miptree");
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index bfb8ad5..57d7b80 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -220,12 +220,6 @@ enum intel_msaa_layout
> enum intel_fast_clear_state
> {
> /**
> - * There is no MCS buffer for this miptree, and one should never be
> - * allocated.
> - */
> - INTEL_FAST_CLEAR_STATE_NO_MCS,
> -
> - /**
> * No deferred clears are pending for this miptree, and the contents
> of the
> * color buffer are entirely correct. An MCS buffer may or may not
> exist
> * for this miptree. If it does exist, it is entirely in the "no
> deferred
> @@ -676,6 +670,12 @@ struct intel_mipmap_tree
> bool disable_aux_buffers;
>
> /**
> + * Fast clear and lossless compression are always disabled for this
> + * miptree.
> + */
> + bool no_msrt_mcs;
>
Please choose a different name for this boolean. What you have right now
unabreviates to "no multisampled render target MCS" but it's for
single-sampled. The single-sampled color compression surface is referred
to in the gen8 docs and prior as a non-multisampled MCS which we abbreviate
various places in the driver as non_msrt_mcs which is 1 character off the
name of this field. When I read this I can't tell if you're saying "no
non-msrt MCS" or "no-msrt MCS enabled".
One option would be "no_ccs". I know that the docs overload CCS to also
include multisampled surfaces. However, in ISL, we chose the convention of
CCS means single-sampled and MCS means multisampled. I'm also open to
other names.
> +
> + /**
> * Tells if the underlying buffer is to be also consumed by entities
> other
> * than the driver. This allows logic to turn off features such as
> lossless
> * compression which is not currently understood by client
> applications.
> --
> 2.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161029/b708be51/attachment.html>
More information about the mesa-dev
mailing list