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

[Jason Ekstrand]

On Thu, May 11, 2017 at 11:00 AM, Nanley Chery <nanleychery at gmail.com>
wrote:

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

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.


> > 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170511/06905a44/attachment.html>