[Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.

[Jason Ekstrand]

On August 10, 2017 2:42:29 AM Kenneth Graunke <kenneth at whitecape.org> wrote:

> On Wednesday, August 9, 2017 1:20:53 PM PDT Jason Ekstrand wrote:
>> On Wed, Aug 9, 2017 at 1:09 PM, Kenneth Graunke <kenneth at whitecape.org>
>> wrote:
>>
>> > Also, silence an obnoxious finishme that started occurring for all
>> > GL applications which use stencil after the i965 ISL conversion.
>> > ---
>> >  src/intel/isl/isl.c | 6 ++++--
>> >  1 file changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
>> > index 6b4203d79d2..c35116214c8 100644
>> > --- a/src/intel/isl/isl.c
>> > +++ b/src/intel/isl/isl.c
>> > @@ -1367,8 +1367,10 @@ isl_calc_row_pitch(const struct isl_device *dev,
>> >         !pitch_in_range(row_pitch, _3DSTATE_HIER_DEPTH_BUFFER_
>> > SurfacePitch_bits(dev->info)))
>> >        return false;
>> >
>> > -   if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
>> > -      isl_finishme("validate row pitch of stencil surfaces");
>> > +   if (dev->use_separate_stencil &&
>> > +       (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT) &&
>> > +       !pitch_in_range(row_pitch, _3DSTATE_STENCIL_BUFFER_
>> > SurfacePitch_bits(dev->info)))
>> >
>>
>> Topi sent the same patch.  This doesn't work on gen4.
>
> Did you see that I have dev->use_separate_stencil in the condition?
>
> That essentially restricts this check to Gen6+, so Gen4-5 won't do
> any checking at all.  Which is exactly how much checking it's doing
> today. :)
>
> Perhaps you'd instead prefer:
>
>    uint32_t stencil_pitch_bits =
>       dev->use_separate_stencil ?
>       _3DSTATE_STENCIL_BUFFER_SurfacePitch_bits(dev->info) :
>       _3DSTATE_DEPTH_BUFFER_SurfacePitch_bits(dev->info);
>
>    if ((surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT) &&
>        !pitch_in_range(row_pitch, stencil_pitch_bits))
>       return false;
>
> so that we use the 3DSTATE_DEPTH_BUFFER pitch limits on Gen4 or whenever
> we're doing combined depth/stencil, and 3DSTATE_STENCIL_BUFFER's limits
> when doing separate stencil.

Works for me

>>
>> > +      return false;
>> >
>> >   done:
>> >     *out_row_pitch = row_pitch;
>> > --
>> > 2.14.0
>> >
>> >
>>
>