[Mesa-dev] [PATCH v2 23/36] isl: Remove duplicate px->sa conversions

[Nanley Chery]

On Tue, Jul 19, 2016 at 04:16:23PM -0700, Jason Ekstrand wrote:
> On Tue, Jul 19, 2016 at 4:12 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> 
> > On Fri, Jul 01, 2016 at 04:08:49PM -0700, Jason Ekstrand wrote:
> > > In all three cases, we start with width and height taken from
> > > isl_surf::phys_slice0_extent_sa which is already in samples.  There is no
> > > need to do the conversion and doing so gives us an incorrect value.
> >
> > Thanks for noticing this bug! I think this patch is missing one
> > necessary change. The level width and height must be adjusted
> > for the sample count.
> >
> > Here's an example that demonstrates the post-patch issue:
> >  * User creates a 2x1px 4xIMS image
> >  * Level 0 is 4x2sa (2x1px)
> >  * Level 1 is 2x1sa (1x1px) but should be 2x2sa
> >
> 
> I thought this was an issue the first time through too.  But then chad
> reminded me that there is no mipmapped multisampling
> 

Thanks for pointing that out. This patch is,
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>

> --Jason
> 
> 
> >
> > - Nanley
> >
> > > ---
> > >  src/intel/isl/isl.c | 20 --------------------
> > >  1 file changed, 20 deletions(-)
> > >
> > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > > index 404cfc1..be3adfc 100644
> > > --- a/src/intel/isl/isl.c
> > > +++ b/src/intel/isl/isl.c
> > > @@ -610,18 +610,6 @@ isl_calc_phys_slice0_extent_sa_gen4_2d(
> > >        uint32_t W = isl_minify(W0, l);
> > >        uint32_t H = isl_minify(H0, l);
> > >
> > > -      if (msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED) {
> > > -         /* From the Broadwell PRM >> Volume 5: Memory Views >>
> > Computing Mip Level
> > > -          * Sizes (p133):
> > > -          *
> > > -          *    If the surface is multisampled and it is a depth or
> > stencil
> > > -          *    surface or Multisampled Surface StorageFormat in
> > > -          *    SURFACE_STATE is MSFMT_DEPTH_STENCIL, W_L and H_L must be
> > > -          *    adjusted as follows before proceeding: [...]
> > > -          */
> > > -         isl_msaa_interleaved_scale_px_to_sa(info->samples, &W, &H);
> > > -      }
> > > -
> > >        uint32_t w = isl_align_npot(W, image_align_sa->w);
> > >        uint32_t h = isl_align_npot(H, image_align_sa->h);
> > >
> > > @@ -1285,17 +1273,9 @@ get_image_offset_sa_gen4_2d(const struct isl_surf
> > *surf,
> > >     for (uint32_t l = 0; l < level; ++l) {
> > >        if (l == 1) {
> > >           uint32_t W = isl_minify(W0, l);
> > > -
> > > -         if (surf->msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED)
> > > -            isl_msaa_interleaved_scale_px_to_sa(surf->samples, &W,
> > NULL);
> > > -
> > >           x += isl_align_npot(W, image_align_sa.w);
> > >        } else {
> > >           uint32_t H = isl_minify(H0, l);
> > > -
> > > -         if (surf->msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED)
> > > -            isl_msaa_interleaved_scale_px_to_sa(surf->samples, NULL,
> > &H);
> > > -
> > >           y += isl_align_npot(H, image_align_sa.h);
> > >        }
> > >     }
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >