[Mesa-dev] [PATCH] anv: ensure that we do not emit negative Depth in 3DSTATE_DEPTH_BUFFER

[Jason Ekstrand]

On Feb 23, 2017 3:23 AM, "Iago Toral" <itoral at igalia.com> wrote:

On Thu, 2017-02-23 at 12:10 +0100, Iago Toral wrote:
> On Wed, 2017-02-22 at 09:38 -0800, Jason Ekstrand wrote:
> >
> > On Wed, Feb 22, 2017 at 6:45 AM, Iago Toral Quiroga <itoral at igalia.
> > co
> > m> wrote:
> > >
> > > This fixes a number of new CTS tests that would crash otherwise:
> > > dEQP-VK.pipeline.render_to_image.*
> > > ---
> > >  src/intel/vulkan/genX_cmd_buffer.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index 40a72f4..cdd4501 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -2270,7 +2270,8 @@ cmd_buffer_emit_depth_stencil(struct
> > > anv_cmd_buffer *cmd_buffer)
> > >           assert(image->depth_surface.isl.dim !=
> > > ISL_SURF_DIM_3D);
> > >           db.Depth =
> > >           db.RenderTargetViewExtent =
> > > -            iview->isl.array_len - iview->isl.base_array_layer -
> > > 1;
> > > +            iview->isl.array_len <= iview->isl.base_array_layer
> > > +               ? 0 : iview->isl.array_len - iview-
> > > >
> > > > isl.base_array_layer - 1;
> > I think both the old code and the new code is broken. :(  I believe
> > what we actually want here is just array_len; we don't want to
> > subtract base_array_layer.
> >
> > Does using just iview->isl.array_len fix the issue?
> Yes it does and there are no regressions according to jenkins. I'll
> send a v2 with that change.

Actually, reviewing the PRMS I think my patch is actually correct:

"This field specifies the total number of levels for a volume texture
or the number of array elements allowed to be accessed starting at the
Minimum Array Element for arrayed surfaces."

Here, MinimumArrayElement is iview->isl.base_array_layer, so my
understanding is that we need to subtract this, right?


No. The issue is in the way ISL defines array_len (which isn't well
documented).  It defines a range [base_array_layer, base_array_layer +
array_len].  In other words, array_len is already relative to
base_array_layer like the hardware wants it to be.


Iago

> Iago
>
> >
> > >
> > >
> > >  #if GEN_GEN >= 8
> > >           db.SurfaceQPitch =
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > 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/20170223/21802456/attachment-0001.html>