On Wed 08 Mar 2017, Jason Ekstrand wrote:
> 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?
Nope. I believe we're seeing the same thing.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
