[Mesa-dev] [PATCH 1/2] anv/cmd_buffer: Set the correct surface type for depth/stencil
Jason Ekstrand
jason at jlekstrand.net
Tue Nov 29 22:08:53 UTC 2016
On Tue, Nov 29, 2016 at 1:56 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, Nov 29, 2016 at 3:44 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Tue, Nov 29, 2016 at 12:24 PM, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> >>
> >> On Mon, Nov 28, 2016 at 03:45:41PM -0800, Jason Ekstrand wrote:
> >> > ---
> >> > src/intel/vulkan/genX_cmd_buffer.c | 51
> >> > ++++++++++++++++++++++++++++++++++++--
> >> > 1 file changed, 49 insertions(+), 2 deletions(-)
> >> >
> >>
> >> This patch does not match the one you've merged
> >> (d4ef87c1bb4290293148cbd6cb782764df38f8f4). The committed code
> >> introduces the following buggy hunk:
> >>
> >> @@ -2106,7 +2152,12 @@ cmd_buffer_emit_depth_stencil(struct
> anv_cmd_buffer
> >> *cmd_buffer)
> >> * be combined with a stencil buffer so we use D32_FLOAT instead.
> >> */
> >> anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_DEPTH_BUFFER),
> db)
> >> {
> >> - db.SurfaceType = SURFTYPE_2D;
> >> + if (has_stencil) {
> >> + db.SurfaceType = SURFTYPE_2D;
> >> +
> >> depth_stencil_surface_type(image->stencil_surface.isl.dim);
>
> this line is a no-op.
>
Drp... Thanks! This is why I should have sent a v2 instead of saying,
"That was a trivial change". Patch incoming.
> >> + } else {
> >> + db.SurfaceType = SURFTYPE_2D;
> >> + }
> >> db.SurfaceFormat = D32_FLOAT;
> >> db.Width = fb->width - 1;
> >> db.Height = fb->height - 1;
> >
> >
> > Yes, it introduces that hunk. What's buggy about it? The old code made
> > Jenkins explode. Maybe I should have sent a v2.
> >
> >>
> >>
> >> -Nanley
> >>
> >> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> >> > b/src/intel/vulkan/genX_cmd_buffer.c
> >> > index a965cd6..c953b93 100644
> >> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> >> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> >> > @@ -2033,6 +2033,51 @@ genX(cmd_buffer_emit_gen7_depth_flush)(struct
> >> > anv_cmd_buffer *cmd_buffer)
> >> > }
> >> > }
> >> >
> >> > +static uint32_t
> >> > +depth_stencil_surface_type(enum isl_surf_dim dim)
> >> > +{
> >> > + switch (dim) {
> >> > + case ISL_SURF_DIM_1D:
> >> > + if (GEN_GEN >= 9) {
> >> > + /* From the Sky Lake PRM, 3DSTATAE_DEPTH_BUFFER::
> SurfaceType
> >> > + *
> >> > + * Programming Notes:
> >> > + * The Surface Type of the depth buffer must be the same
> as
> >> > the
> >> > + * Surface Type of the render target(s) (defined in
> >> > + * SURFACE_STATE), unless either the depth buffer or
> render
> >> > + * targets are SURFTYPE_NULL (see exception below for
> SKL).
> >> > 1D
> >> > + * surface type not allowed for depth surface and stencil
> >> > surface.
> >> > + *
> >> > + * Workaround:
> >> > + * 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.
> >> > + */
> >> > + return SURFTYPE_2D;
> >> > + } else {
> >> > + return SURFTYPE_1D;
> >> > + }
> >> > + case ISL_SURF_DIM_2D:
> >> > + return SURFTYPE_2D;
> >> > + case ISL_SURF_DIM_3D:
> >> > + if (GEN_GEN >= 9) {
> >> > + /* The Sky Lake docs list the value for 3D as "Reserved".
> >> > However,
> >> > + * they have the exact same layout as 2D arrays on gen9+, so
> >> > we can
> >> > + * just use 2D here.
> >> > + */
> >> > + return SURFTYPE_2D;
> >> > + } else {
> >> > + return SURFTYPE_3D;
> >> > + }
> >> > + default:
> >> > + unreachable("Invalid surface dimension");
> >> > + }
> >> > +}
> >> > +
> >> > static void
> >> > cmd_buffer_emit_depth_stencil(struct anv_cmd_buffer *cmd_buffer)
> >> > {
> >> > @@ -2054,7 +2099,8 @@ cmd_buffer_emit_depth_stencil(struct
> >> > anv_cmd_buffer *cmd_buffer)
> >> > /* Emit 3DSTATE_DEPTH_BUFFER */
> >> > if (has_depth) {
> >> > anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_DEPTH_BUFFER),
> >> > db) {
> >> > - db.SurfaceType = SURFTYPE_2D;
> >> > + db.SurfaceType =
> >> > + depth_stencil_surface_type(
> image->depth_surface.isl.dim);
> >> > db.DepthWriteEnable = true;
> >> > db.StencilWriteEnable = has_stencil;
> >> >
> >> > @@ -2106,7 +2152,8 @@ cmd_buffer_emit_depth_stencil(struct
> >> > anv_cmd_buffer *cmd_buffer)
> >> > * be combined with a stencil buffer so we use D32_FLOAT
> instead.
> >> > */
> >> > anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_DEPTH_BUFFER),
> >> > db) {
> >> > - db.SurfaceType = SURFTYPE_2D;
> >> > + db.SurfaceType =
> >> > + depth_stencil_surface_type(
> image->depth_surface.isl.dim);
> >> > db.SurfaceFormat = D32_FLOAT;
> >> > db.Width = fb->width - 1;
> >> > db.Height = fb->height - 1;
> >> > --
> >> > 2.5.0.400.gff86faf
> >> >
> >> > _______________________________________________
> >> > mesa-dev mailing list
> >> > mesa-dev at lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161129/ed1a0a77/attachment.html>
More information about the mesa-dev
mailing list