[Intel-gfx] [PATCH v2 1/4] drm/i915/fbc: Parametrize FBC register offsets

Jani Nikula jani.nikula at intel.com
Mon Dec 13 19:54:04 UTC 2021


On Mon, 13 Dec 2021, Ville Syrjala <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Parametrize ilk+ FBC register offsets based on the FBC instance.
>
> v2: More intel_ namespace (Jani)
>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Some questions below, apart from that,

Reviewed-by: Jani Nikula <jani.nikula at intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++++++-----------
>  drivers/gpu/drm/i915/display/intel_fbc.h |  6 +++++
>  drivers/gpu/drm/i915/i915_reg.h          | 34 ++++++++++++------------
>  drivers/gpu/drm/i915/intel_pm.c          | 31 ++++++++++++---------
>  4 files changed, 60 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 8be01b93015f..112aafa72253 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -85,6 +85,8 @@ struct intel_fbc {
>  	struct drm_mm_node compressed_fb;
>  	struct drm_mm_node compressed_llb;
>  
> +	enum intel_fbc_id id;
> +
>  	u8 limit;
>  
>  	bool false_color;
> @@ -454,10 +456,10 @@ static void ilk_fbc_activate(struct intel_fbc *fbc)
>  	struct intel_fbc_state *fbc_state = &fbc->state;
>  	struct drm_i915_private *i915 = fbc->i915;
>  
> -	intel_de_write(i915, ILK_DPFC_FENCE_YOFF,
> +	intel_de_write(i915, ILK_DPFC_FENCE_YOFF(fbc->id),
>  		       fbc_state->fence_y_offset);
>  
> -	intel_de_write(i915, ILK_DPFC_CONTROL,
> +	intel_de_write(i915, ILK_DPFC_CONTROL(fbc->id),
>  		       DPFC_CTL_EN | g4x_dpfc_ctl(fbc));
>  }
>  
> @@ -467,28 +469,28 @@ static void ilk_fbc_deactivate(struct intel_fbc *fbc)
>  	u32 dpfc_ctl;
>  
>  	/* Disable compression */
> -	dpfc_ctl = intel_de_read(i915, ILK_DPFC_CONTROL);
> +	dpfc_ctl = intel_de_read(i915, ILK_DPFC_CONTROL(fbc->id));
>  	if (dpfc_ctl & DPFC_CTL_EN) {
>  		dpfc_ctl &= ~DPFC_CTL_EN;
> -		intel_de_write(i915, ILK_DPFC_CONTROL, dpfc_ctl);
> +		intel_de_write(i915, ILK_DPFC_CONTROL(fbc->id), dpfc_ctl);
>  	}
>  }
>  
>  static bool ilk_fbc_is_active(struct intel_fbc *fbc)
>  {
> -	return intel_de_read(fbc->i915, ILK_DPFC_CONTROL) & DPFC_CTL_EN;
> +	return intel_de_read(fbc->i915, ILK_DPFC_CONTROL(fbc->id)) & DPFC_CTL_EN;
>  }
>  
>  static bool ilk_fbc_is_compressing(struct intel_fbc *fbc)
>  {
> -	return intel_de_read(fbc->i915, ILK_DPFC_STATUS) & DPFC_COMP_SEG_MASK;
> +	return intel_de_read(fbc->i915, ILK_DPFC_STATUS(fbc->id)) & DPFC_COMP_SEG_MASK;
>  }
>  
>  static void ilk_fbc_program_cfb(struct intel_fbc *fbc)
>  {
>  	struct drm_i915_private *i915 = fbc->i915;
>  
> -	intel_de_write(i915, ILK_DPFC_CB_BASE, fbc->compressed_fb.start);
> +	intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc->compressed_fb.start);
>  }
>  
>  static const struct intel_fbc_funcs ilk_fbc_funcs = {
> @@ -524,8 +526,8 @@ static void snb_fbc_nuke(struct intel_fbc *fbc)
>  {
>  	struct drm_i915_private *i915 = fbc->i915;
>  
> -	intel_de_write(i915, MSG_FBC_REND_STATE, FBC_REND_NUKE);
> -	intel_de_posting_read(i915, MSG_FBC_REND_STATE);
> +	intel_de_write(i915, MSG_FBC_REND_STATE(fbc->id), FBC_REND_NUKE);
> +	intel_de_posting_read(i915, MSG_FBC_REND_STATE(fbc->id));
>  }
>  
>  static const struct intel_fbc_funcs snb_fbc_funcs = {
> @@ -547,7 +549,7 @@ static void glk_fbc_program_cfb_stride(struct intel_fbc *fbc)
>  		val |= FBC_STRIDE_OVERRIDE |
>  			FBC_STRIDE(fbc_state->override_cfb_stride / fbc->limit);
>  
> -	intel_de_write(i915, GLK_FBC_STRIDE, val);
> +	intel_de_write(i915, GLK_FBC_STRIDE(fbc->id), val);
>  }
>  
>  static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc)
> @@ -598,19 +600,19 @@ static void ivb_fbc_activate(struct intel_fbc *fbc)
>  	if (i915->ggtt.num_fences)
>  		snb_fbc_program_fence(fbc);
>  
> -	intel_de_write(i915, ILK_DPFC_CONTROL,
> +	intel_de_write(i915, ILK_DPFC_CONTROL(fbc->id),
>  		       DPFC_CTL_EN | ivb_dpfc_ctl(fbc));
>  }
>  
>  static bool ivb_fbc_is_compressing(struct intel_fbc *fbc)
>  {
> -	return intel_de_read(fbc->i915, ILK_DPFC_STATUS2) & DPFC_COMP_SEG_MASK_IVB;
> +	return intel_de_read(fbc->i915, ILK_DPFC_STATUS2(fbc->id)) & DPFC_COMP_SEG_MASK_IVB;
>  }
>  
>  static void ivb_fbc_set_false_color(struct intel_fbc *fbc,
>  				    bool enable)
>  {
> -	intel_de_rmw(fbc->i915, ILK_DPFC_CONTROL,
> +	intel_de_rmw(fbc->i915, ILK_DPFC_CONTROL(fbc->id),
>  		     DPFC_CTL_FALSE_COLOR, enable ? DPFC_CTL_FALSE_COLOR : 0);
>  }
>  
> @@ -1620,7 +1622,8 @@ void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane)
>  	fbc->possible_framebuffer_bits |= plane->frontbuffer_bit;
>  }
>  
> -static struct intel_fbc *intel_fbc_create(struct drm_i915_private *i915)
> +static struct intel_fbc *intel_fbc_create(struct drm_i915_private *i915,
> +					  enum intel_fbc_id fbc_id)
>  {
>  	struct intel_fbc *fbc;
>  
> @@ -1628,6 +1631,7 @@ static struct intel_fbc *intel_fbc_create(struct drm_i915_private *i915)
>  	if (!fbc)
>  		return NULL;
>  
> +	fbc->id = fbc_id;
>  	fbc->i915 = i915;
>  	INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn);
>  	mutex_init(&fbc->lock);
> @@ -1671,7 +1675,7 @@ void intel_fbc_init(struct drm_i915_private *i915)
>  	if (!HAS_FBC(i915))
>  		return;
>  
> -	fbc = intel_fbc_create(i915);
> +	fbc = intel_fbc_create(i915, INTEL_FBC_A);
>  	if (!fbc)
>  		return;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h
> index 07ad0411fcc3..7b7631aec527 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.h
> @@ -17,6 +17,12 @@ struct intel_fbc;
>  struct intel_plane;
>  struct intel_plane_state;
>  
> +enum intel_fbc_id {
> +	INTEL_FBC_A,
> +
> +	I915_MAX_FBCS,
> +};
> +
>  int intel_fbc_atomic_check(struct intel_atomic_state *state);
>  bool intel_fbc_pre_update(struct intel_atomic_state *state,
>  			  struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d27ba273cc68..698a023e70f5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3386,10 +3386,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define FBC_LL_SIZE		(1536)
>  
>  /* Framebuffer compression for GM45+ */
> -#define DPFC_CB_BASE		_MMIO(0x3200)
> -#define ILK_DPFC_CB_BASE	_MMIO(0x43200)
> -#define DPFC_CONTROL		_MMIO(0x3208)
> -#define ILK_DPFC_CONTROL	_MMIO(0x43208)
> +#define DPFC_CB_BASE			_MMIO(0x3200)
> +#define ILK_DPFC_CB_BASE(fbc_id)	_MMIO_PIPE((fbc_id), 0x43200, 0x43240)
> +#define DPFC_CONTROL			_MMIO(0x3208)
> +#define ILK_DPFC_CONTROL(fbc_id)	_MMIO_PIPE((fbc_id), 0x43208, 0x43248)
>  #define   DPFC_CTL_EN				REG_BIT(31)
>  #define   DPFC_CTL_PLANE_MASK_G4X		REG_BIT(30) /* g4x-snb */
>  #define   DPFC_CTL_PLANE_G4X(i9xx_plane)	REG_FIELD_PREP(DPFC_CTL_PLANE_MASK_G4X, (i9xx_plane))
> @@ -3407,28 +3407,28 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define   DPFC_CTL_LIMIT_4X			REG_FIELD_PREP(DPFC_CTL_LIMIT_MASK, 2)
>  #define   DPFC_CTL_FENCENO_MASK			REG_GENMASK(3, 0)
>  #define   DPFC_CTL_FENCENO(fence)		REG_FIELD_PREP(DPFC_CTL_FENCENO_MASK, (fence))
> -#define DPFC_RECOMP_CTL		_MMIO(0x320c)
> -#define ILK_DPFC_RECOMP_CTL	_MMIO(0x4320c)
> +#define DPFC_RECOMP_CTL			_MMIO(0x320c)
> +#define ILK_DPFC_RECOMP_CTL(fbc_id)	_MMIO_PIPE((fbc_id), 0x4320c, 0x4324c)

This is display 5 and 6 only, right? Will there be a register instance
for fbc_id > INTEL_FBC_A? Or is the parametrization just for
completeness?

This one is only used in gvt, anyway. And that actually makes me wonder
if this should be breaking the build. Does CI not have gvt enabled?

>  #define   DPFC_RECOMP_STALL_EN			REG_BIT(27)
>  #define   DPFC_RECOMP_STALL_WM_MASK		REG_GENMASK(26, 16)
>  #define   DPFC_RECOMP_TIMER_COUNT_MASK		REG_GENMASK(5, 0)
> -#define DPFC_STATUS		_MMIO(0x3210)
> -#define ILK_DPFC_STATUS		_MMIO(0x43210)
> +#define DPFC_STATUS			_MMIO(0x3210)
> +#define ILK_DPFC_STATUS(fbc_id)		_MMIO_PIPE((fbc_id), 0x43210, 0x43250)

Ditto, apart from the gvt part.

>  #define   DPFC_INVAL_SEG_MASK			REG_GENMASK(26, 16)
>  #define   DPFC_COMP_SEG_MASK			REG_GENMASK(10, 0)
> -#define DPFC_STATUS2		_MMIO(0x3214)
> -#define ILK_DPFC_STATUS2		_MMIO(0x43214)
> +#define DPFC_STATUS2			_MMIO(0x3214)
> +#define ILK_DPFC_STATUS2(fbc_id)	_MMIO_PIPE((fbc_id), 0x43214, 0x43254)
>  #define   DPFC_COMP_SEG_MASK_IVB		REG_GENMASK(11, 0)
> -#define DPFC_FENCE_YOFF		_MMIO(0x3218)
> -#define ILK_DPFC_FENCE_YOFF	_MMIO(0x43218)
> -#define DPFC_CHICKEN		_MMIO(0x3224)
> -#define ILK_DPFC_CHICKEN	_MMIO(0x43224)
> +#define DPFC_FENCE_YOFF			_MMIO(0x3218)
> +#define ILK_DPFC_FENCE_YOFF(fbc_id)	_MMIO_PIPE((fbc_id), 0x43218, 0x43258)

Ditto.

BR,
Jani.

> +#define DPFC_CHICKEN			_MMIO(0x3224)
> +#define ILK_DPFC_CHICKEN(fbc_id)	_MMIO_PIPE((fbc_id), 0x43224, 0x43264)
>  #define   DPFC_HT_MODIFY			REG_BIT(31) /* pre-ivb */
>  #define   DPFC_NUKE_ON_ANY_MODIFICATION		REG_BIT(23) /* bdw+ */
>  #define   DPFC_CHICKEN_COMP_DUMMY_PIXEL		REG_BIT(14) /* glk+ */
>  #define   DPFC_DISABLE_DUMMY0			REG_BIT(8) /* ivb+ */
>  
> -#define GLK_FBC_STRIDE		_MMIO(0x43228)
> +#define GLK_FBC_STRIDE(fbc_id)	_MMIO_PIPE((fbc_id), 0x43228, 0x43268)
>  #define   FBC_STRIDE_OVERRIDE	REG_BIT(15)
>  #define   FBC_STRIDE_MASK	REG_GENMASK(14, 0)
>  #define   FBC_STRIDE(x)		REG_FIELD_PREP(FBC_STRIDE_MASK, (x))
> @@ -3471,9 +3471,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define IPS_CTL		_MMIO(0x43408)
>  #define   IPS_ENABLE	(1 << 31)
>  
> -#define MSG_FBC_REND_STATE	_MMIO(0x50380)
> +#define MSG_FBC_REND_STATE(fbc_id)	_MMIO_PIPE((fbc_id), 0x50380, 0x50384)
>  #define   FBC_REND_NUKE			REG_BIT(2)
> -#define   FBC_REND_CACHE_CLEAN			REG_BIT(1)
> +#define   FBC_REND_CACHE_CLEAN		REG_BIT(1)
>  
>  /*
>   * GPIO regs
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 434b1f8b7fe3..bdf97a8c9ef3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -160,8 +160,9 @@ static void bxt_init_clock_gating(struct drm_i915_private *dev_priv)
>  	 * WaFbcHighMemBwCorruptionAvoidance:bxt
>  	 * Display WA #0883: bxt
>  	 */
> -	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN, intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN) |
> -		   DPFC_DISABLE_DUMMY0);
> +	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
> +			   intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A)) |
> +			   DPFC_DISABLE_DUMMY0);
>  }
>  
>  static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
> @@ -7451,8 +7452,8 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
>  static void icl_init_clock_gating(struct drm_i915_private *dev_priv)
>  {
>  	/* Wa_1409120013:icl,ehl */
> -	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN,
> -		   DPFC_CHICKEN_COMP_DUMMY_PIXEL);
> +	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
> +			   DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>  
>  	/*Wa_14010594013:icl, ehl */
>  	intel_uncore_rmw(&dev_priv->uncore, GEN8_CHICKEN_DCPR_1,
> @@ -7464,7 +7465,7 @@ static void gen12lp_init_clock_gating(struct drm_i915_private *dev_priv)
>  	/* Wa_1409120013:tgl,rkl,adl-s,dg1,dg2 */
>  	if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv) ||
>  	    IS_ALDERLAKE_S(dev_priv) || IS_DG1(dev_priv) || IS_DG2(dev_priv))
> -		intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN,
> +		intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
>  				   DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>  
>  	/* Wa_1409825376:tgl (pre-prod)*/
> @@ -7549,8 +7550,9 @@ static void cfl_init_clock_gating(struct drm_i915_private *dev_priv)
>  	 * WaFbcNukeOnHostModify:cfl
>  	 * Display WA #0873: cfl
>  	 */
> -	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN, intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN) |
> -		   DPFC_NUKE_ON_ANY_MODIFICATION);
> +	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
> +			   intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A)) |
> +			   DPFC_NUKE_ON_ANY_MODIFICATION);
>  }
>  
>  static void kbl_init_clock_gating(struct drm_i915_private *dev_priv)
> @@ -7582,8 +7584,9 @@ static void kbl_init_clock_gating(struct drm_i915_private *dev_priv)
>  	 * WaFbcNukeOnHostModify:kbl
>  	 * Display WA #0873: kbl
>  	 */
> -	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN, intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN) |
> -		   DPFC_NUKE_ON_ANY_MODIFICATION);
> +	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
> +			   intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A)) |
> +			   DPFC_NUKE_ON_ANY_MODIFICATION);
>  }
>  
>  static void skl_init_clock_gating(struct drm_i915_private *dev_priv)
> @@ -7609,15 +7612,17 @@ static void skl_init_clock_gating(struct drm_i915_private *dev_priv)
>  	 * WaFbcNukeOnHostModify:skl
>  	 * Display WA #0873: skl
>  	 */
> -	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN, intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN) |
> -		   DPFC_NUKE_ON_ANY_MODIFICATION);
> +	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
> +			   intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A)) |
> +			   DPFC_NUKE_ON_ANY_MODIFICATION);
>  
>  	/*
>  	 * WaFbcHighMemBwCorruptionAvoidance:skl
>  	 * Display WA #0883: skl
>  	 */
> -	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN, intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN) |
> -		   DPFC_DISABLE_DUMMY0);
> +	intel_uncore_write(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
> +			   intel_uncore_read(&dev_priv->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A)) |
> +			   DPFC_DISABLE_DUMMY0);
>  }
>  
>  static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center


More information about the Intel-gfx mailing list