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