On Fri, Mar 24, 2017 at 11:10 AM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Thu 23 Mar 2017, Jason Ekstrand wrote:
> > On Wed, Mar 22, 2017 at 6:04 PM, Chad Versace <chadvers...@chromium.org>
> > wrote:
> >
> > > Validate that isl_surf::row_pitch fits in the below bitfields,
> > > if applicable based on isl_surf::usage.
> > >
> > > RENDER_SURFACE_STATE::SurfacePitch
> > > RENDER_SURFACE_STATE::AuxiliarySurfacePitch
> > > 3DSTATE_DEPTH_BUFFER::SurfacePitch
> > > 3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch
> > >
> > > v2: Add a Makefile dependency on generated header genX_bits.h.
> > > ---
> > > src/intel/Makefile.isl.am | 3 ++
> > > src/intel/isl/isl.c | 72 ++++++++++++++++++++++++++++++
> > > +++++++++++++----
> > > 2 files changed, 69 insertions(+), 6 deletions(-)
>
>
>
>
> > > +
> > > + if (row_pitch == 0)
> > > + return false;
> > > +
> > > + if (dim_layout == ISL_DIM_LAYOUT_GEN9_1D) {
> > > + /* SurfacePitch is ignored for this layout.How should we
> validate
> > > it? */
> > > + isl_finishme("validate row pitch for ISL_DIM_LAYOUT_GEN9_1D");
> > > + goto done;
> > > + }
> > > +
> > > + if (((surf_info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
> > > + (surf_info->usage & ISL_SURF_USAGE_TEXTURE_BIT)) &&
> > >
> >
> > We also want to handle STORAGE_BIT
>
> Done. I added it locally.
>
> > > + !pitch_in_range(row_pitch, RENDER_SURFACE_STATE_
> > > SurfacePitch_bits(gen_10x)))
> > > + return false;
> > > +
> > > + if (((surf_info->usage & ISL_SURF_USAGE_CCS_BIT) ||
> > > + (surf_info->usage & ISL_SURF_USAGE_MCS_BIT)) &&
> > > + !pitch_in_range(row_pitch_tiles, RENDER_SURFACE_STATE_
> > > AuxiliarySurfacePitch_bits(gen_10x)))
> > > + return false;
> > > +
> > > + if ((surf_info->usage & ISL_SURF_USAGE_DEPTH_BIT) &&
> > > + !pitch_in_range(row_pitch, _3DSTATE_DEPTH_BUFFER_
> > > SurfacePitch_bits(gen_10x)))
> > > + return false;
> > > +
> > > + if ((surf_info->usage & ISL_SURF_USAGE_HIZ_BIT) &&
> > > + !pitch_in_range(row_pitch, _3DSTATE_HIER_DEPTH_BUFFER_
> > > SurfacePitch_bits(gen_10x)))
> > > + return false;
> > > +
> > > + if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
> > > + isl_finishme("validate row pitch of stencil surfaces");
> > >
> >
> > How hard is this? I assume it's not trivial or you would have done it.
>
> It's not trivial. There's corner cases, especially for older gens. But
> it's not *too* hard. It's just that I have no confidence that I could
> write it correctly on the first try.
>
> I wanted to land all the obvious pitch validations asap. Where obvious
> means:
>
> if ((usage & bits) && !pitch_in_range())
> return false;
>
> Since stencil pitch validation is the only non-obvious validation,
> I wanted to do it as a follow-up. Because there WILL be errors in v1 of
> the patch. And I didn't want this patch to be blocked while I debugged
> the stencil validation failures and argued over the corner cases.
>
Fine with me.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
