Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On Tue, Aug 22, 2017 at 2:07 AM, Andres Gomez wrote: > On Thu, 2017-08-10 at 12:51 -0700, Jason Ekstrand wrote: > > On August 10, 2017 12:45:43 PM Ilia Mirkin wrote: > > > > > On Wed, Aug 9, 2017 at 4:09 PM, Kenneth Graunke > wrote: > > > > Also, silence an obnoxious finishme that started occurring for all > > > > GL applications which use stencil after the i965 ISL conversion. > > > > > > Without commenting on the correctness of the patch, either this or > > > something like it should really end up in 17.2. > > > > > > Cheers, > > > > Agreed > > Would we want it also in 17.1 ? > I wouldn't bother ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On Thu, 2017-08-10 at 12:51 -0700, Jason Ekstrand wrote: > On August 10, 2017 12:45:43 PM Ilia Mirkin wrote: > > > On Wed, Aug 9, 2017 at 4:09 PM, Kenneth Graunke > > wrote: > > > Also, silence an obnoxious finishme that started occurring for all > > > GL applications which use stencil after the i965 ISL conversion. > > > > Without commenting on the correctness of the patch, either this or > > something like it should really end up in 17.2. > > > > Cheers, > > Agreed Would we want it also in 17.1 ? -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On Thu, Aug 10, 2017 at 07:01:00AM -0700, Jason Ekstrand wrote: > > > On August 10, 2017 2:42:29 AM Kenneth Graunke 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 > > > 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 Thanks Ken, and sorry for leaving it behind. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On August 10, 2017 12:45:43 PM Ilia Mirkin wrote: On Wed, Aug 9, 2017 at 4:09 PM, Kenneth Graunke wrote: Also, silence an obnoxious finishme that started occurring for all GL applications which use stencil after the i965 ISL conversion. Without commenting on the correctness of the patch, either this or something like it should really end up in 17.2. Cheers, Agreed ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On Wed, Aug 9, 2017 at 4:09 PM, Kenneth Graunke wrote: > Also, silence an obnoxious finishme that started occurring for all > GL applications which use stencil after the i965 ISL conversion. Without commenting on the correctness of the patch, either this or something like it should really end up in 17.2. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On August 10, 2017 2:42:29 AM Kenneth Graunke 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 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 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On Wednesday, August 9, 2017 1:20:53 PM PDT Jason Ekstrand wrote: > On Wed, Aug 9, 2017 at 1:09 PM, Kenneth Graunke > 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. > > > + return false; > > > > done: > > *out_row_pitch = row_pitch; > > -- > > 2.14.0 > > > > > signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
On Wed, Aug 9, 2017 at 1:09 PM, Kenneth Graunke 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. > + return false; > > done: > *out_row_pitch = row_pitch; > -- > 2.14.0 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.
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))) + return false; done: *out_row_pitch = row_pitch; -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev