[Mesa-dev] [PATCH 01/22] intel/isl: Limit CCS to one subresource on gen7

[Nanley Chery]

On Tue, May 02, 2017 at 02:48:09PM -0700, Jason Ekstrand wrote:
> On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> 
> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> >  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.

> 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.