On Wed, Mar 8, 2017 at 4:16 PM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Mon 06 Mar 2017, Jason Ekstrand wrote:
> > On Mon, Mar 6, 2017 at 10:12 AM, Chad Versace <chadvers...@chromium.org>
> > 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. :-/
>
> 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?
>
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?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
