[Mesa-dev] [PATCH 07/20] i965/fs: Import image memory offset calculation code.

Jason Ekstrand jason at jlekstrand.net
Wed Jul 22 11:22:16 PDT 2015


This needs a *lot* more commentary.  These calculations are extremely
tricky and there are almost no comments.  For instance, you are
turning a 2D offset on a tiled surface into a new 2D address into the
raw view of the surface.  Nowhere do you explain what the "raw"
surface looks like and how its width/height map to "real" tiled
version.  Somewhere, you need to write all this down in detail and
explain why your calculations are correct.  If you don't write it down
here, then write it down somewhere else and put a comment here
explaining where to go look.

I tried to dig through it, but I'm having a lot of trouble getting any
further splitting the x/y into major/minor.

Cc'ing chad.  He knows miptree layouts far better than I do.  Maybe
this is all obvious to someone familiar with the calculations.

--Jason

On Tue, Jul 21, 2015 at 9:38 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Define a function to calculate the memory address of the image
> location given by a vector of coordinates.  This is required in cases
> where we need to fall back to untyped surface access, which take a raw
> memory offset and know nothing about surface coordinates, type
> conversion or memory tiling and swizzling.  They are still useful
> because typed surface reads don't support any 64 or 128-bit formats on
> IVB, and they don't support any 128-bit formats on HSW and BDW.
>
> The tiling algorithm is implemented based on a number of parameters
> which are passed in as uniforms and determine whether the surface
> layout is X-tiled, Y-tiled or untiled.  This allows binding surfaces
> of different tiling layouts to the pipeline without recompiling the
> program.
>
> v2: Drop VEC4 suport.
> v3: Rebase.
> ---
>  .../drivers/dri/i965/brw_fs_surface_builder.cpp    | 108 +++++++++++++++++++++
>  1 file changed, 108 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
> index 5ee04de..0c879db 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
> @@ -215,4 +215,112 @@ namespace {
>           return BRW_PREDICATE_NORMAL;
>        }
>     }
> +
> +   namespace image_coordinates {
> +      /**
> +       * Calculate the offset in memory of the texel given by \p coord.
> +       *
> +       * This is meant to be used with untyped surface messages to access a
> +       * tiled surface, what involves taking into account the tiling and
> +       * swizzling modes of the surface manually so it will hopefully not
> +       * happen very often.
> +       */
> +      fs_reg
> +      emit_address_calculation(const fs_builder &bld, const fs_reg &image,
> +                               const fs_reg &coord, unsigned dims)
> +      {
> +         const brw_device_info *devinfo = bld.shader->devinfo;
> +         const fs_reg off = offset(image, bld, BRW_IMAGE_PARAM_OFFSET_OFFSET);
> +         const fs_reg stride = offset(image, bld, BRW_IMAGE_PARAM_STRIDE_OFFSET);
> +         const fs_reg tile = offset(image, bld, BRW_IMAGE_PARAM_TILING_OFFSET);
> +         const fs_reg swz = offset(image, bld, BRW_IMAGE_PARAM_SWIZZLING_OFFSET);
> +         const fs_reg addr = bld.vgrf(BRW_REGISTER_TYPE_UD, 2);
> +         const fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_UD, 2);
> +         const fs_reg minor = bld.vgrf(BRW_REGISTER_TYPE_UD, 2);
> +         const fs_reg major = bld.vgrf(BRW_REGISTER_TYPE_UD, 2);
> +         const fs_reg dst = bld.vgrf(BRW_REGISTER_TYPE_UD);
> +
> +         /* Shift the coordinates by the fixed surface offset. */
> +         for (unsigned c = 0; c < 2; ++c)
> +            bld.ADD(offset(addr, bld, c), offset(off, bld, c),
> +                    (c < dims ?
> +                     offset(retype(coord, BRW_REGISTER_TYPE_UD), bld, c) :
> +                     fs_reg(0)));
> +
> +         if (dims > 2) {
> +            /* Decompose z into a major (tmp.y) and a minor (tmp.x)
> +             * index.
> +             */
> +            bld.BFE(offset(tmp, bld, 0), offset(tile, bld, 2), fs_reg(0),
> +                    offset(retype(coord, BRW_REGISTER_TYPE_UD), bld, 2));
> +            bld.SHR(offset(tmp, bld, 1),
> +                    offset(retype(coord, BRW_REGISTER_TYPE_UD), bld, 2),
> +                    offset(tile, bld, 2));
> +
> +            /* Take into account the horizontal (tmp.x) and vertical (tmp.y)
> +             * slice offset.
> +             */
> +            for (unsigned c = 0; c < 2; ++c) {
> +               bld.MUL(offset(tmp, bld, c),
> +                       offset(stride, bld, 2 + c), offset(tmp, bld, c));
> +               bld.ADD(offset(addr, bld, c),
> +                       offset(addr, bld, c), offset(tmp, bld, c));
> +            }

Tiled surfaces are inherently 2-D, but it seems like you're treating
it as if it has 3-D tiles.  What's going on here?

> +         }
> +
> +         if (dims > 1) {
> +            for (unsigned c = 0; c < 2; ++c) {
> +               /* Calculate the minor x and y indices. */
> +               bld.BFE(offset(minor, bld, c), offset(tile, bld, c),
> +                       fs_reg(0), offset(addr, bld, c));
> +
> +               /* Calculate the major x and y indices. */
> +               bld.SHR(offset(major, bld, c),
> +                       offset(addr, bld, c), offset(tile, bld, c));
> +            }
> +
> +            /* Calculate the texel index from the start of the tile row and
> +             * the vertical coordinate of the row.
> +             * Equivalent to:
> +             *   tmp.x = (major.x << tile.y << tile.x) +
> +             *           (minor.y << tile.x) + minor.x
> +             *   tmp.y = major.y << tile.y
> +             */
> +            bld.SHL(tmp, major, offset(tile, bld, 1));
> +            bld.ADD(tmp, tmp, offset(minor, bld, 1));
> +            bld.SHL(tmp, tmp, offset(tile, bld, 0));
> +            bld.ADD(tmp, tmp, minor);
> +            bld.SHL(offset(tmp, bld, 1),
> +                    offset(major, bld, 1), offset(tile, bld, 1));
> +
> +            /* Add it to the start of the tile row. */
> +            bld.MUL(offset(tmp, bld, 1),
> +                    offset(tmp, bld, 1), offset(stride, bld, 1));
> +            bld.ADD(tmp, tmp, offset(tmp, bld, 1));
> +
> +            /* Multiply by the Bpp value. */
> +            bld.MUL(dst, tmp, stride);

Where in these calculations do you take into account the different
ordering for X-tiled vs. Y-tiled (x-major vs. y-major)?

> +
> +            if (devinfo->gen < 8 && !devinfo->is_baytrail) {

Should this be has_swizzling?  Or is that a composite thing in
brw_context that varies based on runtime checks?

> +               /* Take into account the two dynamically specified shifts. */
> +               for (unsigned c = 0; c < 2; ++c)
> +                  bld.SHR(offset(tmp, bld, c), dst, offset(swz, bld, c));
> +
> +               /* XOR tmp.x and tmp.y with bit 6 of the memory address. */
> +               bld.XOR(tmp, tmp, offset(tmp, bld, 1));
> +               bld.AND(tmp, tmp, fs_reg(1 << 6));
> +               bld.XOR(dst, dst, tmp);
> +            }
> +
> +         } else {

Why do we need to do anything for 1-D surfaces?

> +            /* Multiply by the Bpp/stride value. */
> +            bld.MUL(offset(addr, bld, 1),
> +                    offset(addr, bld, 1), offset(stride, bld, 1));
> +            bld.ADD(addr, addr, offset(addr, bld, 1));
> +            bld.MUL(dst, addr, stride);
> +         }
> +
> +         return dst;
> +      }
> +   }
>  }
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list