<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 11, 2017 at 11:00 AM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, May 02, 2017 at 02:48:09PM -0700, Jason Ekstrand wrote: > On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> > wrote: > > > Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> > > --- > > src/intel/isl/isl.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > > index f89f351c15..ce5b35c47c 100644 > > --- a/src/intel/isl/isl.c > > +++ b/src/intel/isl/isl.c > > @@ -1610,14 +1610,19 @@ isl_surf_get_ccs_surf(const struct isl_device *dev, > > return false; > > } > > > > + /* Multi-LOD and multi-layer CCS isn't supported on gen7. */ > > + const uint8_t levels = ISL_DEV_GEN(dev) == 7 ? 1 : surf->levels; > > + const uint32_t array_len = ISL_DEV_GEN(dev) == 7 ? > > + 1 : surf->logical_level0_px.array_len; > > + > > > > The GL driver does > > if (brw->gen < 8 && (mip_mapped || arrayed)) > return false; > > Which is a bit stronger condition. I think this is probably ok though so > long as we're careful. > I actually prefer the above condition because it would remove the need for the helper functions. I went with the current implementation to avoid losing more pre-existing performance features than necessary. I don't know how useful it is for us to retain the feature of fast-clearing LOD0 of multi-LOD and multi-layer CCS images on gen7. </blockquote><div> </div><div>I'm not terribly concerned about that case. It's kind-of a nice to support it since the hardware obviously can, but I don't think it's necessary. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > It might be worth adding the following asserts to blorp_fast_clear and > blorp_ccs_resolve: > > assert(level < surf->aux_surf->levels); > if (surf->surf->dim == ISL_SURF_DIM_3D) { > assert(start_layer < surf->aux_surf->logical_level0_px.depth); > assert(start_layer + num_layers < > surf->aux_surf->logical_level0_px.depth); > } else { > assert(start_layer < surf->aux_surf->logical_level0_px.array_len); > assert(start_layer + num_layers < > surf->aux_surf->logical_level0_px.array_len); > } > > That way we'll catch it if anyone ever tries to resolve/fast-clear a > miplevel or array slice that isn't there. > > --Jason > > Adding asserts to catch this is a good idea. I've locally put some in brw_blorp_surface_info_init and ported the one from blorp_ccs_resolve to blorp_ccs_fast_clear. </blockquote></div> </div></div>