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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev