[6/6] drm/i915/tgl: Add Clear Color supoort for TGL Render Decompression
diff mbox series

Message ID 20190917121155.13197-7-radhakrishna.sripada@intel.com
State New
Headers show
Series
  • Clear Color Support for TGL Render Decompression
Related show

Commit Message

Sripada, Radhakrishna Sept. 17, 2019, 12:11 p.m. UTC
Render Decompression is supported with Y-Tiled main surface. The CCS is
linear and has 4 bits of data for each main surface cache line pair, a
ratio of 1:256. Additional Clear Color information is passed from the
user-space through an offset in the GEM BO. Add a new modifier to identify
and parse new Clear Color information and extend Gen12 render decompression
functionality to the newly added modifier.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjala <ville.syrjala@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Rafael Antognolli <rafael.antognolli@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Nanley G Chery <nanley.g.chery@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 47 +++++++++++++++++--
 .../drm/i915/display/intel_display_types.h    |  3 ++
 drivers/gpu/drm/i915/display/intel_sprite.c   | 10 +++-
 drivers/gpu/drm/i915/i915_reg.h               | 13 +++++
 4 files changed, 69 insertions(+), 4 deletions(-)

Comments

Matt Roper Sept. 17, 2019, 9:52 p.m. UTC | #1
On Tue, Sep 17, 2019 at 05:11:55AM -0700, Radhakrishna Sripada wrote:
> Render Decompression is supported with Y-Tiled main surface. The CCS is
> linear and has 4 bits of data for each main surface cache line pair, a
> ratio of 1:256. Additional Clear Color information is passed from the
> user-space through an offset in the GEM BO. Add a new modifier to identify
> and parse new Clear Color information and extend Gen12 render decompression
> functionality to the newly added modifier.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Ville Syrjala <ville.syrjala@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Rafael Antognolli <rafael.antognolli@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Nanley G Chery <nanley.g.chery@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 47 +++++++++++++++++--
>  .../drm/i915/display/intel_display_types.h    |  3 ++
>  drivers/gpu/drm/i915/display/intel_sprite.c   | 10 +++-
>  drivers/gpu/drm/i915/i915_reg.h               | 13 +++++
>  4 files changed, 69 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2da721a6abab..725b9724da49 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1913,6 +1913,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
>  		/* fall through */
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>  		if (color_plane == 1)
>  			return cpp;
>  		/* fall through */
> @@ -2051,6 +2052,7 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
>  		return 0;
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>  		return 4 * 4 * 1024;
>  	case I915_FORMAT_MOD_Y_TILED_CCS:
>  	case I915_FORMAT_MOD_Yf_TILED_CCS:
> @@ -2256,6 +2258,7 @@ static bool is_surface_linear(u64 modifier, int color_plane)
>  		return true;
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>  		return color_plane == 1;
>  	default:
>  		return false;
> @@ -2448,6 +2451,7 @@ static unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
>  	case I915_FORMAT_MOD_Y_TILED_CCS:
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>  		return I915_TILING_Y;
>  	default:
>  		return I915_TILING_NONE;
> @@ -2497,6 +2501,21 @@ static const struct drm_format_info gen12_ccs_formats[] = {
>  	  .cpp = { 4, 1, }, .hsub = 2, .vsub = 32, .has_alpha = true },
>  };
>  
> +/*
> + * Gen-12 compression uses 4 bits of CCS data for each cache line pair in the
> + * main surface. And each 64B CCS cache line represents an area of 4x1 Y-tiles
> + * in the main surface. With 4 byte pixels and each Y-tile having dimensions of
> + * 32x32 pixels, the ratio turns out to 1B in the CCS for every 2 x 32 pixels in
> + * the main surface. Additional surface is used to pass the Clear Color
> + * structure for the driver to program the DE.
> + */
> +static const struct drm_format_info gen12_ccs_cc_formats[] = {
> +	{ .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> +	{ .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> +	{ .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> +	{ .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },

I think these last two were supposed to have .has_alpha = true?

> +};
> +
>  static const struct drm_format_info *
>  lookup_format_info(const struct drm_format_info formats[],
>  		   int num_formats, u32 format)
> @@ -2525,6 +2544,10 @@ intel_get_format_info(const struct drm_mode_fb_cmd2 *cmd)
>  		return lookup_format_info(gen12_ccs_formats,
>  					  ARRAY_SIZE(gen12_ccs_formats),
>  					  cmd->pixel_format);
> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> +		return lookup_format_info(gen12_ccs_formats,
> +					  ARRAY_SIZE(gen12_ccs_cc_formats),
> +					  cmd->pixel_format);
>  	default:
>  		return NULL;
>  	}
> @@ -2534,6 +2557,7 @@ bool is_ccs_modifier(u64 modifier)
>  {
>  	return modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
>  	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS ||
> +	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC ||
>  	       modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
>  	       modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
>  }
> @@ -4120,6 +4144,9 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
>  	case I915_FORMAT_MOD_Y_TILED_CCS:
>  		/* fall through */
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> +		return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE |
> +			ICL_PLANE_CTL_CLEAR_COLOR_DISABLE;

I think this needs to be squashed back into the previous patch that
added general gen12 CCS.


> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>  		return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
>  		return PLANE_CTL_TILED_Y | PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE;
> @@ -9897,9 +9924,13 @@ skylake_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 = INTEL_GEN(dev_priv) >= 12 ?
> -				I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> -				I915_FORMAT_MOD_Y_TILED_CCS;
> +			if (INTEL_GEN(dev_priv) >= 12)
> +				fb->modifier = val &
> +					ICL_PLANE_CTL_CLEAR_COLOR_DISABLE ?
> +					I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> +					I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC;
> +			else
> +				fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS;
>  		else if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
>  			fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS;
>  		else
> @@ -14322,6 +14353,15 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
>  
>  	plane_state->vma = vma;
>  
> +	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC) {
> +		u32 *ccaddr = kmap_atomic(i915_gem_object_get_page(intel_fb_obj(fb),
> +								  fb->offsets[2] >> PAGE_SHIFT));
> +

Will this work on framebuffers in stolen memory?  Not that it's likely a
regular BIOS would actually setup compressed framebuffers to begin
with...

> +		plane_state->ccval = ((u64)*(ccaddr + CC_VAL_HIGHER_OFFSET) << 32)
> +				     | *(ccaddr + CC_VAL_LOWER_OFFSET);

Is there more data in the clear color page besides the color itself?  If
so, maybe we should just use a struct to make the memory layout explicit?

> +		kunmap_atomic(ccaddr);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -15709,6 +15749,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>  		 * tile widths.
>  		 */
>  		if ((fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> +		     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC ||
>  		     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS) &&
>  		    i == 0)
>  			stride_alignment *= 4;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index d5cc4b810d9e..a82695a3e0b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -579,6 +579,9 @@ struct intel_plane_state {
>  	u32 slave;
>  
>  	struct drm_intel_sprite_colorkey ckey;
> +
> +	/* Clear Color Value */
> +	u64 ccval;
>  };
>  
>  struct intel_initial_plane_config {
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 1655984955ca..3672e8b9c0bc 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -549,6 +549,7 @@ skl_program_plane(struct intel_plane *plane,
>  	u32 plane_color_ctl = 0;
>  	unsigned long irqflags;
>  	u32 keymsk, keymax;
> +	u64 ccval = plane_state->ccval;
>  
>  	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
>  
> @@ -609,6 +610,10 @@ skl_program_plane(struct intel_plane *plane,
>  	if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id))
>  		icl_program_input_csc(plane, crtc_state, plane_state);
>  
> +	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)
> +		intel_uncore_write64_fw(&dev_priv->uncore,
> +					PLANE_CC_VAL(pipe, plane_id), ccval);
> +
>  	skl_write_plane_wm(plane, crtc_state);
>  
>  	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> @@ -1738,7 +1743,8 @@ static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>  	     fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
>  	     fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
>  	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> -	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS)) {
> +	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS ||
> +	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)) {
>  		DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
>  		return -EINVAL;
>  	}
> @@ -2153,6 +2159,7 @@ static const u64 skl_plane_format_modifiers_ccs[] = {
>  static const u64 gen12_plane_format_modifiers_ccs[] = {
>  	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
>  	I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
> +	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC,
>  	I915_FORMAT_MOD_Y_TILED,
>  	I915_FORMAT_MOD_X_TILED,
>  	DRM_FORMAT_MOD_LINEAR,
> @@ -2321,6 +2328,7 @@ static bool gen12_plane_format_mod_supported(struct drm_plane *_plane,
>  	case I915_FORMAT_MOD_X_TILED:
>  	case I915_FORMAT_MOD_Y_TILED:
>  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>  		break;
>  	default:
>  		return false;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 096be95ec7f9..00c59ed8b114 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6681,6 +6681,7 @@ enum {
>  #define   PLANE_CTL_YUV422_VYUY			(3 << 16)
>  #define   PLANE_CTL_RENDER_DECOMPRESSION_ENABLE	(1 << 15)
>  #define   PLANE_CTL_TRICKLE_FEED_DISABLE	(1 << 14)
> +#define	  ICL_PLANE_CTL_CLEAR_COLOR_DISABLE	(1 << 13)

Looks like the whitespace is off here.


Matt

>  #define   PLANE_CTL_PLANE_GAMMA_DISABLE		(1 << 13) /* Pre-GLK */
>  #define   PLANE_CTL_TILED_MASK			(0x7 << 10)
>  #define   PLANE_CTL_TILED_LINEAR		(0 << 10)
> @@ -6721,6 +6722,8 @@ enum {
>  #define _PLANE_KEYMAX_1_A			0x701a0
>  #define _PLANE_KEYMAX_2_A			0x702a0
>  #define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
> +#define _PLANE_CC_VAL_1_A			0x701b4
> +#define _PLANE_CC_VAL_2_A			0x702b4
>  #define _PLANE_AUX_DIST_1_A			0x701c0
>  #define _PLANE_AUX_DIST_2_A			0x702c0
>  #define _PLANE_AUX_OFFSET_1_A			0x701c4
> @@ -6760,6 +6763,16 @@ enum {
>  #define _PLANE_NV12_BUF_CFG_1_A		0x70278
>  #define _PLANE_NV12_BUF_CFG_2_A		0x70378
>  
> +#define _PLANE_CC_VAL_1_B			0x711b4
> +#define _PLANE_CC_VAL_2_B			0x712b4
> +#define _PLANE_CC_VAL_1(pipe)	_PIPE(pipe, _PLANE_CC_VAL_1_A, _PLANE_CC_VAL_1_B)
> +#define _PLANE_CC_VAL_2(pipe)	_PIPE(pipe, _PLANE_CC_VAL_2_A, _PLANE_CC_VAL_2_B)
> +#define PLANE_CC_VAL(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_CC_VAL_1(pipe), _PLANE_CC_VAL_2(pipe))
> +
> +#define CC_VAL_LOWER_OFFSET		4
> +#define CC_VAL_HIGHER_OFFSET		5
> +
>  /* Input CSC Register Definitions */
>  #define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
>  #define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0
> -- 
> 2.20.1
>
Jordan Justen Sept. 19, 2019, 12:53 a.m. UTC | #2
On 2019-09-17 05:11:55, Radhakrishna Sripada wrote:
>                 return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
>         case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
>                 return PLANE_CTL_TILED_Y | PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE;
> @@ -9897,9 +9924,13 @@ skylake_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)

Should {} be added to this `if` since a nested if-else is being added?

> -                       fb->modifier = INTEL_GEN(dev_priv) >= 12 ?
> -                               I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> -                               I915_FORMAT_MOD_Y_TILED_CCS;
> +                       if (INTEL_GEN(dev_priv) >= 12)
> +                               fb->modifier = val &
> +                                       ICL_PLANE_CTL_CLEAR_COLOR_DISABLE ?
> +                                       I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> +                                       I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC;
> +                       else
> +                               fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS;
>                 else if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
>                         fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS;
>                 else
Sripada, Radhakrishna Sept. 19, 2019, 10:16 p.m. UTC | #3
Hi Matt,
> -----Original Message-----
> From: Roper, Matthew D
> Sent: Tuesday, September 17, 2019 2:53 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> Sharma, Shashank <shashank.sharma@intel.com>; Antognolli, Rafael
> <rafael.antognolli@intel.com>; Chery, Nanley G <nanley.g.chery@intel.com>
> Subject: Re: [PATCH 6/6] drm/i915/tgl: Add Clear Color supoort for TGL
> Render Decompression
> 
> On Tue, Sep 17, 2019 at 05:11:55AM -0700, Radhakrishna Sripada wrote:
> > Render Decompression is supported with Y-Tiled main surface. The CCS
> > is linear and has 4 bits of data for each main surface cache line
> > pair, a ratio of 1:256. Additional Clear Color information is passed
> > from the user-space through an offset in the GEM BO. Add a new
> > modifier to identify and parse new Clear Color information and extend
> > Gen12 render decompression functionality to the newly added modifier.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@intel.com>
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Rafael Antognolli <rafael.antognolli@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Nanley G Chery <nanley.g.chery@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  | 47 +++++++++++++++++--
> >  .../drm/i915/display/intel_display_types.h    |  3 ++
> >  drivers/gpu/drm/i915/display/intel_sprite.c   | 10 +++-
> >  drivers/gpu/drm/i915/i915_reg.h               | 13 +++++
> >  4 files changed, 69 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 2da721a6abab..725b9724da49 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1913,6 +1913,7 @@ intel_tile_width_bytes(const struct
> drm_framebuffer *fb, int color_plane)
> >  		/* fall through */
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >  		if (color_plane == 1)
> >  			return cpp;
> >  		/* fall through */
> > @@ -2051,6 +2052,7 @@ static unsigned int intel_surf_alignment(const
> struct drm_framebuffer *fb,
> >  		return 0;
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >  		return 4 * 4 * 1024;
> >  	case I915_FORMAT_MOD_Y_TILED_CCS:
> >  	case I915_FORMAT_MOD_Yf_TILED_CCS:
> > @@ -2256,6 +2258,7 @@ static bool is_surface_linear(u64 modifier, int
> color_plane)
> >  		return true;
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >  		return color_plane == 1;
> >  	default:
> >  		return false;
> > @@ -2448,6 +2451,7 @@ static unsigned int
> intel_fb_modifier_to_tiling(u64 fb_modifier)
> >  	case I915_FORMAT_MOD_Y_TILED_CCS:
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >  		return I915_TILING_Y;
> >  	default:
> >  		return I915_TILING_NONE;
> > @@ -2497,6 +2501,21 @@ static const struct drm_format_info
> gen12_ccs_formats[] = {
> >  	  .cpp = { 4, 1, }, .hsub = 2, .vsub = 32, .has_alpha = true },  };
> >
> > +/*
> > + * Gen-12 compression uses 4 bits of CCS data for each cache line
> > +pair in the
> > + * main surface. And each 64B CCS cache line represents an area of
> > +4x1 Y-tiles
> > + * in the main surface. With 4 byte pixels and each Y-tile having
> > +dimensions of
> > + * 32x32 pixels, the ratio turns out to 1B in the CCS for every 2 x
> > +32 pixels in
> > + * the main surface. Additional surface is used to pass the Clear
> > +Color
> > + * structure for the driver to program the DE.
> > + */
> > +static const struct drm_format_info gen12_ccs_cc_formats[] = {
> > +	{ .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 3,
> .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > +	{ .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 3,
> .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > +	{ .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 3,
> .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > +	{ .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 3,
> .cpp
> > += { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> 
> I think these last two were supposed to have .has_alpha = true?
Sure will include in the next rev.

>  
> > +};
> > +
> >  static const struct drm_format_info *  lookup_format_info(const
> > struct drm_format_info formats[],
> >  		   int num_formats, u32 format)
> > @@ -2525,6 +2544,10 @@ intel_get_format_info(const struct
> drm_mode_fb_cmd2 *cmd)
> >  		return lookup_format_info(gen12_ccs_formats,
> >  					  ARRAY_SIZE(gen12_ccs_formats),
> >  					  cmd->pixel_format);
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > +		return lookup_format_info(gen12_ccs_formats,
> > +
> ARRAY_SIZE(gen12_ccs_cc_formats),
> > +					  cmd->pixel_format);
> >  	default:
> >  		return NULL;
> >  	}
> > @@ -2534,6 +2557,7 @@ bool is_ccs_modifier(u64 modifier)  {
> >  	return modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> >  	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS ||
> > +	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC
> ||
> >  	       modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> >  	       modifier == I915_FORMAT_MOD_Yf_TILED_CCS;  } @@ -4120,6
> > +4144,9 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
> >  	case I915_FORMAT_MOD_Y_TILED_CCS:
> >  		/* fall through */
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > +		return PLANE_CTL_TILED_Y |
> PLANE_CTL_RENDER_DECOMPRESSION_ENABLE |
> > +			ICL_PLANE_CTL_CLEAR_COLOR_DISABLE;
> 
> I think this needs to be squashed back into the previous patch that added
> general gen12 CCS.
Sure. I will work with DK to get this changes.
> 
> 
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >  		return PLANE_CTL_TILED_Y |
> PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> >  		return PLANE_CTL_TILED_Y |
> PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE;
> > @@ -9897,9 +9924,13 @@ skylake_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 = INTEL_GEN(dev_priv) >= 12 ?
> > -
> 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> > -				I915_FORMAT_MOD_Y_TILED_CCS;
> > +			if (INTEL_GEN(dev_priv) >= 12)
> > +				fb->modifier = val &
> > +
> 	ICL_PLANE_CTL_CLEAR_COLOR_DISABLE ?
> > +
> 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> > +
> 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC;
> > +			else
> > +				fb->modifier =
> I915_FORMAT_MOD_Y_TILED_CCS;
> >  		else if (val &
> PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
> >  			fb->modifier =
> I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS;
> >  		else
> > @@ -14322,6 +14353,15 @@ static int intel_plane_pin_fb(struct
> > intel_plane_state *plane_state)
> >
> >  	plane_state->vma = vma;
> >
> > +	if (fb->modifier ==
> I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC) {
> > +		u32 *ccaddr =
> kmap_atomic(i915_gem_object_get_page(intel_fb_obj(fb),
> > +								  fb-
> >offsets[2] >> PAGE_SHIFT));
> > +
> 
> Will this work on framebuffers in stolen memory?  Not that it's likely a regular
> BIOS would actually setup compressed framebuffers to begin with...
It is unlikely that BIOS sets up a compressed fb. However FBC could impact as it works with
Stolen memory. I am not sure about the interaction between fbc and render compression
working at the same time. Apart from that I am unable to think of an issue. Will try to figure
 out during testing.

> 
> > +		plane_state->ccval = ((u64)*(ccaddr +
> CC_VAL_HIGHER_OFFSET) << 32)
> > +				     | *(ccaddr + CC_VAL_LOWER_OFFSET);
> 
> Is there more data in the clear color page besides the color itself?  If so,
> maybe we should just use a struct to make the memory layout explicit?
According to BSpec: 43854 the first 3 offsets are for the raw color which acts as an input to
3d engine which fills the info in the later 2 offsets in DE compatible format. The other data
Therefore is redundant and cannot be consumed by the DE hence ignoring.
> 
> > +		kunmap_atomic(ccaddr);
> > +	}
> > +
> >  	return 0;
> >  }
> >
> > @@ -15709,6 +15749,7 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> >  		 * tile widths.
> >  		 */
> >  		if ((fb->modifier ==
> I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> > +		     fb->modifier ==
> I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC ||
> >  		     fb->modifier ==
> I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS) &&
> >  		    i == 0)
> >  			stride_alignment *= 4;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index d5cc4b810d9e..a82695a3e0b8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -579,6 +579,9 @@ struct intel_plane_state {
> >  	u32 slave;
> >
> >  	struct drm_intel_sprite_colorkey ckey;
> > +
> > +	/* Clear Color Value */
> > +	u64 ccval;
> >  };
> >
> >  struct intel_initial_plane_config {
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index 1655984955ca..3672e8b9c0bc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -549,6 +549,7 @@ skl_program_plane(struct intel_plane *plane,
> >  	u32 plane_color_ctl = 0;
> >  	unsigned long irqflags;
> >  	u32 keymsk, keymax;
> > +	u64 ccval = plane_state->ccval;
> >
> >  	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
> >
> > @@ -609,6 +610,10 @@ skl_program_plane(struct intel_plane *plane,
> >  	if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id))
> >  		icl_program_input_csc(plane, crtc_state, plane_state);
> >
> > +	if (fb->modifier ==
> I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)
> > +		intel_uncore_write64_fw(&dev_priv->uncore,
> > +					PLANE_CC_VAL(pipe, plane_id),
> ccval);
> > +
> >  	skl_write_plane_wm(plane, crtc_state);
> >
> >  	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> @@
> > -1738,7 +1743,8 @@ static int skl_plane_check_fb(const struct
> intel_crtc_state *crtc_state,
> >  	     fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> >  	     fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> >  	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> > -	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS))
> {
> > +	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS
> ||
> > +	     fb->modifier ==
> I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)) {
> >  		DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID
> mode\n");
> >  		return -EINVAL;
> >  	}
> > @@ -2153,6 +2159,7 @@ static const u64
> > skl_plane_format_modifiers_ccs[] = {  static const u64
> gen12_plane_format_modifiers_ccs[] = {
> >  	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
> >  	I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
> > +	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC,
> >  	I915_FORMAT_MOD_Y_TILED,
> >  	I915_FORMAT_MOD_X_TILED,
> >  	DRM_FORMAT_MOD_LINEAR,
> > @@ -2321,6 +2328,7 @@ static bool
> gen12_plane_format_mod_supported(struct drm_plane *_plane,
> >  	case I915_FORMAT_MOD_X_TILED:
> >  	case I915_FORMAT_MOD_Y_TILED:
> >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >  		break;
> >  	default:
> >  		return false;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 096be95ec7f9..00c59ed8b114
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6681,6 +6681,7 @@ enum {
> >  #define   PLANE_CTL_YUV422_VYUY			(3 << 16)
> >  #define   PLANE_CTL_RENDER_DECOMPRESSION_ENABLE	(1 << 15)
> >  #define   PLANE_CTL_TRICKLE_FEED_DISABLE	(1 << 14)
> > +#define	  ICL_PLANE_CTL_CLEAR_COLOR_DISABLE	(1 << 13)
> 
> Looks like the whitespace is off here.
Will fix it next rev.

Thanks,
RK
> 
> 
> Matt
> 
> >  #define   PLANE_CTL_PLANE_GAMMA_DISABLE		(1 << 13) /*
> Pre-GLK */
> >  #define   PLANE_CTL_TILED_MASK			(0x7 << 10)
> >  #define   PLANE_CTL_TILED_LINEAR		(0 << 10)
> > @@ -6721,6 +6722,8 @@ enum {
> >  #define _PLANE_KEYMAX_1_A			0x701a0
> >  #define _PLANE_KEYMAX_2_A			0x702a0
> >  #define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
> > +#define _PLANE_CC_VAL_1_A			0x701b4
> > +#define _PLANE_CC_VAL_2_A			0x702b4
> >  #define _PLANE_AUX_DIST_1_A			0x701c0
> >  #define _PLANE_AUX_DIST_2_A			0x702c0
> >  #define _PLANE_AUX_OFFSET_1_A			0x701c4
> > @@ -6760,6 +6763,16 @@ enum {
> >  #define _PLANE_NV12_BUF_CFG_1_A		0x70278
> >  #define _PLANE_NV12_BUF_CFG_2_A		0x70378
> >
> > +#define _PLANE_CC_VAL_1_B			0x711b4
> > +#define _PLANE_CC_VAL_2_B			0x712b4
> > +#define _PLANE_CC_VAL_1(pipe)	_PIPE(pipe, _PLANE_CC_VAL_1_A,
> _PLANE_CC_VAL_1_B)
> > +#define _PLANE_CC_VAL_2(pipe)	_PIPE(pipe, _PLANE_CC_VAL_2_A,
> _PLANE_CC_VAL_2_B)
> > +#define PLANE_CC_VAL(pipe, plane)	\
> > +	_MMIO_PLANE(plane, _PLANE_CC_VAL_1(pipe),
> _PLANE_CC_VAL_2(pipe))
> > +
> > +#define CC_VAL_LOWER_OFFSET		4
> > +#define CC_VAL_HIGHER_OFFSET		5
> > +
> >  /* Input CSC Register Definitions */
> >  #define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
> >  #define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0
> > --
> > 2.20.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
Matt Roper Sept. 19, 2019, 10:38 p.m. UTC | #4
On Thu, Sep 19, 2019 at 03:16:34PM -0700, Sripada, Radhakrishna wrote:
> Hi Matt,
> > -----Original Message-----
> > From: Roper, Matthew D
> > Sent: Tuesday, September 17, 2019 2:53 PM
> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran
> > <dhinakaran.pandiyan@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> > Sharma, Shashank <shashank.sharma@intel.com>; Antognolli, Rafael
> > <rafael.antognolli@intel.com>; Chery, Nanley G <nanley.g.chery@intel.com>
> > Subject: Re: [PATCH 6/6] drm/i915/tgl: Add Clear Color supoort for TGL
> > Render Decompression
> > 
> > On Tue, Sep 17, 2019 at 05:11:55AM -0700, Radhakrishna Sripada wrote:
> > > Render Decompression is supported with Y-Tiled main surface. The CCS
> > > is linear and has 4 bits of data for each main surface cache line
> > > pair, a ratio of 1:256. Additional Clear Color information is passed
> > > from the user-space through an offset in the GEM BO. Add a new
> > > modifier to identify and parse new Clear Color information and extend
> > > Gen12 render decompression functionality to the newly added modifier.
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@intel.com>
> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > Cc: Rafael Antognolli <rafael.antognolli@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: Nanley G Chery <nanley.g.chery@intel.com>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c  | 47 +++++++++++++++++--
> > >  .../drm/i915/display/intel_display_types.h    |  3 ++
> > >  drivers/gpu/drm/i915/display/intel_sprite.c   | 10 +++-
> > >  drivers/gpu/drm/i915/i915_reg.h               | 13 +++++
> > >  4 files changed, 69 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 2da721a6abab..725b9724da49 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -1913,6 +1913,7 @@ intel_tile_width_bytes(const struct
> > drm_framebuffer *fb, int color_plane)
> > >  		/* fall through */
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > >  		if (color_plane == 1)
> > >  			return cpp;
> > >  		/* fall through */
> > > @@ -2051,6 +2052,7 @@ static unsigned int intel_surf_alignment(const
> > struct drm_framebuffer *fb,
> > >  		return 0;
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > >  		return 4 * 4 * 1024;
> > >  	case I915_FORMAT_MOD_Y_TILED_CCS:
> > >  	case I915_FORMAT_MOD_Yf_TILED_CCS:
> > > @@ -2256,6 +2258,7 @@ static bool is_surface_linear(u64 modifier, int
> > color_plane)
> > >  		return true;
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > >  		return color_plane == 1;
> > >  	default:
> > >  		return false;
> > > @@ -2448,6 +2451,7 @@ static unsigned int
> > intel_fb_modifier_to_tiling(u64 fb_modifier)
> > >  	case I915_FORMAT_MOD_Y_TILED_CCS:
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > >  		return I915_TILING_Y;
> > >  	default:
> > >  		return I915_TILING_NONE;
> > > @@ -2497,6 +2501,21 @@ static const struct drm_format_info
> > gen12_ccs_formats[] = {
> > >  	  .cpp = { 4, 1, }, .hsub = 2, .vsub = 32, .has_alpha = true },  };
> > >
> > > +/*
> > > + * Gen-12 compression uses 4 bits of CCS data for each cache line
> > > +pair in the
> > > + * main surface. And each 64B CCS cache line represents an area of
> > > +4x1 Y-tiles
> > > + * in the main surface. With 4 byte pixels and each Y-tile having
> > > +dimensions of
> > > + * 32x32 pixels, the ratio turns out to 1B in the CCS for every 2 x
> > > +32 pixels in
> > > + * the main surface. Additional surface is used to pass the Clear
> > > +Color
> > > + * structure for the driver to program the DE.
> > > + */
> > > +static const struct drm_format_info gen12_ccs_cc_formats[] = {
> > > +	{ .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 3,
> > .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > > +	{ .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 3,
> > .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > > +	{ .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 3,
> > .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > > +	{ .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 3,
> > .cpp
> > > += { 4, 1, 0}, .hsub = 2, .vsub = 32, },
> > 
> > I think these last two were supposed to have .has_alpha = true?
> Sure will include in the next rev.
> 
> >  
> > > +};
> > > +
> > >  static const struct drm_format_info *  lookup_format_info(const
> > > struct drm_format_info formats[],
> > >  		   int num_formats, u32 format)
> > > @@ -2525,6 +2544,10 @@ intel_get_format_info(const struct
> > drm_mode_fb_cmd2 *cmd)
> > >  		return lookup_format_info(gen12_ccs_formats,
> > >  					  ARRAY_SIZE(gen12_ccs_formats),
> > >  					  cmd->pixel_format);
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > > +		return lookup_format_info(gen12_ccs_formats,
> > > +
> > ARRAY_SIZE(gen12_ccs_cc_formats),
> > > +					  cmd->pixel_format);
> > >  	default:
> > >  		return NULL;
> > >  	}
> > > @@ -2534,6 +2557,7 @@ bool is_ccs_modifier(u64 modifier)  {
> > >  	return modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> > >  	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS ||
> > > +	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC
> > ||
> > >  	       modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > >  	       modifier == I915_FORMAT_MOD_Yf_TILED_CCS;  } @@ -4120,6
> > > +4144,9 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
> > >  	case I915_FORMAT_MOD_Y_TILED_CCS:
> > >  		/* fall through */
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > > +		return PLANE_CTL_TILED_Y |
> > PLANE_CTL_RENDER_DECOMPRESSION_ENABLE |
> > > +			ICL_PLANE_CTL_CLEAR_COLOR_DISABLE;
> > 
> > I think this needs to be squashed back into the previous patch that added
> > general gen12 CCS.
> Sure. I will work with DK to get this changes.
> > 
> > 
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > >  		return PLANE_CTL_TILED_Y |
> > PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> > >  		return PLANE_CTL_TILED_Y |
> > PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE;
> > > @@ -9897,9 +9924,13 @@ skylake_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 = INTEL_GEN(dev_priv) >= 12 ?
> > > -
> > 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> > > -				I915_FORMAT_MOD_Y_TILED_CCS;
> > > +			if (INTEL_GEN(dev_priv) >= 12)
> > > +				fb->modifier = val &
> > > +
> > 	ICL_PLANE_CTL_CLEAR_COLOR_DISABLE ?
> > > +
> > 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
> > > +
> > 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC;
> > > +			else
> > > +				fb->modifier =
> > I915_FORMAT_MOD_Y_TILED_CCS;
> > >  		else if (val &
> > PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
> > >  			fb->modifier =
> > I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS;
> > >  		else
> > > @@ -14322,6 +14353,15 @@ static int intel_plane_pin_fb(struct
> > > intel_plane_state *plane_state)
> > >
> > >  	plane_state->vma = vma;
> > >
> > > +	if (fb->modifier ==
> > I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC) {
> > > +		u32 *ccaddr =
> > kmap_atomic(i915_gem_object_get_page(intel_fb_obj(fb),
> > > +								  fb-
> > >offsets[2] >> PAGE_SHIFT));
> > > +
> > 
> > Will this work on framebuffers in stolen memory?  Not that it's likely a regular
> > BIOS would actually setup compressed framebuffers to begin with...
> It is unlikely that BIOS sets up a compressed fb. However FBC could impact as it works with
> Stolen memory. I am not sure about the interaction between fbc and render compression
> working at the same time. Apart from that I am unable to think of an issue. Will try to figure
>  out during testing.

My main worry here was that if the BIOS or pre-OS bootloader setup a
framebuffer in stolen memory (which is probably unlikely, but
theoretically possible in some highly specialized embedded
environments), we wouldn't be able to kmap the buffer since stolen
memory isn't accessible by the CPU (i.e., the CPU can only access it
indirectly through the aperture).  So if we tried to flip back to a
RC_CCS_CC fb located in stolen memory, the i915_gem_object_get_page()
above would GEM_BUG_ON() because i915_gem_object_stolen_ops.flags
doesn't have I915_GEM_OBJECT_HAS_STRUCT_PAGE.

We don't currently allow direct allocation of framebuffers in stolen
memory, so the only place those can come from is the initial config we
inherit from the BIOS.  Probably the simplest way forward is to just
bail out of skylake_get_initial_plane_config() and give up on inheriting
the fb in the very unlikely case we notice that the BIOS is claiming
that render compression + clear color is enabled.


Matt

> 
> > 
> > > +		plane_state->ccval = ((u64)*(ccaddr +
> > CC_VAL_HIGHER_OFFSET) << 32)
> > > +				     | *(ccaddr + CC_VAL_LOWER_OFFSET);
> > 
> > Is there more data in the clear color page besides the color itself?  If so,
> > maybe we should just use a struct to make the memory layout explicit?
> According to BSpec: 43854 the first 3 offsets are for the raw color which acts as an input to
> 3d engine which fills the info in the later 2 offsets in DE compatible format. The other data
> Therefore is redundant and cannot be consumed by the DE hence ignoring.
> > 
> > > +		kunmap_atomic(ccaddr);
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -15709,6 +15749,7 @@ static int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > >  		 * tile widths.
> > >  		 */
> > >  		if ((fb->modifier ==
> > I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> > > +		     fb->modifier ==
> > I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC ||
> > >  		     fb->modifier ==
> > I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS) &&
> > >  		    i == 0)
> > >  			stride_alignment *= 4;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index d5cc4b810d9e..a82695a3e0b8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -579,6 +579,9 @@ struct intel_plane_state {
> > >  	u32 slave;
> > >
> > >  	struct drm_intel_sprite_colorkey ckey;
> > > +
> > > +	/* Clear Color Value */
> > > +	u64 ccval;
> > >  };
> > >
> > >  struct intel_initial_plane_config {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index 1655984955ca..3672e8b9c0bc 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -549,6 +549,7 @@ skl_program_plane(struct intel_plane *plane,
> > >  	u32 plane_color_ctl = 0;
> > >  	unsigned long irqflags;
> > >  	u32 keymsk, keymax;
> > > +	u64 ccval = plane_state->ccval;
> > >
> > >  	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
> > >
> > > @@ -609,6 +610,10 @@ skl_program_plane(struct intel_plane *plane,
> > >  	if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id))
> > >  		icl_program_input_csc(plane, crtc_state, plane_state);
> > >
> > > +	if (fb->modifier ==
> > I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)
> > > +		intel_uncore_write64_fw(&dev_priv->uncore,
> > > +					PLANE_CC_VAL(pipe, plane_id),
> > ccval);
> > > +
> > >  	skl_write_plane_wm(plane, crtc_state);
> > >
> > >  	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> > @@
> > > -1738,7 +1743,8 @@ static int skl_plane_check_fb(const struct
> > intel_crtc_state *crtc_state,
> > >  	     fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > >  	     fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> > >  	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> > > -	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS))
> > {
> > > +	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS
> > ||
> > > +	     fb->modifier ==
> > I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)) {
> > >  		DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID
> > mode\n");
> > >  		return -EINVAL;
> > >  	}
> > > @@ -2153,6 +2159,7 @@ static const u64
> > > skl_plane_format_modifiers_ccs[] = {  static const u64
> > gen12_plane_format_modifiers_ccs[] = {
> > >  	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
> > >  	I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
> > > +	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC,
> > >  	I915_FORMAT_MOD_Y_TILED,
> > >  	I915_FORMAT_MOD_X_TILED,
> > >  	DRM_FORMAT_MOD_LINEAR,
> > > @@ -2321,6 +2328,7 @@ static bool
> > gen12_plane_format_mod_supported(struct drm_plane *_plane,
> > >  	case I915_FORMAT_MOD_X_TILED:
> > >  	case I915_FORMAT_MOD_Y_TILED:
> > >  	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > > +	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> > >  		break;
> > >  	default:
> > >  		return false;
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 096be95ec7f9..00c59ed8b114
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6681,6 +6681,7 @@ enum {
> > >  #define   PLANE_CTL_YUV422_VYUY			(3 << 16)
> > >  #define   PLANE_CTL_RENDER_DECOMPRESSION_ENABLE	(1 << 15)
> > >  #define   PLANE_CTL_TRICKLE_FEED_DISABLE	(1 << 14)
> > > +#define	  ICL_PLANE_CTL_CLEAR_COLOR_DISABLE	(1 << 13)
> > 
> > Looks like the whitespace is off here.
> Will fix it next rev.
> 
> Thanks,
> RK
> > 
> > 
> > Matt
> > 
> > >  #define   PLANE_CTL_PLANE_GAMMA_DISABLE		(1 << 13) /*
> > Pre-GLK */
> > >  #define   PLANE_CTL_TILED_MASK			(0x7 << 10)
> > >  #define   PLANE_CTL_TILED_LINEAR		(0 << 10)
> > > @@ -6721,6 +6722,8 @@ enum {
> > >  #define _PLANE_KEYMAX_1_A			0x701a0
> > >  #define _PLANE_KEYMAX_2_A			0x702a0
> > >  #define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
> > > +#define _PLANE_CC_VAL_1_A			0x701b4
> > > +#define _PLANE_CC_VAL_2_A			0x702b4
> > >  #define _PLANE_AUX_DIST_1_A			0x701c0
> > >  #define _PLANE_AUX_DIST_2_A			0x702c0
> > >  #define _PLANE_AUX_OFFSET_1_A			0x701c4
> > > @@ -6760,6 +6763,16 @@ enum {
> > >  #define _PLANE_NV12_BUF_CFG_1_A		0x70278
> > >  #define _PLANE_NV12_BUF_CFG_2_A		0x70378
> > >
> > > +#define _PLANE_CC_VAL_1_B			0x711b4
> > > +#define _PLANE_CC_VAL_2_B			0x712b4
> > > +#define _PLANE_CC_VAL_1(pipe)	_PIPE(pipe, _PLANE_CC_VAL_1_A,
> > _PLANE_CC_VAL_1_B)
> > > +#define _PLANE_CC_VAL_2(pipe)	_PIPE(pipe, _PLANE_CC_VAL_2_A,
> > _PLANE_CC_VAL_2_B)
> > > +#define PLANE_CC_VAL(pipe, plane)	\
> > > +	_MMIO_PLANE(plane, _PLANE_CC_VAL_1(pipe),
> > _PLANE_CC_VAL_2(pipe))
> > > +
> > > +#define CC_VAL_LOWER_OFFSET		4
> > > +#define CC_VAL_HIGHER_OFFSET		5
> > > +
> > >  /* Input CSC Register Definitions */
> > >  #define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
> > >  #define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0
> > > --
> > > 2.20.1
> > >
> > 
> > --
> > Matt Roper
> > Graphics Software Engineer
> > VTT-OSGC Platform Enablement
> > Intel Corporation
> > (916) 356-2795

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2da721a6abab..725b9724da49 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1913,6 +1913,7 @@  intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
 		/* fall through */
 	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
 	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
 		if (color_plane == 1)
 			return cpp;
 		/* fall through */
@@ -2051,6 +2052,7 @@  static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 		return 0;
 	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
 	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
 		return 4 * 4 * 1024;
 	case I915_FORMAT_MOD_Y_TILED_CCS:
 	case I915_FORMAT_MOD_Yf_TILED_CCS:
@@ -2256,6 +2258,7 @@  static bool is_surface_linear(u64 modifier, int color_plane)
 		return true;
 	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
 	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
 		return color_plane == 1;
 	default:
 		return false;
@@ -2448,6 +2451,7 @@  static unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
 	case I915_FORMAT_MOD_Y_TILED_CCS:
 	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
 	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
 		return I915_TILING_Y;
 	default:
 		return I915_TILING_NONE;
@@ -2497,6 +2501,21 @@  static const struct drm_format_info gen12_ccs_formats[] = {
 	  .cpp = { 4, 1, }, .hsub = 2, .vsub = 32, .has_alpha = true },
 };
 
+/*
+ * Gen-12 compression uses 4 bits of CCS data for each cache line pair in the
+ * main surface. And each 64B CCS cache line represents an area of 4x1 Y-tiles
+ * in the main surface. With 4 byte pixels and each Y-tile having dimensions of
+ * 32x32 pixels, the ratio turns out to 1B in the CCS for every 2 x 32 pixels in
+ * the main surface. Additional surface is used to pass the Clear Color
+ * structure for the driver to program the DE.
+ */
+static const struct drm_format_info gen12_ccs_cc_formats[] = {
+	{ .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
+	{ .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
+	{ .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
+	{ .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 3, .cpp = { 4, 1, 0}, .hsub = 2, .vsub = 32, },
+};
+
 static const struct drm_format_info *
 lookup_format_info(const struct drm_format_info formats[],
 		   int num_formats, u32 format)
@@ -2525,6 +2544,10 @@  intel_get_format_info(const struct drm_mode_fb_cmd2 *cmd)
 		return lookup_format_info(gen12_ccs_formats,
 					  ARRAY_SIZE(gen12_ccs_formats),
 					  cmd->pixel_format);
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
+		return lookup_format_info(gen12_ccs_formats,
+					  ARRAY_SIZE(gen12_ccs_cc_formats),
+					  cmd->pixel_format);
 	default:
 		return NULL;
 	}
@@ -2534,6 +2557,7 @@  bool is_ccs_modifier(u64 modifier)
 {
 	return modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
 	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS ||
+	       modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC ||
 	       modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
 	       modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
 }
@@ -4120,6 +4144,9 @@  static u32 skl_plane_ctl_tiling(u64 fb_modifier)
 	case I915_FORMAT_MOD_Y_TILED_CCS:
 		/* fall through */
 	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
+		return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE |
+			ICL_PLANE_CTL_CLEAR_COLOR_DISABLE;
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
 		return PLANE_CTL_TILED_Y | PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
 	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
 		return PLANE_CTL_TILED_Y | PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE;
@@ -9897,9 +9924,13 @@  skylake_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 = INTEL_GEN(dev_priv) >= 12 ?
-				I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
-				I915_FORMAT_MOD_Y_TILED_CCS;
+			if (INTEL_GEN(dev_priv) >= 12)
+				fb->modifier = val &
+					ICL_PLANE_CTL_CLEAR_COLOR_DISABLE ?
+					I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS :
+					I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC;
+			else
+				fb->modifier = I915_FORMAT_MOD_Y_TILED_CCS;
 		else if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
 			fb->modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS;
 		else
@@ -14322,6 +14353,15 @@  static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
 
 	plane_state->vma = vma;
 
+	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC) {
+		u32 *ccaddr = kmap_atomic(i915_gem_object_get_page(intel_fb_obj(fb),
+								  fb->offsets[2] >> PAGE_SHIFT));
+
+		plane_state->ccval = ((u64)*(ccaddr + CC_VAL_HIGHER_OFFSET) << 32)
+				     | *(ccaddr + CC_VAL_LOWER_OFFSET);
+		kunmap_atomic(ccaddr);
+	}
+
 	return 0;
 }
 
@@ -15709,6 +15749,7 @@  static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 		 * tile widths.
 		 */
 		if ((fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
+		     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC ||
 		     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS) &&
 		    i == 0)
 			stride_alignment *= 4;
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index d5cc4b810d9e..a82695a3e0b8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -579,6 +579,9 @@  struct intel_plane_state {
 	u32 slave;
 
 	struct drm_intel_sprite_colorkey ckey;
+
+	/* Clear Color Value */
+	u64 ccval;
 };
 
 struct intel_initial_plane_config {
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 1655984955ca..3672e8b9c0bc 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -549,6 +549,7 @@  skl_program_plane(struct intel_plane *plane,
 	u32 plane_color_ctl = 0;
 	unsigned long irqflags;
 	u32 keymsk, keymax;
+	u64 ccval = plane_state->ccval;
 
 	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
 
@@ -609,6 +610,10 @@  skl_program_plane(struct intel_plane *plane,
 	if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id))
 		icl_program_input_csc(plane, crtc_state, plane_state);
 
+	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)
+		intel_uncore_write64_fw(&dev_priv->uncore,
+					PLANE_CC_VAL(pipe, plane_id), ccval);
+
 	skl_write_plane_wm(plane, crtc_state);
 
 	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
@@ -1738,7 +1743,8 @@  static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
 	     fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
 	     fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
 	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
-	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS)) {
+	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS ||
+	     fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC)) {
 		DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
 		return -EINVAL;
 	}
@@ -2153,6 +2159,7 @@  static const u64 skl_plane_format_modifiers_ccs[] = {
 static const u64 gen12_plane_format_modifiers_ccs[] = {
 	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
 	I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
+	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC,
 	I915_FORMAT_MOD_Y_TILED,
 	I915_FORMAT_MOD_X_TILED,
 	DRM_FORMAT_MOD_LINEAR,
@@ -2321,6 +2328,7 @@  static bool gen12_plane_format_mod_supported(struct drm_plane *_plane,
 	case I915_FORMAT_MOD_X_TILED:
 	case I915_FORMAT_MOD_Y_TILED:
 	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
 		break;
 	default:
 		return false;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 096be95ec7f9..00c59ed8b114 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6681,6 +6681,7 @@  enum {
 #define   PLANE_CTL_YUV422_VYUY			(3 << 16)
 #define   PLANE_CTL_RENDER_DECOMPRESSION_ENABLE	(1 << 15)
 #define   PLANE_CTL_TRICKLE_FEED_DISABLE	(1 << 14)
+#define	  ICL_PLANE_CTL_CLEAR_COLOR_DISABLE	(1 << 13)
 #define   PLANE_CTL_PLANE_GAMMA_DISABLE		(1 << 13) /* Pre-GLK */
 #define   PLANE_CTL_TILED_MASK			(0x7 << 10)
 #define   PLANE_CTL_TILED_LINEAR		(0 << 10)
@@ -6721,6 +6722,8 @@  enum {
 #define _PLANE_KEYMAX_1_A			0x701a0
 #define _PLANE_KEYMAX_2_A			0x702a0
 #define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
+#define _PLANE_CC_VAL_1_A			0x701b4
+#define _PLANE_CC_VAL_2_A			0x702b4
 #define _PLANE_AUX_DIST_1_A			0x701c0
 #define _PLANE_AUX_DIST_2_A			0x702c0
 #define _PLANE_AUX_OFFSET_1_A			0x701c4
@@ -6760,6 +6763,16 @@  enum {
 #define _PLANE_NV12_BUF_CFG_1_A		0x70278
 #define _PLANE_NV12_BUF_CFG_2_A		0x70378
 
+#define _PLANE_CC_VAL_1_B			0x711b4
+#define _PLANE_CC_VAL_2_B			0x712b4
+#define _PLANE_CC_VAL_1(pipe)	_PIPE(pipe, _PLANE_CC_VAL_1_A, _PLANE_CC_VAL_1_B)
+#define _PLANE_CC_VAL_2(pipe)	_PIPE(pipe, _PLANE_CC_VAL_2_A, _PLANE_CC_VAL_2_B)
+#define PLANE_CC_VAL(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_CC_VAL_1(pipe), _PLANE_CC_VAL_2(pipe))
+
+#define CC_VAL_LOWER_OFFSET		4
+#define CC_VAL_HIGHER_OFFSET		5
+
 /* Input CSC Register Definitions */
 #define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
 #define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0