> -----Original Message-----
> From: Juha-Pekka Heikkila <juhapekka.heikkila@xxxxxxxxx>
> Sent: Tuesday, February 15, 2022 6:56 AM
> To: Nanley Chery <nanleychery@xxxxxxxxx>; C, Ramalingam
> <ramalingam.c@xxxxxxxxx>
> Cc: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; Chery, Nanley G
> <nanley.g.chery@xxxxxxxxx>; Auld, Matthew <matthew.auld@xxxxxxxxx>; dri-
> devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v5 16/19] uapi/drm/dg2: Introduce format
> modifier for DG2 clear color
>
> On 12.2.2022 3.19, Nanley Chery wrote:
> > On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C <ramalingam.c@xxxxxxxxx>
> wrote:
> >>
> >> From: Mika Kahola <mika.kahola@xxxxxxxxx>
> >>
> >> DG2 clear color render compression uses Tile4 layout. Therefore, we
> >> need to define a new format modifier for uAPI to support clear color
> rendering.
> >>
> >> v2:
> >> Display version is fixed. [Imre]
> >> KDoc is enhanced for cc modifier. [Nanley & Lionel]
> >>
> >> Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> >> cc: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> >> Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@xxxxxxxxx>
> >> Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_fb.c | 8 ++++++++
> >> drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 ++++++++-
> >> include/uapi/drm/drm_fourcc.h | 10 ++++++++++
> >> 3 files changed, 26 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> >> b/drivers/gpu/drm/i915/display/intel_fb.c
> >> index 4d4d01963f15..3df6ef5ffec5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> >> @@ -144,6 +144,12 @@ 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_CC,
> >> + .display_ver = { 13, 13 },
> >> + .plane_caps = INTEL_PLANE_CAP_TILING_4 |
> >> + INTEL_PLANE_CAP_CCS_RC_CC,
> >> +
> >> + .ccs.cc_planes = BIT(1),
> >> }, {
> >> .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS,
> >> .display_ver = { 13, 13 }, @@ -559,6 +565,7 @@
> >> intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
> >> else
> >> return 512;
> >> 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:
> >> case I915_FORMAT_MOD_4_TILED:
> >> /*
> >> @@ -763,6 +770,7 @@ unsigned int intel_surf_alignment(const struct
> drm_framebuffer *fb,
> >> 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:
> >> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> index c38ae0876c15..b4dced1907c5 100644
> >> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> @@ -772,6 +772,8 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
> >> 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;
> >> @@ -2358,10 +2360,15 @@ 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)) {
> >> - if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
> >> + 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 {
> >> diff --git a/include/uapi/drm/drm_fourcc.h
> >> b/include/uapi/drm/drm_fourcc.h index b8fb7b44c03c..697614ea4b84
> >> 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -605,6 +605,16 @@ extern "C" {
> >> */
> >> #define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS
> fourcc_mod_code(INTEL,
> >> 11)
> >>
> >> +/*
> >> + * Intel color control surfaces (CCS) for DG2 clear color render compression.
> >> + *
> >> + * DG2 uses a unified compression format for clear color render
> compression.
> >
> > What's unified about DG2's compression format? If this doesn't affect
> > the layout, maybe we should drop this sentence.
> >
> >> + * The general layout is a tiled layout using 4Kb tiles i.e. Tile4 layout.
> >> + *
> >
> > This also needs a pitch aligned to four tiles, right? I think we can
> > save some effort by referencing the DG2_RC_CCS modifier here.
> >
> >> + * Fast clear color value expected by HW is located in fb at offset
> >> + 0 of plane#1
> >
> > Why is the expected offset hardcoded to 0 instead of relying on the
> > offset provided by the modifier API? This looks like a bug.
>
> Hi Nanley,
>
> can you elaborate a bit, which offset from modifier API that applies to cc surface?
>
Hi Juha-Pekka,
On the kernel-side of things, I'm thinking of drm_mode_fb_cmd2::offsets[1].
-Nanley
> >
> > We should probably give some info about the relevant fields in the
> > fast clear plane (like what's done in the GEN12_RC_CCS_CC modifier).
>
> agree, that's totally missing here.
>
> /Juha-Pekka
>
> >
> >> + */
> >> +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC
> fourcc_mod_code(INTEL,
> >> +12)
> >> +
> >> /*
> >> * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
> >> *
> >> --
> >> 2.20.1
> >>
