All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used
@ 2022-03-21 10:49 Stanislav Lisovskiy
  2022-03-21 11:10 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used (rev2) Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: Stanislav Lisovskiy @ 2022-03-21 10:49 UTC (permalink / raw)
  To: intel-gfx

We are currently getting FIFO underruns, in particular
when PSR2 is enabled. There seem to be no existing workaround
or patches, which can fix that issue(were expecting some recent
selective fetch update and DBuf bw/SAGV fixes to help,
which unfortunately didn't).
Current idea is that it looks like for some reason the
DBuf prefill time isn't enough once we exit PSR2, despite its
theoretically correct.
So bump it up a bit by 15%(minimum experimental amount required
to get it working), if PSR2 is enabled.
For PSR1 there is no need in this hack, so we limit it only
to PSR2 and Alderlake.

v2: - Added comment(Jose Souza)
    - Fixed 15% calculation(Jose Souza)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 8888fda8b701..92d57869983a 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2325,6 +2325,32 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 					dev_priv->max_cdclk_freq));
 	}
 
+
+	/*
+	 * HACK.  We are getting FIFO underruns, in particular
+	 * when PSR2 is enabled. There seem to be no existing workaround
+	 * or patches as of now.
+	 * Current idea is that it looks like for some reason the
+	 * DBuf prefill time isn't enough once we exit PSR2, despite its
+	 * theoretically correct.
+	 * So bump it up a bit by 15%(minimum experimental amount required
+	 * to get it working), if PSR2 is enabled.
+	 * For PSR1 there is no need in this hack, so we limit it only
+	 * to PSR2 and Alderlake.
+	 */
+	if (IS_ALDERLAKE_P(dev_priv)) {
+		struct intel_encoder *encoder;
+
+		for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
+			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+			if (intel_dp->psr.psr2_enabled) {
+				min_cdclk = DIV_ROUND_UP(min_cdclk * 115, 100);
+				break;
+			}
+		}
+	}
+
 	if (min_cdclk > dev_priv->max_cdclk_freq) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
-- 
2.24.1.485.gad05a3d8e5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used
@ 2022-03-18  8:52 Stanislav Lisovskiy
  2022-03-18 12:21 ` Souza, Jose
  0 siblings, 1 reply; 28+ messages in thread
From: Stanislav Lisovskiy @ 2022-03-18  8:52 UTC (permalink / raw)
  To: intel-gfx

We are currently getting FIFO underruns, in particular
when PSR2 is enabled. There seem to be no existing workaround
or patches, which can fix that issue(were expecting some recent
selective fetch update and DBuf bw/SAGV fixes to help,
which unfortunately didn't).
Current idea is that it looks like for some reason the
DBuf prefill time isn't enough once we exit PSR2, despite its
theoretically correct.
So bump it up a bit by 15%(minimum experimental amount required
to get it working), if PSR2 is enabled.
For PSR1 there is no need in this hack, so we limit it only
to PSR2 and Alderlake.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 8888fda8b701..095b79950788 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2325,6 +2325,19 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 					dev_priv->max_cdclk_freq));
 	}
 
+	if (IS_ALDERLAKE_P(dev_priv)) {
+		struct intel_encoder *encoder;
+
+		for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
+			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+			if (intel_dp->psr.psr2_enabled) {
+				min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 85);
+				break;
+			}
+		}
+	}
+
 	if (min_cdclk > dev_priv->max_cdclk_freq) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
-- 
2.24.1.485.gad05a3d8e5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2
@ 2022-01-24  9:06 Stanislav Lisovskiy
  2022-03-18  8:48 ` [Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used Stanislav Lisovskiy
  0 siblings, 1 reply; 28+ messages in thread
From: Stanislav Lisovskiy @ 2022-01-24  9:06 UTC (permalink / raw)
  To: intel-gfx

In terms of async flip optimization we don't to allocate
extra ddb space, so lets skip it.

v2: - Extracted min ddb async flip check to separate function
      (Ville Syrjälä)
    - Used this function to prevent false positive WARN
      to be triggered(Ville Syrjälä)

v3: - Renamed dg2_need_min_ddb to need_min_ddb thus making
      it more universal.
    - Also used DISPLAY_VER instead of IS_DG2(Ville Syrjälä)
    - Use rate = 0 instead of just setting extra = 0, thus
      letting other planes to use extra ddb and avoiding WARN
      (Ville Syrjälä)

v4: - Renamed needs_min_ddb as s/needs/use/ to match
      the wm0 counterpart(Ville Syrjälä)
    - Added plane->async_flip check to use_min_ddb(now
      passing plane as a parameter to do that)(Ville Syrjälä)
    - Account for use_min_ddb also when calculating total data rate
      (Ville Syrjälä)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c |  2 +-
 .../gpu/drm/i915/display/intel_atomic_plane.h |  3 +-
 drivers/gpu/drm/i915/intel_pm.c               | 53 ++++++++++++++-----
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index b20cf2c16691..9d79ab987b2e 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -374,7 +374,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 					       old_plane_state, new_plane_state);
 }
 
-static struct intel_plane *
+struct intel_plane *
 intel_crtc_get_plane(struct intel_crtc *crtc, enum plane_id plane_id)
 {
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index ead789709477..aaddcc636f98 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -23,7 +23,8 @@ unsigned int intel_adjusted_rate(const struct drm_rect *src,
 				 unsigned int rate);
 unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
 				    const struct intel_plane_state *plane_state);
-
+struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
+					  enum plane_id plane_id);
 unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
 				   const struct intel_plane_state *plane_state);
 void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8269f1e9c784..fbe6a45801bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4988,6 +4988,25 @@ skl_get_total_relative_data_rate(struct intel_atomic_state *state,
 	return total_data_rate;
 }
 
+static bool use_min_ddb(struct intel_crtc_state *crtc_state,
+			struct intel_plane *plane)
+{
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
+
+	return DISPLAY_VER(i915) >= 13 && crtc_state->uapi.async_flip &&
+	       plane->async_flip;
+}
+
+static bool use_minimal_wm0_only(const struct intel_crtc_state *crtc_state,
+				 struct intel_plane *plane)
+{
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
+
+	return DISPLAY_VER(i915) >= 13 &&
+	       crtc_state->uapi.async_flip &&
+	       plane->async_flip;
+}
+
 static u64
 icl_get_total_relative_data_rate(struct intel_atomic_state *state,
 				 struct intel_crtc *crtc)
@@ -5033,8 +5052,15 @@ icl_get_total_relative_data_rate(struct intel_atomic_state *state,
 		}
 	}
 
-	for_each_plane_id_on_crtc(crtc, plane_id)
-		total_data_rate += crtc_state->plane_data_rate[plane_id];
+	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
+		/*
+		 * We calculate extra ddb based on ratio plane rate/total data rate
+		 * in case, in some cases we should not allocate extra ddb for the plane,
+		 * so do not count its data rate, if this is the case.
+		 */
+		if (!use_min_ddb(crtc_state, plane))
+			total_data_rate += crtc_state->plane_data_rate[plane->id];
+	}
 
 	return total_data_rate;
 }
@@ -5199,6 +5225,8 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 	for_each_plane_id_on_crtc(crtc, plane_id) {
 		const struct skl_plane_wm *wm =
 			&crtc_state->wm.skl.optimal.planes[plane_id];
+		struct intel_plane *plane =
+			intel_crtc_get_plane(crtc, plane_id);
 		u64 rate;
 		u16 extra;
 
@@ -5213,9 +5241,14 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 			break;
 
 		rate = crtc_state->plane_data_rate[plane_id];
+
+		if (use_min_ddb(crtc_state, plane))
+			rate = 0;
+
 		extra = min_t(u16, alloc_size,
 			      DIV64_U64_ROUND_UP(alloc_size * rate,
 						 total_data_rate));
+
 		total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
@@ -5224,13 +5257,19 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 			break;
 
 		rate = crtc_state->uv_plane_data_rate[plane_id];
+
+		if (use_min_ddb(crtc_state, plane))
+			rate = 0;
+
 		extra = min_t(u16, alloc_size,
 			      DIV64_U64_ROUND_UP(alloc_size * rate,
 						 total_data_rate));
+
 		uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
 	}
+
 	drm_WARN_ON(&dev_priv->drm, alloc_size != 0 || total_data_rate != 0);
 
 	/* Set the actual DDB start/end points for each plane */
@@ -5497,16 +5536,6 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
 		return 31;
 }
 
-static bool use_minimal_wm0_only(const struct intel_crtc_state *crtc_state,
-				 struct intel_plane *plane)
-{
-	struct drm_i915_private *i915 = to_i915(plane->base.dev);
-
-	return DISPLAY_VER(i915) >= 13 &&
-	       crtc_state->uapi.async_flip &&
-	       plane->async_flip;
-}
-
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 				 struct intel_plane *plane,
 				 int level,
-- 
2.24.1.485.gad05a3d8e5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-03-29 13:53 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21 10:49 [Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used Stanislav Lisovskiy
2022-03-21 11:10 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used (rev2) Patchwork
2022-03-21 11:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-03-21 14:39 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-03-21 16:58 ` [Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used Souza, Jose
2022-03-22  7:48   ` Lisovskiy, Stanislav
2022-03-22 13:16     ` Souza, Jose
2022-03-22 13:30       ` Lisovskiy, Stanislav
2022-03-22 13:34         ` Souza, Jose
2022-03-21 17:01 ` Souza, Jose
2022-03-22  7:49   ` Lisovskiy, Stanislav
2022-03-22 13:36     ` Souza, Jose
2022-03-29 13:10       ` Lisovskiy, Stanislav
2022-03-29 13:24         ` Souza, Jose
2022-03-29 13:53           ` Lisovskiy, Stanislav
2022-03-22 13:55 ` [Intel-gfx] " Mark Pearson
2022-03-22 14:18   ` Lisovskiy, Stanislav
2022-03-22 14:23     ` [Intel-gfx] [External] " Mark Pearson
2022-03-23 15:47       ` Souza, Jose
  -- strict thread matches above, loose matches on Subject: below --
2022-03-18  8:52 [Intel-gfx] [PATCH] " Stanislav Lisovskiy
2022-03-18 12:21 ` Souza, Jose
2022-03-18 12:27   ` Souza, Jose
2022-03-18 14:22     ` Lisovskiy, Stanislav
2022-03-18 14:40       ` Souza, Jose
2022-03-18 14:19   ` Lisovskiy, Stanislav
2022-03-18 14:38     ` Souza, Jose
2022-03-18 14:45       ` Lisovskiy, Stanislav
2022-01-24  9:06 [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2 Stanislav Lisovskiy
2022-03-18  8:48 ` [Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used Stanislav Lisovskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.