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

[PATCH v4 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    |   3 +
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 drivers/gpu/drm/i915/i915_reg.h               |   3 +
 drivers/gpu/drm/i915/intel_pm.c               |  73 +++++++++++-
 9 files changed, 236 insertions(+), 29 deletions(-)

-- 
2.17.1

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

[PATCH v4 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.

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/intel_pm.c | 73 +++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 67d171456f59..662a36ff2f43 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);
@@ -3801,8 +3801,8 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
 
 		if (skl_needs_memory_bw_wa(dev_priv) &&
 		    plane->base.state->fb->modifier ==
-		    I915_FORMAT_MOD_X_TILED)
-			latency += 15;
+			    I915_FORMAT_MOD_X_TILED)
+				latency += 15;
 
 		/*
 		 * If any of the planes on this pipe don't enable wm levels that
@@ -3816,6 +3816,73 @@ 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) >= 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,
-- 
2.17.1

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

[PATCH v4 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)

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 40390d855815..8750a34c3e28 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -512,6 +512,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.CHECKPATCH: warning for Refactor Gen11+ SAGV support

[Patchwork]

== Series Details ==

Series: Refactor Gen11+ SAGV support
URL   : https://patchwork.freedesktop.org/series/68028/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
248cbaf741d0 drm/i915: Refactor intel_can_enable_sagv
-:33: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 32)
#33: FILE: drivers/gpu/drm/i915/intel_pm.c:3802:
 		if (skl_needs_memory_bw_wa(dev_priv) &&
[...]
+				latency += 15;

-:66: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#66: FILE: drivers/gpu/drm/i915/intel_pm.c:3839:
+	for_each_new_intel_crtc_in_state(state, crtc,
+					     new_crtc_state, i) {

-:67: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#67: FILE: drivers/gpu/drm/i915/intel_pm.c:3840:
+					     new_crtc_state, i) {
+

-:83: ERROR:OPEN_BRACE: that open brace { should be on the previous line
#83: FILE: drivers/gpu/drm/i915/intel_pm.c:3856:
+			for (level = ilk_wm_max_level(dev_priv);
+			     !wm->wm[level].plane_en; --level)
+			     { }

total: 1 errors, 1 warnings, 2 checks, 91 lines checked
4b87f530f108 drm/i915: Restrict qgv points which don't have enough bandwidth.
-:124: CHECK:LINE_SPACING: Please don't use multiple blank lines
#124: FILE: drivers/gpu/drm/i915/display/intel_bw.c:138:
+
+

-:329: CHECK:BRACES: braces {} should be used on all arms of this statement
#329: FILE: drivers/gpu/drm/i915/display/intel_display.c:14208:
+	if (INTEL_GEN(dev_priv) < 11) {
[...]
+	else
[...]

-:333: ERROR:ELSE_AFTER_BRACE: else should follow close brace '}'
#333: FILE: drivers/gpu/drm/i915/display/intel_display.c:14212:
+	}
+	else

total: 1 errors, 0 warnings, 2 checks, 298 lines checked

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

✗ Fi.CI.SPARSE: warning for Refactor Gen11+ SAGV support

[Patchwork]

== Series Details ==

Series: Refactor Gen11+ SAGV support
URL   : https://patchwork.freedesktop.org/series/68028/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915: Refactor intel_can_enable_sagv
-
+drivers/gpu/drm/i915/intel_pm.c:3753:6: warning: symbol 'skl_can_enable_sagv' was not declared. Should it be static?
+drivers/gpu/drm/i915/intel_pm.c:3819:6: warning: symbol 'icl_can_enable_sagv' was not declared. Should it be static?

Commit: drm/i915: Restrict qgv points which don't have enough bandwidth.
Okay!

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

✓ Fi.CI.BAT: success for Refactor Gen11+ SAGV support

[Patchwork]

== Series Details ==

Series: Refactor Gen11+ SAGV support
URL   : https://patchwork.freedesktop.org/series/68028/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7096 -> Patchwork_14812
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@basic-read-write:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][3] -> [DMESG-WARN][4] ([fdo#102614])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@i915_getparams_basic@basic-eu-total:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/fi-icl-u3/igt@i915_getparams_basic@basic-eu-total.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/fi-icl-u3/igt@i915_getparams_basic@basic-eu-total.html

  * igt@i915_selftest@live_hangcheck:
    - {fi-icl-guc}:       [DMESG-FAIL][7] ([fdo#111144] / [fdo#111678]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/fi-icl-guc/igt@i915_selftest@live_hangcheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/fi-icl-guc/igt@i915_selftest@live_hangcheck.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - {fi-icl-u4}:        [FAIL][9] ([fdo#111045]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/fi-icl-u4/igt@kms_chamelium@hdmi-crc-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/fi-icl-u4/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - fi-snb-2520m:       [FAIL][11] ([fdo#100368]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/fi-snb-2520m/igt@kms_flip@basic-flip-vs-wf_vblank.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/fi-snb-2520m/igt@kms_flip@basic-flip-vs-wf_vblank.html

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

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111144]: https://bugs.freedesktop.org/show_bug.cgi?id=111144
  [fdo#111678]: https://bugs.freedesktop.org/show_bug.cgi?id=111678


Participating hosts (53 -> 46)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7096 -> Patchwork_14812

  CI-20190529: 20190529
  CI_DRM_7096: 2ed00991bec362d1e5fce84b6f0863ba44b79eee @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5231: e293051f8f99c72cb01d21e4b73a5928ea351eb3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14812: 4b87f530f10818b31e1969b4357ca2ad369f8ba8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4b87f530f108 drm/i915: Restrict qgv points which don't have enough bandwidth.
248cbaf741d0 drm/i915: Refactor intel_can_enable_sagv

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Refactor Gen11+ SAGV support

[Patchwork]

== Series Details ==

Series: Refactor Gen11+ SAGV support
URL   : https://patchwork.freedesktop.org/series/68028/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7096_full -> Patchwork_14812_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_switch@vcs0-heavy-queue:
    - shard-apl:          [PASS][1] -> [INCOMPLETE][2] ([fdo#103927]) +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-apl7/igt@gem_ctx_switch@vcs0-heavy-queue.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-apl4/igt@gem_ctx_switch@vcs0-heavy-queue.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb7/igt@gem_exec_async@concurrent-writes-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb1/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_tiled_blits@normal:
    - shard-kbl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#111904])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-kbl4/igt@gem_tiled_blits@normal.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-kbl3/igt@gem_tiled_blits@normal.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-hsw:          [PASS][7] -> [DMESG-WARN][8] ([fdo#111870])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-snb:          [PASS][9] -> [DMESG-WARN][10] ([fdo#111870])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-snb4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-a-ctm-0-75:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108682])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl9/igt@kms_color@pipe-a-ctm-0-75.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl10/igt@kms_color@pipe-a-ctm-0-75.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding:
    - shard-apl:          [PASS][15] -> [FAIL][16] ([fdo#103232])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-apl6/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-apl7/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [PASS][17] -> [FAIL][18] ([fdo#105767])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#105363])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl3/igt@kms_flip@flip-vs-expired-vblank.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl9/igt@kms_flip@flip-vs-expired-vblank.html
    - shard-glk:          [PASS][21] -> [FAIL][22] ([fdo#105363])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-glk4/igt@kms_flip@flip-vs-expired-vblank.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-glk3/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         [PASS][23] -> [FAIL][24] ([fdo#103167]) +6 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_psr@psr2_sprite_plane_onoff:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#109441]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb2/igt@kms_psr@psr2_sprite_plane_onoff.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb4/igt@kms_psr@psr2_sprite_plane_onoff.html

  * igt@prime_busy@after-bsd2:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109276]) +15 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb4/igt@prime_busy@after-bsd2.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb5/igt@prime_busy@after-bsd2.html

  * igt@prime_busy@wait-hang-vebox:
    - shard-iclb:         [PASS][29] -> [INCOMPLETE][30] ([fdo#107713] / [fdo#110428 ])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb2/igt@prime_busy@wait-hang-vebox.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb1/igt@prime_busy@wait-hang-vebox.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][31] ([fdo#110841]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb5/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][33] ([fdo#111325]) -> [PASS][34] +4 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb5/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-snb:          [TIMEOUT][35] -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-snb2/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][37] ([fdo#104108]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl9/igt@gem_softpin@noreloc-s3.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl7/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-snb:          [DMESG-WARN][39] ([fdo#111870]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-snb6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-snb2/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-hsw:          [DMESG-WARN][41] ([fdo#111870]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-hsw1/igt@gem_userptr_blits@sync-unmap-after-close.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-hsw8/igt@gem_userptr_blits@sync-unmap-after-close.html

  * {igt@i915_pm_dc@dc5-dpms}:
    - shard-iclb:         [FAIL][43] ([fdo#111795 ]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb3/igt@i915_pm_dc@dc5-dpms.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb4/igt@i915_pm_dc@dc5-dpms.html

  * igt@i915_selftest@live_gem_contexts:
    - shard-iclb:         [DMESG-FAIL][45] -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb1/igt@i915_selftest@live_gem_contexts.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb2/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding:
    - shard-skl:          [FAIL][47] ([fdo#103232]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [DMESG-WARN][49] ([fdo#106107]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl6/igt@kms_fbcon_fbt@psr-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl7/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [DMESG-WARN][51] ([fdo#108566]) -> [PASS][52] +2 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-apl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-apl7/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move:
    - shard-iclb:         [FAIL][53] ([fdo#103167]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu:
    - shard-skl:          [FAIL][55] ([fdo#103167]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl5/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl3/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][57] ([fdo#108145]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][59] ([fdo#108145] / [fdo#110403]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][61] ([fdo#103166]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][63] ([fdo#109441]) -> [PASS][64] +2 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb8/igt@kms_psr@psr2_primary_mmap_cpu.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][65] ([fdo#109276]) -> [PASS][66] +10 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb3/igt@prime_busy@hang-bsd2.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb4/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [FAIL][67] ([fdo#111330]) -> [SKIP][68] ([fdo#109276]) +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7096/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/shard-iclb6/igt@gem_mocs_settings@mocs-settings-bsd2.html

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108682]: https://bugs.freedesktop.org/show_bug.cgi?id=108682
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110428 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110428 
  [fdo#110548]: https://bugs.freedesktop.org/show_bug.cgi?id=110548
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111795 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111795 
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#111904]: https://bugs.freedesktop.org/show_bug.cgi?id=111904


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7096 -> Patchwork_14812

  CI-20190529: 20190529
  CI_DRM_7096: 2ed00991bec362d1e5fce84b6f0863ba44b79eee @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5231: e293051f8f99c72cb01d21e4b73a5928ea351eb3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14812: 4b87f530f10818b31e1969b4357ca2ad369f8ba8 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14812/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[James Ausmus]

On Tue, Oct 15, 2019 at 04:50:12PM +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.
> 
> 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/intel_pm.c | 73 +++++++++++++++++++++++++++++++--
>  1 file changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67d171456f59..662a36ff2f43 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);
> @@ -3801,8 +3801,8 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
>  
>  		if (skl_needs_memory_bw_wa(dev_priv) &&
>  		    plane->base.state->fb->modifier ==
> -		    I915_FORMAT_MOD_X_TILED)
> -			latency += 15;
> +			    I915_FORMAT_MOD_X_TILED)
> +				latency += 15;

This whitespace change doesn't look right

>  
>  		/*
>  		 * If any of the planes on this pipe don't enable wm levels that
> @@ -3816,6 +3816,73 @@ 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];

This isn't exactly the same for TGL. From BSpec 49325, "Calculate
watermark level 0 with level 0 latency + SAGV block time. If the result
can be supported (does not exceed maximum), then the plane can tolerate
SAGV", so I think it can be simplified for Gen12+ by not having to loop
through all the wm levels.

-James

> +
> +			/*
> +			 * 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) >= 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,
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[James Ausmus]

On Tue, Oct 15, 2019 at 04:50:13PM +0300, Stanislav Lisovskiy wrote:
> 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)
> 
> 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);

This could also be enabling qgv points, so this might better be "Failed
to set qgv point mask".

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

Nit: "Could not mask unusable qgv points"

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

Nit: "Could not unmask allowed qgv points"

With those:

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> +			      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 40390d855815..8750a34c3e28 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -512,6 +512,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

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

[Lisovskiy, Stanislav]

On Thu, 2019-10-17 at 14:53 -0700, James Ausmus wrote:
> On Tue, Oct 15, 2019 at 04:50:12PM +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.
> > 
> > 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/intel_pm.c | 73
> > +++++++++++++++++++++++++++++++--
> >  1 file changed, 70 insertions(+), 3 deletions(-)
> > 
> > +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];
> 
> This isn't exactly the same for TGL. From BSpec 49325, "Calculate
> watermark level 0 with level 0 latency + SAGV block time. If the
> result
> can be supported (does not exceed maximum), then the plane can
> tolerate
> SAGV", so I think it can be simplified for Gen12+ by not having to
> loop
> through all the wm levels.
> 
> -James

Yes, we discussed that with Ville as I understood, properly doing
that for TGL requires changes to watermark/ddb allocation algorithm,
so that we check if dbuf can fit Level 0 watermark increased by SAGV
block time. I was not sure whether should I add this patch to this
series or proceed with that separately, probably this is worth adding
here - however then it would require changes to watermark algorithm,
as we need to recalculate min_ddb_alloc for Level 0 latency + SAGV
and then check if we fit into DBuf again.
So basically we'll have to calculate watermark level 0 twice - once for
SAGV enabled and once for SAGV disabled case, I would probably do it 
already during watermark calculations not when checking bandwidth and
then set some "SAGV can enable" flag, which will be then used by
intel_can_enable_sagv, so that we act accordingly when
bandwidth check and restricting qgv points happens. For pre-gen 12 it
will otherwise use the old algorithm.

> 
> > +
> > +			/*
> > +			 * 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) >= 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,
> > -- 
> > 2.17.1
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[James Ausmus]

On Fri, Oct 18, 2019 at 01:34:35AM -0700, Lisovskiy, Stanislav wrote:
> On Thu, 2019-10-17 at 14:53 -0700, James Ausmus wrote:
> > On Tue, Oct 15, 2019 at 04:50:12PM +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.
> > > 
> > > 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/intel_pm.c | 73
> > > +++++++++++++++++++++++++++++++--
> > >  1 file changed, 70 insertions(+), 3 deletions(-)
> > > 
> > > +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];
> > 
> > This isn't exactly the same for TGL. From BSpec 49325, "Calculate
> > watermark level 0 with level 0 latency + SAGV block time. If the
> > result
> > can be supported (does not exceed maximum), then the plane can
> > tolerate
> > SAGV", so I think it can be simplified for Gen12+ by not having to
> > loop
> > through all the wm levels.
> > 
> > -James
> 
> Yes, we discussed that with Ville as I understood, properly doing
> that for TGL requires changes to watermark/ddb allocation algorithm,
> so that we check if dbuf can fit Level 0 watermark increased by SAGV
> block time. I was not sure whether should I add this patch to this
> series or proceed with that separately, probably this is worth adding
> here - however then it would require changes to watermark algorithm,
> as we need to recalculate min_ddb_alloc for Level 0 latency + SAGV
> and then check if we fit into DBuf again.
> So basically we'll have to calculate watermark level 0 twice - once for
> SAGV enabled and once for SAGV disabled case, I would probably do it 
> already during watermark calculations not when checking bandwidth and
> then set some "SAGV can enable" flag, which will be then used by
> intel_can_enable_sagv, so that we act accordingly when
> bandwidth check and restricting qgv points happens. For pre-gen 12 it
> will otherwise use the old algorithm.

OK, that works then. With the whitespace fix added:

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> 
> > 
> > > +
> > > +			/*
> > > +			 * 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) >= 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,
> > > -- 
> > > 2.17.1
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx