[Intel-gfx] [PATCH 1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values

[Intel-gfx] [PATCH 1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values

[José Roberto de Souza]

From: Caz Yokoyama <caz.yokoyama@intel.com>

B credits set by IFWI do not match with specification default, so here
programming the right value.

Also while at it, taking the oportunity to do a read-modify-write to
all other bit in this register that specification don't ask us to
change.

BSpec: 49213
BSpec: 50343
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7cd586d280883..2e85ae575423a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1825,15 +1825,20 @@ static void icl_pipe_mbus_enable(struct intel_crtc *crtc, bool joined_mbus)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
-	u32 val;
+	u32 val = intel_de_read(dev_priv, PIPE_MBUS_DBOX_CTL(pipe));
 
+	val &= ~MBUS_DBOX_A_CREDIT_MASK;
 	/* Wa_22010947358:adl-p */
 	if (IS_ALDERLAKE_P(dev_priv))
-		val = joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
+		val |= joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
 	else
-		val = MBUS_DBOX_A_CREDIT(2);
+		val |= MBUS_DBOX_A_CREDIT(2);
 
-	if (DISPLAY_VER(dev_priv) >= 12) {
+	val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
+	if (IS_ALDERLAKE_P(dev_priv)) {
+		val |= MBUS_DBOX_BW_CREDIT(2);
+		val |= MBUS_DBOX_B_CREDIT(8);
+	} else if (DISPLAY_VER(dev_priv) >= 12) {
 		val |= MBUS_DBOX_BW_CREDIT(2);
 		val |= MBUS_DBOX_B_CREDIT(12);
 	} else {
-- 
2.35.1

[Intel-gfx] [PATCH 2/3] drm/i915/display: Add HAS_MBUS_JOINING

[José Roberto de Souza]

This will make easy to extend MBUS joining support to future platforms
that also supports this feature.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 ++
 drivers/gpu/drm/i915/intel_pm.c | 6 +++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 97622d3ccfc2a..0f7f7ebe23cb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1379,6 +1379,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define HAS_PERCTX_PREEMPT_CTRL(i915) \
 	((GRAPHICS_VER(i915) >= 9) &&  GRAPHICS_VER_FULL(i915) < IP_VER(12, 55))
 
+#define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915))
+
 static inline bool run_as_guest(void)
 {
 	return !hypervisor_is_type(X86_HYPER_NATIVE);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8ee31c9590a7f..96bb8ecc11668 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6144,7 +6144,7 @@ skl_compute_ddb(struct intel_atomic_state *state)
 			return ret;
 	}
 
-	if (IS_ALDERLAKE_P(dev_priv))
+	if (HAS_MBUS_JOINING(dev_priv))
 		new_dbuf_state->joined_mbus =
 			adlp_check_mbus_joined(new_dbuf_state->active_pipes);
 
@@ -6636,7 +6636,7 @@ void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
 		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
 	struct intel_crtc *crtc;
 
-	if (IS_ALDERLAKE_P(dev_priv))
+	if (HAS_MBUS_JOINING(dev_priv))
 		dbuf_state->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN;
 
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
@@ -8299,7 +8299,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
 	const struct intel_dbuf_state *dbuf_state =
 		intel_atomic_get_new_dbuf_state(state);
 
-	if (!IS_ALDERLAKE_P(dev_priv))
+	if (!HAS_MBUS_JOINING(dev_priv))
 		return;
 
 	/*
-- 
2.35.1

Re: [Intel-gfx] [PATCH 2/3] drm/i915/display: Add HAS_MBUS_JOINING

[Ville Syrjälä]

On Fri, Mar 18, 2022 at 12:55:21PM -0700, José Roberto de Souza wrote:
> This will make easy to extend MBUS joining support to future platforms
> that also supports this feature.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 97622d3ccfc2a..0f7f7ebe23cb0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1379,6 +1379,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define HAS_PERCTX_PREEMPT_CTRL(i915) \
>  	((GRAPHICS_VER(i915) >= 9) &&  GRAPHICS_VER_FULL(i915) < IP_VER(12, 55))
>  
> +#define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915))
> +
>  static inline bool run_as_guest(void)
>  {
>  	return !hypervisor_is_type(X86_HYPER_NATIVE);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8ee31c9590a7f..96bb8ecc11668 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6144,7 +6144,7 @@ skl_compute_ddb(struct intel_atomic_state *state)
>  			return ret;
>  	}
>  
> -	if (IS_ALDERLAKE_P(dev_priv))
> +	if (HAS_MBUS_JOINING(dev_priv))
>  		new_dbuf_state->joined_mbus =
>  			adlp_check_mbus_joined(new_dbuf_state->active_pipes);
>  
> @@ -6636,7 +6636,7 @@ void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
>  		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
>  	struct intel_crtc *crtc;
>  
> -	if (IS_ALDERLAKE_P(dev_priv))
> +	if (HAS_MBUS_JOINING(dev_priv))
>  		dbuf_state->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN;
>  
>  	for_each_intel_crtc(&dev_priv->drm, crtc) {
> @@ -8299,7 +8299,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  	const struct intel_dbuf_state *dbuf_state =
>  		intel_atomic_get_new_dbuf_state(state);
>  
> -	if (!IS_ALDERLAKE_P(dev_priv))
> +	if (!HAS_MBUS_JOINING(dev_priv))
>  		return;
>  
>  	/*
> -- 
> 2.35.1

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 3/3] drm/i915/display/adlp: Fix programing of PIPE_MBUS_DBOX_CTL

[José Roberto de Souza]

PIPE_MBUS_DBOX_CTL was only being programmed when a pipe is being
enabled leaving other pipes with a wrong A_CREDIT value in cases
like when going from one pipe enabled to two pipes and the first
pipe don't need modeset, similar when going from two or more
pipes to ones.

So here moving the PIPE_MBUS_DBOX_CTL programing to be executed before
the function that enables and updates all necessary pipes.
Leaving all pipes with the correct value of A_CREDIT.

As now PIPE_MBUS_DBOX_CTL is being programmed at the right time it
is also waiting the vblanks after adjust PIPE_MBUS_DBOX_CTL
as required by specification.

BSpec: 49213
BSpec: 50343
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 36 +------------
 drivers/gpu/drm/i915/intel_pm.c              | 55 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_pm.h              |  1 +
 3 files changed, 56 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2e85ae575423a..4cd2d76058b8c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1821,34 +1821,6 @@ static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
 	intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe), val);
 }
 
-static void icl_pipe_mbus_enable(struct intel_crtc *crtc, bool joined_mbus)
-{
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe;
-	u32 val = intel_de_read(dev_priv, PIPE_MBUS_DBOX_CTL(pipe));
-
-	val &= ~MBUS_DBOX_A_CREDIT_MASK;
-	/* Wa_22010947358:adl-p */
-	if (IS_ALDERLAKE_P(dev_priv))
-		val |= joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
-	else
-		val |= MBUS_DBOX_A_CREDIT(2);
-
-	val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
-	if (IS_ALDERLAKE_P(dev_priv)) {
-		val |= MBUS_DBOX_BW_CREDIT(2);
-		val |= MBUS_DBOX_B_CREDIT(8);
-	} else if (DISPLAY_VER(dev_priv) >= 12) {
-		val |= MBUS_DBOX_BW_CREDIT(2);
-		val |= MBUS_DBOX_B_CREDIT(12);
-	} else {
-		val |= MBUS_DBOX_BW_CREDIT(1);
-		val |= MBUS_DBOX_B_CREDIT(8);
-	}
-
-	intel_de_write(dev_priv, PIPE_MBUS_DBOX_CTL(pipe), val);
-}
-
 static void hsw_set_linetime_wm(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -1984,13 +1956,6 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 
 	intel_initial_watermarks(state, crtc);
 
-	if (DISPLAY_VER(dev_priv) >= 11) {
-		const struct intel_dbuf_state *dbuf_state =
-				intel_atomic_get_new_dbuf_state(state);
-
-		icl_pipe_mbus_enable(crtc, dbuf_state->joined_mbus);
-	}
-
 	if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
 		intel_crtc_vblank_on(new_crtc_state);
 
@@ -8589,6 +8554,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	intel_encoders_update_prepare(state);
 
 	intel_dbuf_pre_plane_update(state);
+	intel_mbus_dbox_update(state);
 
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		if (new_crtc_state->do_async_flip)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 96bb8ecc11668..08ba32e5eb4ad 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6172,7 +6172,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
 			return ret;
 
 		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
-			/* TODO: Implement vblank synchronized MBUS joining changes */
 			ret = intel_modeset_all_pipes(state);
 			if (ret)
 				return ret;
@@ -8365,3 +8364,57 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
 	gen9_dbuf_slices_update(dev_priv,
 				new_dbuf_state->enabled_slices);
 }
+
+void intel_mbus_dbox_update(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *i915 = to_i915(state->base.dev);
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+	struct intel_dbuf_state *old_dbuf_state, *new_dbuf_state;
+	struct intel_crtc *crtc;
+	int i;
+
+	if (DISPLAY_VER(i915) < 11 || !state->modeset)
+		return;
+
+	if (HAS_MBUS_JOINING(i915)) {
+		new_dbuf_state = intel_atomic_get_dbuf_state(state);
+		old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
+	}
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		u32 val;
+
+		val = intel_de_read(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe));
+		val &= ~MBUS_DBOX_A_CREDIT_MASK;
+
+		/* Wa_22010947358:adl-p */
+		if (IS_ALDERLAKE_P(i915))
+			val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(6) :
+							     MBUS_DBOX_A_CREDIT(4);
+		else
+			val |= MBUS_DBOX_A_CREDIT(2);
+
+		if (IS_ALDERLAKE_P(i915)) {
+			val |= MBUS_DBOX_BW_CREDIT(2);
+			val |= MBUS_DBOX_B_CREDIT(8);
+		} else if (DISPLAY_VER(i915) >= 12) {
+			val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
+			val |= MBUS_DBOX_BW_CREDIT(2);
+			val |= MBUS_DBOX_B_CREDIT(12);
+		} else {
+			val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
+			val |= MBUS_DBOX_BW_CREDIT(1);
+			val |= MBUS_DBOX_B_CREDIT(8);
+		}
+
+		intel_de_write(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe), val);
+
+		/* If going from joined to not joined, wait a vblank */
+		if (HAS_MBUS_JOINING(i915) &&
+		    old_crtc_state->hw.active &&
+		    new_crtc_state->hw.active &&
+		    old_dbuf_state->joined_mbus &&
+		    !new_dbuf_state->joined_mbus)
+			intel_crtc_wait_for_next_vblank(crtc);
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index 51705151b842f..50604cf7398c4 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -94,5 +94,6 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
 int intel_dbuf_init(struct drm_i915_private *dev_priv);
 void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
 void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
+void intel_mbus_dbox_update(struct intel_atomic_state *state);
 
 #endif /* __INTEL_PM_H__ */
-- 
2.35.1

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/adlp: Fix programing of PIPE_MBUS_DBOX_CTL

[Ville Syrjälä]

On Fri, Mar 18, 2022 at 12:55:22PM -0700, José Roberto de Souza wrote:
> PIPE_MBUS_DBOX_CTL was only being programmed when a pipe is being
> enabled leaving other pipes with a wrong A_CREDIT value in cases
> like when going from one pipe enabled to two pipes and the first
> pipe don't need modeset, similar when going from two or more
> pipes to ones.
> 
> So here moving the PIPE_MBUS_DBOX_CTL programing to be executed before
> the function that enables and updates all necessary pipes.
> Leaving all pipes with the correct value of A_CREDIT.
> 
> As now PIPE_MBUS_DBOX_CTL is being programmed at the right time it
> is also waiting the vblanks after adjust PIPE_MBUS_DBOX_CTL
> as required by specification.
> 
> BSpec: 49213
> BSpec: 50343
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 36 +------------
>  drivers/gpu/drm/i915/intel_pm.c              | 55 +++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_pm.h              |  1 +
>  3 files changed, 56 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2e85ae575423a..4cd2d76058b8c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1821,34 +1821,6 @@ static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
>  	intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe), val);
>  }
>  
> -static void icl_pipe_mbus_enable(struct intel_crtc *crtc, bool joined_mbus)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum pipe pipe = crtc->pipe;
> -	u32 val = intel_de_read(dev_priv, PIPE_MBUS_DBOX_CTL(pipe));
> -
> -	val &= ~MBUS_DBOX_A_CREDIT_MASK;
> -	/* Wa_22010947358:adl-p */
> -	if (IS_ALDERLAKE_P(dev_priv))
> -		val |= joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
> -	else
> -		val |= MBUS_DBOX_A_CREDIT(2);
> -
> -	val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> -	if (IS_ALDERLAKE_P(dev_priv)) {
> -		val |= MBUS_DBOX_BW_CREDIT(2);
> -		val |= MBUS_DBOX_B_CREDIT(8);
> -	} else if (DISPLAY_VER(dev_priv) >= 12) {
> -		val |= MBUS_DBOX_BW_CREDIT(2);
> -		val |= MBUS_DBOX_B_CREDIT(12);
> -	} else {
> -		val |= MBUS_DBOX_BW_CREDIT(1);
> -		val |= MBUS_DBOX_B_CREDIT(8);
> -	}
> -
> -	intel_de_write(dev_priv, PIPE_MBUS_DBOX_CTL(pipe), val);
> -}
> -
>  static void hsw_set_linetime_wm(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -1984,13 +1956,6 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>  
>  	intel_initial_watermarks(state, crtc);
>  
> -	if (DISPLAY_VER(dev_priv) >= 11) {
> -		const struct intel_dbuf_state *dbuf_state =
> -				intel_atomic_get_new_dbuf_state(state);
> -
> -		icl_pipe_mbus_enable(crtc, dbuf_state->joined_mbus);
> -	}
> -
>  	if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
>  		intel_crtc_vblank_on(new_crtc_state);
>  
> @@ -8589,6 +8554,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  	intel_encoders_update_prepare(state);
>  
>  	intel_dbuf_pre_plane_update(state);
> +	intel_mbus_dbox_update(state);
>  
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		if (new_crtc_state->do_async_flip)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 96bb8ecc11668..08ba32e5eb4ad 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6172,7 +6172,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
>  			return ret;
>  
>  		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> -			/* TODO: Implement vblank synchronized MBUS joining changes */
>  			ret = intel_modeset_all_pipes(state);
>  			if (ret)
>  				return ret;
> @@ -8365,3 +8364,57 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
>  	gen9_dbuf_slices_update(dev_priv,
>  				new_dbuf_state->enabled_slices);
>  }
> +
> +void intel_mbus_dbox_update(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *i915 = to_i915(state->base.dev);
> +	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> +	struct intel_dbuf_state *old_dbuf_state, *new_dbuf_state;
> +	struct intel_crtc *crtc;
> +	int i;
> +
> +	if (DISPLAY_VER(i915) < 11 || !state->modeset)
> +		return;
> +
> +	if (HAS_MBUS_JOINING(i915)) {
> +		new_dbuf_state = intel_atomic_get_dbuf_state(state);
> +		old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
> +	}
> +
> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +		u32 val;
> +
> +		val = intel_de_read(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe));

In which power well does that live? and are we guaranteed to
have that enabled here?

I guess tou could just do something like
 if (!hw.active || !needs_modeset)
 	continue;
since I don't think there's much point in programming this
for inactive pipes, or pipes that have already been enabled
earlier. That should also avoid any power well issues.

> +		val &= ~MBUS_DBOX_A_CREDIT_MASK;
> +
> +		/* Wa_22010947358:adl-p */
> +		if (IS_ALDERLAKE_P(i915))
> +			val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(6) :
> +							     MBUS_DBOX_A_CREDIT(4);
> +		else
> +			val |= MBUS_DBOX_A_CREDIT(2);
> +
> +		if (IS_ALDERLAKE_P(i915)) {
> +			val |= MBUS_DBOX_BW_CREDIT(2);
> +			val |= MBUS_DBOX_B_CREDIT(8);
> +		} else if (DISPLAY_VER(i915) >= 12) {
> +			val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> +			val |= MBUS_DBOX_BW_CREDIT(2);
> +			val |= MBUS_DBOX_B_CREDIT(12);
> +		} else {
> +			val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> +			val |= MBUS_DBOX_BW_CREDIT(1);
> +			val |= MBUS_DBOX_B_CREDIT(8);
> +		}
> +
> +		intel_de_write(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe), val);
> +
> +		/* If going from joined to not joined, wait a vblank */
> +		if (HAS_MBUS_JOINING(i915) &&
> +		    old_crtc_state->hw.active &&
> +		    new_crtc_state->hw.active &&
> +		    old_dbuf_state->joined_mbus &&
> +		    !new_dbuf_state->joined_mbus)
> +			intel_crtc_wait_for_next_vblank(crtc);

That check does not guarantee the pipe is active when you call this.
It could be doing a enabled->enabled modeset. In fact that is guaranteed
to be the case since we anyway force a full modeset on everything when
changing mbus joining. So you can just nuke this vblank wait.

> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index 51705151b842f..50604cf7398c4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -94,5 +94,6 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
>  int intel_dbuf_init(struct drm_i915_private *dev_priv);
>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
>  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
> +void intel_mbus_dbox_update(struct intel_atomic_state *state);

AFAICS you could just call this from intel_dbuf_pre_plane_update()
instead of making the high level modeset code have to deal with it.

>  
>  #endif /* __INTEL_PM_H__ */
> -- 
> 2.35.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/adlp: Fix programing of PIPE_MBUS_DBOX_CTL

[Souza, Jose]

On Fri, 2022-03-18 at 23:28 +0200, Ville Syrjälä wrote:
> On Fri, Mar 18, 2022 at 12:55:22PM -0700, José Roberto de Souza wrote:
> > PIPE_MBUS_DBOX_CTL was only being programmed when a pipe is being
> > enabled leaving other pipes with a wrong A_CREDIT value in cases
> > like when going from one pipe enabled to two pipes and the first
> > pipe don't need modeset, similar when going from two or more
> > pipes to ones.
> > 
> > So here moving the PIPE_MBUS_DBOX_CTL programing to be executed before
> > the function that enables and updates all necessary pipes.
> > Leaving all pipes with the correct value of A_CREDIT.
> > 
> > As now PIPE_MBUS_DBOX_CTL is being programmed at the right time it
> > is also waiting the vblanks after adjust PIPE_MBUS_DBOX_CTL
> > as required by specification.
> > 
> > BSpec: 49213
> > BSpec: 50343
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 36 +------------
> >  drivers/gpu/drm/i915/intel_pm.c              | 55 +++++++++++++++++++-
> >  drivers/gpu/drm/i915/intel_pm.h              |  1 +
> >  3 files changed, 56 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 2e85ae575423a..4cd2d76058b8c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1821,34 +1821,6 @@ static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
> >  	intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe), val);
> >  }
> >  
> > -static void icl_pipe_mbus_enable(struct intel_crtc *crtc, bool joined_mbus)
> > -{
> > -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > -	enum pipe pipe = crtc->pipe;
> > -	u32 val = intel_de_read(dev_priv, PIPE_MBUS_DBOX_CTL(pipe));
> > -
> > -	val &= ~MBUS_DBOX_A_CREDIT_MASK;
> > -	/* Wa_22010947358:adl-p */
> > -	if (IS_ALDERLAKE_P(dev_priv))
> > -		val |= joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
> > -	else
> > -		val |= MBUS_DBOX_A_CREDIT(2);
> > -
> > -	val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> > -	if (IS_ALDERLAKE_P(dev_priv)) {
> > -		val |= MBUS_DBOX_BW_CREDIT(2);
> > -		val |= MBUS_DBOX_B_CREDIT(8);
> > -	} else if (DISPLAY_VER(dev_priv) >= 12) {
> > -		val |= MBUS_DBOX_BW_CREDIT(2);
> > -		val |= MBUS_DBOX_B_CREDIT(12);
> > -	} else {
> > -		val |= MBUS_DBOX_BW_CREDIT(1);
> > -		val |= MBUS_DBOX_B_CREDIT(8);
> > -	}
> > -
> > -	intel_de_write(dev_priv, PIPE_MBUS_DBOX_CTL(pipe), val);
> > -}
> > -
> >  static void hsw_set_linetime_wm(const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -1984,13 +1956,6 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
> >  
> >  	intel_initial_watermarks(state, crtc);
> >  
> > -	if (DISPLAY_VER(dev_priv) >= 11) {
> > -		const struct intel_dbuf_state *dbuf_state =
> > -				intel_atomic_get_new_dbuf_state(state);
> > -
> > -		icl_pipe_mbus_enable(crtc, dbuf_state->joined_mbus);
> > -	}
> > -
> >  	if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
> >  		intel_crtc_vblank_on(new_crtc_state);
> >  
> > @@ -8589,6 +8554,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >  	intel_encoders_update_prepare(state);
> >  
> >  	intel_dbuf_pre_plane_update(state);
> > +	intel_mbus_dbox_update(state);
> >  
> >  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> >  		if (new_crtc_state->do_async_flip)
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 96bb8ecc11668..08ba32e5eb4ad 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6172,7 +6172,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
> >  			return ret;
> >  
> >  		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> > -			/* TODO: Implement vblank synchronized MBUS joining changes */
> >  			ret = intel_modeset_all_pipes(state);
> >  			if (ret)
> >  				return ret;
> > @@ -8365,3 +8364,57 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
> >  	gen9_dbuf_slices_update(dev_priv,
> >  				new_dbuf_state->enabled_slices);
> >  }
> > +
> > +void intel_mbus_dbox_update(struct intel_atomic_state *state)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(state->base.dev);
> > +	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> > +	struct intel_dbuf_state *old_dbuf_state, *new_dbuf_state;
> > +	struct intel_crtc *crtc;
> > +	int i;
> > +
> > +	if (DISPLAY_VER(i915) < 11 || !state->modeset)
> > +		return;
> > +
> > +	if (HAS_MBUS_JOINING(i915)) {
> > +		new_dbuf_state = intel_atomic_get_dbuf_state(state);
> > +		old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
> > +	}
> > +
> > +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > +		u32 val;
> > +
> > +		val = intel_de_read(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe));
> 
> In which power well does that live? and are we guaranteed to
> have that enabled here?

No guarantee, I missed that and CI caught it too.

> 
> I guess tou could just do something like
>  if (!hw.active || !needs_modeset)
>  	continue;
> since I don't think there's much point in programming this
> for inactive pipes, or pipes that have already been enabled
> earlier. That should also avoid any power well issues.
> 
> > +		val &= ~MBUS_DBOX_A_CREDIT_MASK;
> > +
> > +		/* Wa_22010947358:adl-p */
> > +		if (IS_ALDERLAKE_P(i915))
> > +			val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(6) :
> > +							     MBUS_DBOX_A_CREDIT(4);
> > +		else
> > +			val |= MBUS_DBOX_A_CREDIT(2);
> > +
> > +		if (IS_ALDERLAKE_P(i915)) {
> > +			val |= MBUS_DBOX_BW_CREDIT(2);
> > +			val |= MBUS_DBOX_B_CREDIT(8);
> > +		} else if (DISPLAY_VER(i915) >= 12) {
> > +			val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> > +			val |= MBUS_DBOX_BW_CREDIT(2);
> > +			val |= MBUS_DBOX_B_CREDIT(12);
> > +		} else {
> > +			val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> > +			val |= MBUS_DBOX_BW_CREDIT(1);
> > +			val |= MBUS_DBOX_B_CREDIT(8);
> > +		}
> > +
> > +		intel_de_write(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe), val);
> > +
> > +		/* If going from joined to not joined, wait a vblank */
> > +		if (HAS_MBUS_JOINING(i915) &&
> > +		    old_crtc_state->hw.active &&
> > +		    new_crtc_state->hw.active &&
> > +		    old_dbuf_state->joined_mbus &&
> > +		    !new_dbuf_state->joined_mbus)
> > +			intel_crtc_wait_for_next_vblank(crtc);
> 
> That check does not guarantee the pipe is active when you call this.
> It could be doing a enabled->enabled modeset. In fact that is guaranteed
> to be the case since we anyway force a full modeset on everything when
> changing mbus joining. So you can just nuke this vblank wait.

oh okay, missed that intel_compute_global_watermarks() is called after intel_crtc_check_fastset().
So yep, all active pipes will be disabled when joined_mbus changes.

Will send a new version shortly, thanks for the feedback.

> 
> > +	}
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > index 51705151b842f..50604cf7398c4 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > @@ -94,5 +94,6 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
> >  int intel_dbuf_init(struct drm_i915_private *dev_priv);
> >  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
> >  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
> > +void intel_mbus_dbox_update(struct intel_atomic_state *state);
> 
> AFAICS you could just call this from intel_dbuf_pre_plane_update()
> instead of making the high level modeset code have to deal with it.
> 
> >  
> >  #endif /* __INTEL_PM_H__ */
> > -- 
> > 2.35.1
> 

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values
URL   : https://patchwork.freedesktop.org/series/101545/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7339 bytes --]

== Series Details ==

Series: series starting with [1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values
URL   : https://patchwork.freedesktop.org/series/101545/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11383 -> Patchwork_22615
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_22615 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22615, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/index.html

Participating hosts (44 -> 41)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (4): fi-bsw-cyan bat-adlm-1 shard-tglu fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22615:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_busy@basic@modeset:
    - fi-rkl-guc:         [PASS][1] -> [DMESG-WARN][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/fi-rkl-guc/igt@kms_busy@basic@modeset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-rkl-guc/igt@kms_busy@basic@modeset.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_busy@basic@flip:
    - {bat-jsl-1}:        [PASS][3] -> [DMESG-WARN][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/bat-jsl-1/igt@kms_busy@basic@flip.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/bat-jsl-1/igt@kms_busy@basic@flip.html
    - {bat-adlp-6}:       [PASS][5] -> [DMESG-WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/bat-adlp-6/igt@kms_busy@basic@flip.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/bat-adlp-6/igt@kms_busy@basic@flip.html

  * igt@kms_busy@basic@modeset:
    - {bat-jsl-2}:        [PASS][7] -> [DMESG-WARN][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/bat-jsl-2/igt@kms_busy@basic@modeset.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/bat-jsl-2/igt@kms_busy@basic@modeset.html
    - {fi-ehl-2}:         [PASS][9] -> [DMESG-WARN][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/fi-ehl-2/igt@kms_busy@basic@modeset.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-ehl-2/igt@kms_busy@basic@modeset.html
    - {fi-jsl-1}:         [PASS][11] -> [DMESG-WARN][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/fi-jsl-1/igt@kms_busy@basic@modeset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-jsl-1/igt@kms_busy@basic@modeset.html

  * igt@runner@aborted:
    - {fi-rkl-11600}:     NOTRUN -> [FAIL][13]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-rkl-11600/igt@runner@aborted.html
    - {fi-adl-ddr5}:      NOTRUN -> [FAIL][14]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-adl-ddr5/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_22615 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-gfx:
    - fi-hsw-4770:        NOTRUN -> [SKIP][15] ([fdo#109271] / [fdo#109315]) +17 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-hsw-4770/igt@amdgpu/amd_basic@cs-gfx.html

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-snb-2600:        NOTRUN -> [SKIP][16] ([fdo#109271]) +17 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-snb-2600/igt@amdgpu/amd_cs_nop@sync-fork-compute0.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
    - fi-pnv-d510:        NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#5341])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-pnv-d510/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html

  * igt@prime_vgem@basic-userptr:
    - fi-pnv-d510:        NOTRUN -> [SKIP][18] ([fdo#109271]) +57 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-pnv-d510/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][19] ([i915#2426] / [i915#4312])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-bdw-5557u/igt@runner@aborted.html
    - fi-rkl-guc:         NOTRUN -> [FAIL][20] ([i915#4312])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-rkl-guc/igt@runner@aborted.html
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][21] ([i915#5257])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-tgl-1115g4/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][22] ([i915#3303]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
    - fi-snb-2600:        [INCOMPLETE][24] ([i915#3921]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11383/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5291]: https://gitlab.freedesktop.org/drm/intel/issues/5291
  [i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341


Build changes
-------------

  * Linux: CI_DRM_11383 -> Patchwork_22615

  CI-20190529: 20190529
  CI_DRM_11383: 6e15005a5eec391b0c001d7fe3757f5c8af02d0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6386: 0fcd59ad25b2960c0b654f90dfe4dd9e7c7b874d @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22615: 34e038b460c04d09973bb09868ec014071f0b8eb @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

34e038b460c0 drm/i915/display/adlp: Fix programing of PIPE_MBUS_DBOX_CTL
04369577a128 drm/i915/display: Add HAS_MBUS_JOINING
ac69a50fd4ef drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22615/index.html

[-- Attachment #2: Type: text/html, Size: 8374 bytes --]

Re: [Intel-gfx] [PATCH 1/3] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values

[Jani Nikula]

On Fri, 18 Mar 2022, José Roberto de Souza <jose.souza@intel.com> wrote:
> From: Caz Yokoyama <caz.yokoyama@intel.com>
>
> B credits set by IFWI do not match with specification default, so here
> programming the right value.
>
> Also while at it, taking the oportunity to do a read-modify-write to
> all other bit in this register that specification don't ask us to
> change.
>
> BSpec: 49213
> BSpec: 50343
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7cd586d280883..2e85ae575423a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1825,15 +1825,20 @@ static void icl_pipe_mbus_enable(struct intel_crtc *crtc, bool joined_mbus)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
> -	u32 val;
> +	u32 val = intel_de_read(dev_priv, PIPE_MBUS_DBOX_CTL(pipe));

Please don't do functional stuff in variable initialization.

BR,
Jani.

>  
> +	val &= ~MBUS_DBOX_A_CREDIT_MASK;
>  	/* Wa_22010947358:adl-p */
>  	if (IS_ALDERLAKE_P(dev_priv))
> -		val = joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
> +		val |= joined_mbus ? MBUS_DBOX_A_CREDIT(6) : MBUS_DBOX_A_CREDIT(4);
>  	else
> -		val = MBUS_DBOX_A_CREDIT(2);
> +		val |= MBUS_DBOX_A_CREDIT(2);
>  
> -	if (DISPLAY_VER(dev_priv) >= 12) {
> +	val &= ~(MBUS_DBOX_BW_CREDIT_MASK | MBUS_DBOX_B_CREDIT_MASK);
> +	if (IS_ALDERLAKE_P(dev_priv)) {
> +		val |= MBUS_DBOX_BW_CREDIT(2);
> +		val |= MBUS_DBOX_B_CREDIT(8);
> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
>  		val |= MBUS_DBOX_BW_CREDIT(2);
>  		val |= MBUS_DBOX_B_CREDIT(12);
>  	} else {

-- 
Jani Nikula, Intel Open Source Graphics Center