From: Ramalingam C <ramalingam.c@intel.com>
To: Nanley Chery <nanleychery@gmail.com>
Cc: Tony Ye <tony.ye@intel.com>,
intel-gfx <intel-gfx@lists.freedesktop.org>,
Kenneth Graunke <kenneth@whitecape.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Slawomir Milczarek <slawomir.milczarek@intel.com>,
Hellstrom Thomas <thomas.hellstrom@intel.com>,
Matthew Auld <matthew.auld@intel.com>,
Jordan Justen <jordan.l.justen@intel.com>,
mesa-dev <mesa-dev@lists.freedesktop.org>
Subject: Re: [Mesa-dev] [PATCH v3 13/17] uapi/drm/dg2: Format modifier for DG2 unified compression and clear color
Date: Tue, 14 Dec 2021 05:53:14 +0530 [thread overview]
Message-ID: <20211214002314.GA31233@intel.com> (raw)
In-Reply-To: <CAJDL4uKiQNPm1WMo50m7Q4OPQQF1Ke9Rs8FzSojL_qT7OOaNsw@mail.gmail.com>
On 2021-12-09 at 14:41:57 -0500, Nanley Chery wrote:
> Ping. I see that a v4 has been sent out without these comments being addressed.
Sorry for the miss. I will address it soon.
Ram.
>
> -Nanley
>
> On Tue, Dec 7, 2021 at 6:51 PM Nanley Chery <nanleychery@gmail.com> wrote:
> >
> > Hi Ramalingam,
> >
> > On Wed, Oct 27, 2021 at 5:22 PM 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.
> > >
> > > 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@intel.com>
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v2)
> > > Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > cc: Simon Ser <contact@emersion.fr>
> > > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > > Cc: mesa-dev@lists.freedesktop.org
> > > Cc: Tony Ye <tony.ye@intel.com>
> > > Cc: Slawomir Milczarek <slawomir.milczarek@intel.com>
> > > Acked-by: Simon Ser <contact@emersion.fr>
> > > ---
> > > 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
> > >
next prev parent reply other threads:[~2021-12-14 0:23 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-27 21:23 [PATCH v3 00/17] drm/i915/dg2: Enabling 64k page size and flat ccs Ramalingam C
2021-10-27 21:23 ` [PATCH v3 01/17] drm/i915: Add has_64k_pages flag Ramalingam C
2021-10-27 21:23 ` [PATCH v3 02/17] drm/i915/xehpsdv: set min page-size to 64K Ramalingam C
2021-10-27 21:23 ` [PATCH v3 03/17] drm/i915/xehpsdv: enforce min GTT alignment Ramalingam C
2021-10-27 21:23 ` [PATCH v3 04/17] drm/i915: enforce min page size for scratch Ramalingam C
2021-10-27 21:23 ` [PATCH v3 05/17] drm/i915/gtt/xehpsdv: move scratch page to system memory Ramalingam C
2021-10-27 21:23 ` [PATCH v3 06/17] drm/i915/xehpsdv: support 64K GTT pages Ramalingam C
2021-10-27 21:23 ` [PATCH v3 07/17] drm/i915/xehpsdv: implement memory coloring Ramalingam C
2021-10-27 21:23 ` [PATCH v3 08/17] drm/i915/xehpsdv: Add has_flat_ccs to device info Ramalingam C
2021-10-27 21:23 ` [PATCH v3 09/17] drm/i915/lmem: Enable lmem for platforms with Flat CCS Ramalingam C
2021-10-27 21:23 ` [PATCH v3 10/17] drm/i915/gt: Clear compress metadata for Xe_HP platforms Ramalingam C
2021-10-27 21:23 ` [PATCH v3 11/17] drm/i915/dg2: Prune the Y Tiling modifiers Ramalingam C
2021-10-27 21:23 ` [PATCH v3 12/17] drm/i915/dg2: Tile 4 plane format support Ramalingam C
2021-10-28 7:04 ` Lisovskiy, Stanislav
2021-10-27 21:23 ` [PATCH v3 13/17] uapi/drm/dg2: Format modifier for DG2 unified compression and clear color Ramalingam C
2021-12-07 23:51 ` [Mesa-dev] " Nanley Chery
2021-12-09 19:41 ` Nanley Chery
2021-12-14 0:23 ` Ramalingam C [this message]
2021-10-27 21:23 ` [PATCH v3 14/17] drm/i915/dg2: Plane handling for Flat CCS " Ramalingam C
2021-10-27 21:23 ` [PATCH v3 15/17] drm/i915/uapi: document behaviour for DG2 64K support Ramalingam C
2021-10-27 21:23 ` [PATCH v3 16/17] drm/i915/Flat-CCS: Document on Flat-CCS memory compression Ramalingam C
2021-10-27 21:23 ` [PATCH v3 17/17] Doc/gpu/rfc/i915: i915 DG2 uAPI Ramalingam C
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211214002314.GA31233@intel.com \
--to=ramalingam.c@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jordan.l.justen@intel.com \
--cc=kenneth@whitecape.org \
--cc=matthew.auld@intel.com \
--cc=mesa-dev@lists.freedesktop.org \
--cc=nanleychery@gmail.com \
--cc=slawomir.milczarek@intel.com \
--cc=thomas.hellstrom@intel.com \
--cc=tony.ye@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).