[Mesa-dev] [PATCH 1/8] i965/drm: Make brw_bo_alloc_tiled take tiling by value, not pointer.

Kenneth Graunke kenneth at whitecape.org
Tue Apr 11 16:02:44 UTC 2017


For some reason we passed tiling by pointer, through several layers,
even though the functions only read the initial value, and never
actually change it.  We even had a do-while loop that executed until
the tiling mode matched - except it always did, so it only ran once.
We then had bogus error handling in case it changed the tiling mode
to something nonsensical...which it never did.

Drop all this nonsense.
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c        | 70 ++++++++++++---------------
 src/mesa/drivers/dri/i965/brw_bufmgr.h        |  2 +-
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 24 +++------
 src/mesa/drivers/dri/i965/intel_screen.c      |  7 ++-
 4 files changed, 41 insertions(+), 62 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 1c5ad2467a0..5243078edae 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -138,10 +138,9 @@ hash_find_bo(struct hash_table *ht, unsigned int key)
 }
 
 static unsigned long
-bo_tile_size(struct brw_bufmgr *bufmgr, unsigned long size,
-             uint32_t *tiling_mode)
+bo_tile_size(struct brw_bufmgr *bufmgr, unsigned long size, uint32_t tiling)
 {
-   if (*tiling_mode == I915_TILING_NONE)
+   if (tiling == I915_TILING_NONE)
       return size;
 
    /* 965+ just need multiples of page size for tiling */
@@ -154,18 +153,17 @@ bo_tile_size(struct brw_bufmgr *bufmgr, unsigned long size,
  * change.
  */
 static unsigned long
-bo_tile_pitch(struct brw_bufmgr *bufmgr,
-              unsigned long pitch, uint32_t *tiling_mode)
+bo_tile_pitch(struct brw_bufmgr *bufmgr, unsigned long pitch, uint32_t tiling)
 {
    unsigned long tile_width;
 
    /* If untiled, then just align it so that we can do rendering
     * to it with the 3D engine.
     */
-   if (*tiling_mode == I915_TILING_NONE)
+   if (tiling == I915_TILING_NONE)
       return ALIGN(pitch, 64);
 
-   if (*tiling_mode == I915_TILING_X)
+   if (tiling == I915_TILING_X)
       tile_width = 512;
    else
       tile_width = 128;
@@ -378,42 +376,36 @@ brw_bo_alloc(struct brw_bufmgr *bufmgr,
 
 struct brw_bo *
 brw_bo_alloc_tiled(struct brw_bufmgr *bufmgr, const char *name,
-                   int x, int y, int cpp, uint32_t *tiling_mode,
+                   int x, int y, int cpp, uint32_t tiling,
                    unsigned long *pitch, unsigned long flags)
 {
    unsigned long size, stride;
-   uint32_t tiling;
+   unsigned long aligned_y, height_alignment;
 
-   do {
-      unsigned long aligned_y, height_alignment;
-
-      tiling = *tiling_mode;
-
-      /* If we're tiled, our allocations are in 8 or 32-row blocks,
-       * so failure to align our height means that we won't allocate
-       * enough pages.
-       *
-       * If we're untiled, we still have to align to 2 rows high
-       * because the data port accesses 2x2 blocks even if the
-       * bottom row isn't to be rendered, so failure to align means
-       * we could walk off the end of the GTT and fault.  This is
-       * documented on 965, and may be the case on older chipsets
-       * too so we try to be careful.
-       */
-      aligned_y = y;
-      height_alignment = 2;
-
-      if (tiling == I915_TILING_X)
-         height_alignment = 8;
-      else if (tiling == I915_TILING_Y)
-         height_alignment = 32;
-      aligned_y = ALIGN(y, height_alignment);
-
-      stride = x * cpp;
-      stride = bo_tile_pitch(bufmgr, stride, tiling_mode);
-      size = stride * aligned_y;
-      size = bo_tile_size(bufmgr, size, tiling_mode);
-   } while (*tiling_mode != tiling);
+   /* If we're tiled, our allocations are in 8 or 32-row blocks,
+    * so failure to align our height means that we won't allocate
+    * enough pages.
+    *
+    * If we're untiled, we still have to align to 2 rows high
+    * because the data port accesses 2x2 blocks even if the
+    * bottom row isn't to be rendered, so failure to align means
+    * we could walk off the end of the GTT and fault.  This is
+    * documented on 965, and may be the case on older chipsets
+    * too so we try to be careful.
+    */
+   aligned_y = y;
+   height_alignment = 2;
+
+   if (tiling == I915_TILING_X)
+      height_alignment = 8;
+   else if (tiling == I915_TILING_Y)
+      height_alignment = 32;
+   aligned_y = ALIGN(y, height_alignment);
+
+   stride = x * cpp;
+   stride = bo_tile_pitch(bufmgr, stride, tiling);
+   size = stride * aligned_y;
+   size = bo_tile_size(bufmgr, size, tiling);
    *pitch = stride;
 
    if (tiling == I915_TILING_NONE)
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index b27178b6fe1..aa3d40bb959 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -160,7 +160,7 @@ struct brw_bo *brw_bo_alloc(struct brw_bufmgr *bufmgr, const char *name,
 struct brw_bo *brw_bo_alloc_tiled(struct brw_bufmgr *bufmgr,
                                   const char *name,
                                   int x, int y, int cpp,
-                                  uint32_t *tiling_mode,
+                                  uint32_t tiling_mode,
                                   unsigned long *pitch,
                                   unsigned long flags);
 
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 81e2d6e1e99..137bb075f6a 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -619,12 +619,12 @@ miptree_create(struct brw_context *brw,
       mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "miptree",
                                   ALIGN(mt->total_width, 64),
                                   ALIGN(mt->total_height, 64),
-                                  mt->cpp, &mt->tiling, &pitch,
+                                  mt->cpp, mt->tiling, &pitch,
                                   alloc_flags);
    } else {
       mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "miptree",
                                   mt->total_width, mt->total_height,
-                                  mt->cpp, &mt->tiling, &pitch,
+                                  mt->cpp, mt->tiling, &pitch,
                                   alloc_flags);
    }
 
@@ -668,7 +668,7 @@ intel_miptree_create(struct brw_context *brw,
       brw_bo_unreference(mt->bo);
       mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "miptree",
                                   mt->total_width, mt->total_height, mt->cpp,
-                                  &mt->tiling, &pitch, alloc_flags);
+                                  mt->tiling, &pitch, alloc_flags);
       mt->pitch = pitch;
    }
 
@@ -1544,7 +1544,6 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
     */
    const uint32_t alloc_flags =
       is_lossless_compressed ? 0 : BO_ALLOC_FOR_RENDER;
-   uint32_t tiling = I915_TILING_Y;
    unsigned long pitch;
 
    /* ISL has stricter set of alignment rules then the drm allocator.
@@ -1553,10 +1552,9 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
     */
    buf->bo = brw_bo_alloc_tiled(brw->bufmgr, "ccs-miptree",
                                 buf->pitch, buf->size / buf->pitch,
-                                1, &tiling, &pitch, alloc_flags);
+                                1, I915_TILING_Y, &pitch, alloc_flags);
    if (buf->bo) {
       assert(pitch == buf->pitch);
-      assert(tiling == I915_TILING_Y);
    } else {
       free(buf);
       return false;
@@ -1687,18 +1685,13 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
    }
 
    unsigned long pitch;
-   uint32_t tiling = I915_TILING_Y;
    buf->aux_base.bo = brw_bo_alloc_tiled(brw->bufmgr, "hiz",
                                          hz_width, hz_height, 1,
-                                         &tiling, &pitch,
+                                         I915_TILING_Y, &pitch,
                                          BO_ALLOC_FOR_RENDER);
    if (!buf->aux_base.bo) {
       free(buf);
       return NULL;
-   } else if (tiling != I915_TILING_Y) {
-      brw_bo_unreference(buf->aux_base.bo);
-      free(buf);
-      return NULL;
    }
 
    buf->aux_base.size = hz_width * hz_height;
@@ -1784,18 +1777,13 @@ intel_gen8_hiz_buf_create(struct brw_context *brw,
    }
 
    unsigned long pitch;
-   uint32_t tiling = I915_TILING_Y;
    buf->aux_base.bo = brw_bo_alloc_tiled(brw->bufmgr, "hiz",
                                          hz_width, hz_height, 1,
-                                         &tiling, &pitch,
+                                         I915_TILING_Y, &pitch,
                                          BO_ALLOC_FOR_RENDER);
    if (!buf->aux_base.bo) {
       free(buf);
       return NULL;
-   } else if (tiling != I915_TILING_Y) {
-      brw_bo_unreference(buf->aux_base.bo);
-      free(buf);
-      return NULL;
    }
 
    buf->aux_base.size = hz_width * hz_height;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 76e82b03b23..f1e9d660d94 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -614,7 +614,7 @@ intel_create_image_common(__DRIscreen *dri_screen,
 
    cpp = _mesa_get_format_bytes(image->format);
    image->bo = brw_bo_alloc_tiled(screen->bufmgr, "image",
-                                  width, height, cpp, &tiling,
+                                  width, height, cpp, tiling,
                                   &pitch, 0);
    if (image->bo == NULL) {
       free(image);
@@ -1298,7 +1298,7 @@ intel_detect_swizzling(struct intel_screen *screen)
    uint32_t swizzle_mode = 0;
 
    buffer = brw_bo_alloc_tiled(screen->bufmgr, "swizzle test",
-                               64, 64, 4, &tiling, &aligned_pitch, flags);
+                               64, 64, 4, tiling, &aligned_pitch, flags);
    if (buffer == NULL)
       return false;
 
@@ -2097,7 +2097,6 @@ intelAllocateBuffer(__DRIscreen *dri_screen,
    /* The front and back buffers are color buffers, which are X tiled. GEN9+
     * supports Y tiled and compressed buffers, but there is no way to plumb that
     * through to here. */
-   uint32_t tiling = I915_TILING_X;
    unsigned long pitch;
    int cpp = format / 8;
    intelBuffer->bo = brw_bo_alloc_tiled(screen->bufmgr,
@@ -2105,7 +2104,7 @@ intelAllocateBuffer(__DRIscreen *dri_screen,
                                         width,
                                         height,
                                         cpp,
-                                        &tiling, &pitch,
+                                        I915_TILING_X, &pitch,
                                         BO_ALLOC_FOR_RENDER);
 
    if (intelBuffer->bo == NULL) {
-- 
2.12.1



More information about the mesa-dev mailing list