[Mesa-dev] [PATCH 08/12] intel: Refactor selection of miptree tiling

[Ian Romanick]

On 02/28/2013 03:45 PM, Chad Versace wrote:
> This patch (1) extracts from intel_miptree_create() the spaghetti logic
> that selects the tiling format, (2) rewrites that spaghetti into a lucid
> form, and (3) moves it to a new function, intel_miptree_choose_tiling().
> No behavioral change.
>
> As a bonus, it is now evident that the force_y_tiling parameter to
> intel_miptree_create() does not really force Y tiling.

This is a really nice refactor.  It also seems like it would be a good 
function for a unit test...

>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>   src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 90 +++++++++++++++-----------
>   1 file changed, 54 insertions(+), 36 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index 0a90de2..ce07abf 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -281,6 +281,57 @@ intel_miptree_create_layout(struct intel_context *intel,
>      return mt;
>   }
>
> +/**
> + * \brief Helper function for intel_miptree_create().
> + */
> +static uint32_t
> +intel_miptree_choose_tiling(struct intel_context *intel,
> +                            gl_format format,
> +                            uint32_t width0,
> +                            uint32_t num_samples,
> +                            bool force_y_tiling)
> +{
> +
> +   if (format == MESA_FORMAT_S8) {
> +      /* The stencil buffer is W tiled. However, we request from the kernel a
> +       * non-tiled buffer because the GTT is incapable of W fencing.
> +       */
> +      return I915_TILING_NONE;
> +   }
> +
> +   if (!intel->use_texture_tiling || _mesa_is_format_compressed(format))
> +      return I915_TILING_NONE;
> +
> +   if (force_y_tiling)
> +      return I915_TILING_Y;
> +
> +   if (num_samples > 1) {
> +      /* From p82 of the Sandy Bridge PRM, dw3[1] of SURFACE_STATE ("Tiled
> +       * Surface"):
> +       *
> +       *   [DevSNB+]: For multi-sample render targets, this field must be
> +       *   1. MSRTs can only be tiled.
> +       *
> +       * Our usual reason for preferring X tiling (fast blits using the
> +       * blitting engine) doesn't apply to MSAA, since we'll generally be
> +       * downsampling or upsampling when blitting between the MSAA buffer
> +       * and another buffer, and the blitting engine doesn't support that.
> +       * So use Y tiling, since it makes better use of the cache.
> +       */
> +      return I915_TILING_Y;
> +   }
> +
> +   GLenum base_format = _mesa_get_format_base_format(format);
> +   if (intel->gen >= 4 &&
> +       (base_format == GL_DEPTH_COMPONENT ||
> +        base_format == GL_DEPTH_STENCIL_EXT))
> +      return I915_TILING_Y;
> +
> +   if (width0 >= 64)
> +      return I915_TILING_X;
> +
> +   return I915_TILING_NONE;
> +}
>
>   struct intel_mipmap_tree *
>   intel_miptree_create(struct intel_context *intel,
> @@ -296,8 +347,6 @@ intel_miptree_create(struct intel_context *intel,
>                        bool force_y_tiling)
>   {
>      struct intel_mipmap_tree *mt;
> -   uint32_t tiling = I915_TILING_NONE;
> -   GLenum base_format;
>      gl_format tex_format = format;
>      gl_format etc_format = MESA_FORMAT_NONE;
>      GLuint total_width, total_height;
> @@ -336,35 +385,6 @@ intel_miptree_create(struct intel_context *intel,
>      }
>
>      etc_format = (format != tex_format) ? tex_format : MESA_FORMAT_NONE;
> -   base_format = _mesa_get_format_base_format(format);
> -
> -   if (num_samples > 1) {
> -      /* From p82 of the Sandy Bridge PRM, dw3[1] of SURFACE_STATE ("Tiled
> -       * Surface"):
> -       *
> -       *   [DevSNB+]: For multi-sample render targets, this field must be
> -       *   1. MSRTs can only be tiled.
> -       *
> -       * Our usual reason for preferring X tiling (fast blits using the
> -       * blitting engine) doesn't apply to MSAA, since we'll generally be
> -       * downsampling or upsampling when blitting between the MSAA buffer
> -       * and another buffer, and the blitting engine doesn't support that.
> -       * So use Y tiling, since it makes better use of the cache.
> -       */
> -      force_y_tiling = true;
> -   }
> -
> -   if (intel->use_texture_tiling && !_mesa_is_format_compressed(format)) {
> -      if (intel->gen >= 4 &&
> -	  (base_format == GL_DEPTH_COMPONENT ||
> -	   base_format == GL_DEPTH_STENCIL_EXT))
> -	 tiling = I915_TILING_Y;
> -      else if (force_y_tiling) {
> -         tiling = I915_TILING_Y;
> -      } else if (width0 >= 64)
> -	 tiling = I915_TILING_X;
> -   }
> -
>      mt = intel_miptree_create_layout(intel, target, format,
>   				      first_level, last_level, width0,
>   				      height0, depth0,
> @@ -381,15 +401,13 @@ intel_miptree_create(struct intel_context *intel,
>      total_height = mt->total_height;
>
>      if (format == MESA_FORMAT_S8) {
> -      /* The stencil buffer is W tiled. However, we request from the kernel a
> -       * non-tiled buffer because the GTT is incapable of W fencing.  So round
> -       * up the width and height to match the size of W tiles (64x64).
> -       */
> -      tiling = I915_TILING_NONE;
> +      /* Align to size of W tile, 64x64. */
>         total_width = ALIGN(total_width, 64);
>         total_height = ALIGN(total_height, 64);
>      }
>
> +   uint32_t tiling = intel_miptree_choose_tiling(intel, format, width0,
> +                                                 num_samples, force_y_tiling);
>      mt->etc_format = etc_format;
>      mt->region = intel_region_alloc(intel->intelScreen,
>   				   tiling,
>