[Mesa-dev] [PATCH] intel/isl: Tighten up restrictions for CCS on gen7

[Jason Ekstrand]

On Mon, Jul 31, 2017 at 1:28 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Mon, Jul 31, 2017 at 01:03:38PM -0700, Jason Ekstrand wrote:
> > On Mon, Jul 31, 2017 at 12:11 PM, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> >
> > > On Sat, Jul 22, 2017 at 04:54:24PM -0700, Jason Ekstrand wrote:
> > > > It may technically be possible to enable some sort of fast-clear
> support
> > > > for at least the base slice of a 2D array texture on gen7.  However,
> > > > it's not documented to work, we've never tried to do it in GL, and we
> > > > have no idea what the hardware does if you turn on CCS_D with arrayed
> > > > rendering.  Let's just play it safe and disallow it for now.  If
> someone
> > > > really cares that much about gen7 performance, they can come along
> and
> > > > try to get it working later.
> > > > ---
> > > >  src/intel/isl/isl.c | 34 ++++++++++++++++++++++++----------
> > > >  1 file changed, 24 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > > > index 9cf5821..5465496 100644
> > > > --- a/src/intel/isl/isl.c
> > > > +++ b/src/intel/isl/isl.c
> > > > @@ -1749,9 +1749,30 @@ isl_surf_get_ccs_surf(const struct isl_device
> > > *dev,
> > > >     if (surf->usage & ISL_SURF_USAGE_DISABLE_AUX_BIT)
> > > >        return false;
> > > >
> > > > +   /* The PRM doesn't say this explicitly, but fast-clears don't
> appear
> > > to
> > > > +    * work for 3D textures until gen9 where the layout of 3D
> textures
> > > changes
> > > > +    * to match 2D array textures.
> > > > +    */
> > > >     if (ISL_DEV_GEN(dev) <= 8 && surf->dim != ISL_SURF_DIM_2D)
> > > >        return false;
> > > >
> > > > +   /* From the HSW PRM Volume 7: 3D-Media-GPGPU, page 652 (Color
> Clear
> > > of
> > > > +    * Non-MultiSampler Render Target Restrictions):
> > > > +    *
> > > > +    *    "Support is for non-mip-mapped and non-array surface types
> > > only."
> > > > +    *
> > > > +    * This restriction is lifted on gen8+.  Technically, it may be
> > > possible to
> > > > +    * create a CCS for an arrayed or mipmapped image and only enable
> > > CCS_D
> > > > +    * when rendering to the base slice.  However, there is no
> > > documentation
> > > > +    * tell us what the hardware would do in that case or what it
> does
> > > if you
> > > > +    * walk off the bases slice.  (Does it ignore CCS or does it
> start
> > > > +    * scribbling over random memory?)  We play it safe and just
> follow
> > > the
> > > > +    * docs and don't allow CCS_D for arrayed or mip-mapped surfaces.
> > > > +    */
> > > > +   if (ISL_DEV_GEN(dev) <= 7 &&
> > > > +       (surf->levels > 1 || surf->logical_level0_px.array_len > 1))
> > > > +      return false;
> > > > +
> > >
> > > Why are mipmapped surfaces unsafe? A user is restricted to rendering
> > > into one miplevel at a time.
> > >
> >
> > Same two reasons:
> >
> >  1) The docs say it doesn't work.
> >  2) The miptree layout may be complene nonsense when it comes to the CCS.
> > On gen8+, the surface is re-laid-out for the auxiliary surface but, on
> > gen7, they try to do a direct translation of x/y coordinates in the main
> > surface to x/y in the CCS.
> >
> > --Jason
> >
> >
>
> Sorry, my question wasn't clear. I meant to ask the following: why
> disable CCS completely for mipmapped surfaces instead of enabling it
> only for the base miplevel?
>

Right.  Yes, we could probably enable it in that case.  I'm reasonably
happy to turn it on and just restrict it like you did before.  I was just
trying to get something that I knew worked so we could land the anv patches
in time.

--Jason


> > > -Nanley
> > >
> > > >     if (isl_format_is_compressed(surf->format))
> > > >        return false;
> > > >
> > > > @@ -1789,21 +1810,14 @@ 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;
> > > > -   const uint32_t depth = ISL_DEV_GEN(dev) <= 7 ?
> > > > -                          1 : surf->logical_level0_px.depth;
> > > > -
> > > >     return isl_surf_init(dev, ccs_surf,
> > > >                          .dim = surf->dim,
> > > >                          .format = ccs_format,
> > > >                          .width = surf->logical_level0_px.width,
> > > >                          .height = surf->logical_level0_px.height,
> > > > -                        .depth = depth,
> > > > -                        .levels = levels,
> > > > -                        .array_len = array_len,
> > > > +                        .depth = surf->logical_level0_px.depth,
> > > > +                        .levels = surf->levels,
> > > > +                        .array_len = surf->logical_level0_px.array_
> len,
> > > >                          .samples = 1,
> > > >                          .row_pitch = row_pitch,
> > > >                          .usage = ISL_SURF_USAGE_CCS_BIT,
> > > > --
> > > > 2.5.0.400.gff86faf
> > > >
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > mesa-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170731/1ef36d25/attachment.html>