<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 8, 2017 at 4:16 PM, Chad Versace <<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon 06 Mar 2017, Jason Ekstrand wrote: > On Mon, Mar 6, 2017 at 10:12 AM, Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> > wrote: > > > The caller does so by setting the new field > > isl_surf_init_info::row_pitch, which overrides isl's row pitch > > calculation. > > --- > > src/intel/isl/isl.c | 11 ++++++++++- > > src/intel/isl/isl.h | 9 +++++++++ > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > > index a91c3f92bcf..b3ac305b81b 100644 > > --- a/src/intel/isl/isl.c > > +++ b/src/intel/isl/isl.c > > @@ -1005,6 +1005,9 @@ isl_calc_linear_row_pitch(const struct isl_device > > *dev, > > { > > const struct isl_format_layout *fmtl = isl_format_get_layout(info-> > > format); > > > > + /* Any override of row_pitch should happen earlier. */ > > + assert(info->row_pitch == 0); > > + > > uint32_t row_pitch = info->min_row_pitch; > > > > /* First, align the surface to a cache line boundary, as the PRM > > explains > > @@ -1088,6 +1091,9 @@ isl_calc_tiled_row_pitch(const struct isl_device > > *dev, > > { > > const struct isl_format_layout *fmtl = isl_format_get_layout(info-> > > format); > > > > + /* Any override of row_pitch should happen earlier. */ > > + assert(info->row_pitch == 0); > > + > > assert(fmtl->bpb % tile_info->format_bpb == 0); > > assert(phys_slice0_sa->w % fmtl->bw == 0); > > > > @@ -1112,7 +1118,10 @@ isl_calc_row_pitch(const struct isl_device *dev, > > const struct isl_tile_info *tile_info, > > const struct isl_extent2d *phys_slice0_sa) > > { > > - if (tile_info->tiling == ISL_TILING_LINEAR) { > > + if (info->row_pitch != 0) { > > + /* Override the usual calculation and validation. */ > > + return info->row_pitch; > > + } else if (tile_info->tiling == ISL_TILING_LINEAR) { > > return isl_calc_linear_row_pitch(dev, info, phys_slice0_sa); > > } else { > > return isl_calc_tiled_row_pitch(dev, info, tile_info, > > phys_slice0_sa); > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > > index e517f11cf5e..5f73639fb48 100644 > > --- a/src/intel/isl/isl.h > > +++ b/src/intel/isl/isl.h > > @@ -813,6 +813,15 @@ struct isl_surf_init_info { > > /** Lower bound for isl_surf::row_pitch, in bytes. */ > > uint32_t min_row_pitch; > > > > + /** > > + * Exact value for isl_surf::row_pitch. Ignored if zero. > > + * > > + * WARNING: If set, then isl_surf_init() skips many calculations and > > + * validations. If the given row pitch is incompatible with the > > requested > > + * surface, then behavior is undefined. > > > > I'm not a fan. I would much rather supplying a row_pitch mean "please > validate and use this" rather than "trust me". I don't want isl_surf_init > to succeed and still give you a bad surface. Sorry if that means real > work. :-/ </div></div>Ok. Here's my suggestion. I can fix this patch so that isl_surf_init() continues to calculate the row pitch, just as it always has. But treat that calculated row pitch as the minimum required pitch. If the requested row pitch is 0, use the min pitch. If the user actually requests a row pitch, then check that the requested row pitch is aligned correctly and meets the min pitch requirement. Do you see a cleaner way? </blockquote></div> </div><div class="gmail_extra">Row pitch, in general, is defined by a minimum and an alignment. When you want to choose a row pitch, you allign the minimum to the alignment. If you want to validate a row pitch, you check that it's large enough and aligned. Am I missing something? </div></div>