[PATCH 0/2] Implement Wa 14019834836

[PATCH 0/2] Implement Wa 14019834836

[Jouni Högander]

This patch set implements Wa 14019834836. Also a new helper is added
to apply workarounds for selective update area.

Jouni Högander (2):
  drm/i915/psr: Add new SU area calculation helper to apply workarounds
  drm/i915/psr: Implement Wa 14019834836

 drivers/gpu/drm/i915/display/intel_psr.c | 52 ++++++++++++++++++++----
 1 file changed, 45 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm/i915/psr: Add new SU area calculation helper to apply workarounds

[Jouni Högander]

intel_psr2_sel_fetch_update is already quite long function. Now we are
about to add one more HW workaround. Let's split applying workarounds to
selective update area into a separate function.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 5b355d0a35652..f7dfd9435b589 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2488,11 +2488,24 @@ static bool psr2_sel_fetch_pipe_state_supported(const struct intel_crtc_state *c
 	return true;
 }
 
+static void
+intel_psr_apply_su_area_workarounds(struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+	/* Wa_14014971492 */
+	if (!crtc_state->has_panel_replay &&
+	    ((IS_DISPLAY_VER_STEP(display, IP_VER(14, 0), STEP_A0, STEP_B0) ||
+	      IS_ALDERLAKE_P(i915) || IS_TIGERLAKE(i915))) &&
+	    crtc_state->splitter.enable)
+		crtc_state->psr2_su_area.y1 = 0;
+}
+
 int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 				struct intel_crtc *crtc)
 {
 	struct intel_display *display = to_intel_display(state);
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_plane_state *new_plane_state, *old_plane_state;
 	struct intel_plane *plane;
@@ -2597,12 +2610,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 	if (full_update)
 		goto skip_sel_fetch_set_loop;
 
-	/* Wa_14014971492 */
-	if (!crtc_state->has_panel_replay &&
-	    ((IS_DISPLAY_VER_STEP(display, IP_VER(14, 0), STEP_A0, STEP_B0) ||
-	      IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv))) &&
-	    crtc_state->splitter.enable)
-		crtc_state->psr2_su_area.y1 = 0;
+	intel_psr_apply_su_area_workarounds(crtc_state);
 
 	ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
 	if (ret)
-- 
2.34.1

[PATCH 2/2] drm/i915/psr: Implement Wa 14019834836

[Jouni Högander]

This patch implements HW workaround 14019834836 for display version >= 30.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 30 ++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index f7dfd9435b589..e529132da93a4 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2493,6 +2493,8 @@ intel_psr_apply_su_area_workarounds(struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
 	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+	struct intel_encoder *encoder;
+	int hactive_limit;
 
 	/* Wa_14014971492 */
 	if (!crtc_state->has_panel_replay &&
@@ -2500,6 +2502,34 @@ intel_psr_apply_su_area_workarounds(struct intel_crtc_state *crtc_state)
 	      IS_ALDERLAKE_P(i915) || IS_TIGERLAKE(i915))) &&
 	    crtc_state->splitter.enable)
 		crtc_state->psr2_su_area.y1 = 0;
+
+	/* Rest of the function is for Wa 14019834836 */
+	if (DISPLAY_VER(display) < 30)
+		return;
+
+	if (crtc_state->psr2_su_area.y1 != 0 ||
+	    crtc_state->psr2_su_area.y2 != 0)
+		return;
+
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		hactive_limit = intel_dp_is_uhbr(crtc_state) ? 1230 : 546;
+	else
+		hactive_limit = intel_dp_is_uhbr(crtc_state) ? 615 : 273;
+
+	if (crtc_state->hw.adjusted_mode.hdisplay < hactive_limit)
+		return;
+
+	for_each_intel_encoder_mask_with_psr(display->drm, encoder,
+					     crtc_state->uapi.encoder_mask) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp) &&
+		    intel_dp->psr.panel_replay_enabled &&
+		    intel_dp->psr.sel_update_enabled) {
+			crtc_state->psr2_su_area.y2++;
+			return;
+		}
+	}
 }
 
 int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
-- 
2.34.1

RE: [PATCH 1/2] drm/i915/psr: Add new SU area calculation helper to apply workarounds

[Kahola, Mika]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jouni
> Högander
> Sent: Monday, 23 September 2024 7.32
> To: intel-gfx@lists.freedesktop.org
> Cc: Hogander, Jouni <jouni.hogander@intel.com>
> Subject: [PATCH 1/2] drm/i915/psr: Add new SU area calculation helper to apply
> workarounds
> 
> intel_psr2_sel_fetch_update is already quite long function. Now we are about to
> add one more HW workaround. Let's split applying workarounds to selective
> update area into a separate function.
> 

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5b355d0a35652..f7dfd9435b589 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2488,11 +2488,24 @@ static bool
> psr2_sel_fetch_pipe_state_supported(const struct intel_crtc_state *c
>  	return true;
>  }
> 
> +static void
> +intel_psr_apply_su_area_workarounds(struct intel_crtc_state
> +*crtc_state) {
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> +	/* Wa_14014971492 */
> +	if (!crtc_state->has_panel_replay &&
> +	    ((IS_DISPLAY_VER_STEP(display, IP_VER(14, 0), STEP_A0, STEP_B0) ||
> +	      IS_ALDERLAKE_P(i915) || IS_TIGERLAKE(i915))) &&
> +	    crtc_state->splitter.enable)
> +		crtc_state->psr2_su_area.y1 = 0;
> +}
> +
>  int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc)
>  {
>  	struct intel_display *display = to_intel_display(state);
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc_state *crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
>  	struct intel_plane_state *new_plane_state, *old_plane_state;
>  	struct intel_plane *plane;
> @@ -2597,12 +2610,7 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  	if (full_update)
>  		goto skip_sel_fetch_set_loop;
> 
> -	/* Wa_14014971492 */
> -	if (!crtc_state->has_panel_replay &&
> -	    ((IS_DISPLAY_VER_STEP(display, IP_VER(14, 0), STEP_A0, STEP_B0) ||
> -	      IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv))) &&
> -	    crtc_state->splitter.enable)
> -		crtc_state->psr2_su_area.y1 = 0;
> +	intel_psr_apply_su_area_workarounds(crtc_state);
> 
>  	ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
>  	if (ret)
> --
> 2.34.1

Re: [PATCH 2/2] drm/i915/psr: Implement Wa 14019834836

[Gustavo Sousa]

Quoting Jouni Högander (2024-09-23 01:32:05-03:00)
>This patch implements HW workaround 14019834836 for display version >= 30.
>
>Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_psr.c | 30 ++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>index f7dfd9435b589..e529132da93a4 100644
>--- a/drivers/gpu/drm/i915/display/intel_psr.c
>+++ b/drivers/gpu/drm/i915/display/intel_psr.c
>@@ -2493,6 +2493,8 @@ intel_psr_apply_su_area_workarounds(struct intel_crtc_state *crtc_state)
> {
>         struct intel_display *display = to_intel_display(crtc_state);
>         struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
>+        struct intel_encoder *encoder;
>+        int hactive_limit;
> 
>         /* Wa_14014971492 */
>         if (!crtc_state->has_panel_replay &&
>@@ -2500,6 +2502,34 @@ intel_psr_apply_su_area_workarounds(struct intel_crtc_state *crtc_state)
>               IS_ALDERLAKE_P(i915) || IS_TIGERLAKE(i915))) &&
>             crtc_state->splitter.enable)
>                 crtc_state->psr2_su_area.y1 = 0;
>+
>+        /* Rest of the function is for Wa 14019834836 */
>+        if (DISPLAY_VER(display) < 30)
>+                return;

Should we really have the expectation that this workaround would extend
to future display IPs? Maybe it might be better to use an equality check
here, just so that we do not accidentally apply this to a future display
release that might have the bug fixed.

In that case, I would have the workaround logic in a separate function
and call it only for DISPLAY_VER(display) == 30.

--
Gustavo Sousa

>+
>+        if (crtc_state->psr2_su_area.y1 != 0 ||
>+            crtc_state->psr2_su_area.y2 != 0)
>+                return;
>+
>+        if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>+                hactive_limit = intel_dp_is_uhbr(crtc_state) ? 1230 : 546;
>+        else
>+                hactive_limit = intel_dp_is_uhbr(crtc_state) ? 615 : 273;
>+
>+        if (crtc_state->hw.adjusted_mode.hdisplay < hactive_limit)
>+                return;
>+
>+        for_each_intel_encoder_mask_with_psr(display->drm, encoder,
>+                                             crtc_state->uapi.encoder_mask) {
>+                struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>+
>+                if (!intel_dp_is_edp(intel_dp) &&
>+                    intel_dp->psr.panel_replay_enabled &&
>+                    intel_dp->psr.sel_update_enabled) {
>+                        crtc_state->psr2_su_area.y2++;
>+                        return;
>+                }
>+        }
> }
> 
> int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>-- 
>2.34.1
>