| Message ID | 20220201104132.3050-16-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dg2: Enabling 64k page size and flat ccs | expand |
On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C <ramalingam.c@intel.com> wrote: > > From: Matt Roper <matthew.d.roper@intel.com> > > 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. > > v2: > Display version fix [Imre] > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v2) > cc: Anshuman Gupta <anshuman.gupta@intel.com> > Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fb.c | 13 ++++++++++ > .../drm/i915/display/skl_universal_plane.c | 26 ++++++++++++++++--- > include/uapi/drm/drm_fourcc.h | 22 ++++++++++++++++ > 3 files changed, 57 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c > index 94c57facbb46..4d4d01963f15 100644 > --- a/drivers/gpu/drm/i915/display/intel_fb.c > +++ b/drivers/gpu/drm/i915/display/intel_fb.c > @@ -141,6 +141,14 @@ struct intel_modifier_desc { > > static const struct intel_modifier_desc intel_modifiers[] = { > { > + .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, > + .display_ver = { 13, 13 }, > + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC, > + }, { > + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, > + .display_ver = { 13, 13 }, > + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC, > + }, { > .modifier = I915_FORMAT_MOD_4_TILED, > .display_ver = { 13, 13 }, > .plane_caps = INTEL_PLANE_CAP_TILING_4, > @@ -550,6 +558,8 @@ 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: > /* > * Each 4K tile consists of 64B(8*8) subtiles, with > @@ -752,6 +762,9 @@ 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_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 5299dfe68802..c38ae0876c15 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -764,6 +764,14 @@ 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_Y_TILED_CCS: > case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: > return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; > @@ -2094,6 +2102,10 @@ static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915, > if (IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0)) > return false; > > + /* Wa_14013215631 */ > + if (IS_DG2_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) > + return false; > + > return plane_id < PLANE_SPRITE4; > } > > @@ -2335,9 +2347,10 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > case PLANE_CTL_TILED_Y: > plane_config->tiling = I915_TILING_Y; > if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) > - fb->modifier = DISPLAY_VER(dev_priv) >= 12 ? > - I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS : > - I915_FORMAT_MOD_Y_TILED_CCS; > + if (DISPLAY_VER(dev_priv) >= 12) > + fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS; > + else > + fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS; > else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) > fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS; > else > @@ -2345,7 +2358,12 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > break; > case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */ > if (HAS_4TILE(dev_priv)) { > - fb->modifier = I915_FORMAT_MOD_4_TILED; > + if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) > + 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 > + 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 b73fe6797fc3..b8fb7b44c03c 100644 > --- 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? > + */ > +#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. ....I think the CCS is technically accessible via the blitter engine, so the part about the plane being "hidden" may need some tweaking. -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 >
On 12.2.2022 3.17, Nanley Chery wrote: > On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C <ramalingam.c@intel.com> wrote: >> >> From: Matt Roper <matthew.d.roper@intel.com> >> >> 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. >> >> v2: >> Display version fix [Imre] >> >> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >> cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >> Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v2) >> cc: Anshuman Gupta <anshuman.gupta@intel.com> >> Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_fb.c | 13 ++++++++++ >> .../drm/i915/display/skl_universal_plane.c | 26 ++++++++++++++++--- >> include/uapi/drm/drm_fourcc.h | 22 ++++++++++++++++ >> 3 files changed, 57 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c >> index 94c57facbb46..4d4d01963f15 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fb.c >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c >> @@ -141,6 +141,14 @@ struct intel_modifier_desc { >> >> static const struct intel_modifier_desc intel_modifiers[] = { >> { >> + .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, >> + .display_ver = { 13, 13 }, >> + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC, >> + }, { >> + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, >> + .display_ver = { 13, 13 }, >> + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC, >> + }, { >> .modifier = I915_FORMAT_MOD_4_TILED, >> .display_ver = { 13, 13 }, >> .plane_caps = INTEL_PLANE_CAP_TILING_4, >> @@ -550,6 +558,8 @@ 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: >> /* >> * Each 4K tile consists of 64B(8*8) subtiles, with >> @@ -752,6 +762,9 @@ 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_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 5299dfe68802..c38ae0876c15 100644 >> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c >> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c >> @@ -764,6 +764,14 @@ 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_Y_TILED_CCS: >> case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: >> return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; >> @@ -2094,6 +2102,10 @@ static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915, >> if (IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0)) >> return false; >> >> + /* Wa_14013215631 */ >> + if (IS_DG2_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) >> + return false; >> + >> return plane_id < PLANE_SPRITE4; >> } >> >> @@ -2335,9 +2347,10 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, >> case PLANE_CTL_TILED_Y: >> plane_config->tiling = I915_TILING_Y; >> if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) >> - fb->modifier = DISPLAY_VER(dev_priv) >= 12 ? >> - I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS : >> - I915_FORMAT_MOD_Y_TILED_CCS; >> + if (DISPLAY_VER(dev_priv) >= 12) >> + fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS; >> + else >> + fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS; >> else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) >> fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS; >> else >> @@ -2345,7 +2358,12 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, >> break; >> case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */ >> if (HAS_4TILE(dev_priv)) { >> - fb->modifier = I915_FORMAT_MOD_4_TILED; >> + if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) >> + 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 >> + 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 b73fe6797fc3..b8fb7b44c03c 100644 >> --- 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. 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. > > ....I think the CCS is technically accessible via the blitter engine, > so the part about the plane being "hidden" may need some tweaking. > > > -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 >>
> -----Original Message----- > From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> > Sent: Tuesday, February 15, 2022 6:54 AM > To: Nanley Chery <nanleychery@gmail.com>; C, Ramalingam > <ramalingam.c@intel.com> > Cc: intel-gfx <intel-gfx@lists.freedesktop.org>; Chery, Nanley G > <nanley.g.chery@intel.com>; Auld, Matthew <matthew.auld@intel.com>; dri- > devel <dri-devel@lists.freedesktop.org> > Subject: Re: [Intel-gfx] [PATCH v5 15/19] drm/i915/dg2: Add DG2 unified > compression > > On 12.2.2022 3.17, Nanley Chery wrote: > > On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C <ramalingam.c@intel.com> > wrote: > >> > >> From: Matt Roper <matthew.d.roper@intel.com> > >> > >> 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. > >> > >> v2: > >> Display version fix [Imre] > >> > >> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > >> cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > >> Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v2) > >> cc: Anshuman Gupta <anshuman.gupta@intel.com> > >> Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com> > >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_fb.c | 13 ++++++++++ > >> .../drm/i915/display/skl_universal_plane.c | 26 ++++++++++++++++--- > >> include/uapi/drm/drm_fourcc.h | 22 ++++++++++++++++ > >> 3 files changed, 57 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c > >> b/drivers/gpu/drm/i915/display/intel_fb.c > >> index 94c57facbb46..4d4d01963f15 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_fb.c > >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c > >> @@ -141,6 +141,14 @@ struct intel_modifier_desc { > >> > >> static const struct intel_modifier_desc intel_modifiers[] = { > >> { > >> + .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, > >> + .display_ver = { 13, 13 }, > >> + .plane_caps = INTEL_PLANE_CAP_TILING_4 | > INTEL_PLANE_CAP_CCS_MC, > >> + }, { > >> + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, > >> + .display_ver = { 13, 13 }, > >> + .plane_caps = INTEL_PLANE_CAP_TILING_4 | > INTEL_PLANE_CAP_CCS_RC, > >> + }, { > >> .modifier = I915_FORMAT_MOD_4_TILED, > >> .display_ver = { 13, 13 }, > >> .plane_caps = INTEL_PLANE_CAP_TILING_4, @@ -550,6 > >> +558,8 @@ 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: > >> /* > >> * Each 4K tile consists of 64B(8*8) subtiles, with > >> @@ -752,6 +762,9 @@ 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_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 5299dfe68802..c38ae0876c15 100644 > >> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > >> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > >> @@ -764,6 +764,14 @@ 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_Y_TILED_CCS: > >> case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: > >> return PLANE_CTL_TILED_Y | > >> PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; > >> @@ -2094,6 +2102,10 @@ static bool gen12_plane_has_mc_ccs(struct > drm_i915_private *i915, > >> if (IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0)) > >> return false; > >> > >> + /* Wa_14013215631 */ > >> + if (IS_DG2_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) > >> + return false; > >> + > >> return plane_id < PLANE_SPRITE4; > >> } > >> > >> @@ -2335,9 +2347,10 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > >> case PLANE_CTL_TILED_Y: > >> plane_config->tiling = I915_TILING_Y; > >> if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) > >> - fb->modifier = DISPLAY_VER(dev_priv) >= 12 ? > >> - I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS : > >> - I915_FORMAT_MOD_Y_TILED_CCS; > >> + if (DISPLAY_VER(dev_priv) >= 12) > >> + fb->modifier = > I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS; > >> + else > >> + fb->modifier = > >> + I915_FORMAT_MOD_Y_TILED_CCS; > >> else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) > >> fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS; > >> else > >> @@ -2345,7 +2358,12 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > >> break; > >> case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */ > >> if (HAS_4TILE(dev_priv)) { > >> - fb->modifier = I915_FORMAT_MOD_4_TILED; > >> + if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) > >> + 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 > >> + 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 > >> b73fe6797fc3..b8fb7b44c03c 100644 > >> --- 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. -Nanley > > ....I think the CCS is technically accessible via the blitter engine, > > so the part about the plane being "hidden" may need some tweaking. > > > > > > -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 > >>
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 > > >> >
> -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Friday, March 18, 2022 10:40 AM > To: Chery, Nanley G <nanley.g.chery@intel.com> > Cc: juhapekka.heikkila@gmail.com; Nanley Chery <nanleychery@gmail.com>; C, Ramalingam <ramalingam.c@intel.com>; intel-gfx <intel- > gfx@lists.freedesktop.org>; Auld, Matthew <matthew.auld@intel.com>; dri-devel <dri-devel@lists.freedesktop.org> > Subject: Re: [Intel-gfx] [PATCH v5 15/19] drm/i915/dg2: Add DG2 unified compression > > 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 This kind of implies that the Y plane is the main surface, but it's not more "main" than the UV plane right? Seems like we should specifically call out the Y plane for clarity. Maybe something like: For semi-planar formats like NV12, the Y and UV planes are Tile 4 and are located at plane indices 0 and 1, respectively. The CCS for all 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. > Looks good to me. Main suggestion I have here is to substitute "from all GEM objects" with "for all compressible GEM objects". Happy to look at further revisions, but with that change at least, Acked-by: Nanley Chery <nanley.g.chery@intel.com> > > > > -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 > > > >> > >
On Thu, Mar 24, 2022 at 01:40:37AM +0200, Chery, Nanley G wrote: > > [...] > > 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 > > This kind of implies that the Y plane is the main surface, but it's not more > "main" than the UV plane right? Seems like we should specifically call out the > Y plane for clarity. Maybe something like: > > For semi-planar formats like NV12, the Y and UV planes are Tile 4 and are > located at plane indices 0 and 1, respectively. The CCS for all planes are stored > outside of the GEM object Ok, makes sense. > > 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. > > Looks good to me. Main suggestion I have here is to substitute > "from all GEM objects" with "for all compressible GEM objects". "for all RC/RC_CC/MC CCS compressible GEM objects" would be more precise, in case there are other ways to compress data. Either way looks ok to me. > Happy to look at further revisions, but with that change at least, > Acked-by: Nanley Chery <nanley.g.chery@intel.com> Thanks. --Imre
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 94c57facbb46..4d4d01963f15 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -141,6 +141,14 @@ struct intel_modifier_desc { static const struct intel_modifier_desc intel_modifiers[] = { { + .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS, + .display_ver = { 13, 13 }, + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC, + }, { + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS, + .display_ver = { 13, 13 }, + .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC, + }, { .modifier = I915_FORMAT_MOD_4_TILED, .display_ver = { 13, 13 }, .plane_caps = INTEL_PLANE_CAP_TILING_4, @@ -550,6 +558,8 @@ 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: /* * Each 4K tile consists of 64B(8*8) subtiles, with @@ -752,6 +762,9 @@ 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_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 5299dfe68802..c38ae0876c15 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -764,6 +764,14 @@ 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_Y_TILED_CCS: case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC: return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE; @@ -2094,6 +2102,10 @@ static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915, if (IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0)) return false; + /* Wa_14013215631 */ + if (IS_DG2_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) + return false; + return plane_id < PLANE_SPRITE4; } @@ -2335,9 +2347,10 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, case PLANE_CTL_TILED_Y: plane_config->tiling = I915_TILING_Y; if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) - fb->modifier = DISPLAY_VER(dev_priv) >= 12 ? - I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS : - I915_FORMAT_MOD_Y_TILED_CCS; + if (DISPLAY_VER(dev_priv) >= 12) + fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS; + else + fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS; else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE) fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS; else @@ -2345,7 +2358,12 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, break; case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */ if (HAS_4TILE(dev_priv)) { - fb->modifier = I915_FORMAT_MOD_4_TILED; + if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE) + 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 + 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 b73fe6797fc3..b8fb7b44c03c 100644 --- 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. + */ +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10) + +/* + * 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 *