<div dir="auto"><div><div class="gmail_extra"><div class="gmail_quote">On Feb 23, 2017 3:23 AM, "Iago Toral" <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">On Thu, 2017-02-23 at 12:10 +0100, Iago Toral wrote:<br>
> On Wed, 2017-02-22 at 09:38 -0800, Jason Ekstrand wrote:<br>
> ><br>
> > On Wed, Feb 22, 2017 at 6:45 AM, Iago Toral Quiroga <itoral@igalia.<br>
> > co<br>
> > m> wrote:<br>
> > ><br>
> > > This fixes a number of new CTS tests that would crash otherwise:<br>
> > > dEQP-VK.pipeline.render_to_<wbr>image.*<br>
> > > ---<br>
> > >  src/intel/vulkan/genX_cmd_<wbr>buffer.c | 3 ++-<br>
> > >  1 file changed, 2 insertions(+), 1 deletion(-)<br>
> > ><br>
> > > diff --git a/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> > > b/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> > > index 40a72f4..cdd4501 100644<br>
> > > --- a/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> > > +++ b/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> > > @@ -2270,7 +2270,8 @@ cmd_buffer_emit_depth_stencil(<wbr>struct<br>
> > > anv_cmd_buffer *cmd_buffer)<br>
> > >           assert(image->depth_surface.<wbr>isl.dim !=<br>
> > > ISL_SURF_DIM_3D);<br>
> > >           db.Depth =<br>
> > >           db.RenderTargetViewExtent =<br>
> > > -            iview->isl.array_len - iview->isl.base_array_layer -<br>
> > > 1;<br>
> > > +            iview->isl.array_len <= iview->isl.base_array_layer<br>
> > > +               ? 0 : iview->isl.array_len - iview-<br>
> > > ><br>
> > > > isl.base_array_layer - 1;<br>
> > I think both the old code and the new code is broken. :(  I believe<br>
> > what we actually want here is just array_len; we don't want to<br>
> > subtract base_array_layer.<br>
> ><br>
> > Does using just iview->isl.array_len fix the issue?<br>
> Yes it does and there are no regressions according to jenkins. I'll<br>
> send a v2 with that change.<br>
<br>
</div>Actually, reviewing the PRMS I think my patch is actually correct:<br>
<br>
"This field specifies the total number of levels for a volume texture<br>
or the number of array elements allowed to be accessed starting at the<br>
Minimum Array Element for arrayed surfaces."<br>
<br>
Here, MinimumArrayElement is iview->isl.base_array_<wbr>layer, so my<br>
understanding is that we need to subtract this, right?</blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><font color="#888888"><br>
Iago<br>
</font><div class="elided-text"><br>
> Iago<br>
><br>
> ><br>
> > ><br>
> > ><br>
> > >  #if GEN_GEN >= 8<br>
> > >           db.SurfaceQPitch =<br>
> > > --<br>
> > > 2.7.4<br>
> > ><br>
> > > ______________________________<wbr>_________________<br>
> > > mesa-dev mailing list<br>
> > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</div></blockquote></div><br></div></div></div>