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