[Mesa-dev] [PATCH] intel:i915:i965: enable mipmap layout right mode

Eric Anholt eric at anholt.net
Fri Mar 16 11:13:23 PDT 2012


On Thu, 15 Mar 2012 14:42:53 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> There are two mipmap layout modes: below and right. And we currently just
> use _below_ mode. And in some cases, like height is greater than width,
> it would be better to use the _right_ mode for saving memory.
> 
> And it also fix some issues like the gl-max-texture-dimensions.html webglc
> test case on pineview in a hardware way(no fallback). Since when
> rendering with 1x2048 texture using below mode would make the draw
> offset exceed the max allowed size, but will not when using right mode.

I think you should be able to get hardware rendering without changing
layouts by taking advantage of intel_renderbuffer_get_tile_offsets().

> diff --git a/src/mesa/drivers/dri/i915/i915_texstate.c b/src/mesa/drivers/dri/i915/i915_texstate.c
> index 9022548..54f32a4 100644
> --- a/src/mesa/drivers/dri/i915/i915_texstate.c
> +++ b/src/mesa/drivers/dri/i915/i915_texstate.c
> @@ -192,6 +192,8 @@ i915_update_tex_unit(struct intel_context *intel, GLuint unit, GLuint ss3)
>         (U_FIXED(CLAMP(maxlod, 0.0, 11.0), 2) << MS4_MAX_LOD_SHIFT) |
>         ((firstImage->Depth - 1) << MS4_VOLUME_DEPTH_SHIFT));
>  
> +   if (intel->is_945)
> +      state[I915_TEXREG_MS4] |= intelObj->mt->layout << MS4_MIP_LAYOUT_MODE_SHIFT;
>  
>     {
>        GLuint minFilt, mipFilt, magFilt;

You're checking 945 here, but your decision to choose right layout
didn't check for 945.

> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index a3de2e3..b6565fe 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -654,7 +654,7 @@ brw_update_texture_surface( struct gl_context *ctx, GLuint unit )
>  			  6 * 4, 32, &brw->wm.surf_offset[surf_index]);
>  
>     surf[0] = (translate_tex_target(tObj->Target) << BRW_SURFACE_TYPE_SHIFT |
> -	      BRW_SURFACE_MIPMAPLAYOUT_BELOW << BRW_SURFACE_MIPLAYOUT_SHIFT |
> +	      mt->layout << BRW_SURFACE_MIPLAYOUT_SHIFT |
>  	      BRW_SURFACE_CUBEFACE_ENABLES |
>  	      (translate_tex_format(mt->format,
>  				    firstImage->InternalFormat,

You didn't update gen7.

> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> index 9082864..d175f50 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> @@ -215,6 +215,10 @@ struct intel_mipmap_tree
>     /* These are also refcounted:
>      */
>     GLuint refcount;
> +
> +#define INTEL_LAYOUT_BELOW	0
> +#define INTEL_LAYOUT_RIGHT	1
> +   int layout;
>  };

Make it a bool called "miplayout_right" or something instead of
#defines.

> diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c b/src/mesa/drivers/dri/intel/intel_tex_layout.c
> index 65645bc..4687dd3 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_layout.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_layout.c
> @@ -138,7 +138,27 @@ intel_get_texture_alignment_unit(struct intel_context *intel,
>     *h = intel_vertical_texture_alignment_unit(intel, format);
>  }
>  
> -void i945_miptree_layout_2d(struct intel_mipmap_tree *mt)
> +static int
> +i945_miptree_choose_layout(struct intel_context *intel, GLenum target,
> +                           GLuint width, GLuint height)
> +{
> +   int layout = INTEL_LAYOUT_BELOW; /* Use layout _below_ by default */
> +
> +   /*
> +    * INTEL_LAYOUT_RIGHT is only for:
> +    *   GL_TEXTURE_1D, GL_TEXTURE_2D [945+, aka all platforms here]
> +    *   GL_TEXTURE_1D, GL_TEXTURE_2D, GL_TEXTURE_CUBE_MAP [gen5+]
> +    */
> +   if (target == GL_TEXTURE_1D || target == GL_TEXTURE_2D ||
> +       (intel->gen >= 5 && target == GL_TEXTURE_CUBE_MAP)) {
> +      if ((height >> 1) >= width)
> +         layout = INTEL_LAYOUT_RIGHT;
> +   }
> +
> +   return layout;
> +}

You should explain why the choice is being made here.

> +void i945_miptree_layout_2d(struct intel_context *intel, struct intel_mipmap_tree *mt)
>  {
>     GLuint level;
>     GLuint x = 0;
> @@ -153,24 +173,32 @@ void i945_miptree_layout_2d(struct intel_mipmap_tree *mt)
>         mt->total_width = ALIGN(mt->width0, mt->align_w);
>     }
>  
> -   /* May need to adjust width to accomodate the placement of
> -    * the 2nd mipmap.  This occurs when the alignment
> -    * constraints of mipmap placement push the right edge of the
> -    * 2nd mipmap out past the width of its parent.
> -    */
> +   mt->layout = i945_miptree_choose_layout(intel, mt->target, width, height);
> +   /* Determine the max width */
>     if (mt->first_level != mt->last_level) {
> -       GLuint mip1_width;
> -
> -       if (mt->compressed) {
> -           mip1_width = ALIGN(minify(mt->width0), mt->align_w)
> -               + ALIGN(minify(minify(mt->width0)), mt->align_w);
> +       GLuint tmp;
> +
> +       if (mt->layout == INTEL_LAYOUT_BELOW) {
> +          if (mt->compressed) {
> +              tmp = ALIGN(minify(mt->width0), mt->align_w) +
> +                          ALIGN(minify(minify(mt->width0)), mt->align_w);
> +          } else {
> +              tmp = ALIGN(minify(mt->width0), mt->align_w) +
> +                          minify(minify(mt->width0));
> +          }
> +       } else if (mt->layout == INTEL_LAYOUT_RIGHT) {
> +          if (mt->compressed) {
> +              tmp = ALIGN(mt->width0, mt->align_w) +
> +                    ALIGN(minify(mt->width0), mt->align_w);
> +          } else {
> +              tmp = ALIGN(mt->width0, mt->align_w) + minify(mt->width0);
> +          }
>         } else {
> -           mip1_width = ALIGN(minify(mt->width0), mt->align_w)
> -               + minify(minify(mt->width0));
> +          assert(!"mipmap: wrong layout!\n");
>         }
>  
> -       if (mip1_width > mt->total_width) {
> -           mt->total_width = mip1_width;
> +       if (tmp > mt->total_width) {
> +           mt->total_width = tmp;
>         }
>     }

don't call variables "tmp"

> @@ -191,13 +219,11 @@ void i945_miptree_layout_2d(struct intel_mipmap_tree *mt)
>         */
>        mt->total_height = MAX2(mt->total_height, y + img_height);
>  
> -      /* Layout_below: step right after second mipmap.
> -       */
> -      if (level == mt->first_level + 1) {
> -	 x += ALIGN(width, mt->align_w);
> -      }
> -      else {
> -	 y += img_height;
> +      if ((mt->layout == INTEL_LAYOUT_BELOW && level == mt->first_level + 1) ||
> +          (mt->layout == INTEL_LAYOUT_RIGHT && level == mt->first_level)) {
> +            x += ALIGN(width, mt->align_w);
> +      } else {
> +            y += img_height;
>        }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120316/8890db3d/attachment.pgp>


More information about the mesa-dev mailing list