On Sep 25, 2015 12:05 PM, Chad Versace <chad.vers...@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 <kenn...@whitecape.org> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev