Re: [Intel-gfx] [PATCH v5 15/19] drm/i915/dg2: Add DG2 unified compression

[Imre Deak <imre.deak@intel.com>]

On Thu, Feb 17, 2022 at 05:15:15PM +0000, Chery, Nanley G wrote:
> > >> [...]
> > >> --- a/include/uapi/drm/drm_fourcc.h
> > >> +++ b/include/uapi/drm/drm_fourcc.h
> > >> @@ -583,6 +583,28 @@ extern "C" {
> > >>    */
> > >>   #define I915_FORMAT_MOD_4_TILED         fourcc_mod_code(INTEL, 9)
> > >>
> > >> +/*
> > >> + * Intel color control surfaces (CCS) for DG2 render compression.
> > >> + *
> > >> + * DG2 uses a new compression format for render compression. The general
> > >> + * layout is the same as I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
> > >> + * but a new hashing/compression algorithm is used, so a fresh modifier must
> > >> + * be associated with buffers of this type. Render compression uses 128 byte
> > >> + * compression blocks.
> > >
> > > I think I've seen a way to configure the compression block size on TGL
> > > at least. I can't find the spec text for that at the moment though...
> > > Could we omit these mentions?
> > 
> > Not sure why general possibility of changing compression block size is relevant?
> > All hw features can be changed but this defines how this modifier is being
> > implemented.
> 
> I was concerned about compatibility between the different modes, but I've
> looked into the restrictions here and don't see any problems with this.
> 
> > Say you take I915_FORMAT_MOD_4_TILED_DG2_RC_CCS framebuffer including
> > control surface and copy it out, then come back and restore framebuffer with
> > same information. It is expected to be valid?
>
> > /Juha-Pekka
> > 
> > >> + */
> > >> +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10)
> > >> +
> > >
> > > How about something like:
> > >
> > > The main surface is Tile 4 and at plane index 0. The CCS plane is
> > > hidden from userspace. The main surface pitch is required to be a
> > > multiple of four Tile 4 widths. The CCS is configured with the render
> > > compression format associated with the main surface format.
> 
> Actually, let's omit the last sentence. CCS has always been affected
> by the main surface format, so I don't think there's a need to mention it
> specifically for the DG2 modifier.
>
> We do need to mention the 4-tile-wide pitch requirement though.

Agreed, the DG2 layout of planes and the tile format used - both
different wrt. the GEN12_RC_CCS format - should be described here.

> -Nanley
>  
> > > ....I think the CCS is technically accessible via the blitter engine,
> > > so the part about the plane being "hidden" may need some tweaking.

Maybe outside of the GEM object? Capturing all the above would you be ok
with the following?:

Intel color control surfaces (CCS) for DG2 render compression.

The main surface is Tile 4 and at plane index 0. The CCS data is stored
outside of the GEM object in a reserved memory area dedicated for the
storage of the CCS data from all GEM objects. The main surface pitch is
required to be a multiple of four Tile 4 widths. 


Intel color control surfaces (CCS) for DG2 media compression.

The main surface is Tile 4 and at plane index 0. For semi-planar formats
like NV12, the UV plane is Tile 4 at plane index 1. The CCS data both for
the main and semi-planar UV planes are stored outside of the GEM object
in a reserved memory area dedicated for the storage of the CCS data from
all GEM objects. The main surface pitch is required to be a multiple of
four Tile 4 widths. 

> > > -Nanley
> > >
> > >> +/*
> > >> + * Intel color control surfaces (CCS) for DG2 media compression.
> > >> + *
> > >> + * DG2 uses a new compression format for media compression. The
> > >> +general
> > >> + * layout is the same as I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
> > >> + * but a new hashing/compression algorithm is used, so a fresh
> > >> +modifier must
> > >> + * be associated with buffers of this type. Media compression uses
> > >> +256 byte
> > >> + * compression blocks.
> > >> + */
> > >> +#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS
> > fourcc_mod_code(INTEL,
> > >> +11)
> > >> +
> > >>   /*
> > >>    * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
> > >>    *
> > >> --
> > >> 2.20.1
> > >>
>