On Tue, Jun 6, 2017 at 1:22 PM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Fri 26 May 2017, Jason Ekstrand wrote:
> > This enum describes all of the states that a auxiliary compressed
> > surface can have. All of the states as well as normative language for
> > referring to each of the compression operations is provided in the
> > truly colossal comment for the new isl_aux_state enum. There is also
> > a diagram showing how surfaces move between the different states.
> > ---
> > src/intel/isl/isl.h | 142 ++++++++++++++++++++++++++++++
> ++++++++++++++++++++++
> > 1 file changed, 142 insertions(+)
> >
> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > index b9d8fa8..df6d3e3 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -560,6 +560,148 @@ enum isl_aux_usage {
> > ISL_AUX_USAGE_CCS_E,
> > };
> >
> > +/**
> > + * Enum for keeping track of the state an auxiliary compressed surface.
>
> This is really nice and helpful for everyone.
>
> I also learned something new from it: that a resolve on CCS_E also
> ambiguates the aux surface. Do you have any insight on why the hardware
> does that?
>
> > + *
> > + * For any given auxiliary surface compression format (HiZ, CCS, or
> MCS), any
> > + * given slice (lod + array layer) can be in one of the six states
> described
> > + * by this enum. Draw and resolve operations may cause the slice to
> change
> > + * from one state to another. The six valid states are:
>
> I have one suggestion: please carefully distinguish between CCS_D and
> CCS_E in the documentation. In my experience, muddy thinking where the
> two are not cleanly distinguished leads to confused minds and confusing
> code.
>
> For someone who already has a firm grasp on aux state, the ambiguous
> term "CCS" poses no problem. That wise person automatically infers from
> context if "CCS" applies to CCS_D, to CCS_E, or to both. But for someone
> who's understanding of aux isn't as solid, the term "CCS" can lead to
> incorrect inferences.
>
> For example, below you say that the partial resolve "operation is only
> available for CCS". That's misleading. It should say "only available for
> CCS_E".
>
> Another benefit: It becomes possible to document that
> ISL_AUX_STATE_COMPRESSED_NO_CLEAR is valid only for CCS_E and HIZ, but
> not valid for CCS_D and MCS.
>
It is valid for MCS. If you don't fast-clear but only render, then you're
in that state. It's only invalid for CCS_D.
> Other than the CCS_D/CCS_E distinction, the patch looks good to me. This
> is a really nice addition to the driver.
>
How about a section after the auxiliary compression ops section which goes
into detail on each of the compression types and discusses which states are
valid etc.
> One more comment at the end...
>
> > + *
> > + * 1) Clear: In this state, each block in the auxiliary surface
> contains a
> > + * magic value that indicates that the block is in the clear
> state. If
> > + * a block is in the clear state, it's values in the primary
> surface are
> > + * ignored and the color of the samples in the block is taken
> either the
> > + * RENDER_SURFACE_STATE packet for color or 3DSTATE_CLEAR_PARAMS
> for
> > + * depth. Since neither the primary surface nor the auxiliary
> surface
> > + * contains the clear value, the surface can be cleared to a
> different
> > + * color by simply changing the clear color without modifying
> either
> > + * surface.
> > + *
> > + * 2) Compressed w/ Clear: In this state, neither the auxiliary
> surface
> > + * nor the primary surface has a complete representation of the
> data.
> > + * Instead, both surfaces must be used together or else rendering
> > + * corruption may occur. Depending on the auxiliary compression
> format
> > + * and the data, any given block in the primary surface may
> contain all,
> > + * some, or none of the data required to reconstruct the actual
> sample
> > + * values. Blocks may also be in the clear state (see Clear) and
> have
> > + * their value taken from outside the surface.
> > + *
> > + * 3) Compressed w/o Clear: This state is identical to the state
> above
> > + * except that no blocks are in the clear state. In this state,
> all of
> > + * the data required to reconstruct the final sample values is
> contained
> > + * in the auxiliary and primary surface and the clear value is not
> > + * considered.
> > + *
> > + * 4) Resolved: In this state, the primary surface contains 100% of
> the
> > + * data. The auxiliary surface is also valid so the surface can
> be
> > + * validly used with or without aux enabled. The auxiliary
> surface may,
> > + * however, contain non-trivial data and any update to the primary
> > + * surface with aux disabled will cause the two to get out of
> sync.
> > + *
> > + * 5) Pass-through: In this state, the primary surface contains
> 100% of the
> > + * data and every block in the auxiliary surface contains a magic
> value
> > + * which indicates that the auxiliary surface should be ignored
> and the
> > + * only the primary surface should be considered. Updating the
> primary
> > + * surface without aux works fine and can be done repeatedly in
> this
> > + * mode. Writing to a surface in pass-through mode with aux
> enabled may
> > + * cause the auxiliary buffer to contain non-trivial data and no
> longer
> > + * be in the pass-through state.
> > + *
> > + * 5) Aux Invalid: In this state, the primary surface contains 100%
> of the
> > + * data and the auxiliary surface is completely bogus. Any
> attempt to
> > + * use the auxiliary surface is liable to result in rendering
> > + * corruption. The only thing that one can do to re-enable aux
> once
> > + * this state is reached is to use an ambiguate pass to
> transition into
> > + * the pass-through state.
> > + *
> > + * Drawing with or without aux enabled may implicitly cause the surface
> to
> > + * transition between these states. There are also four types of
> "resolve"
> > + * operations which cause an explicit transition:
> > + *
> > + * 1) Fast Clear: This operation writes the magic "clear" value to
> the
> > + * auxiliary surface. This operation will safely transition any
> slice
> > + * of a surface from any state to the clear state so long as the
> entire
> > + * slice is fast cleared at once.
> > + *
> > + * 2) Full Resolve: This operation combines the auxiliary surface
> data
> > + * with the primary surface data and writes the result to the
> primary.
> > + * For CCS resolves, this operation is destructive in the sense
> that it
> > + * also sets the auxiliary surface to the pass-through mode. For
> HiZ,
> > + * it is not destructive.
> > + *
> > + * 3) Partial Resolve: This operation considers blocks which are in
> the
> > + * "clear" state and writes the clear value directly into the
> primary or
> > + * auxiliary surface. Once this operation completes, the surface
> is
> > + * still compressed but no longer references the clear color.
> This
> > + * operation is only available for CCS.
> > + *
> > + * 4) Ambiguate: This operation throws away the current auxiliary
> data and
> > + * replaces it with the magic pass-through value. If an ambiguate
> > + * operation is performed when the primary surface does not
> contain 100%
> > + * of the data, data will be lost. This operation is only
> implemented
> > + * in hardware for depth where it is called a HiZ resolve.
> > + *
> > + * Not all operations are valid or useful in all states. The diagram
> below
> > + * contains a complete description of the states and all valid and
> useful
> > + * transitions except clear.
> > + *
> > + * Draw w/ Aux
> > + * +----------+
> > + * | |
> > + * | +-------------+ Draw w/ Aux +-------------+
> > + * +------>| Compressed |<---------------------| Clear |
> > + * | w/ Clear | | |
> > + * +-------------+ +-------------+
> > + * | | |
> > + * Partial | | |
> > + * Resolve | | Full Resolve |
> > + * | +----------------------------+ | Full
> > + * | | | Resolve
> > + * Draw w/ aux | | |
> > + * +----------+ | | |
> > + * | | \|/ \|/ \|/
> > + * | +-------------+ Full Resolve +-------------+
> > + * +------>| Compressed |--------------------->| Resolved |
> > + * | w/o Clear |<---------------------| |
> > + * +-------------+ Draw w/ Aux +-------------+
> > + * /|\ | |
> > + * | Draw | | Draw
> > + * | w/ Aux | | w/o Aux
> > + * | Ambiguate | |
> > + * | +----------------------------+ |
> > + * Draw w/o Aux | | | Draw w/o
> Aux
> > + * +----------+ | | |
> +----------+
> > + * | | | \|/ \|/ |
> |
> > + * | +-------------+ Ambiguate +-------------+
> |
> > + * +------>| Pass- |<---------------------| Aux
> |<------+
> > + * | through | | Invalid |
> > + * +-------------+ +-------------+
> > + *
> > + *
> > + * As referenced in the description of the different operations above,
> not all
> > + * auxiliary surface formats actually support all of the above modes.
> With
> > + * HiZ, for instance, does not have a partial resolve operation so the
> two
> > + * "compressed" modes are the same. With CCS, the resolve operation is
> > + * destructive and takes you directly to passthrough so the "resolved"
> state
> > + * doesn't really exist. However, if you consider the CCS resolve
> operation
> > + * as doing a resolve and then an ambiguate, the diagram is still
> accurate.
> > + */
> > +enum isl_aux_state {
>
> One last quibble: I think the code is cleaner with the below comments
> removed. They don't add much, as they basically just restart the each
> enum's name.
>
> > + /** Describes the Clear state */
> > + ISL_AUX_STATE_CLEAR = 0,
> > + /** Describes the Compressed w/ Clear state */
> > + ISL_AUX_STATE_COMPRESSED_CLEAR,
> > + /** Describes the Compressed w/o Clear state */
> > + ISL_AUX_STATE_COMPRESSED_NO_CLEAR,
> > + /** Describes the Resolved state */
> > + ISL_AUX_STATE_RESOLVED,
> > + /** Describes the Pass-through state */
> > + ISL_AUX_STATE_PASS_THROUGH,
> > + /** Describes the Aux Invalid state */
> > + ISL_AUX_STATE_AUX_INVALID,
> > +};
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
