[PATCH v6 0/2] Refactor Gen11+ SAGV support

[PATCH v6 0/2] Refactor Gen11+ SAGV support

[Stanislav Lisovskiy]

For Gen11+ platforms BSpec suggests disabling specific
QGV points separately, depending on bandwidth limitations
and current display configuration. Thus it required adding
a new PCode request for disabling QGV points and some
refactoring of already existing SAGV code.

Stanislav Lisovskiy (2):
  drm/i915: Refactor intel_can_enable_sagv
  drm/i915: Restrict qgv points which don't have enough bandwidth.

 drivers/gpu/drm/i915/display/intel_atomic.c   |  16 ++
 drivers/gpu/drm/i915/display/intel_atomic.h   |   3 +
 drivers/gpu/drm/i915/display/intel_bw.c       | 105 ++++++--
 drivers/gpu/drm/i915/display/intel_bw.h       |   2 +
 drivers/gpu/drm/i915/display/intel_display.c  |  58 ++++-
 .../drm/i915/display/intel_display_types.h    |  11 +
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 drivers/gpu/drm/i915/i915_reg.h               |   3 +
 drivers/gpu/drm/i915/intel_pm.c               | 229 +++++++++++++++++-
 9 files changed, 395 insertions(+), 34 deletions(-)

-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v6 1/2] drm/i915: Refactor intel_can_enable_sagv

[Stanislav Lisovskiy]

Currently intel_can_enable_sagv function contains
a mix of workarounds for different platforms
some of them are not valid for gens >= 11 already,
so lets split it into separate functions.

v2:
    - Rework watermark calculation algorithm to
      attempt to calculate Level 0 watermark
      with added sagv block time latency and
      check if it fits in DBuf in order to
      determine if SAGV can be enabled already
      at this stage, just as BSpec 49325 states.
      if that fails rollback to usual Level 0
      latency and disable SAGV.
    - Remove unneeded tabs(James Ausmus)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Ville Syrjälä <ville.syrjala@intel.com>
Cc: James Ausmus <james.ausmus@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |   8 +
 drivers/gpu/drm/i915/intel_pm.c               | 229 +++++++++++++++++-
 2 files changed, 229 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..891e06af228d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -490,6 +490,13 @@ struct intel_atomic_state {
 	 */
 	u8 active_pipe_changes;
 
+	/*
+	 * For Gen12 only after calculating watermarks with
+	 * additional latency, we can determine if SAGV can be enabled
+	 * or not for that particular configuration.
+	 */
+	bool gen12_can_sagv;
+
 	u8 active_pipes;
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
@@ -642,6 +649,7 @@ struct skl_plane_wm {
 	struct skl_wm_level wm[8];
 	struct skl_wm_level uv_wm[8];
 	struct skl_wm_level trans_wm;
+	struct skl_wm_level sagv_wm_l0;
 	bool is_planar;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 67d171456f59..7a08fa950677 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3750,7 +3750,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-bool intel_can_enable_sagv(struct intel_atomic_state *state)
+bool skl_can_enable_sagv(struct intel_atomic_state *state)
 {
 	struct drm_device *dev = state->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3816,6 +3816,75 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
 	return true;
 }
 
+bool icl_can_enable_sagv(struct intel_atomic_state *state)
+{
+	struct drm_device *dev = state->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *new_crtc_state;
+	int level, latency;
+	int i;
+	int plane_id;
+
+	if (!intel_has_sagv(dev_priv))
+		return false;
+
+	/*
+	 * If there are no active CRTCs, no additional checks need be performed
+	 */
+	if (hweight8(state->active_pipes) == 0)
+		return true;
+
+	for_each_new_intel_crtc_in_state(state, crtc,
+					     new_crtc_state, i) {
+
+		if (crtc->base.state->adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
+			return false;
+
+		if (!new_crtc_state->base.enable)
+			continue;
+
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			struct skl_plane_wm *wm =
+				&new_crtc_state->wm.skl.optimal.planes[plane_id];
+
+			/* Skip this plane if it's not enabled */
+			if (!wm->wm[0].plane_en)
+				continue;
+
+			/* Find the highest enabled wm level for this plane */
+			for (level = ilk_wm_max_level(dev_priv);
+			     !wm->wm[level].plane_en; --level)
+			     { }
+
+			latency = dev_priv->wm.skl_latency[level];
+
+			/*
+			 * If any of the planes on this pipe don't enable wm levels that
+			 * incur memory latencies higher than sagv_block_time_us we
+			 * can't enable SAGV.
+			 */
+			if (latency < dev_priv->sagv_block_time_us)
+				return false;
+		}
+	}
+
+	return true;
+}
+
+bool intel_can_enable_sagv(struct intel_atomic_state *state)
+{
+	struct drm_device *dev = state->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (INTEL_GEN(dev_priv) >= 12)
+		return state->gen12_can_sagv;
+	else if (INTEL_GEN(dev_priv) == 11)
+		return icl_can_enable_sagv(state);
+
+	return skl_can_enable_sagv(state);
+}
+
 static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv,
 			      const struct intel_crtc_state *crtc_state,
 			      const u64 total_data_rate,
@@ -4028,6 +4097,7 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 int color_plane);
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 				 int level,
+				 u32 latency,
 				 const struct skl_wm_params *wp,
 				 const struct skl_wm_level *result_prev,
 				 struct skl_wm_level *result /* out */);
@@ -4050,7 +4120,8 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	WARN_ON(ret);
 
 	for (level = 0; level <= max_level; level++) {
-		skl_compute_plane_wm(crtc_state, level, &wp, &wm, &wm);
+		u32 latency = dev_priv->wm.skl_latency[level];
+		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -4505,6 +4576,73 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *crtc_state,
 	return total_data_rate;
 }
 
+static int
+tgl_check_pipe_fits_sagv_wm(struct intel_crtc_state *crtc_state,
+		      struct skl_ddb_allocation *ddb /* out */)
+{
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct skl_ddb_entry *alloc = &crtc_state->wm.skl.ddb;
+	u16 alloc_size;
+	u16 total[I915_MAX_PLANES] = {};
+	u64 total_data_rate;
+	enum plane_id plane_id;
+	int num_active;
+	u64 plane_data_rate[I915_MAX_PLANES] = {};
+	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
+	u32 blocks;
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		total_data_rate =
+			icl_get_total_relative_data_rate(crtc_state,
+							 plane_data_rate);
+	else
+		total_data_rate =
+			skl_get_total_relative_data_rate(crtc_state,
+							 plane_data_rate,
+							 uv_plane_data_rate);
+
+
+	skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state, total_data_rate,
+					   ddb, alloc, &num_active);
+	alloc_size = skl_ddb_entry_size(alloc);
+	if (alloc_size == 0)
+		return -ENOSPC;
+
+	/* Allocate fixed number of blocks for cursor. */
+	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active);
+	alloc_size -= total[PLANE_CURSOR];
+	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
+		alloc->end - total[PLANE_CURSOR];
+	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
+
+	/*
+	 * Do check if we can fit L0 + sagv_block_time and
+	 * disable SAGV if we can't.
+	 */
+	blocks = 0;
+	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+		const struct skl_plane_wm *wm =
+			&crtc_state->wm.skl.optimal.planes[plane_id];
+
+		if (plane_id == PLANE_CURSOR) {
+			if (WARN_ON(wm->sagv_wm_l0.min_ddb_alloc >
+				    total[PLANE_CURSOR])) {
+				blocks = U32_MAX;
+				break;
+			}
+			continue;
+		}
+
+		blocks += wm->sagv_wm_l0.min_ddb_alloc;
+		if (blocks > alloc_size) {
+			return -ENOSPC;
+		}
+	}
+	return 0;
+}
+
 static int
 skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state,
 		      struct skl_ddb_allocation *ddb /* out */)
@@ -4934,12 +5072,12 @@ static bool skl_wm_has_lines(struct drm_i915_private *dev_priv, int level)
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 				 int level,
+				 u32 latency,
 				 const struct skl_wm_params *wp,
 				 const struct skl_wm_level *result_prev,
 				 struct skl_wm_level *result /* out */)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
-	u32 latency = dev_priv->wm.skl_latency[level];
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
 	u32 res_blocks, res_lines, min_ddb_alloc = 0;
@@ -5060,19 +5198,45 @@ 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 skl_wm_params *wm_params,
-		      struct skl_wm_level *levels)
+		      struct skl_plane_wm *plane_wm,
+		      bool yuv)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	int level, max_level = ilk_wm_max_level(dev_priv);
+	/*
+	 * Check which kind of plane is it and based on that calculate
+	 * correspondent WM levels.
+	 */
+	struct skl_wm_level *levels = yuv ? plane_wm->uv_wm : plane_wm->wm;
 	struct skl_wm_level *result_prev = &levels[0];
 
 	for (level = 0; level <= max_level; level++) {
 		struct skl_wm_level *result = &levels[level];
+		u32 latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, wm_params,
-				     result_prev, result);
+		skl_compute_plane_wm(crtc_state, level, latency,
+				     wm_params, result_prev, result);
 
 		result_prev = result;
+		if (level == 0) {
+			/*
+			 * For Gen12 if it is an L0 we need to also
+			 * consider sagv_block_time when calculating
+			 * L0 watermark - we will need that when making
+			 * a decision whether enable SAGV or not.
+			 * For older gens we agreed to copy L0 value for
+			 * compatibility.
+			 */
+			if ((INTEL_GEN(dev_priv) >= 12)) {
+				latency += dev_priv->sagv_block_time_us;
+				skl_compute_plane_wm(crtc_state, level, latency,
+				     wm_params, result_prev,
+				    &plane_wm->sagv_wm_l0);
+			}
+			else 
+				memcpy(&plane_wm->sagv_wm_l0, &levels[0],
+					sizeof(struct skl_wm_level));
+		}
 	}
 }
 
@@ -5166,7 +5330,7 @@ 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, &wm_params, wm, false);
 	skl_compute_transition_wm(crtc_state, &wm_params, wm);
 
 	return 0;
@@ -5188,7 +5352,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, &wm_params, wm, true);
 
 	return 0;
 }
@@ -5768,20 +5932,26 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state,
 static int
 skl_compute_wm(struct intel_atomic_state *state)
 {
+	struct drm_device *dev = state->base.dev;
+	const struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *new_crtc_state;
 	struct intel_crtc_state *old_crtc_state;
 	struct skl_ddb_values *results = &state->wm_results;
+	struct skl_ddb_allocation *ddb = &state->wm_results.ddb;
 	bool changed = false;
 	int ret, i;
 
 	/* Clear all dirty flags */
 	results->dirty_pipes = 0;
 
+	state->gen12_can_sagv = false;
+
 	ret = skl_ddb_add_affected_pipes(state, &changed);
 	if (ret || !changed)
 		return ret;
 
+
 	/*
 	 * Calculate WM's for all pipes that are part of this transaction.
 	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
@@ -5804,6 +5974,49 @@ skl_compute_wm(struct intel_atomic_state *state)
 			results->dirty_pipes |= drm_crtc_mask(&crtc->base);
 	}
 
+	if (INTEL_GEN(dev_priv) < 12)
+		goto compute_ddb;
+
+	/*
+	 * Lets assume we can tolerate SAGV for now,
+	 * until watermark calculations prove the opposite
+	 * if any of the pipe planes in the state will
+	 * fail the requirements it will be assigned to false
+	 * in skl_compute_ddb.
+	 */
+	state->gen12_can_sagv = true;
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		ret = tgl_check_pipe_fits_sagv_wm(new_crtc_state, ddb);
+		if (ret) {
+			state->gen12_can_sagv = false;
+			break;
+		}
+	}
+
+	if (state->gen12_can_sagv) {
+		/*
+		 * If we determined that we can actually enable SAGV, then
+		 * actually use those levels tgl_check_pipe_fits_sagv_wm
+		 * has already taken care of checking if L0 + sagv block time
+		 * fits into ddb.
+		 */
+		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+			struct intel_plane *plane;
+			for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+				enum plane_id plane_id = plane->id;
+				struct skl_plane_wm *plane_wm = \
+				    &new_crtc_state->wm.skl.optimal.planes[plane_id];
+				struct skl_wm_level *sagv_wm0 = &plane_wm->sagv_wm_l0;
+				struct skl_wm_level *l0_wm0 = &plane_wm->wm[0];
+				memcpy(l0_wm0, sagv_wm0, sizeof(struct skl_wm_level));
+			}
+		}
+	}
+
+compute_ddb:
 	ret = skl_compute_ddb(state);
 	if (ret)
 		return ret;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v6 2/2] drm/i915: Restrict qgv points which don't have enough bandwidth.

[Stanislav Lisovskiy]

According to BSpec 53998, we should try to
restrict qgv points, which can't provide
enough bandwidth for desired display configuration.

Currently we are just comparing against all of
those and take minimum(worst case).

v2: Fixed wrong PCode reply mask, removed hardcoded
    values.

v3: Forbid simultaneous legacy SAGV PCode requests and
    restricting qgv points. Put the actual restriction
    to commit function, added serialization(thanks to Ville)
    to prevent commit being applied out of order in case of
    nonblocking and/or nomodeset commits.

v4:
    - Minor code refactoring, fixed few typos(thanks to James Ausmus)
    - Change the naming of qgv point
      masking/unmasking functions(James Ausmus).
    - Simplify the masking/unmasking operation itself,
      as we don't need to mask only single point per request(James Ausmus)
    - Reject and stick to highest bandwidth point if SAGV
      can't be enabled(BSpec)

Reviewed-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Ville Syrjälä <ville.syrjala@intel.com>
Cc: James Ausmus <james.ausmus@intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |  16 +++
 drivers/gpu/drm/i915/display/intel_atomic.h   |   3 +
 drivers/gpu/drm/i915/display/intel_bw.c       | 105 ++++++++++++++----
 drivers/gpu/drm/i915/display/intel_bw.h       |   2 +
 drivers/gpu/drm/i915/display/intel_display.c  |  58 +++++++++-
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 drivers/gpu/drm/i915/i915_reg.h               |   3 +
 8 files changed, 166 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index c5a552a69752..b3f4f02f380b 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -207,6 +207,22 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	return &crtc_state->base;
 }
 
+int intel_atomic_serialize_global_state(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc *crtc;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_crtc_state *crtc_state;
+
+		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	return 0;
+}
+
 /**
  * intel_crtc_destroy_state - destroy crtc state
  * @crtc: drm crtc
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h
index 58065d3161a3..fd17b3ca257f 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
@@ -7,6 +7,7 @@
 #define __INTEL_ATOMIC_H__
 
 #include <linux/types.h>
+#include "intel_display_types.h"
 
 struct drm_atomic_state;
 struct drm_connector;
@@ -38,6 +39,8 @@ void intel_crtc_destroy_state(struct drm_crtc *crtc,
 struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
 void intel_atomic_state_clear(struct drm_atomic_state *state);
 
+int intel_atomic_serialize_global_state(struct intel_atomic_state *state);
+
 struct intel_crtc_state *
 intel_atomic_get_crtc_state(struct drm_atomic_state *state,
 			    struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 22e83f857de8..09f786cfdfaa 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -8,6 +8,8 @@
 #include "intel_bw.h"
 #include "intel_display_types.h"
 #include "intel_sideband.h"
+#include "intel_atomic.h"
+#include "intel_pm.h"
 
 /* Parameters for Qclk Geyserville (QGV) */
 struct intel_qgv_point {
@@ -113,6 +115,27 @@ static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
 	return 0;
 }
 
+int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
+				  u32 points_mask)
+{
+	int ret;
+
+	/* bspec says to keep retrying for at least 1 ms */
+	ret = skl_pcode_request(dev_priv, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
+				points_mask,
+				GEN11_PCODE_POINTS_RESTRICTED_MASK,
+				GEN11_PCODE_POINTS_RESTRICTED,
+				1);
+
+	if (ret < 0) {
+		DRM_ERROR("Failed to disable qgv points (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+
 static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
 			      struct intel_qgv_info *qi)
 {
@@ -270,22 +293,6 @@ void intel_bw_init_hw(struct drm_i915_private *dev_priv)
 		icl_get_bw_info(dev_priv, &icl_sa_info);
 }
 
-static unsigned int intel_max_data_rate(struct drm_i915_private *dev_priv,
-					int num_planes)
-{
-	if (INTEL_GEN(dev_priv) >= 11)
-		/*
-		 * FIXME with SAGV disabled maybe we can assume
-		 * point 1 will always be used? Seems to match
-		 * the behaviour observed in the wild.
-		 */
-		return min3(icl_max_bw(dev_priv, num_planes, 0),
-			    icl_max_bw(dev_priv, num_planes, 1),
-			    icl_max_bw(dev_priv, num_planes, 2));
-	else
-		return UINT_MAX;
-}
-
 static unsigned int intel_bw_crtc_num_active_planes(const struct intel_crtc_state *crtc_state)
 {
 	/*
@@ -377,7 +384,10 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
 	unsigned int data_rate, max_data_rate;
 	unsigned int num_active_planes;
 	struct intel_crtc *crtc;
-	int i;
+	int i, ret;
+	struct intel_qgv_info qi = {};
+	u32 allowed_points = 0;
+	unsigned int max_bw_point = 0, max_bw = 0;
 
 	/* FIXME earlier gens need some checks too */
 	if (INTEL_GEN(dev_priv) < 11)
@@ -421,16 +431,67 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
 	data_rate = intel_bw_data_rate(dev_priv, bw_state);
 	num_active_planes = intel_bw_num_active_planes(dev_priv, bw_state);
 
-	max_data_rate = intel_max_data_rate(dev_priv, num_active_planes);
-
 	data_rate = DIV_ROUND_UP(data_rate, 1000);
 
-	if (data_rate > max_data_rate) {
-		DRM_DEBUG_KMS("Bandwidth %u MB/s exceeds max available %d MB/s (%d active planes)\n",
-			      data_rate, max_data_rate, num_active_planes);
+	ret = icl_get_qgv_points(dev_priv, &qi);
+	if (ret < 0)
+		return 0;
+
+	for (i = 0; i < qi.num_points; i++) {
+		max_data_rate = icl_max_bw(dev_priv, num_active_planes, i);
+		/*
+		 * We need to know which qgv point gives us
+		 * maximum bandwidth in order to disable SAGV
+		 * if we find that we exceed SAGV block time
+		 * with watermarks. By that moment we already
+		 * have those, as it is calculated earlier in
+		 * intel_atomic_check,
+		 */
+		if (max_data_rate > max_bw) {
+			max_bw_point = i;
+			max_bw = max_data_rate;
+		}
+		if (max_data_rate >= data_rate)
+			allowed_points |= 1 << i;
+		DRM_DEBUG_KMS("QGV point %d: max bw %d required %d\n",
+			      i, max_data_rate, data_rate);
+	}
+
+	/*
+	 * BSpec states that we always should have at least one allowed point
+	 * left, so if we couldn't - simply reject the configuration for obvious
+	 * reasons.
+	 */
+	if (allowed_points == 0) {
+		DRM_DEBUG_KMS("Could not find any suitable QGV points\n");
 		return -EINVAL;
 	}
 
+	/*
+	 * Leave only single point with highest bandwidth, if
+	 * we can't enable SAGV according to BSpec.
+	 */
+	if (!intel_can_enable_sagv(state))
+		allowed_points = 1 << max_bw_point;
+
+	/*
+	 * We store the ones which need to be masked as that is what PCode
+	 * actually accepts as a parameter.
+	 */
+	state->qgv_points_mask = (~allowed_points) & ((1 << qi.num_points) - 1);
+
+	/*
+	 * If the actual mask had changed we need to make sure that
+	 * the commits are serialized(in case this is a nomodeset, nonblocking)
+	 */
+	if (state->qgv_points_mask != dev_priv->qgv_points_mask) {
+		ret = intel_atomic_serialize_global_state(state);
+		if (ret) {
+			DRM_DEBUG_KMS("Could not serialize global state\n");
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index 9db10af012f4..66bf9bc10b73 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -28,5 +28,7 @@ int intel_bw_init(struct drm_i915_private *dev_priv);
 int intel_bw_atomic_check(struct intel_atomic_state *state);
 void intel_bw_crtc_update(struct intel_bw_state *bw_state,
 			  const struct intel_crtc_state *crtc_state);
+int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
+				  u32 points_mask);
 
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a8168e2aacd5..4aaf66955149 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14003,6 +14003,48 @@ static void intel_atomic_cleanup_work(struct work_struct *work)
 	intel_atomic_helper_free_state(i915);
 }
 
+static void intel_qgv_points_mask(struct intel_atomic_state *state)
+{
+	struct drm_device *dev = state->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	int ret;
+	u32 new_mask = dev_priv->qgv_points_mask | state->qgv_points_mask;
+
+	/*
+	 * Restrict required qgv points before updating the configuration.
+	 * According to BSpec we can't mask and unmask qgv points at the same
+	 * time. Also masking should be done before updating the configuration
+	 * and unmasking afterwards.
+	 */
+	ret = icl_pcode_restrict_qgv_points(dev_priv, new_mask);
+	if (ret < 0)
+		DRM_DEBUG_KMS("Could not restrict required qgv points(%d)\n",
+			      ret);
+	else
+		dev_priv->qgv_points_mask = new_mask;
+}
+
+static void intel_qgv_points_unmask(struct intel_atomic_state *state)
+{
+	struct drm_device *dev = state->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	int ret;
+	u32 new_mask = dev_priv->qgv_points_mask & state->qgv_points_mask;
+
+	/*
+	 * Allow required qgv points after updating the configuration.
+	 * According to BSpec we can't mask and unmask qgv points at the same
+	 * time. Also masking should be done before updating the configuration
+	 * and unmasking afterwards.
+	 */
+	ret = icl_pcode_restrict_qgv_points(dev_priv, new_mask);
+	if (ret < 0)
+		DRM_DEBUG_KMS("Could not restrict required qgv points(%d)\n",
+			      ret);
+	else
+		dev_priv->qgv_points_mask = new_mask;
+}
+
 static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 {
 	struct drm_device *dev = state->base.dev;
@@ -14030,6 +14072,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		}
 	}
 
+	if ((INTEL_GEN(dev_priv) >= 11))
+		intel_qgv_points_mask(state);
+
 	intel_commit_modeset_disables(state);
 
 	/* FIXME: Eventually get rid of our crtc->config pointer */
@@ -14048,8 +14093,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		 * SKL workaround: bspec recommends we disable the SAGV when we
 		 * have more then one pipe enabled
 		 */
-		if (!intel_can_enable_sagv(state))
-			intel_disable_sagv(dev_priv);
+		if (INTEL_GEN(dev_priv) < 11)
+			if (!intel_can_enable_sagv(state))
+				intel_disable_sagv(dev_priv);
 
 		intel_modeset_verify_disabled(dev_priv, state);
 	}
@@ -14127,8 +14173,12 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	if (state->modeset)
 		intel_verify_planes(state);
 
-	if (state->modeset && intel_can_enable_sagv(state))
-		intel_enable_sagv(dev_priv);
+	if (INTEL_GEN(dev_priv) < 11) {
+		if (state->modeset && intel_can_enable_sagv(state))
+			intel_enable_sagv(dev_priv);
+	}
+	else
+		intel_qgv_points_unmask(state);
 
 	drm_atomic_helper_commit_hw_done(&state->base);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 891e06af228d..c99f561e9fa5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -519,6 +519,9 @@ struct intel_atomic_state {
 	struct i915_sw_fence commit_ready;
 
 	struct llist_node freed;
+
+	/* Gen11+ only */
+	u32 qgv_points_mask;
 };
 
 struct intel_plane_state {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c46b339064c0..0b1b641a280f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1335,6 +1335,8 @@ struct drm_i915_private {
 		u8 num_planes;
 	} max_bw[6];
 
+	u32 qgv_points_mask;
+
 	struct drm_private_obj bw_obj;
 
 	struct intel_runtime_pm runtime_pm;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e24991e54897..1f3402547ba5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8869,6 +8869,7 @@ enum {
 #define   ICL_PCODE_MEM_SUBSYSYSTEM_INFO	0xd
 #define     ICL_PCODE_MEM_SS_READ_GLOBAL_INFO	(0x0 << 8)
 #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((point) << 16) | (0x1 << 8))
+#define   ICL_PCODE_SAGV_DE_MEM_SS_CONFIG	0xe
 #define   GEN6_PCODE_READ_D_COMP		0x10
 #define   GEN6_PCODE_WRITE_D_COMP		0x11
 #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
@@ -8881,6 +8882,8 @@ enum {
 #define     GEN9_SAGV_IS_DISABLED		0x1
 #define     GEN9_SAGV_ENABLE			0x3
 #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
+#define GEN11_PCODE_POINTS_RESTRICTED		0x0
+#define GEN11_PCODE_POINTS_RESTRICTED_MASK	0x1
 #define GEN6_PCODE_DATA				_MMIO(0x138128)
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BUILD: failure for Refactor Gen11+ SAGV support (rev2)

[Patchwork]

== Series Details ==

Series: Refactor Gen11+ SAGV support (rev2)
URL   : https://patchwork.freedesktop.org/series/68028/
State : failure

== Summary ==

Applying: drm/i915: Refactor intel_can_enable_sagv
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' to see the failed patch
Patch failed at 0001 drm/i915: Refactor intel_can_enable_sagv
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 mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v6 1/2] drm/i915: Refactor intel_can_enable_sagv

[James Ausmus]

On Wed, Oct 23, 2019 at 12:08:03PM +0300, Stanislav Lisovskiy wrote:
> Currently intel_can_enable_sagv function contains
> a mix of workarounds for different platforms
> some of them are not valid for gens >= 11 already,
> so lets split it into separate functions.
> 
> v2:
>     - Rework watermark calculation algorithm to
>       attempt to calculate Level 0 watermark
>       with added sagv block time latency and
>       check if it fits in DBuf in order to
>       determine if SAGV can be enabled already
>       at this stage, just as BSpec 49325 states.
>       if that fails rollback to usual Level 0
>       latency and disable SAGV.
>     - Remove unneeded tabs(James Ausmus)
> 
> v3: Rebased the patch
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@intel.com>
> Cc: James Ausmus <james.ausmus@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |   8 +
>  drivers/gpu/drm/i915/intel_pm.c               | 228 +++++++++++++++++-
>  2 files changed, 228 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 8358152e403e..f09c80c96470 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -490,6 +490,13 @@ struct intel_atomic_state {
>  	 */
>  	u8 active_pipe_changes;
>  
> +	/*
> +	 * For Gen12 only after calculating watermarks with
> +	 * additional latency, we can determine if SAGV can be enabled
> +	 * or not for that particular configuration.
> +	 */
> +	bool gen12_can_sagv;
> +
>  	u8 active_pipes;
>  	/* minimum acceptable cdclk for each pipe */
>  	int min_cdclk[I915_MAX_PIPES];
> @@ -642,6 +649,7 @@ struct skl_plane_wm {
>  	struct skl_wm_level wm[8];
>  	struct skl_wm_level uv_wm[8];
>  	struct skl_wm_level trans_wm;
> +	struct skl_wm_level sagv_wm_l0;
>  	bool is_planar;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 362234449087..c0419e4d83de 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3751,7 +3751,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
>  	return 0;
>  }
>  
> -bool intel_can_enable_sagv(struct intel_atomic_state *state)
> +bool skl_can_enable_sagv(struct intel_atomic_state *state)
>  {
>  	struct drm_device *dev = state->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -3817,6 +3817,75 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
>  	return true;
>  }
>  
> +bool icl_can_enable_sagv(struct intel_atomic_state *state)
> +{
> +	struct drm_device *dev = state->base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *crtc;
> +	struct intel_crtc_state *new_crtc_state;
> +	int level, latency;
> +	int i;
> +	int plane_id;
> +
> +	if (!intel_has_sagv(dev_priv))
> +		return false;
> +
> +	/*
> +	 * If there are no active CRTCs, no additional checks need be performed
> +	 */
> +	if (hweight8(state->active_pipes) == 0)
> +		return true;
> +
> +	for_each_new_intel_crtc_in_state(state, crtc,
> +					     new_crtc_state, i) {
> +
> +		if (crtc->base.state->adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
> +			return false;
> +
> +		if (!new_crtc_state->base.enable)
> +			continue;
> +
> +		for_each_plane_id_on_crtc(crtc, plane_id) {
> +			struct skl_plane_wm *wm =
> +				&new_crtc_state->wm.skl.optimal.planes[plane_id];
> +
> +			/* Skip this plane if it's not enabled */
> +			if (!wm->wm[0].plane_en)
> +				continue;
> +
> +			/* Find the highest enabled wm level for this plane */
> +			for (level = ilk_wm_max_level(dev_priv);
> +			     !wm->wm[level].plane_en; --level)
> +			     { }
> +
> +			latency = dev_priv->wm.skl_latency[level];
> +
> +			/*
> +			 * If any of the planes on this pipe don't enable wm levels that
> +			 * incur memory latencies higher than sagv_block_time_us we
> +			 * can't enable SAGV.
> +			 */
> +			if (latency < dev_priv->sagv_block_time_us)
> +				return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
> +bool intel_can_enable_sagv(struct intel_atomic_state *state)
> +{
> +	struct drm_device *dev = state->base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (INTEL_GEN(dev_priv) >= 12)
> +		return state->gen12_can_sagv;

This loses the interlaced mode check that the skl and icl functions
have, which is still needed for TGL


> +	else if (INTEL_GEN(dev_priv) == 11)
> +		return icl_can_enable_sagv(state);
> +
> +	return skl_can_enable_sagv(state);
> +}
> +
>  static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv,
>  			      const struct intel_crtc_state *crtc_state,
>  			      const u64 total_data_rate,
> @@ -3936,6 +4005,7 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>  				 int color_plane);
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  				 int level,
> +				 u32 latency,
>  				 const struct skl_wm_params *wp,
>  				 const struct skl_wm_level *result_prev,
>  				 struct skl_wm_level *result /* out */);
> @@ -3958,7 +4028,8 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>  	WARN_ON(ret);
>  
>  	for (level = 0; level <= max_level; level++) {
> -		skl_compute_plane_wm(crtc_state, level, &wp, &wm, &wm);
> +		u32 latency = dev_priv->wm.skl_latency[level];
> +		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
>  		if (wm.min_ddb_alloc == U16_MAX)
>  			break;
>  
> @@ -4310,6 +4381,73 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *crtc_state,
>  	return total_data_rate;
>  }
>  
> +static int
> +tgl_check_pipe_fits_sagv_wm(struct intel_crtc_state *crtc_state,
> +		      struct skl_ddb_allocation *ddb /* out */)
> +{
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct skl_ddb_entry *alloc = &crtc_state->wm.skl.ddb;
> +	u16 alloc_size;
> +	u16 total[I915_MAX_PLANES] = {};
> +	u64 total_data_rate;
> +	enum plane_id plane_id;
> +	int num_active;
> +	u64 plane_data_rate[I915_MAX_PLANES] = {};
> +	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
> +	u32 blocks;
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		total_data_rate =
> +			icl_get_total_relative_data_rate(crtc_state,
> +							 plane_data_rate);

This function is already only called on gen12+ - could drop the if
check, and the entire else block.


> +	else
> +		total_data_rate =
> +			skl_get_total_relative_data_rate(crtc_state,
> +							 plane_data_rate,
> +							 uv_plane_data_rate);
> +
> +
> +	skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state, total_data_rate,
> +					   ddb, alloc, &num_active);
> +	alloc_size = skl_ddb_entry_size(alloc);
> +	if (alloc_size == 0)
> +		return -ENOSPC;
> +
> +	/* Allocate fixed number of blocks for cursor. */
> +	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active);
> +	alloc_size -= total[PLANE_CURSOR];
> +	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
> +		alloc->end - total[PLANE_CURSOR];
> +	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
> +
> +	/*
> +	 * Do check if we can fit L0 + sagv_block_time and
> +	 * disable SAGV if we can't.
> +	 */
> +	blocks = 0;
> +	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> +		const struct skl_plane_wm *wm =
> +			&crtc_state->wm.skl.optimal.planes[plane_id];
> +
> +		if (plane_id == PLANE_CURSOR) {
> +			if (WARN_ON(wm->sagv_wm_l0.min_ddb_alloc >
> +				    total[PLANE_CURSOR])) {
> +				blocks = U32_MAX;
> +				break;
> +			}
> +			continue;
> +		}
> +
> +		blocks += wm->sagv_wm_l0.min_ddb_alloc;
> +		if (blocks > alloc_size) {
> +			return -ENOSPC;
> +		}
> +	}
> +	return 0;
> +}
> +
>  static int
>  skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state,
>  		      struct skl_ddb_allocation *ddb /* out */)
> @@ -4739,12 +4877,12 @@ static bool skl_wm_has_lines(struct drm_i915_private *dev_priv, int level)
>  
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  				 int level,
> +				 u32 latency,
>  				 const struct skl_wm_params *wp,
>  				 const struct skl_wm_level *result_prev,
>  				 struct skl_wm_level *result /* out */)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> -	u32 latency = dev_priv->wm.skl_latency[level];
>  	uint_fixed_16_16_t method1, method2;
>  	uint_fixed_16_16_t selected_result;
>  	u32 res_blocks, res_lines, min_ddb_alloc = 0;
> @@ -4865,19 +5003,45 @@ 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 skl_wm_params *wm_params,
> -		      struct skl_wm_level *levels)
> +		      struct skl_plane_wm *plane_wm,
> +		      bool yuv)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>  	int level, max_level = ilk_wm_max_level(dev_priv);
> +	/*
> +	 * Check which kind of plane is it and based on that calculate
> +	 * correspondent WM levels.
> +	 */
> +	struct skl_wm_level *levels = yuv ? plane_wm->uv_wm : plane_wm->wm;
>  	struct skl_wm_level *result_prev = &levels[0];
>  
>  	for (level = 0; level <= max_level; level++) {
>  		struct skl_wm_level *result = &levels[level];
> +		u32 latency = dev_priv->wm.skl_latency[level];
>  
> -		skl_compute_plane_wm(crtc_state, level, wm_params,
> -				     result_prev, result);
> +		skl_compute_plane_wm(crtc_state, level, latency,
> +				     wm_params, result_prev, result);
>  
>  		result_prev = result;
> +		if (level == 0) {
> +			/*
> +			 * For Gen12 if it is an L0 we need to also
> +			 * consider sagv_block_time when calculating
> +			 * L0 watermark - we will need that when making
> +			 * a decision whether enable SAGV or not.
> +			 * For older gens we agreed to copy L0 value for
> +			 * compatibility.
> +			 */
> +			if ((INTEL_GEN(dev_priv) >= 12)) {
> +				latency += dev_priv->sagv_block_time_us;
> +				skl_compute_plane_wm(crtc_state, level, latency,
> +				     wm_params, result_prev,
> +				    &plane_wm->sagv_wm_l0);
> +			}
> +			else 
> +				memcpy(&plane_wm->sagv_wm_l0, &levels[0],
> +					sizeof(struct skl_wm_level));
> +		}
>  	}
>  }
>  
> @@ -4971,7 +5135,7 @@ 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, &wm_params, wm, false);
>  	skl_compute_transition_wm(crtc_state, &wm_params, wm);
>  
>  	return 0;
> @@ -4993,7 +5157,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, &wm_params, wm, true);
>  
>  	return 0;
>  }
> @@ -5544,10 +5708,13 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state,
>  static int
>  skl_compute_wm(struct intel_atomic_state *state)
>  {
> +	struct drm_device *dev = state->base.dev;
> +	const struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *new_crtc_state;
>  	struct intel_crtc_state *old_crtc_state;
>  	struct skl_ddb_values *results = &state->wm_results;
> +	struct skl_ddb_allocation *ddb = &state->wm_results.ddb;
>  	int ret, i;
>  
>  	/* Clear all dirty flags */
> @@ -5557,6 +5724,8 @@ skl_compute_wm(struct intel_atomic_state *state)
>  	if (ret)
>  		return ret;
>  
> +	state->gen12_can_sagv = false;
> +
>  	/*
>  	 * Calculate WM's for all pipes that are part of this transaction.
>  	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
> @@ -5579,6 +5748,49 @@ skl_compute_wm(struct intel_atomic_state *state)
>  			results->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) < 12)
> +		goto compute_ddb;

I understand why you are goto'ing to avoid the extra indent below - can
the block between here and compute_ddb just be extracted to a separate
function instead?

-James

> +
> +	/*
> +	 * Lets assume we can tolerate SAGV for now,
> +	 * until watermark calculations prove the opposite
> +	 * if any of the pipe planes in the state will
> +	 * fail the requirements it will be assigned to false
> +	 * in skl_compute_ddb.
> +	 */
> +	state->gen12_can_sagv = true;
> +
> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> +					    new_crtc_state, i) {
> +		ret = tgl_check_pipe_fits_sagv_wm(new_crtc_state, ddb);
> +		if (ret) {
> +			state->gen12_can_sagv = false;
> +			break;
> +		}
> +	}
> +
> +	if (state->gen12_can_sagv) {
> +		/*
> +		 * If we determined that we can actually enable SAGV, then
> +		 * actually use those levels tgl_check_pipe_fits_sagv_wm
> +		 * has already taken care of checking if L0 + sagv block time
> +		 * fits into ddb.
> +		 */
> +		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> +					    new_crtc_state, i) {
> +			struct intel_plane *plane;
> +			for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
> +				enum plane_id plane_id = plane->id;
> +				struct skl_plane_wm *plane_wm = \
> +				    &new_crtc_state->wm.skl.optimal.planes[plane_id];
> +				struct skl_wm_level *sagv_wm0 = &plane_wm->sagv_wm_l0;
> +				struct skl_wm_level *l0_wm0 = &plane_wm->wm[0];
> +				memcpy(l0_wm0, sagv_wm0, sizeof(struct skl_wm_level));
> +			}
> +		}
> +	}
> +
> +compute_ddb:
>  	ret = skl_compute_ddb(state);
>  	if (ret)
>  		return ret;
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v6 1/2] drm/i915: Refactor intel_can_enable_sagv

[Stanislav Lisovskiy]

Currently intel_can_enable_sagv function contains
a mix of workarounds for different platforms
some of them are not valid for gens >= 11 already,
so lets split it into separate functions.

v2:
    - Rework watermark calculation algorithm to
      attempt to calculate Level 0 watermark
      with added sagv block time latency and
      check if it fits in DBuf in order to
      determine if SAGV can be enabled already
      at this stage, just as BSpec 49325 states.
      if that fails rollback to usual Level 0
      latency and disable SAGV.
    - Remove unneeded tabs(James Ausmus)

v3: Rebased the patch

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Ville Syrjälä <ville.syrjala@intel.com>
Cc: James Ausmus <james.ausmus@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |   8 +
 drivers/gpu/drm/i915/intel_pm.c               | 228 +++++++++++++++++-
 2 files changed, 228 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8358152e403e..f09c80c96470 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -490,6 +490,13 @@ struct intel_atomic_state {
 	 */
 	u8 active_pipe_changes;
 
+	/*
+	 * For Gen12 only after calculating watermarks with
+	 * additional latency, we can determine if SAGV can be enabled
+	 * or not for that particular configuration.
+	 */
+	bool gen12_can_sagv;
+
 	u8 active_pipes;
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
@@ -642,6 +649,7 @@ struct skl_plane_wm {
 	struct skl_wm_level wm[8];
 	struct skl_wm_level uv_wm[8];
 	struct skl_wm_level trans_wm;
+	struct skl_wm_level sagv_wm_l0;
 	bool is_planar;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 362234449087..c0419e4d83de 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3751,7 +3751,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-bool intel_can_enable_sagv(struct intel_atomic_state *state)
+bool skl_can_enable_sagv(struct intel_atomic_state *state)
 {
 	struct drm_device *dev = state->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3817,6 +3817,75 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
 	return true;
 }
 
+bool icl_can_enable_sagv(struct intel_atomic_state *state)
+{
+	struct drm_device *dev = state->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *new_crtc_state;
+	int level, latency;
+	int i;
+	int plane_id;
+
+	if (!intel_has_sagv(dev_priv))
+		return false;
+
+	/*
+	 * If there are no active CRTCs, no additional checks need be performed
+	 */
+	if (hweight8(state->active_pipes) == 0)
+		return true;
+
+	for_each_new_intel_crtc_in_state(state, crtc,
+					     new_crtc_state, i) {
+
+		if (crtc->base.state->adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
+			return false;
+
+		if (!new_crtc_state->base.enable)
+			continue;
+
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			struct skl_plane_wm *wm =
+				&new_crtc_state->wm.skl.optimal.planes[plane_id];
+
+			/* Skip this plane if it's not enabled */
+			if (!wm->wm[0].plane_en)
+				continue;
+
+			/* Find the highest enabled wm level for this plane */
+			for (level = ilk_wm_max_level(dev_priv);
+			     !wm->wm[level].plane_en; --level)
+			     { }
+
+			latency = dev_priv->wm.skl_latency[level];
+
+			/*
+			 * If any of the planes on this pipe don't enable wm levels that
+			 * incur memory latencies higher than sagv_block_time_us we
+			 * can't enable SAGV.
+			 */
+			if (latency < dev_priv->sagv_block_time_us)
+				return false;
+		}
+	}
+
+	return true;
+}
+
+bool intel_can_enable_sagv(struct intel_atomic_state *state)
+{
+	struct drm_device *dev = state->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (INTEL_GEN(dev_priv) >= 12)
+		return state->gen12_can_sagv;
+	else if (INTEL_GEN(dev_priv) == 11)
+		return icl_can_enable_sagv(state);
+
+	return skl_can_enable_sagv(state);
+}
+
 static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv,
 			      const struct intel_crtc_state *crtc_state,
 			      const u64 total_data_rate,
@@ -3936,6 +4005,7 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 int color_plane);
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 				 int level,
+				 u32 latency,
 				 const struct skl_wm_params *wp,
 				 const struct skl_wm_level *result_prev,
 				 struct skl_wm_level *result /* out */);
@@ -3958,7 +4028,8 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	WARN_ON(ret);
 
 	for (level = 0; level <= max_level; level++) {
-		skl_compute_plane_wm(crtc_state, level, &wp, &wm, &wm);
+		u32 latency = dev_priv->wm.skl_latency[level];
+		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -4310,6 +4381,73 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *crtc_state,
 	return total_data_rate;
 }
 
+static int
+tgl_check_pipe_fits_sagv_wm(struct intel_crtc_state *crtc_state,
+		      struct skl_ddb_allocation *ddb /* out */)
+{
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct skl_ddb_entry *alloc = &crtc_state->wm.skl.ddb;
+	u16 alloc_size;
+	u16 total[I915_MAX_PLANES] = {};
+	u64 total_data_rate;
+	enum plane_id plane_id;
+	int num_active;
+	u64 plane_data_rate[I915_MAX_PLANES] = {};
+	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
+	u32 blocks;
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		total_data_rate =
+			icl_get_total_relative_data_rate(crtc_state,
+							 plane_data_rate);
+	else
+		total_data_rate =
+			skl_get_total_relative_data_rate(crtc_state,
+							 plane_data_rate,
+							 uv_plane_data_rate);
+
+
+	skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state, total_data_rate,
+					   ddb, alloc, &num_active);
+	alloc_size = skl_ddb_entry_size(alloc);
+	if (alloc_size == 0)
+		return -ENOSPC;
+
+	/* Allocate fixed number of blocks for cursor. */
+	total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active);
+	alloc_size -= total[PLANE_CURSOR];
+	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
+		alloc->end - total[PLANE_CURSOR];
+	crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
+
+	/*
+	 * Do check if we can fit L0 + sagv_block_time and
+	 * disable SAGV if we can't.
+	 */
+	blocks = 0;
+	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+		const struct skl_plane_wm *wm =
+			&crtc_state->wm.skl.optimal.planes[plane_id];
+
+		if (plane_id == PLANE_CURSOR) {
+			if (WARN_ON(wm->sagv_wm_l0.min_ddb_alloc >
+				    total[PLANE_CURSOR])) {
+				blocks = U32_MAX;
+				break;
+			}
+			continue;
+		}
+
+		blocks += wm->sagv_wm_l0.min_ddb_alloc;
+		if (blocks > alloc_size) {
+			return -ENOSPC;
+		}
+	}
+	return 0;
+}
+
 static int
 skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state,
 		      struct skl_ddb_allocation *ddb /* out */)
@@ -4739,12 +4877,12 @@ static bool skl_wm_has_lines(struct drm_i915_private *dev_priv, int level)
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 				 int level,
+				 u32 latency,
 				 const struct skl_wm_params *wp,
 				 const struct skl_wm_level *result_prev,
 				 struct skl_wm_level *result /* out */)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
-	u32 latency = dev_priv->wm.skl_latency[level];
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
 	u32 res_blocks, res_lines, min_ddb_alloc = 0;
@@ -4865,19 +5003,45 @@ 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 skl_wm_params *wm_params,
-		      struct skl_wm_level *levels)
+		      struct skl_plane_wm *plane_wm,
+		      bool yuv)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	int level, max_level = ilk_wm_max_level(dev_priv);
+	/*
+	 * Check which kind of plane is it and based on that calculate
+	 * correspondent WM levels.
+	 */
+	struct skl_wm_level *levels = yuv ? plane_wm->uv_wm : plane_wm->wm;
 	struct skl_wm_level *result_prev = &levels[0];
 
 	for (level = 0; level <= max_level; level++) {
 		struct skl_wm_level *result = &levels[level];
+		u32 latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, wm_params,
-				     result_prev, result);
+		skl_compute_plane_wm(crtc_state, level, latency,
+				     wm_params, result_prev, result);
 
 		result_prev = result;
+		if (level == 0) {
+			/*
+			 * For Gen12 if it is an L0 we need to also
+			 * consider sagv_block_time when calculating
+			 * L0 watermark - we will need that when making
+			 * a decision whether enable SAGV or not.
+			 * For older gens we agreed to copy L0 value for
+			 * compatibility.
+			 */
+			if ((INTEL_GEN(dev_priv) >= 12)) {
+				latency += dev_priv->sagv_block_time_us;
+				skl_compute_plane_wm(crtc_state, level, latency,
+				     wm_params, result_prev,
+				    &plane_wm->sagv_wm_l0);
+			}
+			else 
+				memcpy(&plane_wm->sagv_wm_l0, &levels[0],
+					sizeof(struct skl_wm_level));
+		}
 	}
 }
 
@@ -4971,7 +5135,7 @@ 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, &wm_params, wm, false);
 	skl_compute_transition_wm(crtc_state, &wm_params, wm);
 
 	return 0;
@@ -4993,7 +5157,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, &wm_params, wm, true);
 
 	return 0;
 }
@@ -5544,10 +5708,13 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state,
 static int
 skl_compute_wm(struct intel_atomic_state *state)
 {
+	struct drm_device *dev = state->base.dev;
+	const struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *new_crtc_state;
 	struct intel_crtc_state *old_crtc_state;
 	struct skl_ddb_values *results = &state->wm_results;
+	struct skl_ddb_allocation *ddb = &state->wm_results.ddb;
 	int ret, i;
 
 	/* Clear all dirty flags */
@@ -5557,6 +5724,8 @@ skl_compute_wm(struct intel_atomic_state *state)
 	if (ret)
 		return ret;
 
+	state->gen12_can_sagv = false;
+
 	/*
 	 * Calculate WM's for all pipes that are part of this transaction.
 	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
@@ -5579,6 +5748,49 @@ skl_compute_wm(struct intel_atomic_state *state)
 			results->dirty_pipes |= BIT(crtc->pipe);
 	}
 
+	if (INTEL_GEN(dev_priv) < 12)
+		goto compute_ddb;
+
+	/*
+	 * Lets assume we can tolerate SAGV for now,
+	 * until watermark calculations prove the opposite
+	 * if any of the pipe planes in the state will
+	 * fail the requirements it will be assigned to false
+	 * in skl_compute_ddb.
+	 */
+	state->gen12_can_sagv = true;
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		ret = tgl_check_pipe_fits_sagv_wm(new_crtc_state, ddb);
+		if (ret) {
+			state->gen12_can_sagv = false;
+			break;
+		}
+	}
+
+	if (state->gen12_can_sagv) {
+		/*
+		 * If we determined that we can actually enable SAGV, then
+		 * actually use those levels tgl_check_pipe_fits_sagv_wm
+		 * has already taken care of checking if L0 + sagv block time
+		 * fits into ddb.
+		 */
+		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+			struct intel_plane *plane;
+			for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+				enum plane_id plane_id = plane->id;
+				struct skl_plane_wm *plane_wm = \
+				    &new_crtc_state->wm.skl.optimal.planes[plane_id];
+				struct skl_wm_level *sagv_wm0 = &plane_wm->sagv_wm_l0;
+				struct skl_wm_level *l0_wm0 = &plane_wm->wm[0];
+				memcpy(l0_wm0, sagv_wm0, sizeof(struct skl_wm_level));
+			}
+		}
+	}
+
+compute_ddb:
 	ret = skl_compute_ddb(state);
 	if (ret)
 		return ret;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx