[Mesa-dev] [PATCH 2/5] i965/miptree: Use cpu tiling/detiling when mapping

Nanley Chery nanleychery at gmail.com
Mon Mar 5 19:39:11 UTC 2018


On Tue, Jan 09, 2018 at 11:16:59PM -0800, Scott D Phillips wrote:
> Rename the (un)map_gtt functions to (un)map_map (map by
> returning a map) and add new functions (un)map_tiled_memcpy that
> return a shadow buffer populated with the intel_tiled_memcpy
> functions.

Could you mention some of the rationale?

> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 95 ++++++++++++++++++++++++---
>  1 file changed, 86 insertions(+), 9 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index ead0c359c0..7a90dafa1e 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -31,6 +31,7 @@
>  #include "intel_image.h"
>  #include "intel_mipmap_tree.h"
>  #include "intel_tex.h"
> +#include "intel_tiled_memcpy.h"
>  #include "intel_blit.h"
>  #include "intel_fbo.h"
>  
> @@ -3031,10 +3032,10 @@ intel_miptree_unmap_raw(struct intel_mipmap_tree *mt)
>  }
>  
>  static void
> -intel_miptree_map_gtt(struct brw_context *brw,
> -		      struct intel_mipmap_tree *mt,
> -		      struct intel_miptree_map *map,
> -		      unsigned int level, unsigned int slice)
> +intel_miptree_map_map(struct brw_context *brw,
> +                      struct intel_mipmap_tree *mt,
> +                      struct intel_miptree_map *map,
> +                      unsigned int level, unsigned int slice)
>  {
>     unsigned int bw, bh;
>     void *base;
> @@ -3052,7 +3053,7 @@ intel_miptree_map_gtt(struct brw_context *brw,
>     y /= bh;
>     x /= bw;
>  
> -   base = intel_miptree_map_raw(brw, mt, map->mode);
> +   base = intel_miptree_map_raw(brw, mt, map->mode | MAP_RAW);
>  
>     if (base == NULL)
>        map->ptr = NULL;
> @@ -3078,11 +3079,80 @@ intel_miptree_map_gtt(struct brw_context *brw,
>  }
>  
>  static void
> -intel_miptree_unmap_gtt(struct intel_mipmap_tree *mt)
> +intel_miptree_unmap_map(struct intel_mipmap_tree *mt)
>  {
>     intel_miptree_unmap_raw(mt);
>  }
>  
> +/* Compute extent parameters for use with tiled_memcpy functions.
> + * xs are in units of bytes and ys are in units of strides. */
> +static inline void
> +tile_extents(struct intel_mipmap_tree *mt, struct intel_miptree_map *map,
> +             unsigned int level, unsigned int slice, unsigned int *x1,
> +             unsigned int *x2, unsigned int *y1, unsigned int *y2)

It would be nice to give these variables units:
x1_B, y1_el, etc.

> +{
> +   unsigned int block_width, block_height, block_bytes;
> +   unsigned int x0_el, y0_el;
> +
> +   _mesa_get_format_block_size(mt->format, &block_width, &block_height);
> +   block_bytes = _mesa_get_format_bytes(mt->format);

block_bytes == mt->cpp (in theory anyways)

> +
> +   assert(map->x % block_width == 0);
> +   assert(map->y % block_height == 0);
> +
> +   intel_miptree_get_image_offset(mt, level, slice, &x0_el, &y0_el);
> +   *x1 = (map->x / block_width + x0_el) * block_bytes;
> +   *y1 = map->y / block_height + y0_el;
> +   *x2 = *x1 + DIV_ROUND_UP(map->w, block_width) * block_bytes;
> +   *y2 = *y1 + DIV_ROUND_UP(map->h, block_height);
> +}
> +
> +static void
> +intel_miptree_map_tiled_memcpy(struct brw_context *brw,
> +                               struct intel_mipmap_tree *mt,
> +                               struct intel_miptree_map *map,
> +                               unsigned int level, unsigned int slice)
> +{
> +   unsigned int x1, x2, y1, y2;
> +   tile_extents(mt, map, level, slice, &x1, &x2, &y1, &y2);
> +   map->stride = _mesa_format_row_stride(mt->format, map->w);
> +   map->buffer = map->ptr = malloc(map->stride * (y2 - y1));

Using _mesa_align_malloc should improve memory accesses for our 128bit formats.

We should probably also assert(map->ptr) or throw a GL_OUT_OF_MEMORY
error if the alloc fails.

> +
> +   if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
> +      char *src = intel_miptree_map_raw(brw, mt, map->mode | MAP_RAW);
> +      src += mt->offset;
> +
> +      tiled_to_linear(x1, x2, y1, y2, map->ptr, src, map->stride,
> +                      mt->surf.row_pitch, brw->has_swizzling, mt->surf.tiling,
> +                      memcpy);
> +
> +      intel_miptree_unmap_raw(mt);
> +   }
> +}
> +
> +static void
> +intel_miptree_unmap_tiled_memcpy(struct brw_context *brw,
> +                                 struct intel_mipmap_tree *mt,
> +                                 struct intel_miptree_map *map,
> +                                 unsigned int level,
> +                                 unsigned int slice)
> +{
> +   if (map->mode & GL_MAP_WRITE_BIT) {
> +      unsigned int x1, x2, y1, y2;
> +      tile_extents(mt, map, level, slice, &x1, &x2, &y1, &y2);
> +
> +      char *dst = intel_miptree_map_raw(brw, mt, map->mode | MAP_RAW);
> +      dst += mt->offset;
> +
> +      linear_to_tiled(x1, x2, y1, y2, dst, map->ptr, mt->surf.row_pitch,
> +                      map->stride, brw->has_swizzling, mt->surf.tiling, memcpy);
> +
> +      intel_miptree_unmap_raw(mt);
> +   }
> +   free(map->buffer);
> +   map->buffer = map->ptr = NULL;
> +}
> +
>  static void
>  intel_miptree_map_blit(struct brw_context *brw,
>  		       struct intel_mipmap_tree *mt,
> @@ -3640,8 +3710,10 @@ intel_miptree_map(struct brw_context *brw,
>                (mt->surf.row_pitch % 16 == 0)) {
>        intel_miptree_map_movntdqa(brw, mt, map, level, slice);
>  #endif
> +   } else if (mt->surf.tiling != ISL_TILING_LINEAR) {

What happens to clients who try to map a tiled surface with the MAP_RAW
flag set? It looks like they expect a tiled buffer but get a linear one
instead.

Also, in a number of places where tiling memcpy is used, the following
restriction is present (though the comment varies):

   /* linear_to_tiled() assumes that if the object is swizzled, it is using
    * I915_BIT6_SWIZZLE_9_10 for X and I915_BIT6_SWIZZLE_9 for Y.  This is only
    * true on gen5 and above.
    *
    * The killer on top is that some gen4 have an L-shaped swizzle mode, where
    * parts of the memory aren't swizzled at all. Userspace just can't handle
    * that.
    */
   if (devinfo->gen < 5 && brw->has_swizzling)
      return false;

> +      intel_miptree_map_tiled_memcpy(brw, mt, map, level, slice);
>     } else {
> -      intel_miptree_map_gtt(brw, mt, map, level, slice);
> +      intel_miptree_map_map(brw, mt, map, level, slice);
>     }
>  
>     *out_ptr = map->ptr;
> @@ -3677,11 +3749,16 @@ intel_miptree_unmap(struct brw_context *brw,
>     } else if (map->linear_mt) {
>        intel_miptree_unmap_blit(brw, mt, map, level, slice);
>  #if defined(USE_SSE41)
> -   } else if (map->buffer && cpu_has_sse4_1) {
> +   } else if (!(map->mode & GL_MAP_WRITE_BIT) &&
> +              !mt->compressed && cpu_has_sse4_1 &&
> +              (mt->surf.row_pitch % 16 == 0) &&
> +              map->buffer) {

This hunk looks unrelated. Should this be a separate patch?

-Nanley
>        intel_miptree_unmap_movntdqa(brw, mt, map, level, slice);
>  #endif
> +   } else if (mt->surf.tiling != ISL_TILING_LINEAR) {
> +      intel_miptree_unmap_tiled_memcpy(brw, mt, map, level, slice);
>     } else {
> -      intel_miptree_unmap_gtt(mt);
> +      intel_miptree_unmap_map(mt);
>     }
>  
>     intel_miptree_release_map(mt, level, slice);
> -- 
> 2.14.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list