<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 21, 2017 at 3:44 PM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon 20 Feb 2017, Jason Ekstrand wrote:<br>
> On Mon, Feb 20, 2017 at 10:33 AM, Lionel Landwerlin <<br>
> <a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a>> wrote:<br>
><br>
</span><div><div class="h5">> >> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c<br>
> >> index 1a47da5..6979063 100644<br>
> >> --- a/src/intel/isl/isl.c<br>
> >> +++ b/src/intel/isl/isl.c<br>
> >> @@ -1417,6 +1417,16 @@ isl_surf_get_mcs_surf(const struct isl_device *dev,<br>
> >>      assert(surf->levels == 1);<br>
> >>      assert(surf->logical_level0_<wbr>px.depth == 1);<br>
> >>   +   /* The "Auxiliary Surface Pitch" field in RENDER_SURFACE_STATE is<br>
> >> only 9<br>
> >> +    * bits which means the maximum pitch of a compression surface is 512<br>
> >> +    * tiles or 64KB (since MCS is always Y-tiled).  Since a 16x MCS<br>
> >> buffer is<br>
> >> +    * 64bpp, this gives us a maximum width of 8192 pixels.  We can create<br>
> >> +    * larger multisampled surfaces, we just can't compress them.   For<br>
> >> 2x, 4x,<br>
> >> +    * and 8x, we have enough room for the full 16k supported by the<br>
> >> hardware.<br>
> >> +    */<br>
> >> +   if (surf->samples == 16 && surf->width > 8192)<br>
> >> +      return false;<br>
> >> +<br>
> >><br>
> ><br>
> > I was about to write something like this :<br>
> ><br>
> >    struct isl_tile_info tile_info;<br>
> >    isl_surf_get_tile_info(dev, surf, &tile_info);<br>
> >    if ((surf->row_pitch / tile_info.phys_extent_B.width) > 512)<br>
> >       return false;<br>
> ><br>
><br>
> That would work too and it is a bit more general.  However, ISL currently<br>
> doesn't touch the isl_surf if creation fails.  I wouldn't mind keeping<br>
> that.  Also, I like that the end result of the restriction is clearly<br>
> spelled out with the old check.  I can't say that I care all that much one<br>
> way or the other so long as both the effect (16x 16k surfaces not working)<br>
> and the reason (pitch) are documented.<br>
<br>
</div></div>I don't understand how Lionel's suggestion would modify the isl_surf on<br>
failure. the isl_surf paramater to isl_surf_get_tile_info() is const.<br></blockquote><div><br></div><div>It modifies the out isl_surf regardless of whether the function succeeds or not so you a partial fill-out.  Meh.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Either way looks good to me.<br>
Reviewed-by: Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>><br>
</blockquote></div><br></div></div>