[Mesa-dev] [PATCH V2 1/8] i965: Add a helper function intel_get_tile_dims()

[Anuj Phogat]

On Thu, Sep 10, 2015 at 2:38 PM, Ville Syrjälä <
ville.syrjala at linux.intel.com> wrote:

> On Thu, Sep 10, 2015 at 12:20:10PM -0700, Chad Versace wrote:
> > On Wed 19 Aug 2015, Anuj Phogat wrote:
> > > V2:
> > > - Do the tile width/height computations in the new helper
> > >   function and use it later in intel_miptree_get_tile_masks().
> > > - Change the name to intel_get_tile_dims().
> > >
> > > 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 | 81
> +++++++++++++++++++--------
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  4 ++
> > >  2 files changed, 63 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 e85c3f0..c282e94 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_get_tile_dims(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,63 @@ 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.
> > > + */
> >
> > Is the tile_w parameter in units of bytes or pixels? That should be
> > documented at the top of the function.
> >
> > Also, just to be clear, "tile height" is always unitless. The hw docs
> > sometime express it in units of "rows". But "rows" itself is unitless.
> >
> > > +void
> > > +intel_get_tile_dims(uint32_t tiling, uint32_t tr_mode, uint32_t cpp,
> > > +                    uint32_t *tile_w, uint32_t *tile_h)
> > > +{
> > > +   if (tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
> > > +      switch (tiling) {
> > > +      case I915_TILING_X:
> > > +         *tile_w = 512;
> > > +         *tile_h = 8 * cpp;
> >
> > For legacy tiling formats, the height of a tile is independent of the
> > pixel size,  because the height is unitless. For Tile X, it's always
> > 2^3. For Tile Y Legacy, it's always 2^5.
> >
> > If tile_w is in units of bytes, then it's also independent of pixel
> > size. If tile_w is in units of pixels, though, then
> >
> >     tile_w_pixels = tile_w_bytes / 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 aspect_ratio = 1;
> > > +      assert(_mesa_is_pow_two(cpp));
> > > +
> > > +      switch (cpp) {
> > > +      case 1:
> > > +         *tile_h = 64 * cpp;
> >
> > I'm still reading the docs for the non-legay tiling formats Yf, and Ys.
> > So I can't comment on this part of the patch.
>
> As it turns out I was just looking at Yf and whatnot from display POV,
> and I came to the conclusion that I'll change the kernel to just have a
> function to return the tile width in bytes based on the cpp passed in,
> and then I can simply compute tile height as 'tile_size / tile_width',
> or tile size in pixels (if needed) as 'tile_width / cpp'
>
> And what I understood about Yf (the docs are no good IME, at least the
> part I was looking at) is the following:
>
I agree that docs don't explain the new layouts very well. I also
came up with the same table. Using your suggestion I will use
'tile_size / tile_width' to compute tile_height instead of using aspect
ratio.


>
> cpp w_bytes w_pixels h  aspect
> 1   64      64       64 1
> 2   128     64       32 2
> 4   128     32       32 1
> 8   256     32       16 2
> 16  256     16       16 1
>
> So all you really need to know is cpp and w_bytes and the rest can all
> be computed as needed.
>
I'll send out V3 with fewer function parameters. Thanks for the suggestions.

>
> --
> Ville Syrjälä
> Intel OTC
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150915/a39df2d0/attachment-0001.html>