[Mesa-dev] [PATCH 2/3] isl: add MCS width constraint 16 samples

[Chad Versace]

On Mon 20 Feb 2017, Jason Ekstrand wrote:
> On Mon, Feb 20, 2017 at 10:33 AM, Lionel Landwerlin <
> lionel.g.landwerlin at intel.com> wrote:
> 
> >> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> >> index 1a47da5..6979063 100644
> >> --- a/src/intel/isl/isl.c
> >> +++ b/src/intel/isl/isl.c
> >> @@ -1417,6 +1417,16 @@ isl_surf_get_mcs_surf(const struct isl_device *dev,
> >>      assert(surf->levels == 1);
> >>      assert(surf->logical_level0_px.depth == 1);
> >>   +   /* The "Auxiliary Surface Pitch" field in RENDER_SURFACE_STATE is
> >> only 9
> >> +    * bits which means the maximum pitch of a compression surface is 512
> >> +    * tiles or 64KB (since MCS is always Y-tiled).  Since a 16x MCS
> >> buffer is
> >> +    * 64bpp, this gives us a maximum width of 8192 pixels.  We can create
> >> +    * larger multisampled surfaces, we just can't compress them.   For
> >> 2x, 4x,
> >> +    * and 8x, we have enough room for the full 16k supported by the
> >> hardware.
> >> +    */
> >> +   if (surf->samples == 16 && surf->width > 8192)
> >> +      return false;
> >> +
> >>
> >
> > I was about to write something like this :
> >
> >    struct isl_tile_info tile_info;
> >    isl_surf_get_tile_info(dev, surf, &tile_info);
> >    if ((surf->row_pitch / tile_info.phys_extent_B.width) > 512)
> >       return false;
> >
> 
> That would work too and it is a bit more general.  However, ISL currently
> doesn't touch the isl_surf if creation fails.  I wouldn't mind keeping
> that.  Also, I like that the end result of the restriction is clearly
> spelled out with the old check.  I can't say that I care all that much one
> way or the other so long as both the effect (16x 16k surfaces not working)
> and the reason (pitch) are documented.

I don't understand how Lionel's suggestion would modify the isl_surf on
failure. the isl_surf paramater to isl_surf_get_tile_info() is const.

Either way looks good to me.
Reviewed-by: Chad Versace <chadversary at chromium.org>