[Mesa-dev] [PATCH 6/8] intel/isl: Add support for emitting depth/stencil/hiz
Jason Ekstrand
jason at jlekstrand.net
Wed Apr 5 19:58:35 UTC 2017
On Wed, Apr 5, 2017 at 11:59 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:
> On Wed, Apr 05, 2017 at 09:10:45AM -0700, Jason Ekstrand wrote:
> > On Wed, Apr 5, 2017 at 1:04 AM, Pohjolainen, Topi <
> > topi.pohjolainen at gmail.com> wrote:
> >
> > > On Tue, Apr 04, 2017 at 03:56:35PM -0700, Jason Ekstrand wrote:
> > > > ---
> > > > src/intel/Makefile.sources | 7 ++
> > > > src/intel/isl/isl.c | 93 ++++++++++++++++
> > > > src/intel/isl/isl.h | 82 ++++++++++++++
> > > > src/intel/isl/isl_emit_depth_stencil.c | 189
> > > +++++++++++++++++++++++++++++++++
> > > > src/intel/isl/isl_priv.h | 28 +++++
> > > > 5 files changed, 399 insertions(+)
> > > > create mode 100644 src/intel/isl/isl_emit_depth_stencil.c
> > > >
> > > > diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> > > > index c568916..df8c868 100644
> > > > --- a/src/intel/Makefile.sources
> > > > +++ b/src/intel/Makefile.sources
> > > > @@ -150,32 +150,39 @@ ISL_FILES = \
> > > > ISL_GEN4_FILES = \
> > > > isl/isl_gen4.c \
> > > > isl/isl_gen4.h \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GEN5_FILES = \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GEN6_FILES = \
> > > > isl/isl_gen6.c \
> > > > isl/isl_gen6.h \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GEN7_FILES = \
> > > > isl/isl_gen7.c \
> > > > isl/isl_gen7.h \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GEN75_FILES = \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GEN8_FILES = \
> > > > isl/isl_gen8.c \
> > > > isl/isl_gen8.h \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GEN9_FILES = \
> > > > isl/isl_gen9.c \
> > > > isl/isl_gen9.h \
> > > > + isl/isl_emit_depth_stencil.c \
> > > > isl/isl_surface_state.c
> > > >
> > > > ISL_GENERATED_FILES = \
> > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > > > index 4e89991..f89f351 100644
> > > > --- a/src/intel/isl/isl.c
> > > > +++ b/src/intel/isl/isl.c
> > > > @@ -83,6 +83,32 @@ isl_device_init(struct isl_device *dev,
> > > > */
> > > > dev->ss.aux_addr_offset =
> > > > (RENDER_SURFACE_STATE_AuxiliarySurfaceBaseAddress_start(info)
> &
> > > ~31) / 8;
> > > > +
> > > > + dev->ds.size =
> > > > + _3DSTATE_DEPTH_BUFFER_length(info) * 4 +
> > > > + _3DSTATE_STENCIL_BUFFER_length(info) * 4 +
> > > > + _3DSTATE_HIER_DEPTH_BUFFER_length(info) * 4 +
> > > > + _3DSTATE_CLEAR_PARAMS_length(info) * 4;
> > > > +
> > > > + assert(_3DSTATE_DEPTH_BUFFER_SurfaceBaseAddress_start(info) % 8
> ==
> > > 0);
> > > > + dev->ds.depth_offset =
> > > > + _3DSTATE_DEPTH_BUFFER_SurfaceBaseAddress_start(info) / 8;
> > > > +
> > > > + if (info->has_hiz_and_separate_stencil) {
> > > > + assert(_3DSTATE_STENCIL_BUFFER_SurfaceBaseAddress_start(info)
> %
> > > 8 == 0);
> > > > + dev->ds.stencil_offset =
> > > > + _3DSTATE_DEPTH_BUFFER_length(info) * 4 +
> > > > + _3DSTATE_STENCIL_BUFFER_SurfaceBaseAddress_start(info) /
> 8;
> > > > +
> > > > + assert(_3DSTATE_HIER_DEPTH_BUFFER_SurfaceBaseAddress_
> start(info)
> > > % 8 == 0);
> > > > + dev->ds.hiz_offset =
> > > > + _3DSTATE_DEPTH_BUFFER_length(info) * 4 +
> > > > + _3DSTATE_STENCIL_BUFFER_length(info) * 4 +
> > > > + _3DSTATE_HIER_DEPTH_BUFFER_SurfaceBaseAddress_start(info)
> / 8;
> > > > + } else {
> > > > + dev->ds.stencil_offset = 0;
> > > > + dev->ds.hiz_offset = 0;
> > > > + }
> > > > }
> > > >
> > > > /**
> > > > @@ -1684,6 +1710,73 @@ isl_buffer_fill_state_s(const struct
> isl_device
> > > *dev, void *state,
> > > > }
> > > > }
> > > >
> > > > +void
> > > > +isl_emit_depth_stencil_hiz_s(const struct isl_device *dev, void
> *batch,
> > > > + const struct
> isl_depth_stencil_hiz_emit_info
> > > *restrict info)
> > > > +{
> > > > + if (info->depth_surf && info->stencil_surf) {
> > > > + if (!dev->info->has_hiz_and_separate_stencil) {
> > > > + assert(info->depth_surf == info->stencil_surf);
> > > > + assert(info->depth_address == info->stencil_address);
> > > > + }
> > > > + assert(info->depth_surf->dim == info->stencil_surf->dim);
> > > > + }
> > > > +
> > > > + if (info->depth_surf) {
> > > > + assert((info->depth_surf->usage & ISL_SURF_USAGE_DEPTH_BIT));
> > > > + if (info->depth_surf->dim == ISL_SURF_DIM_3D) {
> > > > + assert(info->view->base_array_layer +
> info->view->array_len <=
> > > > + info->depth_surf->logical_level0_px.depth);
> > > > + } else {
> > > > + assert(info->view->base_array_layer +
> info->view->array_len <=
> > > > + info->depth_surf->logical_level0_px.array_len);
> > > > + }
> > > > + }
> > > > +
> > > > + if (info->stencil_surf) {
> > > > + assert((info->stencil_surf->usage &
> ISL_SURF_USAGE_STENCIL_BIT));
> > > > + if (info->stencil_surf->dim == ISL_SURF_DIM_3D) {
> > > > + assert(info->view->base_array_layer +
> info->view->array_len <=
> > > > + info->stencil_surf->logical_level0_px.depth);
> > > > + } else {
> > > > + assert(info->view->base_array_layer +
> info->view->array_len <=
> > > > + info->stencil_surf->logical_level0_px.array_len);
> > > > + }
> > > > + }
> > > > +
> > > > + switch (ISL_DEV_GEN(dev)) {
> > > > + case 4:
> > > > + if (ISL_DEV_IS_G4X(dev)) {
> > > > + /* G45 surface state is the same as gen5 */
> > > > + isl_gen5_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + } else {
> > > > + isl_gen4_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + }
> > > > + break;
> > > > + case 5:
> > > > + isl_gen5_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + break;
> > > > + case 6:
> > > > + isl_gen6_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + break;
> > > > + case 7:
> > > > + if (ISL_DEV_IS_HASWELL(dev)) {
> > > > + isl_gen75_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + } else {
> > > > + isl_gen7_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + }
> > > > + break;
> > > > + case 8:
> > > > + isl_gen8_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + break;
> > > > + case 9:
> > > > + isl_gen9_emit_depth_stencil_hiz_s(dev, batch, info);
> > > > + break;
> > > > + default:
> > > > + assert(!"Cannot fill surface state for this gen");
> > > > + }
> > > > +}
> > > > +
> > > > /**
> > > > * A variant of isl_surf_get_image_offset_sa() specific to
> > > > * ISL_DIM_LAYOUT_GEN4_2D.
> > > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > > > index 17b52cf..73ef2b6 100644
> > > > --- a/src/intel/isl/isl.h
> > > > +++ b/src/intel/isl/isl.h
> > > > @@ -682,6 +682,17 @@ struct isl_device {
> > > > uint8_t addr_offset;
> > > > uint8_t aux_addr_offset;
> > > > } ss;
> > > > +
> > > > + /**
> > > > + * Describes the layout of the depth/stencil/hiz commands as
> emitted
> > > by
> > > > + * isl_emit_depth_stencil_hiz.
> > > > + */
> > > > + struct {
> > > > + uint8_t size;
> > > > + uint8_t depth_offset;
> > > > + uint8_t stencil_offset;
> > > > + uint8_t hiz_offset;
> > > > + } ds;
> > > > };
> > > >
> > > > struct isl_extent2d {
> > > > @@ -1017,6 +1028,69 @@ struct isl_buffer_fill_state_info {
> > > > uint32_t stride;
> > > > };
> > > >
> > > > +struct isl_depth_stencil_hiz_emit_info {
> > > > + /**
> > > > + * The depth surface
> > > > + */
> > > > + const struct isl_surf *depth_surf;
> > > > +
> > > > + /**
> > > > + * The stencil surface
> > > > + *
> > > > + * If separate stencil is not available, this must point to the
> same
> > > > + * isl_surf as depth_surf.
> > > > + */
> > > > + const struct isl_surf *stencil_surf;
> > > > +
> > > > + /**
> > > > + * The view into the depth and stencil surfaces.
> > > > + *
> > > > + * This view applies to both surfaces simultaneously.
> > > > + */
> > > > + const struct isl_view *view;
> > > > +
> > > > + /**
> > > > + * The size of the framebuffer
> > > > + *
> > > > + * This is used as a back-up to provide a width and height if
> both
> > > > + * depth_surf and stencil_surf are NULL.
> > > > + */
> > > > + struct isl_extent2d fb_extent;
> > > > +
> > > > + /**
> > > > + * The address of the depth surface in GPU memory
> > > > + */
> > > > + uint64_t depth_address;
> > > > +
> > > > + /**
> > > > + * The address of the stencil surface in GPU memory
> > > > + *
> > > > + * If separate stencil is not available, this must have the same
> > > value as
> > > > + * depth_address.
> > > > + */
> > > > + uint64_t stencil_address;
> > > > +
> > > > + /**
> > > > + * The Memory Object Control state for depth and stencil buffers
> > > > + *
> > > > + * Both depth and stencil will get the same MOCS value. The
> exact
> > > format
> > > > + * of this value depends on hardware generation.
> > > > + */
> > > > + uint32_t mocs;
> > > > +
> > > > + /**
> > > > + * The HiZ surfae or NULL if HiZ is disabled.
> > >
> > > surface
> > >
> >
> > Yup. Fixed locally.
> >
> >
> > > > + */
> > > > + const struct isl_surf *hiz_surf;
> > > > + enum isl_aux_usage hiz_usage;
> > > > + uint64_t hiz_address;
> > > > +
> > > > + /**
> > > > + * The depth clear value
> > > > + */
> > > > + float depth_clear_value;
> > > > +};
> > > > +
> > > > extern const struct isl_format_layout isl_format_layouts[];
> > > >
> > > > void
> > > > @@ -1315,6 +1389,14 @@ void
> > > > isl_buffer_fill_state_s(const struct isl_device *dev, void *state,
> > > > const struct isl_buffer_fill_state_info
> > > *restrict info);
> > > >
> > > > +#define isl_emit_depth_stencil_hiz(dev, batch, ...) \
> > > > + isl_emit_depth_stencil_hiz_s((dev), (batch), \
> > > > + &(struct isl_depth_stencil_hiz_emit_
> info)
> > > { __VA_ARGS__ })
> > > > +
> > > > +void
> > > > +isl_emit_depth_stencil_hiz_s(const struct isl_device *dev, void
> *batch,
> > > > + const struct
> isl_depth_stencil_hiz_emit_info
> > > *restrict info);
> > > > +
> > > > void
> > > > isl_surf_fill_image_param(const struct isl_device *dev,
> > > > struct brw_image_param *param,
> > > > diff --git a/src/intel/isl/isl_emit_depth_stencil.c
> > > b/src/intel/isl/isl_emit_depth_stencil.c
> > > > new file mode 100644
> > > > index 0000000..d641b5b
> > > > --- /dev/null
> > > > +++ b/src/intel/isl/isl_emit_depth_stencil.c
> > > > @@ -0,0 +1,189 @@
> > > > +/*
> > > > + * Copyright 2016 Intel Corporation
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > > obtaining a
> > > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > > + * to deal in the Software without restriction, including without
> > > limitation
> > > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > > + * and/or sell copies of the Software, and to permit persons to
> whom
> > > the
> > > > + * Software is furnished to do so, subject to the following
> conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice (including
> > > the next
> > > > + * paragraph) shall be included in all copies or substantial
> portions
> > > of the
> > > > + * Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT
> > > SHALL
> > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES
> > > OR OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > ARISING
> > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER
> > > DEALINGS
> > > > + * IN THE SOFTWARE.
> > > > + */
> > > > +
> > > > +#include <stdint.h>
> > > > +
> > > > +#define __gen_address_type uint64_t
> > > > +#define __gen_user_data void
> > > > +
> > > > +static inline uint64_t
> > > > +__gen_combine_address(void *data, void *loc, uint64_t addr, uint32_t
> > > delta)
> > > > +{
> > > > + return addr + delta;
> > > > +}
> > > > +
> > > > +#include "genxml/gen_macros.h"
> > > > +#include "genxml/genX_pack.h"
> > > > +
> > > > +#include "isl_priv.h"
> > > > +
> > > > +#define __PASTE2(x, y) x ## y
> > > > +#define __PASTE(x, y) __PASTE2(x, y)
> > > > +#define isl_genX(x) __PASTE(isl_, genX(x))
> > > > +
> > > > +static const uint32_t isl_to_gen_ds_surftype [] = {
> > >
> > > Intentional space before []?
> > >
> >
> > Thanks. Fixed locally.
> >
> >
> > > > +#if GEN_GEN >= 9
> > > > + /* From the SKL PRM, "3DSTATE_DEPTH_STENCIL::SurfaceType":
> > > > + *
> > > > + * "If depth/stencil is enabled with 1D render target,
> > > depth/stencil
> > > > + * surface type needs to be set to 2D surface type and height
> set
> > > to 1.
> > > > + * Depth will use (legacy) TileY and stencil will use TileW.
> For
> > > this
> > > > + * case only, the Surface Type of the depth buffer can be 2D
> > > while the
> > > > + * Surface Type of the render target(s) are 1D, representing
> an
> > > > + * exception to a programming note above.
> > > > + */
> > > > + [ISL_SURF_DIM_1D] = SURFTYPE_2D,
> > > > +#else
> > > > + [ISL_SURF_DIM_1D] = SURFTYPE_1D,
> > > > +#endif
> > > > + [ISL_SURF_DIM_2D] = SURFTYPE_2D,
> > > > + [ISL_SURF_DIM_3D] = SURFTYPE_3D,
> > > > +};
> > > > +
> > > > +void
> > > > +isl_genX(emit_depth_stencil_hiz_s)(const struct isl_device *dev,
> void
> > > *batch,
> > > > + const struct
> > > isl_depth_stencil_hiz_emit_info *restrict info)
> > > > +{
> > > > + struct GENX(3DSTATE_DEPTH_BUFFER) db = {
> > > > + GENX(3DSTATE_DEPTH_BUFFER_header),
> > > > + };
> > > > +
> > > > + if (info->depth_surf) {
> > > > + db.SurfaceType = isl_to_gen_ds_surftype[info->
> depth_surf->dim];
> > > > + db.SurfaceFormat = isl_surf_get_depth_format(dev,
> > > info->depth_surf);
> > > > + db.Width = info->depth_surf->logical_level0_px.width - 1;
> > > > + db.Height = info->depth_surf->logical_level0_px.height - 1;
> > > > + } else if (info->stencil_surf) {
> > > > + db.SurfaceType = isl_to_gen_ds_surftype[info->
> stencil_surf->dim];
> > > > + db.SurfaceFormat = D32_FLOAT;
> > > > + db.Width = info->stencil_surf->logical_level0_px.width - 1;
> > > > + db.Height = info->stencil_surf->logical_level0_px.height - 1;
> > > > + } else {
> > > > + db.SurfaceType = SURFTYPE_2D;
> > > > + db.SurfaceFormat = D32_FLOAT;
> > > > + db.Width = MAX2(info->fb_extent.width, 1) - 1;
> > > > + db.Height = MAX2(info->fb_extent.height, 1) - 1;
> > > > + }
> > > > +
> > > > + if (info->depth_surf || info->stencil_surf) {
> > > > + /* These are based entirely on the view */
> > > > + db.Depth = db.RenderTargetViewExtent = info->view->array_len
> - 1;
> > > > + db.LOD = info->view->base_level;
> > > > + db.MinimumArrayElement = info->view->base_array_layer;
> > > > + }
> > > > +
> > > > + if (info->depth_surf) {
> > > > +#if GEN_GEN >= 7
> > > > + db.DepthWriteEnable = true;
> > > > +#endif
> > > > + db.SurfaceBaseAddress = info->depth_address;
> > > > +#if GEN_GEN >= 6
> > > > + db.DepthBufferMOCS = info->mocs;
> > > > +#endif
> > > > + db.SurfacePitch = info->depth_surf->row_pitch - 1;
> > > > +#if GEN_GEN >= 8
> > > > + db.SurfaceQPitch =
> > > > + isl_surf_get_array_pitch_el_rows(info->depth_surf) >> 2;
> > > > +#endif
> > > > + }
> > > > +
> > > > +#if GEN_GEN >= 6
> > > > + struct GENX(3DSTATE_STENCIL_BUFFER) sb = {
> > > > + GENX(3DSTATE_STENCIL_BUFFER_header),
> > > > + };
> > > > +#else
> > > > +# define sb db
> > > > +#endif
> > > > +
> > > > + if (info->stencil_surf) {
> > > > +#if GEN_GEN >= 7
> > > > + db.StencilWriteEnable = true;
> > > > +#endif
> > > > +#if GEN_GEN >= 8 || GEN_IS_HASWELL
> > > > + sb.StencilBufferEnable = true;
> > > > +#endif
> > > > + sb.SurfaceBaseAddress = info->stencil_address;
> > >
> > > Shouldn't this check for depth not existing on gen < 6? Drop the
> "define
> > > sb db"
> > > above and:
> > >
> > > #if GEN_GEN >= 7
> > > sb.SurfaceBaseAddress = info->stencil_address;
> > > #else
> > > if (!info->depth_surf)
> > > db.SurfaceBaseAddress = info->stencil_address;
> > > #endif
> > >
> > > Same for SurfacePitch further down?
> > >
> > > Or are depth_surf and stencil_surf mutually exclusive on gen < 6? if
> so,
> > > perhaps add an assert?
> > >
> >
> > My intention was that if you provide both a depth and a stencil surface
> on
> > gen6, they must point to the same isl_surf and their addresses must be
> the
> > same. There are asserts for this in isl_emit_depth_stencil_hiz_s above
> but
> > those are a long way away from this code. Maybe that was an
> ill-conceived
> > notion? I'm happy to change the semantics to something else if it's more
> > convenient.
>
> I wasn't concerned about gen6, it has 3DSTATE_STENCIL_BUFFER and therefore
> one doesn't try to overwrite depth. My concern was gen4 and gen5. Do they
> actually ever have valid stencil_surf?
I don't know, do they? You're the one writing the gen4 code to use ISL. :-)
> If there is stencil only, it is still
> carried in depth_surf, right?
> If that holds then we should be able to simply guard the entire
> "if (info->stencil_surf)" behind "#if GEN_GEN >= 6"?
>
I don't really have an opinion on how we handle it and I think it needs to
be guided by the work you're doing more than anything else. If you'd like
to just use depth_surf even when it's stencil-only, I'm totally fine with
that. Whatever makes your life the easiest.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170405/7a476edc/attachment-0001.html>
More information about the mesa-dev
mailing list