[Intel-gfx] [PATCH 0/4] Async flip optimization for DG2

[Intel-gfx] [PATCH 0/4] Async flip optimization for DG2

[Stanislav Lisovskiy]

Limitting the WM levels to 0 for DG2 during async flips,
allows to slightly increase the performance, as recommended
by HW team.

Stanislav Lisovskiy (4):
  drm/i915: Pass plane to watermark calculation functions
  drm/i915: Introduce do_async_flip flag to intel_plane_state
  drm/i915: Use wm0 only during async flips for DG2
  drm/i915: Don't allocate extra ddb during async flip for DG2

 .../gpu/drm/i915/display/intel_atomic_plane.c |  5 +-
 .../gpu/drm/i915/display/intel_atomic_plane.h |  4 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 31 ++++++-
 .../drm/i915/display/intel_display_types.h    |  3 +
 drivers/gpu/drm/i915/intel_pm.c               | 83 ++++++++++++++-----
 5 files changed, 102 insertions(+), 24 deletions(-)

-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Stanislav Lisovskiy]

Sometimes we might need to change the way we calculate
watermarks, based on which particular plane it is calculated
for. Thus it would be convenient to pass plane struct to those
functions.

v2: Pass plane instead of plane_id
v3: Do not pass plane to skl_cursor_allocation(Ville Syrjälä)
v4: - Make intel_crtc_get_plane static again(Ville Syrjälä)
    - s/cursor_plane/plane(Ville Syrjälä)
    - Pass plane to skl_compute_wm_* instead of plane_id(Ville Syrjälä)

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

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index 7907f601598e..ead789709477 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -16,6 +16,7 @@ struct intel_crtc;
 struct intel_crtc_state;
 struct intel_plane;
 struct intel_plane_state;
+enum plane_id;
 
 unsigned int intel_adjusted_rate(const struct drm_rect *src,
 				 const struct drm_rect *dst,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2ec8e48806b6..06707d2b5fc5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4240,7 +4240,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 u64 modifier, unsigned int rotation,
 				 u32 plane_pixel_rate, struct skl_wm_params *wp,
 				 int color_plane);
+
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -4251,6 +4253,7 @@ static unsigned int
 skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 		      int num_active)
 {
+	struct intel_plane *plane = to_intel_plane(crtc_state->uapi.crtc->cursor);
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
 	int level, max_level = ilk_wm_max_level(dev_priv);
 	struct skl_wm_level wm = {};
@@ -4267,7 +4270,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	for (level = 0; level <= max_level; level++) {
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
+		skl_compute_plane_wm(crtc_state, plane, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -5495,6 +5498,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
 }
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -5622,6 +5626,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static void
 skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
+		      struct intel_plane *plane,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -5633,7 +5638,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 		struct skl_wm_level *result = &levels[level];
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency,
+		skl_compute_plane_wm(crtc_state, plane, level, latency,
 				     wm_params, result_prev, result);
 
 		result_prev = result;
@@ -5641,6 +5646,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 }
 
 static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
+				struct intel_plane *plane,
 				const struct skl_wm_params *wm_params,
 				struct skl_plane_wm *plane_wm)
 {
@@ -5649,7 +5655,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
 	struct skl_wm_level *levels = plane_wm->wm;
 	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
 
-	skl_compute_plane_wm(crtc_state, 0, latency,
+	skl_compute_plane_wm(crtc_state, plane, 0, latency,
 			     wm_params, &levels[0],
 			     sagv_wm);
 }
@@ -5719,11 +5725,11 @@ static void skl_compute_transition_wm(struct drm_i915_private *dev_priv,
 
 static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 				     const struct intel_plane_state *plane_state,
-				     enum plane_id plane_id, int color_plane)
+				     struct intel_plane *plane, int color_plane)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane->id];
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5732,13 +5738,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
 
 	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
 				  &wm->wm[0], &wm_params);
 
 	if (DISPLAY_VER(dev_priv) >= 12) {
-		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
+		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
 
 		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
 					  &wm->sagv.wm0, &wm_params);
@@ -5749,9 +5755,9 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 
 static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 				 const struct intel_plane_state *plane_state,
-				 enum plane_id plane_id)
+				 struct intel_plane *plane)
 {
-	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane->id];
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5763,7 +5769,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
 
 	return 0;
 }
@@ -5783,13 +5789,13 @@ static int skl_build_plane_wm(struct intel_crtc_state *crtc_state,
 		return 0;
 
 	ret = skl_build_plane_wm_single(crtc_state, plane_state,
-					plane_id, 0);
+					plane, 0);
 	if (ret)
 		return ret;
 
 	if (fb->format->is_yuv && fb->format->num_planes > 1) {
 		ret = skl_build_plane_wm_uv(crtc_state, plane_state,
-					    plane_id);
+					    plane);
 		if (ret)
 			return ret;
 	}
@@ -5814,7 +5820,6 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
 
 	if (plane_state->planar_linked_plane) {
 		const struct drm_framebuffer *fb = plane_state->hw.fb;
-		enum plane_id y_plane_id = plane_state->planar_linked_plane->id;
 
 		drm_WARN_ON(&dev_priv->drm,
 			    !intel_wm_plane_visible(crtc_state, plane_state));
@@ -5822,17 +5827,17 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
 			    fb->format->num_planes == 1);
 
 		ret = skl_build_plane_wm_single(crtc_state, plane_state,
-						y_plane_id, 0);
+						plane_state->planar_linked_plane, 0);
 		if (ret)
 			return ret;
 
 		ret = skl_build_plane_wm_single(crtc_state, plane_state,
-						plane_id, 1);
+						plane, 1);
 		if (ret)
 			return ret;
 	} else if (intel_wm_plane_visible(crtc_state, plane_state)) {
 		ret = skl_build_plane_wm_single(crtc_state, plane_state,
-						plane_id, 0);
+						plane, 0);
 		if (ret)
 			return ret;
 	}
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 2/4] drm/i915: Introduce do_async_flip flag to intel_plane_state

[Stanislav Lisovskiy]

There might be various logical contructs when we might want
to enable async flip, so lets calculate those and set this
flag, so that there is no need in long conditions in other
places.

v2: - Set do_async_flip flag to False, if no async flip needed.
      Lets not rely that it will be 0-initialized, but set
      explicitly, so that the logic is clear as well.

v3: - Clear do_async_flip in intel_plane_duplicate_state(Ville Syrjälä)
    - Check with do_async_flip also when calling
      intel_crtc_{enable,disable}_flip_done(Ville Syrjälä)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic_plane.c  | 3 ++-
 drivers/gpu/drm/i915/display/intel_display.c       | 9 +++++++--
 drivers/gpu/drm/i915/display/intel_display_types.h | 3 +++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index c2c512cd8ec0..b20cf2c16691 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -109,6 +109,7 @@ intel_plane_duplicate_state(struct drm_plane *plane)
 	intel_state->ggtt_vma = NULL;
 	intel_state->dpt_vma = NULL;
 	intel_state->flags = 0;
+	intel_state->do_async_flip = false;
 
 	/* add reference to fb */
 	if (intel_state->hw.fb)
@@ -491,7 +492,7 @@ void intel_plane_update_arm(struct intel_plane *plane,
 
 	trace_intel_plane_update_arm(&plane->base, crtc);
 
-	if (crtc_state->uapi.async_flip && plane->async_flip)
+	if (plane_state->do_async_flip)
 		plane->async_flip(plane, crtc_state, plane_state, true);
 	else
 		plane->update_arm(plane, crtc_state, plane_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 0964b2403e2d..a65bae1f0c35 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1369,7 +1369,8 @@ static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
 	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
 		if (plane->enable_flip_done &&
 		    plane->pipe == crtc->pipe &&
-		    update_planes & BIT(plane->id))
+		    update_planes & BIT(plane->id) &&
+		    plane_state->do_async_flip)
 			plane->enable_flip_done(plane);
 	}
 }
@@ -1387,7 +1388,8 @@ static void intel_crtc_disable_flip_done(struct intel_atomic_state *state,
 	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
 		if (plane->disable_flip_done &&
 		    plane->pipe == crtc->pipe &&
-		    update_planes & BIT(plane->id))
+		    update_planes & BIT(plane->id) &&
+		    plane_state->do_async_flip)
 			plane->disable_flip_done(plane);
 	}
 }
@@ -5027,6 +5029,9 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 			 needs_scaling(new_plane_state))))
 		new_crtc_state->disable_lp_wm = true;
 
+	if (new_crtc_state->uapi.async_flip && plane->async_flip)
+		new_plane_state->do_async_flip = true;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 41e3dd25a78f..6b107872ad39 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -634,6 +634,9 @@ struct intel_plane_state {
 
 	struct intel_fb_view view;
 
+	/* Indicates if async flip is required */
+	bool do_async_flip;
+
 	/* Plane pxp decryption state */
 	bool decrypt;
 
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Stanislav Lisovskiy]

This optimization allows to achieve higher perfomance
during async flips.
For the first async flip we have to still temporarily
switch to sync flip, in order to reprogram plane
watermarks, so this requires taking into account
old plane state's do_async_flip flag.

v2: - Removed redundant new_plane_state->do_async_flip
      check from needs_async_flip_wm_override condition
      (Ville Syrjälä)
    - Extract dg2_async_flip_optimization to separate
      function(Ville Syrjälä)
    - Check for plane->async_flip instead of plane_id
      (Ville Syrjälä)

v3: - Rename "needs_async_flip_wm_override" to
      "intel_plane_do_async_flip" and move all the required
      checks there (Ville Syrjälä)
    - Rename "dg2_async_flip_optimization" to
      "use_minimal_wm0_only" (Ville Syrjälä)

v4: - Swap new/old_crtc_state in intel_plane_do_async_flip
      argument list(Ville Syrjälä)
    - Use plane->base.dev to grab i915 pointer in
      intel_plane_do_async_flip(Ville Syrjälä)
    - Remove const modifier from plane parameter in
      use_minimal_wm0_only(Ville Syrjälä)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 24 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_pm.c              | 13 ++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a65bae1f0c35..d4ba6c2910fc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4910,6 +4910,28 @@ static bool needs_scaling(const struct intel_plane_state *state)
 	return (src_w != dst_w || src_h != dst_h);
 }
 
+static bool intel_plane_do_async_flip(struct intel_plane *plane,
+				      const struct intel_crtc_state *old_crtc_state,
+				      const struct intel_crtc_state *new_crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
+
+	if (!plane->async_flip)
+		return false;
+
+	if (!new_crtc_state->uapi.async_flip)
+		return false;
+
+	/*
+	 * In platforms after DISPLAY13, we might need to override
+	 * first async flip in order to change watermark levels
+	 * as part of optimization.
+	 * So for those, we are checking if this is a first async flip.
+	 * For platforms earlier than DISPLAY13 we always do async flip.
+	 */
+	return DISPLAY_VER(i915) < 13 || old_crtc_state->uapi.async_flip;
+}
+
 int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
 				    struct intel_crtc_state *new_crtc_state,
 				    const struct intel_plane_state *old_plane_state,
@@ -5029,7 +5051,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 			 needs_scaling(new_plane_state))))
 		new_crtc_state->disable_lp_wm = true;
 
-	if (new_crtc_state->uapi.async_flip && plane->async_flip)
+	if (intel_plane_do_async_flip(plane, new_crtc_state, old_crtc_state))
 		new_plane_state->do_async_flip = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 06707d2b5fc5..8269f1e9c784 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5497,6 +5497,16 @@ 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,
@@ -5510,7 +5520,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 	uint_fixed_16_16_t selected_result;
 	u32 blocks, lines, min_ddb_alloc = 0;
 
-	if (latency == 0) {
+	if (latency == 0 ||
+	    (use_minimal_wm0_only(crtc_state, plane) && level > 0)) {
 		/* reject it */
 		result->min_ddb_alloc = U16_MAX;
 		return;
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Stanislav Lisovskiy]

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

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Ville Syrjälä]

On Mon, Jan 24, 2022 at 11:06:52AM +0200, Stanislav Lisovskiy wrote:
> This optimization allows to achieve higher perfomance
> during async flips.
> For the first async flip we have to still temporarily
> switch to sync flip, in order to reprogram plane
> watermarks, so this requires taking into account
> old plane state's do_async_flip flag.
> 
> v2: - Removed redundant new_plane_state->do_async_flip
>       check from needs_async_flip_wm_override condition
>       (Ville Syrjälä)
>     - Extract dg2_async_flip_optimization to separate
>       function(Ville Syrjälä)
>     - Check for plane->async_flip instead of plane_id
>       (Ville Syrjälä)
> 
> v3: - Rename "needs_async_flip_wm_override" to
>       "intel_plane_do_async_flip" and move all the required
>       checks there (Ville Syrjälä)
>     - Rename "dg2_async_flip_optimization" to
>       "use_minimal_wm0_only" (Ville Syrjälä)
> 
> v4: - Swap new/old_crtc_state in intel_plane_do_async_flip
>       argument list(Ville Syrjälä)
>     - Use plane->base.dev to grab i915 pointer in
>       intel_plane_do_async_flip(Ville Syrjälä)
>     - Remove const modifier from plane parameter in
>       use_minimal_wm0_only(Ville Syrjälä)
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 24 +++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_pm.c              | 13 ++++++++++-
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a65bae1f0c35..d4ba6c2910fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4910,6 +4910,28 @@ static bool needs_scaling(const struct intel_plane_state *state)
>  	return (src_w != dst_w || src_h != dst_h);
>  }
>  
> +static bool intel_plane_do_async_flip(struct intel_plane *plane,
> +				      const struct intel_crtc_state *old_crtc_state,
> +				      const struct intel_crtc_state *new_crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> +
> +	if (!plane->async_flip)
> +		return false;
> +
> +	if (!new_crtc_state->uapi.async_flip)
> +		return false;
> +
> +	/*
> +	 * In platforms after DISPLAY13, we might need to override
> +	 * first async flip in order to change watermark levels
> +	 * as part of optimization.
> +	 * So for those, we are checking if this is a first async flip.
> +	 * For platforms earlier than DISPLAY13 we always do async flip.
> +	 */
> +	return DISPLAY_VER(i915) < 13 || old_crtc_state->uapi.async_flip;
> +}
> +
>  int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
>  				    struct intel_crtc_state *new_crtc_state,
>  				    const struct intel_plane_state *old_plane_state,
> @@ -5029,7 +5051,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
>  			 needs_scaling(new_plane_state))))
>  		new_crtc_state->disable_lp_wm = true;
>  
> -	if (new_crtc_state->uapi.async_flip && plane->async_flip)
> +	if (intel_plane_do_async_flip(plane, new_crtc_state, old_crtc_state))

need to swap the new vs. old here too.

>  		new_plane_state->do_async_flip = true;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 06707d2b5fc5..8269f1e9c784 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5497,6 +5497,16 @@ 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,
> @@ -5510,7 +5520,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  	uint_fixed_16_16_t selected_result;
>  	u32 blocks, lines, min_ddb_alloc = 0;
>  
> -	if (latency == 0) {
> +	if (latency == 0 ||
> +	    (use_minimal_wm0_only(crtc_state, plane) && level > 0)) {
>  		/* reject it */
>  		result->min_ddb_alloc = U16_MAX;
>  		return;
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Lisovskiy, Stanislav]

On Mon, Jan 24, 2022 at 11:11:14AM +0200, Ville Syrjälä wrote:
> On Mon, Jan 24, 2022 at 11:06:52AM +0200, Stanislav Lisovskiy wrote:
> > This optimization allows to achieve higher perfomance
> > during async flips.
> > For the first async flip we have to still temporarily
> > switch to sync flip, in order to reprogram plane
> > watermarks, so this requires taking into account
> > old plane state's do_async_flip flag.
> > 
> > v2: - Removed redundant new_plane_state->do_async_flip
> >       check from needs_async_flip_wm_override condition
> >       (Ville Syrjälä)
> >     - Extract dg2_async_flip_optimization to separate
> >       function(Ville Syrjälä)
> >     - Check for plane->async_flip instead of plane_id
> >       (Ville Syrjälä)
> > 
> > v3: - Rename "needs_async_flip_wm_override" to
> >       "intel_plane_do_async_flip" and move all the required
> >       checks there (Ville Syrjälä)
> >     - Rename "dg2_async_flip_optimization" to
> >       "use_minimal_wm0_only" (Ville Syrjälä)
> > 
> > v4: - Swap new/old_crtc_state in intel_plane_do_async_flip
> >       argument list(Ville Syrjälä)
> >     - Use plane->base.dev to grab i915 pointer in
> >       intel_plane_do_async_flip(Ville Syrjälä)
> >     - Remove const modifier from plane parameter in
> >       use_minimal_wm0_only(Ville Syrjälä)
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 24 +++++++++++++++++++-
> >  drivers/gpu/drm/i915/intel_pm.c              | 13 ++++++++++-
> >  2 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index a65bae1f0c35..d4ba6c2910fc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -4910,6 +4910,28 @@ static bool needs_scaling(const struct intel_plane_state *state)
> >  	return (src_w != dst_w || src_h != dst_h);
> >  }
> >  
> > +static bool intel_plane_do_async_flip(struct intel_plane *plane,
> > +				      const struct intel_crtc_state *old_crtc_state,
> > +				      const struct intel_crtc_state *new_crtc_state)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> > +
> > +	if (!plane->async_flip)
> > +		return false;
> > +
> > +	if (!new_crtc_state->uapi.async_flip)
> > +		return false;
> > +
> > +	/*
> > +	 * In platforms after DISPLAY13, we might need to override
> > +	 * first async flip in order to change watermark levels
> > +	 * as part of optimization.
> > +	 * So for those, we are checking if this is a first async flip.
> > +	 * For platforms earlier than DISPLAY13 we always do async flip.
> > +	 */
> > +	return DISPLAY_VER(i915) < 13 || old_crtc_state->uapi.async_flip;
> > +}
> > +
> >  int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
> >  				    struct intel_crtc_state *new_crtc_state,
> >  				    const struct intel_plane_state *old_plane_state,
> > @@ -5029,7 +5051,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
> >  			 needs_scaling(new_plane_state))))
> >  		new_crtc_state->disable_lp_wm = true;
> >  
> > -	if (new_crtc_state->uapi.async_flip && plane->async_flip)
> > +	if (intel_plane_do_async_flip(plane, new_crtc_state, old_crtc_state))
> 
> need to swap the new vs. old here too.

Damn right, thanks for spotting. Stupid mistake..

Stan

> 
> >  		new_plane_state->do_async_flip = true;
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 06707d2b5fc5..8269f1e9c784 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5497,6 +5497,16 @@ 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,
> > @@ -5510,7 +5520,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> >  	uint_fixed_16_16_t selected_result;
> >  	u32 blocks, lines, min_ddb_alloc = 0;
> >  
> > -	if (latency == 0) {
> > +	if (latency == 0 ||
> > +	    (use_minimal_wm0_only(crtc_state, plane) && level > 0)) {
> >  		/* reject it */
> >  		result->min_ddb_alloc = U16_MAX;
> >  		return;
> > -- 
> > 2.24.1.485.gad05a3d8e5
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Ville Syrjälä]

On Mon, Jan 24, 2022 at 11:06:53AM +0200, Stanislav Lisovskiy wrote:
> 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)

Please put this function into the right spot to begin with.
Avoids polluting this patch with unrelated code movement.

> +{
> +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> +
> +	return DISPLAY_VER(i915) >= 13 &&
> +	       crtc_state->uapi.async_flip &&
> +	       plane->async_flip;

Line wrapping is different between the two functions despite the
identical contents, which looks a bit weird.

> +}
> +
>  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) {

for_each_intel_plane_on_crtc() otherwise we miss all
the planes not in the state.

> +		/*
> +		 * 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) {

for_each_intel_plane_on_crtc()...

>  		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);

... avoids needing that function.

>  		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;

else
	rate = crtc_state->plane_data_rate[plane_id];

is how I'd write that probably.

> +
>  		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

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Lisovskiy, Stanislav]

On Mon, Jan 24, 2022 at 11:16:36AM +0200, Ville Syrjälä wrote:
> On Mon, Jan 24, 2022 at 11:06:53AM +0200, Stanislav Lisovskiy wrote:
> > 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)
> 
> Please put this function into the right spot to begin with.
> Avoids polluting this patch with unrelated code movement.

It happened this way, because initially use_minimal_wm0_only which
is needed only by skl_compute_plane_wm was much lower, however as
you recommended to group it with use_min_ddb, so had to move it higher,
because use_min_ddb is required way higher in the file.

However I will then change the patch where it was introduced, so that
its already there. 

> 
> > +{
> > +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> > +
> > +	return DISPLAY_VER(i915) >= 13 &&
> > +	       crtc_state->uapi.async_flip &&
> > +	       plane->async_flip;
> 
> Line wrapping is different between the two functions despite the
> identical contents, which looks a bit weird.
> 
> > +}
> > +
> >  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) {
> 
> for_each_intel_plane_on_crtc() otherwise we miss all
> the planes not in the state.

Right, we recalculate those which changed first, then get total data rate.
For some reason I was looking at first cycle so just took same macro.

> 
> > +		/*
> > +		 * 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) {
> 
> for_each_intel_plane_on_crtc()...
> 
> >  		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);
> 
> ... avoids needing that function.
> 
> >  		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;
> 
> else
> 	rate = crtc_state->plane_data_rate[plane_id];
> 
> is how I'd write that probably.
> 
> > +
> >  		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
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Async flip optimization for DG2 (rev5)

[Patchwork]

== Series Details ==

Series: Async flip optimization for DG2 (rev5)
URL   : https://patchwork.freedesktop.org/series/98981/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
58dc909288f9 drm/i915: Pass plane to watermark calculation functions
a8601bc34503 drm/i915: Introduce do_async_flip flag to intel_plane_state
aeefcd12bb9c drm/i915: Use wm0 only during async flips for DG2
-:9: WARNING:TYPO_SPELLING: 'perfomance' may be misspelled - perhaps 'performance'?
#9: 
This optimization allows to achieve higher perfomance
                                           ^^^^^^^^^^

total: 0 errors, 1 warnings, 0 checks, 61 lines checked
bdb0b08414b0 drm/i915: Don't allocate extra ddb during async flip for DG2
-:56: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#56: FILE: drivers/gpu/drm/i915/display/intel_atomic_plane.h:27:
+struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
+					  enum plane_id plane_id);

total: 0 errors, 0 warnings, 1 checks, 116 lines checked

[Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Stanislav Lisovskiy]

This optimization allows to achieve higher perfomance
during async flips.
For the first async flip we have to still temporarily
switch to sync flip, in order to reprogram plane
watermarks, so this requires taking into account
old plane state's do_async_flip flag.

v2: - Removed redundant new_plane_state->do_async_flip
      check from needs_async_flip_wm_override condition
      (Ville Syrjälä)
    - Extract dg2_async_flip_optimization to separate
      function(Ville Syrjälä)
    - Check for plane->async_flip instead of plane_id
      (Ville Syrjälä)

v3: - Rename "needs_async_flip_wm_override" to
      "intel_plane_do_async_flip" and move all the required
      checks there (Ville Syrjälä)
    - Rename "dg2_async_flip_optimization" to
      "use_minimal_wm0_only" (Ville Syrjälä)

v4: - Swap new/old_crtc_state in intel_plane_do_async_flip
      argument list(Ville Syrjälä)
    - Use plane->base.dev to grab i915 pointer in
      intel_plane_do_async_flip(Ville Syrjälä)
    - Remove const modifier from plane parameter in
      use_minimal_wm0_only(Ville Syrjälä)
    - Swap also new/old_crtc_state at intel_plane_do_async_flip
      call site(Ville Syrjälä)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 24 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_pm.c              | 13 ++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a65bae1f0c35..54cb7e9bfa59 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4910,6 +4910,28 @@ static bool needs_scaling(const struct intel_plane_state *state)
 	return (src_w != dst_w || src_h != dst_h);
 }
 
+static bool intel_plane_do_async_flip(struct intel_plane *plane,
+				      const struct intel_crtc_state *old_crtc_state,
+				      const struct intel_crtc_state *new_crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
+
+	if (!plane->async_flip)
+		return false;
+
+	if (!new_crtc_state->uapi.async_flip)
+		return false;
+
+	/*
+	 * In platforms after DISPLAY13, we might need to override
+	 * first async flip in order to change watermark levels
+	 * as part of optimization.
+	 * So for those, we are checking if this is a first async flip.
+	 * For platforms earlier than DISPLAY13 we always do async flip.
+	 */
+	return DISPLAY_VER(i915) < 13 || old_crtc_state->uapi.async_flip;
+}
+
 int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
 				    struct intel_crtc_state *new_crtc_state,
 				    const struct intel_plane_state *old_plane_state,
@@ -5029,7 +5051,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 			 needs_scaling(new_plane_state))))
 		new_crtc_state->disable_lp_wm = true;
 
-	if (new_crtc_state->uapi.async_flip && plane->async_flip)
+	if (intel_plane_do_async_flip(plane, old_crtc_state, new_crtc_state))
 		new_plane_state->do_async_flip = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 06707d2b5fc5..f6c742b583c1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4988,6 +4988,16 @@ skl_get_total_relative_data_rate(struct intel_atomic_state *state,
 	return total_data_rate;
 }
 
+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)
@@ -5510,7 +5520,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 	uint_fixed_16_16_t selected_result;
 	u32 blocks, lines, min_ddb_alloc = 0;
 
-	if (latency == 0) {
+	if (latency == 0 ||
+	    (use_minimal_wm0_only(crtc_state, plane) && level > 0)) {
 		/* reject it */
 		result->min_ddb_alloc = U16_MAX;
 		return;
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Stanislav Lisovskiy]

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ä)

v5:
    - Use for_each_intel_plane_on_crtc instead of for_each_intel_plane_id
      to get plane->async_flip check and account for all planes(Ville Syrjälä)
    - Fix line wrapping(Ville Syrjälä)
    - Set plane data rate conditionally, avoiding on redundant assignment
      (Ville Syrjälä)

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

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index ead789709477..c238177e5563 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -23,7 +23,6 @@ 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);
-
 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 f6c742b583c1..7ce26f22e10e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4988,6 +4988,16 @@ 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)
 {
@@ -5002,6 +5012,7 @@ static u64
 icl_get_total_relative_data_rate(struct intel_atomic_state *state,
 				 struct intel_crtc *crtc)
 {
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	struct intel_crtc_state *crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct intel_plane_state *plane_state;
@@ -5043,8 +5054,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_intel_plane_on_crtc(&i915->drm, crtc, plane) {
+		/*
+		 * 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;
 }
@@ -5130,6 +5148,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 	u16 alloc_size, start = 0;
 	u16 total[I915_MAX_PLANES] = {};
 	u16 uv_total[I915_MAX_PLANES] = {};
+	struct intel_plane *plane;
 	u64 total_data_rate;
 	enum plane_id plane_id;
 	u32 blocks;
@@ -5206,7 +5225,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 	 * watermark level, plus an extra share of the leftover blocks
 	 * proportional to its relative data rate.
 	 */
-	for_each_plane_id_on_crtc(crtc, plane_id) {
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
 		const struct skl_plane_wm *wm =
 			&crtc_state->wm.skl.optimal.planes[plane_id];
 		u64 rate;
@@ -5222,10 +5241,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 		if (total_data_rate == 0)
 			break;
 
-		rate = crtc_state->plane_data_rate[plane_id];
+		if (use_min_ddb(crtc_state, plane))
+			rate = 0;
+		else
+			rate = crtc_state->plane_data_rate[plane_id];
+
 		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;
@@ -5233,14 +5257,20 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 		if (total_data_rate == 0)
 			break;
 
-		rate = crtc_state->uv_plane_data_rate[plane_id];
+		if (use_min_ddb(crtc_state, plane))
+			rate = 0;
+		else
+			rate = crtc_state->uv_plane_data_rate[plane_id];
+
 		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 */
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] ✓ Fi.CI.BAT: success for Async flip optimization for DG2 (rev5)

[Patchwork]

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

== Series Details ==

Series: Async flip optimization for DG2 (rev5)
URL   : https://patchwork.freedesktop.org/series/98981/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11120 -> Patchwork_22076
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (49 -> 37)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (13): fi-ilk-m540 fi-bdw-samus shard-tglu bat-dg1-6 fi-hsw-4200u fi-bsw-cyan bat-adlp-6 fi-ctg-p8600 bat-rpls-1 shard-rkl shard-dg1 bat-jsl-2 bat-jsl-1 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
    - fi-pnv-d510:        NOTRUN -> [SKIP][2] ([fdo#109271]) +57 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-pnv-d510/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-skl-6600u:       NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-kefka:       [PASS][4] -> [INCOMPLETE][5] ([i915#2940])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11120/fi-bsw-kefka/igt@i915_selftest@live@execlists.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-bsw-kefka/igt@i915_selftest@live@execlists.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-skl-6600u:       NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][7] ([fdo#109271]) +21 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-skl-6600u:       NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#533])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@runner@aborted:
    - fi-icl-u2:          NOTRUN -> [FAIL][9] ([i915#4312])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-icl-u2/igt@runner@aborted.html
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][10] ([i915#4312])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-tgl-1115g4/igt@runner@aborted.html
    - fi-rkl-11600:       NOTRUN -> [FAIL][11] ([i915#4312])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-rkl-11600/igt@runner@aborted.html
    - fi-bdw-5557u:       NOTRUN -> [FAIL][12] ([i915#2426] / [i915#4312])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-bdw-5557u/igt@runner@aborted.html
    - fi-rkl-guc:         NOTRUN -> [FAIL][13] ([i915#4312])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-rkl-guc/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_flink_basic@bad-flink:
    - fi-skl-6600u:       [INCOMPLETE][14] ([i915#4547]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11120/fi-skl-6600u/igt@gem_flink_basic@bad-flink.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-skl-6600u/igt@gem_flink_basic@bad-flink.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cml-u2:          [DMESG-WARN][16] ([i915#4269]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11120/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22076/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.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#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533


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

  * Linux: CI_DRM_11120 -> Patchwork_22076

  CI-20190529: 20190529
  CI_DRM_11120: d8e524ded1a6cb24bbd2da0785b04f199c03f1b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6330: f73008bac9a8db0779264b170f630483e9165764 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22076: bdb0b08414b09e900ee3dc07a6e5d31f5de0b318 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bdb0b08414b0 drm/i915: Don't allocate extra ddb during async flip for DG2
aeefcd12bb9c drm/i915: Use wm0 only during async flips for DG2
a8601bc34503 drm/i915: Introduce do_async_flip flag to intel_plane_state
58dc909288f9 drm/i915: Pass plane to watermark calculation functions

== Logs ==

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

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Ville Syrjälä]

On Mon, Jan 24, 2022 at 11:51:23AM +0200, Stanislav Lisovskiy wrote:
> 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ä)
> 
> v5:
>     - Use for_each_intel_plane_on_crtc instead of for_each_intel_plane_id
>       to get plane->async_flip check and account for all planes(Ville Syrjälä)
>     - Fix line wrapping(Ville Syrjälä)
>     - Set plane data rate conditionally, avoiding on redundant assignment
>       (Ville Syrjälä)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.h |  1 -
>  drivers/gpu/drm/i915/intel_pm.c               | 40 ++++++++++++++++---
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> index ead789709477..c238177e5563 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -23,7 +23,6 @@ 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);
> -

supurious whitespace changes

>  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 f6c742b583c1..7ce26f22e10e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4988,6 +4988,16 @@ 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)
>  {
> @@ -5002,6 +5012,7 @@ static u64
>  icl_get_total_relative_data_rate(struct intel_atomic_state *state,
>  				 struct intel_crtc *crtc)
>  {
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	struct intel_crtc_state *crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	const struct intel_plane_state *plane_state;
> @@ -5043,8 +5054,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];

Hmm. I was going to say we should remove plane_id now, but looks like
the other loop still uses it. I would move the declaration into that
loop now to make it less likely we mess up and use the wrong thing.

> +	for_each_intel_plane_on_crtc(&i915->drm, crtc, plane) {
> +		/*
> +		 * 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;
>  }
> @@ -5130,6 +5148,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  	u16 alloc_size, start = 0;
>  	u16 total[I915_MAX_PLANES] = {};
>  	u16 uv_total[I915_MAX_PLANES] = {};
> +	struct intel_plane *plane;
>  	u64 total_data_rate;
>  	enum plane_id plane_id;
>  	u32 blocks;
> @@ -5206,7 +5225,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  	 * watermark level, plus an extra share of the leftover blocks
>  	 * proportional to its relative data rate.
>  	 */
> -	for_each_plane_id_on_crtc(crtc, plane_id) {
> +	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
>  		const struct skl_plane_wm *wm =
>  			&crtc_state->wm.skl.optimal.planes[plane_id];

And here we do use the wrong thing. Should be plane->id.

Apparently some other loops still use plane_id, so can't remove it
fully :(

It's looking a bit messy overall. Might just be much cleaner to
calculate the plane_data_rate[] stuff as zero to start with so we
wouldn't have to deal with any of this here. Looks like you could
just handle it in skl_plane_relative_data_rate() in fact.

>  		u64 rate;
> @@ -5222,10 +5241,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  		if (total_data_rate == 0)
>  			break;
>  
> -		rate = crtc_state->plane_data_rate[plane_id];
> +		if (use_min_ddb(crtc_state, plane))
> +			rate = 0;
> +		else
> +			rate = crtc_state->plane_data_rate[plane_id];
> +
>  		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;
> @@ -5233,14 +5257,20 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  		if (total_data_rate == 0)
>  			break;
>  
> -		rate = crtc_state->uv_plane_data_rate[plane_id];
> +		if (use_min_ddb(crtc_state, plane))
> +			rate = 0;
> +		else
> +			rate = crtc_state->uv_plane_data_rate[plane_id];
> +
>  		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 */
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Stanislav Lisovskiy]

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ä)

v5:
    - Use for_each_intel_plane_on_crtc instead of for_each_intel_plane_id
      to get plane->async_flip check and account for all planes(Ville Syrjälä)
    - Fix line wrapping(Ville Syrjälä)
    - Set plane data rate conditionally, avoiding on redundant assignment
      (Ville Syrjälä)
    - Removed redundant whitespace(Ville Syrjälä)
    - Handle use_min_ddb case in skl_plane_relative_data_rate instead of
      icl_get_total_relative_data_rate(Ville Syrjälä)

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0bb4c941f950..bb147e5a77b6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4906,6 +4906,16 @@ static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes)
 	return active_pipes & BIT(pipe) ? BIT(DBUF_S1) : 0;
 }
 
+static bool use_min_ddb(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 bool use_minimal_wm0_only(const struct intel_crtc_state *crtc_state,
 				 struct intel_plane *plane)
 {
@@ -4934,6 +4944,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
 	if (plane->id == PLANE_CURSOR)
 		return 0;
 
+	/*
+	 * 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))
+		return 0;
+
 	if (color_plane == 1 &&
 	    !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
 		return 0;
-- 
2.24.1.485.gad05a3d8e5

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Ville Syrjälä]

On Mon, Jan 24, 2022 at 03:52:34PM +0200, Stanislav Lisovskiy wrote:
> 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ä)
> 
> v5:
>     - Use for_each_intel_plane_on_crtc instead of for_each_intel_plane_id
>       to get plane->async_flip check and account for all planes(Ville Syrjälä)
>     - Fix line wrapping(Ville Syrjälä)
>     - Set plane data rate conditionally, avoiding on redundant assignment
>       (Ville Syrjälä)
>     - Removed redundant whitespace(Ville Syrjälä)
>     - Handle use_min_ddb case in skl_plane_relative_data_rate instead of
>       icl_get_total_relative_data_rate(Ville Syrjälä)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0bb4c941f950..bb147e5a77b6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4906,6 +4906,16 @@ static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes)
>  	return active_pipes & BIT(pipe) ? BIT(DBUF_S1) : 0;
>  }
>  
> +static bool use_min_ddb(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 bool use_minimal_wm0_only(const struct intel_crtc_state *crtc_state,
>  				 struct intel_plane *plane)
>  {
> @@ -4934,6 +4944,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
>  	if (plane->id == PLANE_CURSOR)
>  		return 0;
>  
> +	/*
> +	 * 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))
> +		return 0;
> +
>  	if (color_plane == 1 &&
>  	    !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
>  		return 0;

Yeah this looks nice and simple. Only minor nit is that I'd probably
have put it after this ccs vs. planar related thing which is a more
static decision than the async flip optimization.

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

> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

[Intel-gfx] ✗ Fi.CI.BUILD: failure for Async flip optimization for DG2 (rev8)

[Patchwork]

== Series Details ==

Series: Async flip optimization for DG2 (rev8)
URL   : https://patchwork.freedesktop.org/series/98981/
State : failure

== Summary ==

Applying: drm/i915: Pass plane to watermark calculation functions
Applying: drm/i915: Introduce do_async_flip flag to intel_plane_state
Applying: drm/i915: Use wm0 only during async flips for DG2
Applying: drm/i915: Don't allocate extra ddb during async flip for DG2
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/intel_pm.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 drm/i915: Don't allocate extra ddb during async flip for DG2
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Lisovskiy, Stanislav]

On Mon, Jan 24, 2022 at 08:55:57PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 24, 2022 at 03:52:34PM +0200, Stanislav Lisovskiy wrote:
> > 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ä)
> > 
> > v5:
> >     - Use for_each_intel_plane_on_crtc instead of for_each_intel_plane_id
> >       to get plane->async_flip check and account for all planes(Ville Syrjälä)
> >     - Fix line wrapping(Ville Syrjälä)
> >     - Set plane data rate conditionally, avoiding on redundant assignment
> >       (Ville Syrjälä)
> >     - Removed redundant whitespace(Ville Syrjälä)
> >     - Handle use_min_ddb case in skl_plane_relative_data_rate instead of
> >       icl_get_total_relative_data_rate(Ville Syrjälä)
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 0bb4c941f950..bb147e5a77b6 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4906,6 +4906,16 @@ static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes)
> >  	return active_pipes & BIT(pipe) ? BIT(DBUF_S1) : 0;
> >  }
> >  
> > +static bool use_min_ddb(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 bool use_minimal_wm0_only(const struct intel_crtc_state *crtc_state,
> >  				 struct intel_plane *plane)
> >  {
> > @@ -4934,6 +4944,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
> >  	if (plane->id == PLANE_CURSOR)
> >  		return 0;
> >  
> > +	/*
> > +	 * 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))
> > +		return 0;
> > +
> >  	if (color_plane == 1 &&
> >  	    !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
> >  		return 0;
> 
> Yeah this looks nice and simple. Only minor nit is that I'd probably
> have put it after this ccs vs. planar related thing which is a more
> static decision than the async flip optimization.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I guess the first patch in this series is also ok?

Just wondering, if I can push it now.

Stan

> 
> > -- 
> > 2.24.1.485.gad05a3d8e5
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Ville Syrjälä]

On Mon, Jan 24, 2022 at 11:06:50AM +0200, Stanislav Lisovskiy wrote:
> Sometimes we might need to change the way we calculate
> watermarks, based on which particular plane it is calculated
> for. Thus it would be convenient to pass plane struct to those
> functions.
> 
> v2: Pass plane instead of plane_id
> v3: Do not pass plane to skl_cursor_allocation(Ville Syrjälä)
> v4: - Make intel_crtc_get_plane static again(Ville Syrjälä)
>     - s/cursor_plane/plane(Ville Syrjälä)
>     - Pass plane to skl_compute_wm_* instead of plane_id(Ville Syrjälä)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

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

> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c               | 37 +++++++++++--------
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> index 7907f601598e..ead789709477 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -16,6 +16,7 @@ struct intel_crtc;
>  struct intel_crtc_state;
>  struct intel_plane;
>  struct intel_plane_state;
> +enum plane_id;
>  
>  unsigned int intel_adjusted_rate(const struct drm_rect *src,
>  				 const struct drm_rect *dst,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2ec8e48806b6..06707d2b5fc5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4240,7 +4240,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>  				 u64 modifier, unsigned int rotation,
>  				 u32 plane_pixel_rate, struct skl_wm_params *wp,
>  				 int color_plane);
> +
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> +				 struct intel_plane *plane,
>  				 int level,
>  				 unsigned int latency,
>  				 const struct skl_wm_params *wp,
> @@ -4251,6 +4253,7 @@ static unsigned int
>  skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>  		      int num_active)
>  {
> +	struct intel_plane *plane = to_intel_plane(crtc_state->uapi.crtc->cursor);
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
>  	int level, max_level = ilk_wm_max_level(dev_priv);
>  	struct skl_wm_level wm = {};
> @@ -4267,7 +4270,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>  	for (level = 0; level <= max_level; level++) {
>  		unsigned int latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
> +		skl_compute_plane_wm(crtc_state, plane, level, latency, &wp, &wm, &wm);
>  		if (wm.min_ddb_alloc == U16_MAX)
>  			break;
>  
> @@ -5495,6 +5498,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
>  }
>  
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> +				 struct intel_plane *plane,
>  				 int level,
>  				 unsigned int latency,
>  				 const struct skl_wm_params *wp,
> @@ -5622,6 +5626,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  
>  static void
>  skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
> +		      struct intel_plane *plane,
>  		      const struct skl_wm_params *wm_params,
>  		      struct skl_wm_level *levels)
>  {
> @@ -5633,7 +5638,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
>  		struct skl_wm_level *result = &levels[level];
>  		unsigned int latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, latency,
> +		skl_compute_plane_wm(crtc_state, plane, level, latency,
>  				     wm_params, result_prev, result);
>  
>  		result_prev = result;
> @@ -5641,6 +5646,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
>  }
>  
>  static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
> +				struct intel_plane *plane,
>  				const struct skl_wm_params *wm_params,
>  				struct skl_plane_wm *plane_wm)
>  {
> @@ -5649,7 +5655,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
>  	struct skl_wm_level *levels = plane_wm->wm;
>  	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
>  
> -	skl_compute_plane_wm(crtc_state, 0, latency,
> +	skl_compute_plane_wm(crtc_state, plane, 0, latency,
>  			     wm_params, &levels[0],
>  			     sagv_wm);
>  }
> @@ -5719,11 +5725,11 @@ static void skl_compute_transition_wm(struct drm_i915_private *dev_priv,
>  
>  static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  				     const struct intel_plane_state *plane_state,
> -				     enum plane_id plane_id, int color_plane)
> +				     struct intel_plane *plane, int color_plane)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
> +	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane->id];
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5732,13 +5738,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
> +	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
>  
>  	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
>  				  &wm->wm[0], &wm_params);
>  
>  	if (DISPLAY_VER(dev_priv) >= 12) {
> -		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
> +		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
>  
>  		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
>  					  &wm->sagv.wm0, &wm_params);
> @@ -5749,9 +5755,9 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  
>  static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  				 const struct intel_plane_state *plane_state,
> -				 enum plane_id plane_id)
> +				 struct intel_plane *plane)
>  {
> -	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
> +	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane->id];
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5763,7 +5769,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
> +	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
>  
>  	return 0;
>  }
> @@ -5783,13 +5789,13 @@ static int skl_build_plane_wm(struct intel_crtc_state *crtc_state,
>  		return 0;
>  
>  	ret = skl_build_plane_wm_single(crtc_state, plane_state,
> -					plane_id, 0);
> +					plane, 0);
>  	if (ret)
>  		return ret;
>  
>  	if (fb->format->is_yuv && fb->format->num_planes > 1) {
>  		ret = skl_build_plane_wm_uv(crtc_state, plane_state,
> -					    plane_id);
> +					    plane);
>  		if (ret)
>  			return ret;
>  	}
> @@ -5814,7 +5820,6 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
>  
>  	if (plane_state->planar_linked_plane) {
>  		const struct drm_framebuffer *fb = plane_state->hw.fb;
> -		enum plane_id y_plane_id = plane_state->planar_linked_plane->id;
>  
>  		drm_WARN_ON(&dev_priv->drm,
>  			    !intel_wm_plane_visible(crtc_state, plane_state));
> @@ -5822,17 +5827,17 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
>  			    fb->format->num_planes == 1);
>  
>  		ret = skl_build_plane_wm_single(crtc_state, plane_state,
> -						y_plane_id, 0);
> +						plane_state->planar_linked_plane, 0);
>  		if (ret)
>  			return ret;
>  
>  		ret = skl_build_plane_wm_single(crtc_state, plane_state,
> -						plane_id, 1);
> +						plane, 1);
>  		if (ret)
>  			return ret;
>  	} else if (intel_wm_plane_visible(crtc_state, plane_state)) {
>  		ret = skl_build_plane_wm_single(crtc_state, plane_state,
> -						plane_id, 0);
> +						plane, 0);
>  		if (ret)
>  			return ret;
>  	}
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 2/2] drm/i915/mtl: Don't use PIN_MAPPABLE for dpt

[Stanislav Lisovskiy]

Cannot use PIN_MAPPABLE pin on MTL because there's no mappable window.
Change dpt allocation as per suggestion from Chris.

v2: - Added forgotten/dropped include

Signed-off-by: Stanslav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
Cc: Chris Wilson <chris.p.wilson@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpt.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
index 15b2716172f7..11f328a42e19 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -4,6 +4,7 @@
  */
 
 #include "gem/i915_gem_domain.h"
+#include "gem/i915_gem_internal.h"
 #include "gt/gen8_ppgtt.h"
 
 #include "i915_drv.h"
@@ -128,6 +129,10 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm)
 	void __iomem *iomem;
 	struct i915_gem_ww_ctx ww;
 	int err;
+	u64 pin_flags = 0;
+
+	if (i915_gem_object_is_stolen(dpt->obj))
+		pin_flags |= PIN_MAPPABLE; /* for i915_vma_pin_iomap(stolen) */
 
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 	atomic_inc(&i915->gpu_error.pending_fb_pin);
@@ -138,7 +143,7 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm)
 			continue;
 
 		vma = i915_gem_object_ggtt_pin_ww(dpt->obj, &ww, NULL, 0, 4096,
-						  HAS_LMEM(i915) ? 0 : PIN_MAPPABLE);
+						  pin_flags);
 		if (IS_ERR(vma)) {
 			err = PTR_ERR(vma);
 			continue;
@@ -248,10 +253,11 @@ intel_dpt_create(struct intel_framebuffer *fb)
 
 	size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
 
-	if (HAS_LMEM(i915))
-		dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
-	else
+	dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
+	if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
 		dpt_obj = i915_gem_object_create_stolen(i915, size);
+	if (IS_ERR(dpt_obj))
+		dpt_obj = i915_gem_object_create_internal(i915, size);
 	if (IS_ERR(dpt_obj))
 		return ERR_CAST(dpt_obj);
 
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used

[Stanislav Lisovskiy]

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

[Intel-gfx] ✗ Fi.CI.BUILD: failure for Async flip optimization for DG2 (rev10)

[Patchwork]

== Series Details ==

Series: Async flip optimization for DG2 (rev10)
URL   : https://patchwork.freedesktop.org/series/98981/
State : failure

== Summary ==

Applying: drm/i915: Pass plane to watermark calculation functions
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_atomic_plane.h
M	drivers/gpu/drm/i915/intel_pm.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_pm.c
Auto-merging drivers/gpu/drm/i915/display/intel_atomic_plane.h
No changes -- Patch already applied.
Applying: drm/i915: Introduce do_async_flip flag to intel_plane_state
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_atomic_plane.c
M	drivers/gpu/drm/i915/display/intel_display.c
M	drivers/gpu/drm/i915/display/intel_display_types.h
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_display_types.h
Auto-merging drivers/gpu/drm/i915/display/intel_display.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display.c
Auto-merging drivers/gpu/drm/i915/display/intel_atomic_plane.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_atomic_plane.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 drm/i915: Introduce do_async_flip flag to intel_plane_state
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

[Intel-gfx] ✗ Fi.CI.BUILD: failure for Async flip optimization for DG2 (rev10)

[Patchwork]

== Series Details ==

Series: Async flip optimization for DG2 (rev10)
URL   : https://patchwork.freedesktop.org/series/98981/
State : failure

== Summary ==

Applying: drm/i915: Pass plane to watermark calculation functions
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_atomic_plane.h
M	drivers/gpu/drm/i915/intel_pm.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_pm.c
Auto-merging drivers/gpu/drm/i915/display/intel_atomic_plane.h
No changes -- Patch already applied.
Applying: drm/i915: Introduce do_async_flip flag to intel_plane_state
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_atomic_plane.c
M	drivers/gpu/drm/i915/display/intel_display.c
M	drivers/gpu/drm/i915/display/intel_display_types.h
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_display_types.h
Auto-merging drivers/gpu/drm/i915/display/intel_display.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display.c
Auto-merging drivers/gpu/drm/i915/display/intel_atomic_plane.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_atomic_plane.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 drm/i915: Introduce do_async_flip flag to intel_plane_state
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Ville Syrjälä]

On Fri, Jan 21, 2022 at 10:06:12AM +0200, Stanislav Lisovskiy wrote:
> Sometimes we might need to change the way we calculate
> watermarks, based on which particular plane it is calculated
> for. Thus it would be convenient to pass plane struct to those
> functions.
> 
> v2: Pass plane instead of plane_id
> v3: Do not pass plane to skl_cursor_allocation(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               | 20 +++++++++++++------
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index c2c512cd8ec0..d1344e9c06de 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -373,7 +373,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 7907f601598e..c1499bb7370e 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -16,10 +16,13 @@ struct intel_crtc;
>  struct intel_crtc_state;
>  struct intel_plane;
>  struct intel_plane_state;
> +enum plane_id;
>  
>  unsigned int intel_adjusted_rate(const struct drm_rect *src,
>  				 const struct drm_rect *dst,
>  				 unsigned int rate);
> +struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
> +					 enum plane_id plane_id);

You're no longer using that, so can stay static.

>  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
>  				    const 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 3981aa856cc2..35d0bd8c6e57 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4252,7 +4252,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>  				 u64 modifier, unsigned int rotation,
>  				 u32 plane_pixel_rate, struct skl_wm_params *wp,
>  				 int color_plane);
> +
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane *plane,
>  				 int level,
>  				 unsigned int latency,
>  				 const struct skl_wm_params *wp,
> @@ -4268,6 +4270,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>  	struct skl_wm_level wm = {};
>  	int ret, min_ddb_alloc = 0;
>  	struct skl_wm_params wp;
> +	const struct intel_plane *cursor_plane = to_intel_plane(crtc_state->uapi.crtc->cursor);

I think just 'plane' would suffice since we know from the context what
it is. Also sticking to the reverse christmas tree order would look a
bit neater imo.

>  
>  	ret = skl_compute_wm_params(crtc_state, 256,
>  				    drm_format_info(DRM_FORMAT_ARGB8888),
> @@ -4279,7 +4282,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>  	for (level = 0; level <= max_level; level++) {
>  		unsigned int latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
> +		skl_compute_plane_wm(crtc_state, cursor_plane, level, latency, &wp, &wm, &wm);
>  		if (wm.min_ddb_alloc == U16_MAX)
>  			break;
>  
> @@ -5508,6 +5511,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
>  }
>  
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane *plane,
>  				 int level,
>  				 unsigned int latency,
>  				 const struct skl_wm_params *wp,
> @@ -5635,6 +5639,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  
>  static void
>  skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
> +		      const struct intel_plane *plane,
>  		      const struct skl_wm_params *wm_params,
>  		      struct skl_wm_level *levels)
>  {
> @@ -5646,7 +5651,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
>  		struct skl_wm_level *result = &levels[level];
>  		unsigned int latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, latency,
> +		skl_compute_plane_wm(crtc_state, plane, level, latency,
>  				     wm_params, result_prev, result);
>  
>  		result_prev = result;
> @@ -5654,6 +5659,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
>  }
>  
>  static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
> +				const struct intel_plane *plane,
>  				const struct skl_wm_params *wm_params,
>  				struct skl_plane_wm *plane_wm)
>  {
> @@ -5662,7 +5668,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
>  	struct skl_wm_level *levels = plane_wm->wm;
>  	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
>  
> -	skl_compute_plane_wm(crtc_state, 0, latency,
> +	skl_compute_plane_wm(crtc_state, plane, 0, latency,
>  			     wm_params, &levels[0],
>  			     sagv_wm);
>  }
> @@ -5737,6 +5743,7 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
> +	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);

Hmm. I was first going to suggest not plumbing plane_id through
to here at all anymore, but thanks to the Y plane shenanigans we
can't actually do that.

I think to make this totally consistent we should actually plumb
the whole plane into skl_build_plane_wm_{single,uv}() instead
of just the plane_id.

>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5745,13 +5752,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
> +	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
>  
>  	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
>  				  &wm->wm[0], &wm_params);
>  
>  	if (DISPLAY_VER(dev_priv) >= 12) {
> -		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
> +		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
>  
>  		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
>  					  &wm->sagv.wm0, &wm_params);
> @@ -5765,6 +5772,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  				 enum plane_id plane_id)
>  {
>  	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
> +	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5776,7 +5784,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
> +	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
>  
>  	return 0;
>  }
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Stanislav Lisovskiy]

Sometimes we might need to change the way we calculate
watermarks, based on which particular plane it is calculated
for. Thus it would be convenient to pass plane struct to those
functions.

v2: Pass plane instead of plane_id
v3: Do not pass plane to skl_cursor_allocation(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               | 20 +++++++++++++------
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index c2c512cd8ec0..d1344e9c06de 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -373,7 +373,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 7907f601598e..c1499bb7370e 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -16,10 +16,13 @@ struct intel_crtc;
 struct intel_crtc_state;
 struct intel_plane;
 struct intel_plane_state;
+enum plane_id;
 
 unsigned int intel_adjusted_rate(const struct drm_rect *src,
 				 const struct drm_rect *dst,
 				 unsigned int rate);
+struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
+					 enum plane_id plane_id);
 unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
 				    const 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 3981aa856cc2..35d0bd8c6e57 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4252,7 +4252,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 u64 modifier, unsigned int rotation,
 				 u32 plane_pixel_rate, struct skl_wm_params *wp,
 				 int color_plane);
+
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -4268,6 +4270,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	struct skl_wm_level wm = {};
 	int ret, min_ddb_alloc = 0;
 	struct skl_wm_params wp;
+	const struct intel_plane *cursor_plane = to_intel_plane(crtc_state->uapi.crtc->cursor);
 
 	ret = skl_compute_wm_params(crtc_state, 256,
 				    drm_format_info(DRM_FORMAT_ARGB8888),
@@ -4279,7 +4282,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	for (level = 0; level <= max_level; level++) {
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
+		skl_compute_plane_wm(crtc_state, cursor_plane, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -5508,6 +5511,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
 }
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -5635,6 +5639,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static void
 skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
+		      const struct intel_plane *plane,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -5646,7 +5651,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 		struct skl_wm_level *result = &levels[level];
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency,
+		skl_compute_plane_wm(crtc_state, plane, level, latency,
 				     wm_params, result_prev, result);
 
 		result_prev = result;
@@ -5654,6 +5659,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 }
 
 static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
+				const struct intel_plane *plane,
 				const struct skl_wm_params *wm_params,
 				struct skl_plane_wm *plane_wm)
 {
@@ -5662,7 +5668,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
 	struct skl_wm_level *levels = plane_wm->wm;
 	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
 
-	skl_compute_plane_wm(crtc_state, 0, latency,
+	skl_compute_plane_wm(crtc_state, plane, 0, latency,
 			     wm_params, &levels[0],
 			     sagv_wm);
 }
@@ -5737,6 +5743,7 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5745,13 +5752,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
 
 	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
 				  &wm->wm[0], &wm_params);
 
 	if (DISPLAY_VER(dev_priv) >= 12) {
-		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
+		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
 
 		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
 					  &wm->sagv.wm0, &wm_params);
@@ -5765,6 +5772,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 				 enum plane_id plane_id)
 {
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5776,7 +5784,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
 
 	return 0;
 }
-- 
2.24.1.485.gad05a3d8e5

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Ville Syrjälä]

On Tue, Jan 18, 2022 at 12:48:36PM +0200, Stanislav Lisovskiy wrote:
> Sometimes we might need to change the way we calculate
> watermarks, based on which particular plane it is calculated
> for. Thus it would be convenient to pass plane struct to those
> functions.
> 
> v2: Pass plane instead of plane_id
> 
> 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               | 23 +++++++++++++------
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index c2c512cd8ec0..d1344e9c06de 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -373,7 +373,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 7907f601598e..c1499bb7370e 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -16,10 +16,13 @@ struct intel_crtc;
>  struct intel_crtc_state;
>  struct intel_plane;
>  struct intel_plane_state;
> +enum plane_id;
>  
>  unsigned int intel_adjusted_rate(const struct drm_rect *src,
>  				 const struct drm_rect *dst,
>  				 unsigned int rate);
> +struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
> +					 enum plane_id plane_id);
>  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
>  				    const 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 62fde21fac39..dc1203d21c46 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4252,7 +4252,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>  				 u64 modifier, unsigned int rotation,
>  				 u32 plane_pixel_rate, struct skl_wm_params *wp,
>  				 int color_plane);
> +
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane *plane,
>  				 int level,
>  				 unsigned int latency,
>  				 const struct skl_wm_params *wp,
> @@ -4261,6 +4263,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  
>  static unsigned int
>  skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
> +		      const struct intel_plane *plane,

I don't see a reason for having the caller pass this in. We can just
keep it local to this function.

Also we don't usually const these things.

>  		      int num_active)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> @@ -4279,7 +4282,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>  	for (level = 0; level <= max_level; level++) {
>  		unsigned int latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
> +		skl_compute_plane_wm(crtc_state, plane, level, latency, &wp, &wm, &wm);
>  		if (wm.min_ddb_alloc == U16_MAX)
>  			break;
>  
> @@ -5124,6 +5127,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	const struct intel_dbuf_state *dbuf_state =
>  		intel_atomic_get_new_dbuf_state(state);
> +	const struct intel_plane *cursor_plane = intel_crtc_get_plane(crtc, PLANE_CURSOR);

Could be just to_intel_plane(crtc->base.cursor) 

Apart from that looks OK.

>  	const struct skl_ddb_entry *alloc = &dbuf_state->ddb[crtc->pipe];
>  	int num_active = hweight8(dbuf_state->active_pipes);
>  	u16 alloc_size, start = 0;
> @@ -5153,7 +5157,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  		return 0;
>  
>  	/* Allocate fixed number of blocks for cursor. */
> -	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active);
> +	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, cursor_plane, num_active);
>  	alloc_size -= total[PLANE_CURSOR];
>  	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
>  		alloc->end - total[PLANE_CURSOR];
> @@ -5507,6 +5511,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
>  }
>  
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane *plane,
>  				 int level,
>  				 unsigned int latency,
>  				 const struct skl_wm_params *wp,
> @@ -5634,6 +5639,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  
>  static void
>  skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
> +		      const struct intel_plane *plane,
>  		      const struct skl_wm_params *wm_params,
>  		      struct skl_wm_level *levels)
>  {
> @@ -5645,7 +5651,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
>  		struct skl_wm_level *result = &levels[level];
>  		unsigned int latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, latency,
> +		skl_compute_plane_wm(crtc_state, plane, level, latency,
>  				     wm_params, result_prev, result);
>  
>  		result_prev = result;
> @@ -5653,6 +5659,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
>  }
>  
>  static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
> +				const struct intel_plane *plane,
>  				const struct skl_wm_params *wm_params,
>  				struct skl_plane_wm *plane_wm)
>  {
> @@ -5661,7 +5668,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
>  	struct skl_wm_level *levels = plane_wm->wm;
>  	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
>  
> -	skl_compute_plane_wm(crtc_state, 0, latency,
> +	skl_compute_plane_wm(crtc_state, plane, 0, latency,
>  			     wm_params, &levels[0],
>  			     sagv_wm);
>  }
> @@ -5736,6 +5743,7 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
> +	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5744,13 +5752,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
> +	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
>  
>  	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
>  				  &wm->wm[0], &wm_params);
>  
>  	if (DISPLAY_VER(dev_priv) >= 12) {
> -		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
> +		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
>  
>  		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
>  					  &wm->sagv.wm0, &wm_params);
> @@ -5764,6 +5772,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  				 enum plane_id plane_id)
>  {
>  	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
> +	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5775,7 +5784,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
> +	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
>  
>  	return 0;
>  }
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Stanislav Lisovskiy]

Sometimes we might need to change the way we calculate
watermarks, based on which particular plane it is calculated
for. Thus it would be convenient to pass plane struct to those
functions.

v2: Pass plane instead of plane_id

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               | 23 +++++++++++++------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index c2c512cd8ec0..d1344e9c06de 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -373,7 +373,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 7907f601598e..c1499bb7370e 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -16,10 +16,13 @@ struct intel_crtc;
 struct intel_crtc_state;
 struct intel_plane;
 struct intel_plane_state;
+enum plane_id;
 
 unsigned int intel_adjusted_rate(const struct drm_rect *src,
 				 const struct drm_rect *dst,
 				 unsigned int rate);
+struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
+					 enum plane_id plane_id);
 unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
 				    const 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 62fde21fac39..dc1203d21c46 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4252,7 +4252,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 u64 modifier, unsigned int rotation,
 				 u32 plane_pixel_rate, struct skl_wm_params *wp,
 				 int color_plane);
+
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -4261,6 +4263,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static unsigned int
 skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
+		      const struct intel_plane *plane,
 		      int num_active)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
@@ -4279,7 +4282,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	for (level = 0; level <= max_level; level++) {
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
+		skl_compute_plane_wm(crtc_state, plane, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -5124,6 +5127,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct intel_dbuf_state *dbuf_state =
 		intel_atomic_get_new_dbuf_state(state);
+	const struct intel_plane *cursor_plane = intel_crtc_get_plane(crtc, PLANE_CURSOR);
 	const struct skl_ddb_entry *alloc = &dbuf_state->ddb[crtc->pipe];
 	int num_active = hweight8(dbuf_state->active_pipes);
 	u16 alloc_size, start = 0;
@@ -5153,7 +5157,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 		return 0;
 
 	/* Allocate fixed number of blocks for cursor. */
-	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active);
+	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, cursor_plane, num_active);
 	alloc_size -= total[PLANE_CURSOR];
 	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
 		alloc->end - total[PLANE_CURSOR];
@@ -5507,6 +5511,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
 }
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -5634,6 +5639,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static void
 skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
+		      const struct intel_plane *plane,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -5645,7 +5651,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 		struct skl_wm_level *result = &levels[level];
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency,
+		skl_compute_plane_wm(crtc_state, plane, level, latency,
 				     wm_params, result_prev, result);
 
 		result_prev = result;
@@ -5653,6 +5659,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 }
 
 static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
+				const struct intel_plane *plane,
 				const struct skl_wm_params *wm_params,
 				struct skl_plane_wm *plane_wm)
 {
@@ -5661,7 +5668,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
 	struct skl_wm_level *levels = plane_wm->wm;
 	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
 
-	skl_compute_plane_wm(crtc_state, 0, latency,
+	skl_compute_plane_wm(crtc_state, plane, 0, latency,
 			     wm_params, &levels[0],
 			     sagv_wm);
 }
@@ -5736,6 +5743,7 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5744,13 +5752,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
 
 	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
 				  &wm->wm[0], &wm_params);
 
 	if (DISPLAY_VER(dev_priv) >= 12) {
-		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
+		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
 
 		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
 					  &wm->sagv.wm0, &wm_params);
@@ -5764,6 +5772,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 				 enum plane_id plane_id)
 {
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5775,7 +5784,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
 
 	return 0;
 }
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 1/4] drm/i915: Pass plane to watermark calculation functions

[Stanislav Lisovskiy]

Sometimes we might need to change the way we calculate
watermarks, based on which particular plane it is calculated
for. Thus it would be convenient to pass plane struct to those
functions.

v2: Pass plane instead of plane_id

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               | 23 +++++++++++++------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 27b8f99dd099..023747fb5052 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -372,7 +372,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 7907f601598e..c1499bb7370e 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -16,10 +16,13 @@ struct intel_crtc;
 struct intel_crtc_state;
 struct intel_plane;
 struct intel_plane_state;
+enum plane_id;
 
 unsigned int intel_adjusted_rate(const struct drm_rect *src,
 				 const struct drm_rect *dst,
 				 unsigned int rate);
+struct intel_plane *intel_crtc_get_plane(struct intel_crtc *crtc,
+					 enum plane_id plane_id);
 unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
 				    const 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 fe3787425780..79dac38d9eb2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4238,7 +4238,9 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 u64 modifier, unsigned int rotation,
 				 u32 plane_pixel_rate, struct skl_wm_params *wp,
 				 int color_plane);
+
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -4247,6 +4249,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static unsigned int
 skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
+		      const struct intel_plane *plane,
 		      int num_active)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
@@ -4265,7 +4268,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	for (level = 0; level <= max_level; level++) {
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
+		skl_compute_plane_wm(crtc_state, plane, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -5111,6 +5114,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct intel_dbuf_state *dbuf_state =
 		intel_atomic_get_new_dbuf_state(state);
+	const struct intel_plane *cursor_plane = intel_crtc_get_plane(crtc, PLANE_CURSOR);
 	const struct skl_ddb_entry *alloc = &dbuf_state->ddb[crtc->pipe];
 	int num_active = hweight8(dbuf_state->active_pipes);
 	u16 alloc_size, start = 0;
@@ -5140,7 +5144,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 		return 0;
 
 	/* Allocate fixed number of blocks for cursor. */
-	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active);
+	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, cursor_plane, num_active);
 	alloc_size -= total[PLANE_CURSOR];
 	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
 		alloc->end - total[PLANE_CURSOR];
@@ -5494,6 +5498,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
 }
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane *plane,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -5621,6 +5626,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static void
 skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
+		      const struct intel_plane *plane,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -5632,7 +5638,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 		struct skl_wm_level *result = &levels[level];
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency,
+		skl_compute_plane_wm(crtc_state, plane, level, latency,
 				     wm_params, result_prev, result);
 
 		result_prev = result;
@@ -5640,6 +5646,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 }
 
 static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
+				const struct intel_plane *plane,
 				const struct skl_wm_params *wm_params,
 				struct skl_plane_wm *plane_wm)
 {
@@ -5648,7 +5655,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
 	struct skl_wm_level *levels = plane_wm->wm;
 	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
 
-	skl_compute_plane_wm(crtc_state, 0, latency,
+	skl_compute_plane_wm(crtc_state, plane, 0, latency,
 			     wm_params, &levels[0],
 			     sagv_wm);
 }
@@ -5723,6 +5730,7 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5731,13 +5739,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->wm);
 
 	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
 				  &wm->wm[0], &wm_params);
 
 	if (DISPLAY_VER(dev_priv) >= 12) {
-		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
+		tgl_compute_sagv_wm(crtc_state, plane, &wm_params, wm);
 
 		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
 					  &wm->sagv.wm0, &wm_params);
@@ -5751,6 +5759,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 				 enum plane_id plane_id)
 {
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.raw.planes[plane_id];
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5762,7 +5771,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
+	skl_compute_wm_levels(crtc_state, plane, &wm_params, wm->uv_wm);
 
 	return 0;
 }
-- 
2.24.1.485.gad05a3d8e5