Re: [Mesa-dev] [PATCH v3 13/17] uapi/drm/dg2: Format modifier for DG2 unified compression and clear color

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux