On Tue, Feb 21, 2017 at 3:44 PM, Chad Versace <chadvers...@chromium.org>
wrote:

> On Mon 20 Feb 2017, Jason Ekstrand wrote:
> > On Mon, Feb 20, 2017 at 10:33 AM, Lionel Landwerlin <
> > lionel.g.landwer...@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.
>

It modifies the out isl_surf regardless of whether the function succeeds or
not so you a partial fill-out.  Meh.


> Either way looks good to me.
> Reviewed-by: Chad Versace <chadvers...@chromium.org>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to