On Tue, Mar 28, 2017 at 9:15 AM, Chad Versace <[email protected]> wrote:
> On Sat 25 Mar 2017, Jason Ekstrand wrote: > > > > > > On March 24, 2017 7:29:05 PM Chad Versace <[email protected]> > 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. > > > v3: > > > - Test ISL_SURF_USAGE_STORAGE_BIT too. [for jekstrand] > > > - Drop explicity dependency on generated header. [for emil] > > > v4: > > > - Rebase for new gen_bits_header.py script. > > > - Replace gen_10x with gen_device_info*. > > > --- > > > src/intel/isl/isl.c | 71 ++++++++++++++++++++++++++++++ > ++++++++++++++++++----- > > > 1 file changed, 65 insertions(+), 6 deletions(-) > > > > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > > > index 81f40b6a6fb..4777113de84 100644 > > > > > > + 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"); > > > > We validate qpitch if anything. But the regular pitch isn't just > ignored, > > it's meaningless. I think we can drop the finishme. > > Done. > > > > + if (((surf_info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) || > > > + (surf_info->usage & ISL_SURF_USAGE_TEXTURE_BIT) || > > > + (surf_info->usage & ISL_SURF_USAGE_STORAGE_BIT)) && > > > + !pitch_in_range(row_pitch, > > > RENDER_SURFACE_STATE_SurfacePitch_bits(dev->info))) > > > + return false; > > > > You know... "(thing & foo) || (thing & bar)" is equivalent to "thing & > (foo > > | bar)". Not that you need to change but sometimes its a but nicer. > > I know :). I think my style looks nicer, and the compiler should emit > the same code. But I changed it to use '|' locally so that it matches > the style in other Intel code. > I don't really care all that much. In my brain one is a "does it have this bit or does it have this bit or does it have that bit" vs. "does it have one of these bits". It really doesn't matter... > > > + > > > + 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(dev->info))) > > > + return false; > > > + > > > + if ((surf_info->usage & ISL_SURF_USAGE_DEPTH_BIT) && > > > + !pitch_in_range(row_pitch, > > > _3DSTATE_DEPTH_BUFFER_SurfacePitch_bits(dev->info))) > > > + return false; > > > + > > > + if ((surf_info->usage & ISL_SURF_USAGE_HIZ_BIT) && > > > + !pitch_in_range(row_pitch, > > > _3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits(dev->info))) > > > > On gen8+, if the surface has the TEXTURE bit set we should check it > against > > AuxiliarySurfacePitch as well. > > Good catch. > > I've already test the branch as-is, so I hesitate to add this fixup. It > can come as an immediate follow-up patch. > That's fine.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
