[Mesa-dev] [PATCH V2 03/22] i965: Move intel_miptree_choose_tiling() to brw_tex_layout.c

Anuj Phogat anuj.phogat at gmail.com
Fri Apr 24 10:04:31 PDT 2015


On Thu, Apr 23, 2015 at 11:38 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> wrote:
> On Fri, Apr 17, 2015 at 04:51:24PM -0700, Anuj Phogat wrote:
>> Patch continues code refactoring.
>>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_tex_layout.c    | 105 ++++++++++++++++++++++++++
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 104 -------------------------
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   8 --
>>  3 files changed, 105 insertions(+), 112 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> index b8408d3..08ef7a6 100644
>> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> @@ -377,6 +377,111 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
>>     align_cube(mt);
>>  }
>>
>> +/**
>> + * \brief Helper function for intel_miptree_create().
>> + */
>> +static uint32_t
>> +intel_miptree_choose_tiling(struct brw_context *brw,
>
> All the other functions in this file use "brw_miptree"-prefix, perhaps
> this should be as well?
>
I'll add the prefix.

Many static functions in the file don't use brw_miptree prefix. e.g:
intel_horizontal_texture_alignment_unit
intel_vertical_texture_alignment_unit
gen9_miptree_layout_1d
use_linear_1d_layout

It might be a good idea to add this prefix to all of them. I'll to do
this in a separate series.

>> +                            mesa_format format,
>> +                            uint32_t width0,
>> +                            uint32_t num_samples,
>> +                            enum intel_miptree_tiling_mode requested,
>> +                            struct intel_mipmap_tree *mt)
>
> You could change both 'brw' and 'mt' to constant pointers, they are only
> used for reading.
>
Consider fixed.

> With that:
>
> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>
>> +{
>> +   if (format == MESA_FORMAT_S_UINT8) {
>> +      /* 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;
>> +   }
>> +
>> +   /* Some usages may want only one type of tiling, like depth miptrees (Y
>> +    * tiled), or temporary BOs for uploading data once (linear).
>> +    */
>> +   switch (requested) {
>> +   case INTEL_MIPTREE_TILING_ANY:
>> +      break;
>> +   case INTEL_MIPTREE_TILING_Y:
>> +      return I915_TILING_Y;
>> +   case INTEL_MIPTREE_TILING_NONE:
>> +      return I915_TILING_NONE;
>> +   }
>> +
>> +   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 (base_format == GL_DEPTH_COMPONENT ||
>> +       base_format == GL_DEPTH_STENCIL_EXT)
>> +      return I915_TILING_Y;
>> +
>> +   /* 1D textures (and 1D array textures) don't get any benefit from tiling,
>> +    * in fact it leads to a less efficient use of memory space and bandwidth
>> +    * due to tile alignment.
>> +    */
>> +   if (mt->logical_height0 == 1)
>> +      return I915_TILING_NONE;
>> +
>> +   int minimum_pitch = mt->total_width * mt->cpp;
>> +
>> +   /* If the width is much smaller than a tile, don't bother tiling. */
>> +   if (minimum_pitch < 64)
>> +      return I915_TILING_NONE;
>> +
>> +   if (ALIGN(minimum_pitch, 512) >= 32768 ||
>> +       mt->total_width >= 32768 || mt->total_height >= 32768) {
>> +      perf_debug("%dx%d miptree too large to blit, falling back to untiled",
>> +                 mt->total_width, mt->total_height);
>> +      return I915_TILING_NONE;
>> +   }
>> +
>> +   /* Pre-gen6 doesn't have BLORP to handle Y-tiling, so use X-tiling. */
>> +   if (brw->gen < 6)
>> +      return I915_TILING_X;
>> +
>> +   /* From the Sandybridge PRM, Volume 1, Part 2, page 32:
>> +    * "NOTE: 128BPE Format Color Buffer ( render target ) MUST be either TileX
>> +    *  or Linear."
>> +    * 128 bits per pixel translates to 16 bytes per pixel. This is necessary
>> +    * all the way back to 965, but is permitted on Gen7+.
>> +    */
>> +   if (brw->gen < 7 && mt->cpp >= 16)
>> +      return I915_TILING_X;
>> +
>> +   /* From the Ivy Bridge PRM, Vol4 Part1 2.12.2.1 (SURFACE_STATE for most
>> +    * messages), on p64, under the heading "Surface Vertical Alignment":
>> +    *
>> +    *     This field must be set to VALIGN_4 for all tiled Y Render Target
>> +    *     surfaces.
>> +    *
>> +    * So if the surface is renderable and uses a vertical alignment of 2,
>> +    * force it to be X tiled.  This is somewhat conservative (it's possible
>> +    * that the client won't ever render to this surface), but it's difficult
>> +    * to know that ahead of time.  And besides, since we use a vertical
>> +    * alignment of 4 as often as we can, this shouldn't happen very often.
>> +    */
>> +   if (brw->gen == 7 && mt->align_h == 2 &&
>> +       brw->format_supported_as_render_target[format]) {
>> +      return I915_TILING_X;
>> +   }
>> +
>> +   return I915_TILING_Y | I915_TILING_X;
>> +}
>> +
>> +
>>  void
>>  brw_miptree_layout(struct brw_context *brw,
>>                     mesa_format format,
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 7a64282..c1414b3 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -438,110 +438,6 @@ intel_miptree_create_layout(struct brw_context *brw,
>>     return mt;
>>  }
>>
>> -/**
>> - * \brief Helper function for intel_miptree_create().
>> - */
>> -uint32_t
>> -intel_miptree_choose_tiling(struct brw_context *brw,
>> -                            mesa_format format,
>> -                            uint32_t width0,
>> -                            uint32_t num_samples,
>> -                            enum intel_miptree_tiling_mode requested,
>> -                            struct intel_mipmap_tree *mt)
>> -{
>> -   if (format == MESA_FORMAT_S_UINT8) {
>> -      /* 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;
>> -   }
>> -
>> -   /* Some usages may want only one type of tiling, like depth miptrees (Y
>> -    * tiled), or temporary BOs for uploading data once (linear).
>> -    */
>> -   switch (requested) {
>> -   case INTEL_MIPTREE_TILING_ANY:
>> -      break;
>> -   case INTEL_MIPTREE_TILING_Y:
>> -      return I915_TILING_Y;
>> -   case INTEL_MIPTREE_TILING_NONE:
>> -      return I915_TILING_NONE;
>> -   }
>> -
>> -   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 (base_format == GL_DEPTH_COMPONENT ||
>> -       base_format == GL_DEPTH_STENCIL_EXT)
>> -      return I915_TILING_Y;
>> -
>> -   /* 1D textures (and 1D array textures) don't get any benefit from tiling,
>> -    * in fact it leads to a less efficient use of memory space and bandwidth
>> -    * due to tile alignment.
>> -    */
>> -   if (mt->logical_height0 == 1)
>> -      return I915_TILING_NONE;
>> -
>> -   int minimum_pitch = mt->total_width * mt->cpp;
>> -
>> -   /* If the width is much smaller than a tile, don't bother tiling. */
>> -   if (minimum_pitch < 64)
>> -      return I915_TILING_NONE;
>> -
>> -   if (ALIGN(minimum_pitch, 512) >= 32768 ||
>> -       mt->total_width >= 32768 || mt->total_height >= 32768) {
>> -      perf_debug("%dx%d miptree too large to blit, falling back to untiled",
>> -                 mt->total_width, mt->total_height);
>> -      return I915_TILING_NONE;
>> -   }
>> -
>> -   /* Pre-gen6 doesn't have BLORP to handle Y-tiling, so use X-tiling. */
>> -   if (brw->gen < 6)
>> -      return I915_TILING_X;
>> -
>> -   /* From the Sandybridge PRM, Volume 1, Part 2, page 32:
>> -    * "NOTE: 128BPE Format Color Buffer ( render target ) MUST be either TileX
>> -    *  or Linear."
>> -    * 128 bits per pixel translates to 16 bytes per pixel. This is necessary
>> -    * all the way back to 965, but is permitted on Gen7+.
>> -    */
>> -   if (brw->gen < 7 && mt->cpp >= 16)
>> -      return I915_TILING_X;
>> -
>> -   /* From the Ivy Bridge PRM, Vol4 Part1 2.12.2.1 (SURFACE_STATE for most
>> -    * messages), on p64, under the heading "Surface Vertical Alignment":
>> -    *
>> -    *     This field must be set to VALIGN_4 for all tiled Y Render Target
>> -    *     surfaces.
>> -    *
>> -    * So if the surface is renderable and uses a vertical alignment of 2,
>> -    * force it to be X tiled.  This is somewhat conservative (it's possible
>> -    * that the client won't ever render to this surface), but it's difficult
>> -    * to know that ahead of time.  And besides, since we use a vertical
>> -    * alignment of 4 as often as we can, this shouldn't happen very often.
>> -    */
>> -   if (brw->gen == 7 && mt->align_h == 2 &&
>> -       brw->format_supported_as_render_target[format]) {
>> -      return I915_TILING_X;
>> -   }
>> -
>> -   return I915_TILING_Y | I915_TILING_X;
>> -}
>> -
>>
>>  /**
>>   * Choose an appropriate uncompressed format for a requested
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> index f03715b..b03ffe7 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> @@ -772,14 +772,6 @@ intel_miptree_unmap(struct brw_context *brw,
>>                   unsigned int level,
>>                   unsigned int slice);
>>
>> -uint32_t
>> -intel_miptree_choose_tiling(struct brw_context *brw,
>> -                            mesa_format format,
>> -                            uint32_t width0,
>> -                            uint32_t num_samples,
>> -                            enum intel_miptree_tiling_mode requested,
>> -                            struct intel_mipmap_tree *mt);
>> -
>>  void
>>  intel_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt,
>>              unsigned int level, unsigned int layer, enum gen6_hiz_op op);
>> --
>> 2.3.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list