[Mesa-dev] [PATCH 3/8] i965: Add a helper function intel_miptree_get_tile_dimensions()

[Anuj Phogat]

On Mon, Aug 17, 2015 at 12:06 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Fri, Aug 14, 2015 at 04:51:54PM -0700, Anuj Phogat wrote:
>> Cc: Ben Widawsky <ben at bwidawsk.net>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 67 ++++++++++++++++++---------
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  5 ++
>>  2 files changed, 50 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 55dc80d..12f1a4d 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -563,35 +563,15 @@ static unsigned long
>>  intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
>>                          unsigned long *pitch)
>>  {
>> -   const uint32_t bpp = mt->cpp * 8;
>> -   const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
>>     uint32_t tile_width, tile_height;
>>     unsigned long stride, size, aligned_y;
>>
>>     assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
>> -
>> -   switch (bpp) {
>> -   case 8:
>> -      tile_height = 64;
>> -      break;
>> -   case 16:
>> -   case 32:
>> -      tile_height = 32;
>> -      break;
>> -   case 64:
>> -   case 128:
>> -      tile_height = 16;
>> -      break;
>> -   default:
>> -      unreachable("not reached");
>> -   }
>> -
>> -   if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
>> -      tile_height *= 4;
>> +   intel_miptree_get_tile_dimensions(mt->tiling, mt->tr_mode, mt->cpp,
>> +                                     &tile_width, &tile_height);
>>
>>     aligned_y = ALIGN(mt->total_height, tile_height);
>>     stride = mt->total_width * mt->cpp;
>> -   tile_width = tile_height * mt->cpp * aspect_ratio;
>>     stride = ALIGN(stride, tile_width);
>>     size = stride * aligned_y;
>>
>> @@ -1081,6 +1061,49 @@ intel_miptree_get_image_offset(const struct intel_mipmap_tree *mt,
>>     *y = mt->level[level].slice[slice].y_offset;
>>  }
>>
>> +
>> +/**
>> + * This function computes the width and height in bytes of different tiling
>> + * patterns. If the BO is untiled, the dimensions are set to cpp.
>> + */
>> +void
>> +intel_miptree_get_tile_dimensions(uint32_t tiling, uint32_t tr_mode,
>> +                                  uint32_t cpp, uint32_t *tile_w,
>> +                                  uint32_t *tile_h)
>> +{
>> +   /* Y tiled surfaces with TRMODE_NONE allows non power of 2 cpp. Using
>> +    * intel_miptree_get_tile_masks() in those cases will give incorrect
>> +    * tile dimensions. So handle the TRMODE_NONE here.
>> +    */
>> +   if (tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
>> +      switch (tiling) {
>> +      case I915_TILING_X:
>> +         *tile_w = 512;
>> +         *tile_h = 8 * cpp;
>> +         break;
>> +      case I915_TILING_Y:
>> +         *tile_w = 128;
>> +         *tile_h = 32 * cpp;
>> +         break;
>> +      case I915_TILING_NONE:
>> +         *tile_w = cpp;
>> +         *tile_h = cpp;
>> +         break;
>> +      default:
>> +         unreachable("not reached");
>> +      }
>> +   } else {
>> +      uint32_t mask_x, mask_y;
>> +      assert(_mesa_is_pow_two(cpp));
>> +
>> +      intel_miptree_get_tile_masks(tiling, tr_mode, cpp,
>> +                                   &mask_x, &mask_y, false);
>> +      *tile_w = (mask_x + 1) * cpp;
>> +      *tile_h = (mask_y + 1) * cpp;
>> +   }
>> +}
>> +
>> +
>
> I think logically it makes a lot more sense if the masks are calculated as a
> function of the dimension (reverse of what you did). That should have the effect
> of putting all the logic in get dimensions, and then get_tile_masks is a really
> simple helper (I think). What do you think?
>
Right. Why it didn't click me. I'll send out V2 of the series.

>>  /**
>>   * This function computes masks that may be used to select the bits of the X
>>   * and Y coordinates that indicate the offset within a tile.  If the BO is
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> index fc68146..54cce1b 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> @@ -622,6 +622,11 @@ intel_miptree_get_dimensions_for_image(struct gl_texture_image *image,
>>                                         int *width, int *height, int *depth);
>>
>>  void
>> +intel_miptree_get_tile_dimensions(uint32_t tiling, uint32_t tr_mode,
>> +                                  uint32_t cpp, uint32_t *tile_w,
>> +                                  uint32_t *tile_h);
>> +
>> +void
>>  intel_miptree_get_tile_masks(uint32_t tiling, uint32_t tr_mode, uint32_t cpp,
>>                               uint32_t *mask_x, uint32_t *mask_y,
>>                               bool map_stencil_as_y_tiled);
>> --
>> 2.4.3
>>