[Mesa-dev] [PATCH 5/5] i965/miptree: Add PRM references for most struct members
Kenneth Graunke
kenneth at whitecape.org
Thu Nov 26 18:29:02 PST 2015
On Sep 25, 2015 12:05 PM, Chad Versace <chad.versace at intel.com> wrote:
>
> Add comments that link the driver's miptree structures to the hardware
> structures documented in the PRM. This provides sorely needed
> orientation to developers new to the miptree code. And for miptree
> veterans, this clarifies some of the more obscure miptree data.
>
> For each driver struct field that closely corresponds to a
> hardware struct field, add a PRM reference to that hardware field's
> name. For example,
>
> struct intel_mipmap_tree {
> ...
> /**
> * @brief One of GL_TEXTURE_2D, GL_TEXTURE_2D_ARRAY, etc.
> *
> * @see RENDER_SURFACE_STATE.SurfaceType
> * @see RENDER_SURFACE_STATE.SurfaceArray
> * @see 3DSTATE_DEPTH_BUFFER.SurfaceType
> */
> GLenum target;
> ...
> };
>
> Also annotate the INTEL_MSAA_LAYOUT_* enums with the name of the PRM
> sections that documents the layout.
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 167 ++++++++++++++++++++++----
> 1 file changed, 146 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index bfcecea..671065d 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -144,6 +144,9 @@ struct intel_mipmap_level
> * \code
> * x = mt->level[l].slice[s].x_offset
> * y = mt->level[l].slice[s].y_offset
> + *
> + * On some hardware generations, we program these offsets into
On G45 and Ironlake we program...
> + * RENDER_SURFACE_STATE.XOffset and RENDER_SURFACE_STATE.YOffset.
> */
> GLuint x_offset;
> GLuint y_offset;
> @@ -172,12 +175,16 @@ enum intel_msaa_layout
> * accommodated by scaling up the width and the height of the surface so
> * that all the samples corresponding to a pixel are located at nearby
> * memory locations.
> + *
> + * @see PRM section "Interleaved Multisampled Surfaces"
> */
> INTEL_MSAA_LAYOUT_IMS,
>
> /**
> * Uncompressed Multisample Surface. The surface is stored as a 2D array,
> * with array slice n containing all pixel data for sample n.
> + *
> + * @see PRM section "Uncompressed Multisampled Surfaces"
> */
> INTEL_MSAA_LAYOUT_UMS,
>
> @@ -189,6 +196,8 @@ enum intel_msaa_layout
> * the common case (where all samples constituting a pixel have the same
> * color value) to be stored efficiently by just using a single array
> * slice.
> + *
> + * @see PRM section "Compressed Multisampled Surfaces"
> */
> INTEL_MSAA_LAYOUT_CMS,
> };
> @@ -322,14 +331,34 @@ enum miptree_array_layout {
> */
> struct intel_miptree_aux_buffer
> {
> - /** Buffer object containing the pixel data. */
> + /**
> + * Buffer object containing the pixel data.
> + *
> + * @see RENDER_SURFACE_STATE.AuxiliarySurfaceBaseAddress
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.AuxiliarySurfaceBaseAddress
Is it worth noting that this is HiZ or MCS before Gen8, when the auxiliary term was introduced?
> + */
> drm_intel_bo *bo;
>
> - uint32_t pitch; /**< pitch in bytes. */
> + /**
> + * Pitch in bytes.
> + *
> + * @see RENDER_SURFACE_STATE.AuxiliarySurfacePitch
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfacePitch
> + */
> + uint32_t pitch;
>
> - uint32_t qpitch; /**< The distance in rows between array slices. */
> + /**
> + * The distance in rows between array slices.
> + *
> + * @see RENDER_SURFACE_STATE.AuxiliarySurfaceQPitch
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfaceQPitch
> + */
> + uint32_t qpitch;
>
> - struct intel_mipmap_tree *mt; /**< hiz miptree used with Gen6 */
> + /**
> + * Hiz miptree. Used only by Gen6.
> + */
> + struct intel_mipmap_tree *mt;
> };
>
> /* Tile resource modes */
> @@ -341,15 +370,49 @@ enum intel_miptree_tr_mode {
>
> struct intel_mipmap_tree
> {
> - /** Buffer object containing the pixel data. */
> + /**
> + * Buffer object containing the surface.
> + *
> + * @see intel_mipmap_tree::offset
> + * @see RENDER_SURFACE_STATE.SurfaceBaseAddress
> + * @see RENDER_SURFACE_STATE.AuxiliarySurfaceBaseAddress
> + * @see 3DSTATE_DEPTH_BUFFER.SurfaceBaseAddress
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfaceBaseAddress
> + * @see 3DSTATE_STENCIL_BUFFER.SurfaceBaseAddress
> + */
> drm_intel_bo *bo;
>
> - uint32_t pitch; /**< pitch in bytes. */
> + /**
> + * Pitch in bytes.
> + *
> + * @see RENDER_SURFACE_STATE.SurfacePitch
> + * @see RENDER_SURFACE_STATE.AuxiliarySurfacePitch
> + * @see 3DSTATE_DEPTH_BUFFER.SurfacePitch
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfacePitch
> + * @see 3DSTATE_STENCIL_BUFFER.SurfacePitch
> + */
> + uint32_t pitch;
>
> - uint32_t tiling; /**< One of the I915_TILING_* flags */
> + /**
> + * One of the I915_TILING_* flags.
> + *
> + * @see RENDER_SURFACE_STATE.TileMode
> + * @see 3DSTATE_DEPTH_BUFFER.TileMode
> + */
> + uint32_t tiling;
> +
> + /**
> + * @see RENDER_SURFACE_STATE.TiledResourceMode
> + * @see 3DSTATE_DEPTH_BUFFER.TiledResourceMode
> + */
> enum intel_miptree_tr_mode tr_mode;
>
> - /* Effectively the key:
> + /**
> + * @brief One of GL_TEXTURE_2D, GL_TEXTURE_2D_ARRAY, etc.
> + *
> + * @see RENDER_SURFACE_STATE.SurfaceType
> + * @see RENDER_SURFACE_STATE.SurfaceArray
> + * @see 3DSTATE_DEPTH_BUFFER.SurfaceType
> */
> GLenum target;
>
> @@ -366,10 +429,17 @@ struct intel_mipmap_tree
> *
> * For ETC1/ETC2 textures, this is one of the uncompressed mesa texture
> * formats if the hardware lacks support for ETC1/ETC2. See @ref etc_format.
> + *
> + * @see RENDER_SURFACE_STATE.SurfaceFormat
> + * @see 3DSTATE_DEPTH_BUFFER.SurfaceFormat
> */
> mesa_format format;
>
> - /** This variable stores the value of ETC compressed texture format */
> + /**
> + * This variable stores the value of ETC compressed texture format
> + *
> + * @see RENDER_SURFACE_STATE.SurfaceFormat
> + */
> mesa_format etc_format;
>
> /**
> @@ -385,8 +455,12 @@ struct intel_mipmap_tree
> * horizontal and vertical surface alignments often appear as variables 'i'
> * and 'j'.
> */
> - uint32_t halign; /**< RENDER_SURFACE_STATE.SurfaceHorizontalAlignment */
> - uint32_t valign; /**< RENDER_SURFACE_STATE.SurfaceVerticalAlignment */
> +
> + /** @see RENDER_SURFACE_STATE.SurfaceHorizontalAlignment */
> + uint32_t halign;
> +
> + /** @see RENDER_SURFACE_STATE.SurfaceVerticalAlignment */
> + uint32_t valign;
> /** @} */
>
> GLuint first_level;
> @@ -402,19 +476,47 @@ struct intel_mipmap_tree
> */
> GLuint physical_width0, physical_height0, physical_depth0;
>
> - GLuint cpp; /**< bytes per pixel (or bytes per block if compressed) */
> + /** Bytes per pixel (or bytes per block if compressed) */
> + GLuint cpp;
> +
> + /**
> + * @see RENDER_SURFACE_STATE.NumberOfMultisamples
> + * @see 3DSTATE_MULTISAMPLE.NumberOfMultisamples
> + */
> GLuint num_samples;
> +
> bool compressed;
>
> /**
> - * Level zero image dimensions. These dimensions correspond to the
> + * @name Level zero image dimensions
> + * @{
> + *
> + * These dimensions correspond to the
> * logical width, height, and depth of the texture as seen by client code.
> * Accordingly, they do not account for the extra width, height, and/or
> * depth that must be allocated in order to accommodate multisample
> * formats, nor do they account for the extra factor of 6 in depth that
> * must be allocated in order to accommodate cubemap textures.
> */
> - uint32_t logical_width0, logical_height0, logical_depth0;
> +
> + /**
> + * @see RENDER_SURFACE_STATE.Width
> + * @see 3DSTATE_DEPTH_BUFFER.Width
> + */
> + uint32_t logical_width0;
> +
> + /**
> + * @see RENDER_SURFACE_STATE.Height
> + * @see 3DSTATE_DEPTH_BUFFER.Height
> + */
> + uint32_t logical_height0;
> +
> + /**
> + * @see RENDER_SURFACE_STATE.Depth
> + * @see 3DSTATE_DEPTH_BUFFER.Depth
> + */
> + uint32_t logical_depth0;
> + /** @} */
>
> /**
> * Indicates if we use the standard miptree layout (ALL_LOD_IN_EACH_SLICE),
> @@ -431,11 +533,18 @@ struct intel_mipmap_tree
> * surfaces it is the number of blocks. For 1D array surfaces that have the
> * mipmap tree stored horizontally it is the number of pixels between each
> * slice.
> + *
> + * @see RENDER_SURFACE_STATE.SurfaceQPitch
> + * @see 3DSTATE_DEPTH_BUFFER.SurfaceQPitch
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfaceQPitch
> + * @see 3DSTATE_STENCIL_BUFFER.SurfaceQPitch
I'm not really sure how valuable comments like this are - we're using the name QPitch, matching the docs, and e use the field when creating those packets. It seems pretty clear, and this is a lot of text. But it's up to you.
> */
> uint32_t qpitch;
>
> /**
> * MSAA layout used by this buffer.
> + *
> + * @see RENDER_SURFACE_STATE.MultisampledSurfaceStorageFormat
> */
> enum intel_msaa_layout msaa_layout;
>
> @@ -444,24 +553,34 @@ struct intel_mipmap_tree
> GLuint total_width;
> GLuint total_height;
>
> - /* The 3DSTATE_CLEAR_PARAMS value associated with the last depth clear to
> - * this depth mipmap tree, if any.
> + /**
> + * The depth value used during the most recent fast depth clear performed
> + * on the surface. This field is invalid only if surface has never
> + * underwent a fast depth clear.
How about "this field is valid unless the surface has never been fast depth cleared."
> + *
> + * @see 3DSTATE_CLEAR_PARAMS.DepthClearValue
> */
> uint32_t depth_clear_value;
>
> - /* Includes image offset tables:
> - */
> + /* Includes image offset tables: */
> struct intel_mipmap_level level[MAX_TEXTURE_LEVELS];
>
> - /* Offset into bo where miptree starts:
> + /**
> + * Offset into bo where the surface starts.
> + *
> + * @see intel_mipmap_tree::bo
> + *
> + * @see RENDER_SURFACE_STATE.AuxiliarySurfaceBaseAddress
> + * @see 3DSTATE_DEPTH_BUFFER.SurfaceBaseAddress
> + * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfaceBaseAddress
> + * @see 3DSTATE_STENCIL_BUFFER.SurfaceBaseAddress
> */
> uint32_t offset;
>
> /**
> * \brief HiZ aux buffer
> *
> - * The hiz miptree contains the miptree's hiz buffer. To allocate the hiz
> - * buffer, use intel_miptree_alloc_hiz().
> + * To allocate the hiz buffer, use intel_miptree_alloc_hiz().
> *
> * To determine if hiz is enabled, do not check this pointer. Instead, use
> * intel_miptree_slice_has_hiz().
> @@ -486,6 +605,7 @@ struct intel_mipmap_tree
> * require separate stencil. It always has the true copy of the stencil
> * bits, regardless of mt->format.
> *
> + * \see 3DSTATE_STENCIL_BUFFER
> * \see intel_miptree_map_depthstencil()
> * \see intel_miptree_unmap_depthstencil()
> */
> @@ -513,6 +633,11 @@ struct intel_mipmap_tree
> *
> * This value will only ever contain ones in bits 28-31, so it is safe to
> * OR into dword 7 of SURFACE_STATE.
> + *
> + * @see RENDER_SURFACE_STATE.RedClearColor
> + * @see RENDER_SURFACE_STATE.GreenClearColor
> + * @see RENDER_SURFACE_STATE.BlueClearColor
> + * @see RENDER_SURFACE_STATE.AlphaClearColor
> */
> uint32_t fast_clear_color_value;
>
> --
> 2.5.0.342.g44e0223
With no changes, the series is
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
More information about the mesa-dev
mailing list