[Mesa-dev] [PATCH 10/14] isl: Add support for HiZ surfaces
Nanley Chery
nanleychery at gmail.com
Fri Jul 15 17:29:03 UTC 2016
On Thu, Jul 14, 2016 at 04:18:33PM -0700, Jason Ekstrand wrote:
> On Thu, Jul 14, 2016 at 4:13 PM, Nanley Chery <nanleychery at gmail.com> wrote:
>
> > On Thu, Jul 14, 2016 at 03:57:09PM -0700, Jason Ekstrand wrote:
> > > On Thu, Jul 14, 2016 at 3:45 PM, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> > >
> > > > On Sat, Jul 09, 2016 at 12:17:28PM -0700, Jason Ekstrand wrote:
> > > > > ---
> > > > > src/intel/isl/isl.c | 11 +++++++++++
> > > > > src/intel/isl/isl.h | 17 +++++++++++++++++
> > > > > src/intel/isl/isl_format_layout.csv | 1 +
> > > > > src/intel/isl/isl_gen6.c | 8 ++++++++
> > > > > src/intel/isl/isl_gen7.c | 10 +++++++++-
> > > > > src/intel/isl/isl_gen8.c | 3 ++-
> > > > > 6 files changed, 48 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > > > > index 8c114a2..9ccdea2 100644
> > > > > --- a/src/intel/isl/isl.c
> > > > > +++ b/src/intel/isl/isl.c
> > > > > @@ -167,6 +167,16 @@ isl_tiling_get_info(const struct isl_device
> > *dev,
> > > > > break;
> > > > > }
> > > > >
> > > > > + case ISL_TILING_HIZ:
> > > > > + /* HiZ buffers are required to have ISL_FORMAT_HIZ which is
> > an 8x4
> > > > > + * 128bpb format. The tiling has the same physical
> > dimensions as
> > > > > + * Y-tiling but actually has two HiZ columns per Y-tiled
> > column.
> > > > > + */
> > > > > + assert(bs == 16);
> > > > > + logical_el = isl_extent2d(16, 16);
> > > > > + phys_B = isl_extent2d(128, 32);
> > > > > + break;
> > > > > +
> > > > > default:
> > > > > unreachable("not reached");
> > > > > } /* end switch */
> > > > > @@ -221,6 +231,7 @@ isl_surf_choose_tiling(const struct isl_device
> > *dev,
> > > > > CHOOSE(ISL_TILING_LINEAR);
> > > > > }
> > > > >
> > > > > + CHOOSE(ISL_TILING_HIZ);
> > > > > CHOOSE(ISL_TILING_Ys);
> > > > > CHOOSE(ISL_TILING_Yf);
> > > > > CHOOSE(ISL_TILING_Y0);
> > > > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > > > > index 85af2d1..9a60bbd 100644
> > > > > --- a/src/intel/isl/isl.h
> > > > > +++ b/src/intel/isl/isl.h
> > > > > @@ -345,6 +345,14 @@ enum isl_format {
> > > > > ISL_FORMAT_ASTC_LDR_2D_12X10_FLT16 = 638,
> > > > > ISL_FORMAT_ASTC_LDR_2D_12X12_FLT16 = 639,
> > > > >
> > > > > + /* The formats that follow are internal to ISL and as such don't
> > > > have an
> > > > > + * explicit number. We'll just let the C compiler assign it for
> > > > us. Any
> > > > > + * actual hardware formats *must* come before these in the list.
> > > > > + */
> > > > > +
> > > > > + /* Formats for color compression surfaces */
> > > > > + ISL_FORMAT_HIZ,
> > > >
> > > > Why is HiZ a color compression surface instead of a depth compression
> > > > surface?
> > > >
> > >
> > > Aux surfaces then? It's not really "color" compression...
> > >
> >
> > That works.
> >
> > >
> > > >
> > > > > +
> > > > > /* Hardware doesn't understand this out-of-band value */
> > > > > ISL_FORMAT_UNSUPPORTED = UINT16_MAX,
> > > > > };
> > > > > @@ -392,6 +400,9 @@ enum isl_txc {
> > > > > ISL_TXC_ETC1,
> > > > > ISL_TXC_ETC2,
> > > > > ISL_TXC_ASTC,
> > > > > +
> > > > > + /* Used for auxiliary surface formats */
> > > > > + ISL_TXC_HIZ,
> > > > > };
> > > > >
> > > > > /**
> > > > > @@ -410,6 +421,7 @@ enum isl_tiling {
> > > > > ISL_TILING_Y0, /**< Legacy Y tiling */
> > > > > ISL_TILING_Yf, /**< Standard 4K tiling. The 'f' means "four". */
> > > > > ISL_TILING_Ys, /**< Standard 64K tiling. The 's' means
> > "sixty-four".
> > > > */
> > > > > + ISL_TILING_HIZ, /**< Tiling format for HiZ surfaces */
> > > > > };
> > > > >
> > > > > /**
> > > > > @@ -423,6 +435,7 @@ typedef uint32_t isl_tiling_flags_t;
> > > > > #define ISL_TILING_Y0_BIT (1u << ISL_TILING_Y0)
> > > > > #define ISL_TILING_Yf_BIT (1u << ISL_TILING_Yf)
> > > > > #define ISL_TILING_Ys_BIT (1u << ISL_TILING_Ys)
> > > > > +#define ISL_TILING_HIZ_BIT (1u << ISL_TILING_HIZ)
> > > > > #define ISL_TILING_ANY_MASK (~0u)
> > > > > #define ISL_TILING_NON_LINEAR_MASK (~ISL_TILING_LINEAR_BIT)
> > > > >
> > > > > @@ -505,6 +518,7 @@ typedef uint64_t isl_surf_usage_flags_t;
> > > > > #define ISL_SURF_USAGE_DISPLAY_FLIP_X_BIT (1u << 10)
> > > > > #define ISL_SURF_USAGE_DISPLAY_FLIP_Y_BIT (1u << 11)
> > > > > #define ISL_SURF_USAGE_STORAGE_BIT (1u << 12)
> > > > > +#define ISL_SURF_USAGE_HIZ_BIT (1u << 13)
> > > > > /** @} */
> > > > >
> > > > > /**
> > > > > @@ -966,6 +980,9 @@ isl_format_has_bc_compression(enum isl_format
> > fmt)
> > > > > case ISL_TXC_ETC2:
> > > > > case ISL_TXC_ASTC:
> > > > > return false;
> > > > > +
> > > > > + case ISL_TXC_HIZ:
> > > > > + unreachable("Should not be called on an aux surface");
> > > > > }
> > > > >
> > > > > unreachable("bad texture compression mode");
> > > > > diff --git a/src/intel/isl/isl_format_layout.csv
> > > > b/src/intel/isl/isl_format_layout.csv
> > > > > index f90fbe0..3e681e8 100644
> > > > > --- a/src/intel/isl/isl_format_layout.csv
> > > > > +++ b/src/intel/isl/isl_format_layout.csv
> > > > > @@ -314,3 +314,4 @@ ASTC_LDR_2D_10X8_FLT16 , 128, 10, 8, 1,
> > sf16,
> > > > sf16, sf16, sf16, ,
> > > > > ASTC_LDR_2D_10X10_FLT16 , 128, 10, 10, 1, sf16, sf16, sf16,
> > sf16,
> > > > , , , linear, astc
> > > > > ASTC_LDR_2D_12X10_FLT16 , 128, 12, 10, 1, sf16, sf16, sf16,
> > sf16,
> > > > , , , linear, astc
> > > > > ASTC_LDR_2D_12X12_FLT16 , 128, 12, 12, 1, sf16, sf16, sf16,
> > sf16,
> > > > , , , linear, astc
> > > > > +HIZ , 128, 8, 4, 1, , , ,
> > ,
> > > > , , , , hiz
> > > > > diff --git a/src/intel/isl/isl_gen6.c b/src/intel/isl/isl_gen6.c
> > > > > index 699aa41..b5050ed 100644
> > > > > --- a/src/intel/isl/isl_gen6.c
> > > > > +++ b/src/intel/isl/isl_gen6.c
> > > > > @@ -89,6 +89,14 @@ gen6_choose_image_alignment_el(const struct
> > > > isl_device *dev,
> > > > > enum isl_msaa_layout msaa_layout,
> > > > > struct isl_extent3d *image_align_el)
> > > > > {
> > > > > + if (info->format == ISL_FORMAT_HIZ) {
> > > > > + /* HiZ surfaces are always aligned to 8x16 pixels in the
> > primary
> > > > surface
> > > > > + * which works out to 2x2 HiZ elments.
> > > > > + */
> > > > > + *image_align_el = isl_extent3d(2, 2, 1);
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > /* Note that the surface's horizontal image alignment is not
> > > > programmable
> > > > > * on Sandybridge.
> > > > > *
> > > > > diff --git a/src/intel/isl/isl_gen7.c b/src/intel/isl/isl_gen7.c
> > > > > index d9b0c08..349875f 100644
> > > > > --- a/src/intel/isl/isl_gen7.c
> > > > > +++ b/src/intel/isl/isl_gen7.c
> > > > > @@ -111,7 +111,8 @@ gen7_choose_msaa_layout(const struct isl_device
> > *dev,
> > > > > * In the table above, MSFMT_MSS refers to
> > ISL_MSAA_LAYOUT_ARRAY, and
> > > > > * MSFMT_DEPTH_STENCIL refers to ISL_MSAA_LAYOUT_INTERLEAVED.
> > > > > */
> > > > > - if (isl_surf_usage_is_depth_or_stencil(info->usage))
> > > > > + if (isl_surf_usage_is_depth_or_stencil(info->usage) ||
> > > > > + (info->usage & ISL_SURF_USAGE_HIZ_BIT))
> > > > > require_interleaved = true;
> > > > >
> > > >
> > > > Higher up in this function, HiZ will be recognized as an compressed
> > > > texture, and the msaa_layout function will bail. The recognition occurs
> > > > because in isl_format_layout.csv, HIZ has the txc field set. I'll send
> > > > a patch that moves the compressed texture check below this hunk.
> > > >
> > >
> > > I've been thinking about this and I think we want make is_compressed
> > return
> > > false for aux surfaces. I'm not sure exactly how we want to do this.
> > One
> > > option would be to add a ISL_TXC_FIRST_AUX enum and check for txc !=
> > > ISL_TXC_NONE && txc < ISL_TXC_FIRST_AUX.
> > >
> >
> > If it's the case that we don't want to consider them as compressed, we can
> > avoid such issues by simply omitting the txc fields for all aux surfaces in
> > isl_format_layout.csv.
> >
>
> I thought about that....
>
> It turns out it's very useful to be able to do things such as
> "isl_format_get_layout(format)->txc == ISL_TXC_CCS" to identify a format as
> being a CCS format. I'm not sure how that usefulness compares against the
> annoyance of having to make a distinction between gl-style compressed
> textures and auxiliary surfaces.
>
While we could check the usage bit for CCS instead of the txc field, the
recurring theme in these solutions seems to be that we want to distinguish
the between gl-style compressed textures and aux compressed textures. I
propose we rename isl_format_is_compressed() to
isl_format_is_compressed_non_aux() to avoid misleading the user, and use your
idea of modifying the internals to resemble something like:
txc != ISL_TXC_NONE && txc < ISL_TXC_FIRST_AUX.
I don't know of any AUX formats that want to be recognized as compressed,
therefore I see no need for an additional isl_format_is_compressed_aux()
function.
- Nanley
>
> >
> > - Nanley
> >
> > > --Jason
> > >
> > >
> > > >
> > > > - Nanley
> > > >
> > > > > /* From the Ivybridge PRM, Volume 4 Part 1 p72, SURFACE_STATE,
> > > > Multisampled
> > > > > @@ -230,6 +231,13 @@ gen7_filter_tiling(const struct isl_device *dev,
> > > > > *flags &= ~ISL_TILING_W_BIT;
> > > > > }
> > > > >
> > > > > + /* The HiZ format and tiling always go together */
> > > > > + if (info->format == ISL_FORMAT_HIZ) {
> > > > > + *flags &= ISL_TILING_HIZ_BIT;
> > > > > + } else {
> > > > > + *flags &= ~ISL_TILING_HIZ_BIT;
> > > > > + }
> > > > > +
> > > > > if (info->usage & (ISL_SURF_USAGE_DISPLAY_ROTATE_90_BIT |
> > > > > ISL_SURF_USAGE_DISPLAY_ROTATE_180_BIT |
> > > > > ISL_SURF_USAGE_DISPLAY_ROTATE_270_BIT)) {
> > > > > diff --git a/src/intel/isl/isl_gen8.c b/src/intel/isl/isl_gen8.c
> > > > > index a46427a..62c331c 100644
> > > > > --- a/src/intel/isl/isl_gen8.c
> > > > > +++ b/src/intel/isl/isl_gen8.c
> > > > > @@ -84,7 +84,8 @@ gen8_choose_msaa_layout(const struct isl_device
> > *dev,
> > > > > if (isl_format_is_yuv(info->format))
> > > > > return false;
> > > > >
> > > > > - if (isl_surf_usage_is_depth_or_stencil(info->usage))
> > > > > + if (isl_surf_usage_is_depth_or_stencil(info->usage) ||
> > > > > + (info->usage & ISL_SURF_USAGE_HIZ_BIT))
> > > > > require_interleaved = true;
> > > > >
> > > > > if (require_array && require_interleaved)
> > > > > --
> > > > > 2.5.0.400.gff86faf
> > > > >
> > > > > _______________________________________________
> > > > > 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