Re: [Mesa-dev] [PATCH 5/5] i965/miptree: Add PRM references for most struct members

2015-11-26 Thread Kenneth Graunke

On Sep 25, 2015 12:05 PM, Chad Versace  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_* 

[Mesa-dev] [PATCH 5/5] i965/miptree: Add PRM references for most struct members

2015-09-25 Thread Chad Versace
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
+   * 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
+*/
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