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

2017-08-22 Thread Jason Ekstrand
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.

2017-08-22 Thread Andres Gomez
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.

2017-08-14 Thread Pohjolainen, Topi
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.

2017-08-10 Thread Jason Ekstrand

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.

2017-08-10 Thread Ilia Mirkin
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.

2017-08-10 Thread Jason Ekstrand



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.

2017-08-10 Thread Kenneth Graunke
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.

2017-08-09 Thread Jason Ekstrand
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.

2017-08-09 Thread Kenneth Graunke
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