Hi Ramalingam, On Wed, Oct 27, 2021 at 5:22 PM Ramalingam C <ramalingam.c@xxxxxxxxx> wrote: > > From: Matt Roper <matthew.d.roper@xxxxxxxxx> > > DG2 unifies render compression and media compression into a single > format for the first time. The programming and buffer layout is > supposed to match compression on older gen12 platforms, but the > actual compression algorithm is different from any previous platform; as > such, we need a new framebuffer modifier to represent buffers in this > format, but otherwise we can re-use the existing gen12 compression driver > logic. > > DG2 clear color render compression uses Tile4 layout. Therefore, we need > to define a new format modifier for uAPI to support clear color rendering. > I left some feedback on the modifier texts below, but I think it also applies to this commit message. > v2: Rebased on new format modifier check [Ram] > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx> (v2) > Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@xxxxxxxxx> > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx> > cc: Simon Ser <contact@xxxxxxxxxxx> > Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx> > Cc: Jordan Justen <jordan.l.justen@xxxxxxxxx> > Cc: Kenneth Graunke <kenneth@xxxxxxxxxxxxx> > Cc: mesa-dev@xxxxxxxxxxxxxxxxxxxxx > Cc: Tony Ye <tony.ye@xxxxxxxxx> > Cc: Slawomir Milczarek <slawomir.milczarek@xxxxxxxxx> > Acked-by: Simon Ser <contact@xxxxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_fb.c | 43 +++++++++++++++++++ > .../drm/i915/display/skl_universal_plane.c | 29 ++++++++++++- > include/uapi/drm/drm_fourcc.h | 30 +++++++++++++ > 3 files changed, 101 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c > index 562d5244688d..484ae1fd0e94 100644 > --- a/drivers/gpu/drm/i915/display/intel_fb.c > +++ b/drivers/gpu/drm/i915/display/intel_fb.c > @@ -106,6 +106,21 @@ static const struct drm_format_info gen12_ccs_cc_formats[] = { > .hsub = 1, .vsub = 1, .has_alpha = true }, > }; > > +static const struct drm_format_info gen12_flat_ccs_cc_formats[] = { > + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, > + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, > + .hsub = 1, .vsub = 1, }, > + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, > + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, > + .hsub = 1, .vsub = 1, }, > + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, > + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, > + .hsub = 1, .vsub = 1, .has_alpha = true }, > + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, > + .char_per_block = { 4, 0 }, .block_w = { 1, 2 }, .block_h = { 1, 1 }, > + .hsub = 1, .vsub = 1, .has_alpha = true }, > +}; > + > struct intel_modifier_desc { > u64 modifier; > struct { > @@ -166,6 +181,27 @@ static const struct intel_modifier_desc intel_modifiers[] = { > .ccs.packed_aux_planes = BIT(1), > > FORMAT_OVERRIDE(gen12_ccs_cc_formats), > + }, { > + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, > + .display_ver = { 12, 13 }, > + .tiling = I915_TILING_NONE, > + > + .ccs.type = INTEL_CCS_RC, > + }, { > + .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, > + .display_ver = { 12, 13 }, > + .tiling = I915_TILING_NONE, > + > + .ccs.type = INTEL_CCS_MC, > + }, { > + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC, > + .display_ver = { 12, 13 }, > + .tiling = I915_TILING_NONE, > + > + .ccs.type = INTEL_CCS_RC_CC, > + .ccs.cc_planes = BIT(1), > + > + FORMAT_OVERRIDE(gen12_flat_ccs_cc_formats), > }, { > .modifier = I915_FORMAT_MOD_Yf_TILED_CCS, > .display_ver = { 9, 11 }, > @@ -582,6 +618,9 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane) > return 128; > else > return 512; > + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: > + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: > + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC: > case I915_FORMAT_MOD_4_TILED: > /* > * Each 4K tile consists of 64B(8*8) subtiles, with > @@ -759,6 +798,10 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, > case I915_FORMAT_MOD_4_TILED: > case I915_FORMAT_MOD_Yf_TILED: > return 1 * 1024 * 1024; > + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: > + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC: > + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: > + return 16 * 1024; > default: > MISSING_CASE(fb->modifier); > return 0; > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index aeca96925feb..136b3f74a290 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -753,6 +753,16 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier) > return PLANE_CTL_TILED_Y; > case I915_FORMAT_MOD_4_TILED: > return PLANE_CTL_TILED_4; > + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS: > + return PLANE_CTL_TILED_4 | > + PLANE_CTL_RENDER_DECOMPRESSION_ENABLE | > + PLANE_CTL_CLEAR_COLOR_DISABLE; > + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS: > + return PLANE_CTL_TILED_4 | > + PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE | > + PLANE_CTL_CLEAR_COLOR_DISABLE; > + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC: > + return PLANE_CTL_TILED_4 | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; > case I915_FORMAT_MOD_Y_TILED_CCS: > case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: > return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; > @@ -1983,6 +1993,9 @@ skl_plane_disable_flip_done(struct intel_plane *plane) > static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915, > enum pipe pipe, enum plane_id plane_id) > { > + if (IS_DG2(i915) && !HAS_4TILE(i915)) > + return false; > + > /* Wa_22011186057 */ > if (IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0)) > return false; > @@ -2001,6 +2014,10 @@ static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915, > static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915, > enum plane_id plane_id) > { > + /* Wa_14013215631:dg2[a0,b0] */ > + if (IS_DG2_DISP_STEP(i915, STEP_A0, STEP_C0)) > + return false; > + > /* Wa_14010477008:tgl[a0..c0],rkl[all],dg1[all] */ > if (IS_DG1(i915) || IS_ROCKETLAKE(i915) || > IS_TGL_DISPLAY_STEP(i915, STEP_A0, STEP_D0)) > @@ -2243,7 +2260,17 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > break; > case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */ > if (DISPLAY_VER(dev_priv) >= 13) { > - fb->modifier = I915_FORMAT_MOD_4_TILED; > + u32 rc_mask = PLANE_CTL_RENDER_DECOMPRESSION_ENABLE | > + PLANE_CTL_CLEAR_COLOR_DISABLE; > + > + if ((val & rc_mask) == rc_mask) > + fb->modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS; > + else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) > + fb->modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS; > + else if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) > + fb->modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC; > + else > + fb->modifier = I915_FORMAT_MOD_4_TILED; > } else { > if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) > fb->modifier = I915_FORMAT_MOD_Yf_TILED_CCS; > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 982b0a9fa78b..719c17847e07 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -567,6 +567,36 @@ extern "C" { > */ > #define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 12) > > +/* > + * 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. > + */ > +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 13) > + > +/* > + * 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_MC_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. > + */ More so than new compression algorithms, these modifiers are needed due to the lack of CCS planes. The layout of these surfaces are more like I915_FORMAT_MOD_4_TILED, right? > +#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 14) > + > +/* > + * Intel color control surfaces (CCS) for DG2 clear color render compression. > + * > + * DG2 uses a unified compression format for clear color render compression. This seems to imply a different format than RC_CCS (with additional clear color tracking). > + * The general layout is a tiled layout using 4Kb tiles i.e. Tile4 layout. > + */ It's unclear where the clear color is located from this description (by comparison, the GEN12 modifier states that the clear color can be found at plane index 2). Regards, Nanley > +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC fourcc_mod_code(INTEL, 15) > + > /* > * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks > * > -- > 2.20.1 >