On Thu, Apr 23, 2015 at 11:38 AM, Pohjolainen, Topi
<topi.pohjolai...@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.pho...@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.pohjolai...@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
