intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK
@ 2020-03-16 11:37 Stanislav Lisovskiy
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks Stanislav Lisovskiy
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Stanislav Lisovskiy @ 2020-03-16 11:37 UTC (permalink / raw)
  To: intel-gfx

We need to calculate cdclk after watermarks/ddb has been calculated
as with recent hw CDCLK needs to be adjusted accordingly to DBuf
requirements, which is not possible with current code organization.

Setting CDCLK according to DBuf BW requirements and not just rejecting
if it doesn't satisfy BW requirements, will allow us to save power when
it is possible and gain additional bandwidth when it's needed - i.e
boosting both our power management and perfomance capabilities.

This patch is preparation for that, first we now extract modeset
calculation from modeset checks, in order to call it after wm/ddb
has been calculated.

Stanislav Lisovskiy (3):
  drm/i915: Decouple cdclk calculation from modeset checks
  drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
  drm/i915: Remove unneeded hack now for CDCLK

 drivers/gpu/drm/i915/display/intel_bw.c       | 46 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_bw.h       |  1 +
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 28 +++++++----
 drivers/gpu/drm/i915/display/intel_display.c  | 32 +++++++++++--
 .../drm/i915/display/intel_display_power.h    |  1 +
 drivers/gpu/drm/i915/intel_pm.c               | 34 +++++++++++++-
 drivers/gpu/drm/i915/intel_pm.h               |  3 ++
 7 files changed, 129 insertions(+), 16 deletions(-)

-- 
2.24.1.485.gad05a3d8e5

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
@ 2020-03-16 11:37 ` Stanislav Lisovskiy
  2020-03-16 23:36   ` Manasi Navare
  2020-03-17 13:21   ` Ville Syrjälä
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs Stanislav Lisovskiy
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 20+ messages in thread
From: Stanislav Lisovskiy @ 2020-03-16 11:37 UTC (permalink / raw)
  To: intel-gfx

We need to calculate cdclk after watermarks/ddb has been calculated
as with recent hw CDCLK needs to be adjusted accordingly to DBuf
requirements, which is not possible with current code organization.

Setting CDCLK according to DBuf BW requirements and not just rejecting
if it doesn't satisfy BW requirements, will allow us to save power when
it is possible and gain additional bandwidth when it's needed - i.e
boosting both our power management and perfomance capabilities.

This patch is preparation for that, first we now extract modeset
calculation from modeset checks, in order to call it after wm/ddb
has been calculated.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8f23c4d51c33..cdff3054b344 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14542,6 +14542,14 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 			return ret;
 	}
 
+	return 0;
+}
+
+static int intel_modeset_cdclk(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	int ret;
+
 	ret = intel_modeset_calc_cdclk(state);
 	if (ret)
 		return ret;
@@ -14879,10 +14887,6 @@ static int intel_atomic_check(struct drm_device *dev,
 			goto fail;
 	}
 
-	ret = intel_atomic_check_crtcs(state);
-	if (ret)
-		goto fail;
-
 	intel_fbc_choose_crtc(dev_priv, state);
 	ret = calc_watermark_data(state);
 	if (ret)
@@ -14892,6 +14896,16 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	if (any_ms) {
+		ret = intel_modeset_cdclk(state);
+		if (ret)
+			goto fail;
+	}
+
+	ret = intel_atomic_check_crtcs(state);
+	if (ret)
+		goto fail;
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (!needs_modeset(new_crtc_state) &&
-- 
2.24.1.485.gad05a3d8e5

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH v1 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks Stanislav Lisovskiy
@ 2020-03-16 11:37 ` Stanislav Lisovskiy
  2020-03-16 22:43   ` [Intel-gfx] [PATCH v2 " Stanislav Lisovskiy
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 3/3] drm/i915: Remove unneeded hack now for CDCLK Stanislav Lisovskiy
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Stanislav Lisovskiy @ 2020-03-16 11:37 UTC (permalink / raw)
  To: intel-gfx

According to BSpec max BW per slice is calculated using formula
Max BW = CDCLK * 64. Currently when calculating min CDCLK we
account only per plane requirements, however in order to avoid
FIFO underruns we need to estimate accumulated BW consumed by
all planes(ddb entries basically) residing on that particular
DBuf slice. This will allow us to put CDCLK lower and save power
when we don't need that much bandwidth or gain additional
perfomance once plane consumption grows.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c       | 46 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_bw.h       |  1 +
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 22 +++++++++
 drivers/gpu/drm/i915/display/intel_display.c  | 10 +++-
 .../drm/i915/display/intel_display_power.h    |  1 +
 drivers/gpu/drm/i915/intel_pm.c               | 34 +++++++++++++-
 drivers/gpu/drm/i915/intel_pm.h               |  3 ++
 7 files changed, 114 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 58b264bc318d..a85125110d7e 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -6,6 +6,7 @@
 #include <drm/drm_atomic_state_helper.h>
 
 #include "intel_bw.h"
+#include "intel_pm.h"
 #include "intel_display_types.h"
 #include "intel_sideband.h"
 
@@ -334,6 +335,51 @@ static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_
 	return data_rate;
 }
 
+int intel_bw_calc_min_cdclk(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	int max_bw_per_dbuf[DBUF_SLICE_MAX];
+	int i = 0;
+	enum plane_id plane_id;
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int max_bw = 0;
+	int min_cdclk;
+
+	memset(max_bw_per_dbuf, 0, sizeof(max_bw_per_dbuf[0]) * DBUF_SLICE_MAX);
+
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			struct skl_ddb_entry *plane_alloc =
+				&crtc_state->wm.skl.plane_ddb_y[plane_id];
+			struct skl_ddb_entry *uv_plane_alloc =
+				&crtc_state->wm.skl.plane_ddb_uv[plane_id];
+			unsigned int data_rate = crtc_state->data_rate[plane_id];
+			int slice_id = 0;
+			u32 dbuf_mask = skl_ddb_dbuf_slice_mask(dev_priv, plane_alloc);
+
+			dbuf_mask |= skl_ddb_dbuf_slice_mask(dev_priv, uv_plane_alloc);
+
+			DRM_DEBUG_KMS("Got dbuf mask %x for pipe %c ddb %d-%d plane %d data rate %d\n",
+				      dbuf_mask, pipe_name(crtc->pipe), plane_alloc->start,
+				      plane_alloc->end, plane_id, data_rate);
+
+			while (dbuf_mask != 0) {
+				if (dbuf_mask & 1) {
+					max_bw_per_dbuf[slice_id] += data_rate;
+					max_bw = max(max_bw, max_bw_per_dbuf[slice_id]);
+				}
+				slice_id++;
+				dbuf_mask >>= 1;
+			}
+		}
+	}
+
+	min_cdclk = max_bw / 64;
+
+	return min_cdclk;
+}
+
 void intel_bw_crtc_update(struct intel_bw_state *bw_state,
 			  const struct intel_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index a8aa7624c5aa..8a522b571c51 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -29,5 +29,6 @@ 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 intel_bw_calc_min_cdclk(struct intel_atomic_state *state);
 
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 0741d643455b..f0dcea4d6357 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -25,6 +25,7 @@
 #include "intel_cdclk.h"
 #include "intel_display_types.h"
 #include "intel_sideband.h"
+#include "intel_bw.h"
 
 /**
  * DOC: CDCLK / RAWCLK
@@ -1979,11 +1980,19 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->uapi.crtc->dev);
+	struct intel_atomic_state *state = NULL;
 	int min_cdclk;
 
 	if (!crtc_state->hw.enable)
 		return 0;
 
+	/*
+	 * FIXME: Unfortunately when this gets called from intel_modeset_setup_hw_state
+	 * there is no intel_atomic_state at all. So lets not then use it.
+	 */
+	if (crtc_state->uapi.state)
+		state = to_intel_atomic_state(crtc_state->uapi.state);
+
 	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
 
 	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
@@ -2058,6 +2067,19 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	if (IS_TIGERLAKE(dev_priv))
 		min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
 
+	/*
+	 * Similar story as with skl_write_plane_wm and intel_enable_sagv
+	 * - in some certain driver parts, we don't have any guarantee that
+	 * parent exists. So we might be having a crtc_state without
+	 * parent state.
+	 */
+	if (state) {
+		int dbuf_bw_cdclk = intel_bw_calc_min_cdclk(state);
+
+		DRM_DEBUG_KMS("DBuf bw min cdclk %d current min_cdclk %d\n", dbuf_bw_cdclk, min_cdclk);
+		min_cdclk = max(min_cdclk, dbuf_bw_cdclk);
+	}
+
 	if (min_cdclk > dev_priv->max_cdclk_freq) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cdff3054b344..aae5521424c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14632,7 +14632,7 @@ static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
 	/* See {hsw,vlv,ivb}_plane_ratio() */
 	return IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv) ||
 		IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
-		IS_IVYBRIDGE(dev_priv);
+		IS_IVYBRIDGE(dev_priv) || (INTEL_GEN(dev_priv) >= 11);
 }
 
 static int intel_atomic_check_planes(struct intel_atomic_state *state,
@@ -14681,6 +14681,14 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state,
 		if (hweight8(old_active_planes) == hweight8(new_active_planes))
 			continue;
 
+		/*
+		 * active_planes bitmask has been updated, whenever amount
+		 * of active planes had changed we need to recalculate CDCLK
+		 * as it depends on total bandwidth now, not only min_cdclk
+		 * per plane.
+		 */
+		*need_cdclk_calc = true;
+
 		ret = intel_crtc_add_planes_to_state(state, crtc, new_active_planes);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index da64a5edae7a..3446c3ce6a4f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -311,6 +311,7 @@ intel_display_power_put_async(struct drm_i915_private *i915,
 enum dbuf_slice {
 	DBUF_S1,
 	DBUF_S2,
+	DBUF_SLICE_MAX
 };
 
 #define with_intel_display_power(i915, domain, wf) \
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8375054ba27d..15300c44d9dc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3844,10 +3844,9 @@ icl_get_first_dbuf_slice_offset(u32 dbuf_slice_mask,
 	return offset;
 }
 
-static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
+u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
 {
 	u16 ddb_size = INTEL_INFO(dev_priv)->ddb_size;
-
 	drm_WARN_ON(&dev_priv->drm, ddb_size == 0);
 
 	if (INTEL_GEN(dev_priv) < 11)
@@ -3856,6 +3855,37 @@ static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
 	return ddb_size;
 }
 
+u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
+			    const struct skl_ddb_entry *entry)
+{
+	u32 slice_mask = 0;
+	u16 ddb_size = intel_get_ddb_size(dev_priv);
+	u16 num_supported_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
+	u16 slice_size = ddb_size / num_supported_slices;
+	u16 start_slice;
+	u16 end_slice;
+
+	if (!skl_ddb_entry_size(entry))
+		return 0;
+
+	start_slice = entry->start / slice_size;
+	end_slice = (entry->end - 1) / slice_size;
+
+	DRM_DEBUG_KMS("ddb size %d slices %d slice size %d start slice %d end slice %d\n",
+		      ddb_size, num_supported_slices, slice_size, start_slice, end_slice);
+
+	/*
+	 * Per plane DDB entry can in a really worst case be on multiple slices
+	 * but single entry is anyway contigious.
+	 */
+	while (start_slice <= end_slice) {
+		slice_mask |= 1 << start_slice;
+		start_slice++;
+	}
+
+	return slice_mask;
+}
+
 static u8 skl_compute_dbuf_slices(const struct intel_crtc_state *crtc_state,
 				  u8 active_pipes);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index d60a85421c5a..a9e3835927a8 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -37,6 +37,9 @@ void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
 			       struct skl_ddb_entry *ddb_y,
 			       struct skl_ddb_entry *ddb_uv);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv);
+u16 intel_get_ddb_size(struct drm_i915_private *dev_priv);
+u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
+			    const struct skl_ddb_entry *entry);
 void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 			      struct skl_pipe_wm *out);
 void g4x_wm_sanitize(struct drm_i915_private *dev_priv);
-- 
2.24.1.485.gad05a3d8e5

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH v1 3/3] drm/i915: Remove unneeded hack now for CDCLK
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks Stanislav Lisovskiy
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs Stanislav Lisovskiy
@ 2020-03-16 11:37 ` Stanislav Lisovskiy
  2020-03-16 23:51   ` Manasi Navare
  2020-03-16 20:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Consider DBuf bandwidth when calculating CDCLK Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Stanislav Lisovskiy @ 2020-03-16 11:37 UTC (permalink / raw)
  To: intel-gfx

No need to bump up CDCLK now, as it is now correctly
calculated, accounting for DBuf BW as BSpec says.

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

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index f0dcea4d6357..45469f6833b8 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2055,18 +2055,6 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	/* Account for additional needs from the planes */
 	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
 
-	/*
-	 * HACK. Currently for TGL platforms we calculate
-	 * min_cdclk initially based on pixel_rate divided
-	 * by 2, accounting for also plane requirements,
-	 * however in some cases the lowest possible CDCLK
-	 * doesn't work and causing the underruns.
-	 * Explicitly stating here that this seems to be currently
-	 * rather a Hack, than final solution.
-	 */
-	if (IS_TIGERLAKE(dev_priv))
-		min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
-
 	/*
 	 * Similar story as with skl_write_plane_wm and intel_enable_sagv
 	 * - in some certain driver parts, we don't have any guarantee that
-- 
2.24.1.485.gad05a3d8e5

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Consider DBuf bandwidth when calculating CDCLK
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
                   ` (2 preceding siblings ...)
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 3/3] drm/i915: Remove unneeded hack now for CDCLK Stanislav Lisovskiy
@ 2020-03-16 20:06 ` Patchwork
  2020-03-16 20:34 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-03-16 20:06 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Consider DBuf bandwidth when calculating CDCLK
URL   : https://patchwork.freedesktop.org/series/74739/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
70a984e1e806 drm/i915: Decouple cdclk calculation from modeset checks
01a1b24738fd drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
-:137: WARNING:LONG_LINE: line over 100 characters
#137: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2101:
+		DRM_DEBUG_KMS("DBuf bw min cdclk %d current min_cdclk %d\n", dbuf_bw_cdclk, min_cdclk);

total: 0 errors, 1 warnings, 0 checks, 195 lines checked
a2e501b7cf7c drm/i915: Remove unneeded hack now for CDCLK

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for Consider DBuf bandwidth when calculating CDCLK
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
                   ` (3 preceding siblings ...)
  2020-03-16 20:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Consider DBuf bandwidth when calculating CDCLK Patchwork
@ 2020-03-16 20:34 ` Patchwork
  2020-03-17  0:36 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2) Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-03-16 20:34 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Consider DBuf bandwidth when calculating CDCLK
URL   : https://patchwork.freedesktop.org/series/74739/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8137 -> Patchwork_16978
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16978 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16978, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16978:

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-hsw-peppy:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-hsw-peppy/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-hsw-peppy/igt@debugfs_test@read_all_entries.html

  * igt@runner@aborted:
    - fi-pnv-d510:        NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-pnv-d510/igt@runner@aborted.html
    - fi-hsw-peppy:       NOTRUN -> [FAIL][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-hsw-peppy/igt@runner@aborted.html
    - fi-gdg-551:         NOTRUN -> [FAIL][5]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-gdg-551/igt@runner@aborted.html
    - fi-snb-2520m:       NOTRUN -> [FAIL][6]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-snb-2520m/igt@runner@aborted.html
    - fi-byt-j1900:       NOTRUN -> [FAIL][7]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-byt-j1900/igt@runner@aborted.html
    - fi-elk-e7500:       NOTRUN -> [FAIL][8]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-elk-e7500/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-bdw-5557u:       [PASS][9] -> [INCOMPLETE][10] ([i915#146])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-bdw-5557u/igt@gem_exec_suspend@basic-s0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-bdw-5557u/igt@gem_exec_suspend@basic-s0.html
    - fi-bsw-n3050:       [PASS][11] -> [INCOMPLETE][12] ([i915#392])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-bsw-n3050/igt@gem_exec_suspend@basic-s0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-bsw-n3050/igt@gem_exec_suspend@basic-s0.html
    - fi-byt-j1900:       [PASS][13] -> [INCOMPLETE][14] ([i915#45])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-byt-j1900/igt@gem_exec_suspend@basic-s0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-byt-j1900/igt@gem_exec_suspend@basic-s0.html

  * igt@i915_selftest@live@execlists:
    - fi-apl-guc:         [PASS][15] -> [INCOMPLETE][16] ([fdo#103927] / [i915#1430] / [i915#656])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-apl-guc/igt@i915_selftest@live@execlists.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-apl-guc/igt@i915_selftest@live@execlists.html
    - fi-icl-dsi:         [PASS][17] -> [INCOMPLETE][18] ([i915#140])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-icl-dsi/igt@i915_selftest@live@execlists.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-icl-dsi/igt@i915_selftest@live@execlists.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-tgl-y:           [FAIL][19] ([CI#94]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][21] ([i915#217]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16978/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#1430]: https://gitlab.freedesktop.org/drm/intel/issues/1430
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#392]: https://gitlab.freedesktop.org/drm/intel/issues/392
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656


Participating hosts (48 -> 36)
------------------------------

  Additional (1): fi-tgl-dsi 
  Missing    (13): fi-bdw-samus fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-8700k fi-ivb-3770 fi-cfl-8109u fi-bsw-kefka fi-skl-lmem fi-blb-e6850 fi-byt-clapper fi-skl-6600u fi-snb-2600 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8137 -> Patchwork_16978

  CI-20190529: 20190529
  CI_DRM_8137: 5786b5e77cc17a1b494b9bdf3c3f29eedc2e2e7d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5510: e100092d50105463f58db531fa953c70cc58bb10 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16978: a2e501b7cf7c01c4cf7e6fc7052a1fc875c9a08f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a2e501b7cf7c drm/i915: Remove unneeded hack now for CDCLK
01a1b24738fd drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
70a984e1e806 drm/i915: Decouple cdclk calculation from modeset checks

== Logs ==

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH v2 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs Stanislav Lisovskiy
@ 2020-03-16 22:43   ` Stanislav Lisovskiy
  2020-03-17 13:46     ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Stanislav Lisovskiy @ 2020-03-16 22:43 UTC (permalink / raw)
  To: intel-gfx

According to BSpec max BW per slice is calculated using formula
Max BW = CDCLK * 64. Currently when calculating min CDCLK we
account only per plane requirements, however in order to avoid
FIFO underruns we need to estimate accumulated BW consumed by
all planes(ddb entries basically) residing on that particular
DBuf slice. This will allow us to put CDCLK lower and save power
when we don't need that much bandwidth or gain additional
performance once plane consumption grows.

v2: - Fix long line warning
    - Limited new DBuf bw checks to only gens >= 11

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c       | 46 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_bw.h       |  1 +
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 25 ++++++++++
 drivers/gpu/drm/i915/display/intel_display.c  | 10 +++-
 .../drm/i915/display/intel_display_power.h    |  1 +
 drivers/gpu/drm/i915/intel_pm.c               | 34 +++++++++++++-
 drivers/gpu/drm/i915/intel_pm.h               |  3 ++
 7 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 58b264bc318d..a85125110d7e 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -6,6 +6,7 @@
 #include <drm/drm_atomic_state_helper.h>
 
 #include "intel_bw.h"
+#include "intel_pm.h"
 #include "intel_display_types.h"
 #include "intel_sideband.h"
 
@@ -334,6 +335,51 @@ static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_
 	return data_rate;
 }
 
+int intel_bw_calc_min_cdclk(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	int max_bw_per_dbuf[DBUF_SLICE_MAX];
+	int i = 0;
+	enum plane_id plane_id;
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int max_bw = 0;
+	int min_cdclk;
+
+	memset(max_bw_per_dbuf, 0, sizeof(max_bw_per_dbuf[0]) * DBUF_SLICE_MAX);
+
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			struct skl_ddb_entry *plane_alloc =
+				&crtc_state->wm.skl.plane_ddb_y[plane_id];
+			struct skl_ddb_entry *uv_plane_alloc =
+				&crtc_state->wm.skl.plane_ddb_uv[plane_id];
+			unsigned int data_rate = crtc_state->data_rate[plane_id];
+			int slice_id = 0;
+			u32 dbuf_mask = skl_ddb_dbuf_slice_mask(dev_priv, plane_alloc);
+
+			dbuf_mask |= skl_ddb_dbuf_slice_mask(dev_priv, uv_plane_alloc);
+
+			DRM_DEBUG_KMS("Got dbuf mask %x for pipe %c ddb %d-%d plane %d data rate %d\n",
+				      dbuf_mask, pipe_name(crtc->pipe), plane_alloc->start,
+				      plane_alloc->end, plane_id, data_rate);
+
+			while (dbuf_mask != 0) {
+				if (dbuf_mask & 1) {
+					max_bw_per_dbuf[slice_id] += data_rate;
+					max_bw = max(max_bw, max_bw_per_dbuf[slice_id]);
+				}
+				slice_id++;
+				dbuf_mask >>= 1;
+			}
+		}
+	}
+
+	min_cdclk = max_bw / 64;
+
+	return min_cdclk;
+}
+
 void intel_bw_crtc_update(struct intel_bw_state *bw_state,
 			  const struct intel_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index a8aa7624c5aa..8a522b571c51 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -29,5 +29,6 @@ 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 intel_bw_calc_min_cdclk(struct intel_atomic_state *state);
 
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 0741d643455b..9f2de613642e 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -25,6 +25,7 @@
 #include "intel_cdclk.h"
 #include "intel_display_types.h"
 #include "intel_sideband.h"
+#include "intel_bw.h"
 
 /**
  * DOC: CDCLK / RAWCLK
@@ -1979,11 +1980,19 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->uapi.crtc->dev);
+	struct intel_atomic_state *state = NULL;
 	int min_cdclk;
 
 	if (!crtc_state->hw.enable)
 		return 0;
 
+	/*
+	 * FIXME: Unfortunately when this gets called from intel_modeset_setup_hw_state
+	 * there is no intel_atomic_state at all. So lets not then use it.
+	 */
+	if (crtc_state->uapi.state)
+		state = to_intel_atomic_state(crtc_state->uapi.state);
+
 	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
 
 	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
@@ -2058,6 +2067,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	if (IS_TIGERLAKE(dev_priv))
 		min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
 
+	/*
+	 * Similar story as with skl_write_plane_wm and intel_enable_sagv
+	 * - in some certain driver parts, we don't have any guarantee that
+	 * parent exists. So we might be having a crtc_state without
+	 * parent state.
+	 */
+	if (INTEL_GEN(dev_priv) >= 11) {
+		if (state) {
+			int dbuf_bw_cdclk = intel_bw_calc_min_cdclk(state);
+
+			DRM_DEBUG_KMS("DBuf bw min cdclk %d current min_cdclk %d\n",
+				      dbuf_bw_cdclk, min_cdclk);
+			min_cdclk = max(min_cdclk, dbuf_bw_cdclk);
+		}
+	}
+
 	if (min_cdclk > dev_priv->max_cdclk_freq) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cdff3054b344..aae5521424c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14632,7 +14632,7 @@ static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
 	/* See {hsw,vlv,ivb}_plane_ratio() */
 	return IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv) ||
 		IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
-		IS_IVYBRIDGE(dev_priv);
+		IS_IVYBRIDGE(dev_priv) || (INTEL_GEN(dev_priv) >= 11);
 }
 
 static int intel_atomic_check_planes(struct intel_atomic_state *state,
@@ -14681,6 +14681,14 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state,
 		if (hweight8(old_active_planes) == hweight8(new_active_planes))
 			continue;
 
+		/*
+		 * active_planes bitmask has been updated, whenever amount
+		 * of active planes had changed we need to recalculate CDCLK
+		 * as it depends on total bandwidth now, not only min_cdclk
+		 * per plane.
+		 */
+		*need_cdclk_calc = true;
+
 		ret = intel_crtc_add_planes_to_state(state, crtc, new_active_planes);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index da64a5edae7a..3446c3ce6a4f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -311,6 +311,7 @@ intel_display_power_put_async(struct drm_i915_private *i915,
 enum dbuf_slice {
 	DBUF_S1,
 	DBUF_S2,
+	DBUF_SLICE_MAX
 };
 
 #define with_intel_display_power(i915, domain, wf) \
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8375054ba27d..15300c44d9dc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3844,10 +3844,9 @@ icl_get_first_dbuf_slice_offset(u32 dbuf_slice_mask,
 	return offset;
 }
 
-static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
+u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
 {
 	u16 ddb_size = INTEL_INFO(dev_priv)->ddb_size;
-
 	drm_WARN_ON(&dev_priv->drm, ddb_size == 0);
 
 	if (INTEL_GEN(dev_priv) < 11)
@@ -3856,6 +3855,37 @@ static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
 	return ddb_size;
 }
 
+u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
+			    const struct skl_ddb_entry *entry)
+{
+	u32 slice_mask = 0;
+	u16 ddb_size = intel_get_ddb_size(dev_priv);
+	u16 num_supported_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
+	u16 slice_size = ddb_size / num_supported_slices;
+	u16 start_slice;
+	u16 end_slice;
+
+	if (!skl_ddb_entry_size(entry))
+		return 0;
+
+	start_slice = entry->start / slice_size;
+	end_slice = (entry->end - 1) / slice_size;
+
+	DRM_DEBUG_KMS("ddb size %d slices %d slice size %d start slice %d end slice %d\n",
+		      ddb_size, num_supported_slices, slice_size, start_slice, end_slice);
+
+	/*
+	 * Per plane DDB entry can in a really worst case be on multiple slices
+	 * but single entry is anyway contigious.
+	 */
+	while (start_slice <= end_slice) {
+		slice_mask |= 1 << start_slice;
+		start_slice++;
+	}
+
+	return slice_mask;
+}
+
 static u8 skl_compute_dbuf_slices(const struct intel_crtc_state *crtc_state,
 				  u8 active_pipes);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index d60a85421c5a..a9e3835927a8 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -37,6 +37,9 @@ void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
 			       struct skl_ddb_entry *ddb_y,
 			       struct skl_ddb_entry *ddb_uv);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv);
+u16 intel_get_ddb_size(struct drm_i915_private *dev_priv);
+u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
+			    const struct skl_ddb_entry *entry);
 void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 			      struct skl_pipe_wm *out);
 void g4x_wm_sanitize(struct drm_i915_private *dev_priv);
-- 
2.24.1.485.gad05a3d8e5

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks Stanislav Lisovskiy
@ 2020-03-16 23:36   ` Manasi Navare
  2020-03-17 13:21   ` Ville Syrjälä
  1 sibling, 0 replies; 20+ messages in thread
From: Manasi Navare @ 2020-03-16 23:36 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

On Mon, Mar 16, 2020 at 01:37:42PM +0200, Stanislav Lisovskiy wrote:
> We need to calculate cdclk after watermarks/ddb has been calculated
> as with recent hw CDCLK needs to be adjusted accordingly to DBuf
> requirements, which is not possible with current code organization.
> 
> Setting CDCLK according to DBuf BW requirements and not just rejecting
> if it doesn't satisfy BW requirements, will allow us to save power when
> it is possible and gain additional bandwidth when it's needed - i.e
> boosting both our power management and perfomance capabilities.
> 
> This patch is preparation for that, first we now extract modeset
> calculation from modeset checks, in order to call it after wm/ddb

Did you mean to type extract cdclk calculation from modeset checks?

Manasi

> has been calculated.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f23c4d51c33..cdff3054b344 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14542,6 +14542,14 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
>  			return ret;
>  	}
>  
> +	return 0;
> +}
> +
> +static int intel_modeset_cdclk(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	int ret;
> +
>  	ret = intel_modeset_calc_cdclk(state);
>  	if (ret)
>  		return ret;
> @@ -14879,10 +14887,6 @@ static int intel_atomic_check(struct drm_device *dev,
>  			goto fail;
>  	}
>  
> -	ret = intel_atomic_check_crtcs(state);
> -	if (ret)
> -		goto fail;
> -
>  	intel_fbc_choose_crtc(dev_priv, state);
>  	ret = calc_watermark_data(state);
>  	if (ret)
> @@ -14892,6 +14896,16 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	if (any_ms) {
> +		ret = intel_modeset_cdclk(state);
> +		if (ret)
> +			goto fail;
> +	}
> +
> +	ret = intel_atomic_check_crtcs(state);
> +	if (ret)
> +		goto fail;
> +
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (!needs_modeset(new_crtc_state) &&
> -- 
> 2.24.1.485.gad05a3d8e5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH v1 3/3] drm/i915: Remove unneeded hack now for CDCLK
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 3/3] drm/i915: Remove unneeded hack now for CDCLK Stanislav Lisovskiy
@ 2020-03-16 23:51   ` Manasi Navare
  0 siblings, 0 replies; 20+ messages in thread
From: Manasi Navare @ 2020-03-16 23:51 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

On Mon, Mar 16, 2020 at 01:37:44PM +0200, Stanislav Lisovskiy wrote:
> No need to bump up CDCLK now, as it is now correctly
> calculated, accounting for DBuf BW as BSpec says.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Logic looks good,

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index f0dcea4d6357..45469f6833b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2055,18 +2055,6 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	/* Account for additional needs from the planes */
>  	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
>  
> -	/*
> -	 * HACK. Currently for TGL platforms we calculate
> -	 * min_cdclk initially based on pixel_rate divided
> -	 * by 2, accounting for also plane requirements,
> -	 * however in some cases the lowest possible CDCLK
> -	 * doesn't work and causing the underruns.
> -	 * Explicitly stating here that this seems to be currently
> -	 * rather a Hack, than final solution.
> -	 */
> -	if (IS_TIGERLAKE(dev_priv))
> -		min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
> -
>  	/*
>  	 * Similar story as with skl_write_plane_wm and intel_enable_sagv
>  	 * - in some certain driver parts, we don't have any guarantee that
> -- 
> 2.24.1.485.gad05a3d8e5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
                   ` (4 preceding siblings ...)
  2020-03-16 20:34 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2020-03-17  0:36 ` Patchwork
  2020-03-17 10:43   ` Lisovskiy, Stanislav
  2020-03-17  0:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-03-17  9:26 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 1 reply; 20+ messages in thread
From: Patchwork @ 2020-03-17  0:36 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Consider DBuf bandwidth when calculating CDCLK (rev2)
URL   : https://patchwork.freedesktop.org/series/74739/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -internal ./drivers/gpu/drm/i915/i915_gem_fence_reg.c' failed with return code 2
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -function fence register handling ./drivers/gpu/drm/i915/i915_gem_fence_reg.c' failed with return code 1
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -function tiling swizzling details ./drivers/gpu/drm/i915/i915_gem_fence_reg.c' failed with return code 1

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
                   ` (5 preceding siblings ...)
  2020-03-17  0:36 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2) Patchwork
@ 2020-03-17  0:56 ` Patchwork
  2020-03-17  9:26 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-03-17  0:56 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Consider DBuf bandwidth when calculating CDCLK (rev2)
URL   : https://patchwork.freedesktop.org/series/74739/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8138 -> Patchwork_16987
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@execlists:
    - fi-cfl-guc:         [PASS][1] -> [INCOMPLETE][2] ([i915#656])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/fi-cfl-guc/igt@i915_selftest@live@execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/fi-cfl-guc/igt@i915_selftest@live@execlists.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@perf:
    - fi-bwr-2160:        [INCOMPLETE][3] -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/fi-bwr-2160/igt@i915_selftest@live@perf.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/fi-bwr-2160/igt@i915_selftest@live@perf.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-cml-u2:          [FAIL][5] ([i915#217]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html

  
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656


Participating hosts (40 -> 36)
------------------------------

  Additional (5): fi-bdw-5557u fi-hsw-peppy fi-gdg-551 fi-cfl-8109u fi-snb-2600 
  Missing    (9): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7500u fi-elk-e7500 fi-skl-lmem fi-bdw-samus fi-tgl-y fi-skl-6600u 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8138 -> Patchwork_16987

  CI-20190529: 20190529
  CI_DRM_8138: 652084cff0971058d1acb1746001f89ef8ea7321 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5512: f6fef7eff6f121e5e89afd7e70116f471ccd5b8b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16987: 49c577d1644782a7a8e1dfdf41c57436c825880a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

49c577d16447 drm/i915: Remove unneeded hack now for CDCLK
bc0c89d0cbd8 drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
7c4650598e71 drm/i915: Decouple cdclk calculation from modeset checks

== Logs ==

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
                   ` (6 preceding siblings ...)
  2020-03-17  0:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-03-17  9:26 ` Patchwork
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-03-17  9:26 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Consider DBuf bandwidth when calculating CDCLK (rev2)
URL   : https://patchwork.freedesktop.org/series/74739/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8138_full -> Patchwork_16987_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb5/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#112146]) +5 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb6/igt@gem_exec_async@concurrent-writes-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_exec_schedule@implicit-both-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([i915#677])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb5/igt@gem_exec_schedule@implicit-both-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb2/igt@gem_exec_schedule@implicit-both-bsd.html

  * igt@gem_exec_schedule@implicit-write-read-bsd1:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#109276] / [i915#677]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb4/igt@gem_exec_schedule@implicit-write-read-bsd1.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb3/igt@gem_exec_schedule@implicit-write-read-bsd1.html

  * igt@gem_exec_whisper@basic-fds-all:
    - shard-tglb:         [PASS][9] -> [INCOMPLETE][10] ([i915#1401])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-tglb5/igt@gem_exec_whisper@basic-fds-all.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-tglb8/igt@gem_exec_whisper@basic-fds-all.html

  * igt@i915_pm_sseu@full-enable:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([i915#286])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl8/igt@i915_pm_sseu@full-enable.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl8/igt@i915_pm_sseu@full-enable.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-apl7/igt@i915_suspend@fence-restore-tiled2untiled.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-apl6/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-c-ctm-max:
    - shard-hsw:          [PASS][15] -> [INCOMPLETE][16] ([i915#61])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-hsw2/igt@kms_color@pipe-c-ctm-max.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-hsw1/igt@kms_color@pipe-c-ctm-max.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x128-random:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([i915#54]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl2/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
    - shard-kbl:          [PASS][19] -> [FAIL][20] ([i915#54])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
    - shard-apl:          [PASS][21] -> [FAIL][22] ([i915#54])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          [PASS][23] -> [FAIL][24] ([i915#79])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-glk8/igt@kms_flip@flip-vs-expired-vblank.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-glk8/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@plain-flip-ts-check-interruptible:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#34])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl8/igt@kms_flip@plain-flip-ts-check-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl8/igt@kms_flip@plain-flip-ts-check-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-move:
    - shard-glk:          [PASS][27] -> [FAIL][28] ([i915#49])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-glk6/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-move.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][29] -> [DMESG-WARN][30] ([i915#180]) +5 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([i915#1188]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][33] -> [FAIL][34] ([fdo#108145])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#109642] / [fdo#111068])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb7/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#109441]) +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb3/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_vblank@pipe-a-query-busy-hang:
    - shard-glk:          [PASS][39] -> [INCOMPLETE][40] ([i915#58] / [k.org#198133])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-glk8/igt@kms_vblank@pipe-a-query-busy-hang.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-glk9/igt@kms_vblank@pipe-a-query-busy-hang.html

  * igt@kms_vblank@pipe-b-query-idle-hang:
    - shard-skl:          [PASS][41] -> [SKIP][42] ([fdo#109271]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl1/igt@kms_vblank@pipe-b-query-idle-hang.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl7/igt@kms_vblank@pipe-b-query-idle-hang.html

  * igt@perf_pmu@busy-vcs1:
    - shard-iclb:         [PASS][43] -> [SKIP][44] ([fdo#112080]) +14 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb1/igt@perf_pmu@busy-vcs1.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb6/igt@perf_pmu@busy-vcs1.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][45] -> [SKIP][46] ([fdo#109276]) +15 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb4/igt@prime_vgem@fence-wait-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb3/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@close-replace-race:
    - shard-tglb:         [INCOMPLETE][47] ([i915#1402]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-tglb1/igt@gem_ctx_persistence@close-replace-race.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-tglb7/igt@gem_ctx_persistence@close-replace-race.html
    - shard-kbl:          [TIMEOUT][49] ([i915#1340]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-kbl4/igt@gem_ctx_persistence@close-replace-race.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-kbl4/igt@gem_ctx_persistence@close-replace-race.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [SKIP][51] ([fdo#112080]) -> [PASS][52] +15 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb5/igt@gem_exec_parallel@vcs1-fds.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@implicit-both-bsd1:
    - shard-iclb:         [SKIP][53] ([fdo#109276] / [i915#677]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb3/igt@gem_exec_schedule@implicit-both-bsd1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb2/igt@gem_exec_schedule@implicit-both-bsd1.html

  * igt@gem_exec_schedule@pi-distinct-iova-render:
    - shard-skl:          [DMESG-WARN][55] ([i915#836]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl10/igt@gem_exec_schedule@pi-distinct-iova-render.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl2/igt@gem_exec_schedule@pi-distinct-iova-render.html

  * igt@gem_exec_schedule@pi-shared-iova-bsd:
    - shard-iclb:         [SKIP][57] ([i915#677]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb2/igt@gem_exec_schedule@pi-shared-iova-bsd.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb7/igt@gem_exec_schedule@pi-shared-iova-bsd.html

  * igt@gem_exec_schedule@preempt-queue-contexts-render:
    - shard-skl:          [FAIL][59] -> [PASS][60] +4 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl3/igt@gem_exec_schedule@preempt-queue-contexts-render.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl1/igt@gem_exec_schedule@preempt-queue-contexts-render.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [SKIP][61] ([fdo#112146]) -> [PASS][62] +5 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb7/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-snb:          [DMESG-WARN][63] ([i915#42]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-snb5/igt@gem_softpin@noreloc-s3.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-snb6/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][65] ([i915#454]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb1/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rps@reset:
    - shard-tglb:         [FAIL][67] ([i915#413]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-tglb2/igt@i915_pm_rps@reset.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-tglb6/igt@i915_pm_rps@reset.html
    - shard-iclb:         [FAIL][69] ([i915#413]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb3/igt@i915_pm_rps@reset.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb6/igt@i915_pm_rps@reset.html

  * igt@kms_color@pipe-c-ctm-0-5:
    - shard-skl:          [FAIL][71] ([i915#182]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl1/igt@kms_color@pipe-c-ctm-0-5.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl6/igt@kms_color@pipe-c-ctm-0-5.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          [FAIL][73] ([i915#72]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-glk5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-glk7/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-hsw:          [FAIL][75] ([i915#57]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-hsw4/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [FAIL][77] ([IGT#5]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][79] ([fdo#109349]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb5/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [DMESG-WARN][81] ([i915#180]) -> [PASS][82] +2 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip@plain-flip-ts-check-interruptible:
    - shard-glk:          [FAIL][83] ([i915#34]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-glk2/igt@kms_flip@plain-flip-ts-check-interruptible.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-glk4/igt@kms_flip@plain-flip-ts-check-interruptible.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-skl:          [INCOMPLETE][85] ([i915#69]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
    - shard-kbl:          [DMESG-WARN][87] ([i915#180]) -> [PASS][88] +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][89] ([fdo#108145]) -> [PASS][90] +1 similar issue
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][91] ([fdo#108145] / [i915#265]) -> [PASS][92] +1 similar issue
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][93] ([fdo#109441]) -> [PASS][94] +3 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_vblank@pipe-a-ts-continuation-modeset-hang:
    - shard-skl:          [SKIP][95] ([fdo#109271]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl6/igt@kms_vblank@pipe-a-ts-continuation-modeset-hang.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl3/igt@kms_vblank@pipe-a-ts-continuation-modeset-hang.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][97] ([fdo#109276]) -> [PASS][98] +16 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb3/igt@prime_busy@hang-bsd2.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb2/igt@prime_busy@hang-bsd2.html

  * {igt@sysfs_heartbeat_interval@precise@rcs0}:
    - shard-skl:          [FAIL][99] ([i915#1459]) -> [PASS][100] +1 similar issue
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-skl3/igt@sysfs_heartbeat_interval@precise@rcs0.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-skl1/igt@sysfs_heartbeat_interval@precise@rcs0.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][101] ([i915#588]) -> [SKIP][102] ([i915#658])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-iclb7/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_rpm@dpms-non-lpsp:
    - shard-snb:          [INCOMPLETE][103] ([i915#82]) -> [SKIP][104] ([fdo#109271])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-snb5/igt@i915_pm_rpm@dpms-non-lpsp.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-snb6/igt@i915_pm_rpm@dpms-non-lpsp.html

  * igt@kms_content_protection@uevent:
    - shard-apl:          [FAIL][105] ([i915#357]) -> [INCOMPLETE][106] ([fdo#103927])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-apl1/igt@kms_content_protection@uevent.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-apl3/igt@kms_content_protection@uevent.html

  * igt@runner@aborted:
    - shard-tglb:         [FAIL][107] ([i915#1389]) -> [FAIL][108] ([i915#1401])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8138/shard-tglb1/igt@runner@aborted.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16987/shard-tglb8/igt@runner@aborted.html

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

  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1340]: https://gitlab.freedesktop.org/drm/intel/issues/1340
  [i915#1389]: https://gitlab.freedesktop.org/drm/intel/issues/1389
  [i915#1401]: https://gitlab.freedesktop.org/drm/intel/issues/1401
  [i915#1402]: https://gitlab.freedesktop.org/drm/intel/issues/1402
  [i915#1459]: https://gitlab.freedesktop.org/drm/intel/issues/1459
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#182]: https://gitlab.freedesktop.org/drm/intel/issues/182
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#286]: https://gitlab.freedesktop.org/drm/intel/issues/286
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#357]: https://gitlab.freedesktop.org/drm/intel/issues/357
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#42]: https://gitlab.freedesktop.org/drm/intel/issues/42
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#57]: https://gitlab.freedesktop.org/drm/intel/issues/57
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#836]: https://gitlab.freedesktop.org/drm/intel/issues/836
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8138 -> Patchwork_16987

  CI-20190529: 20190529
  CI_DRM_8138: 652084cff0971058d1acb1746001f89ef8ea7321 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5512: f6fef7eff6f121e5e89afd7e70116f471ccd5b8b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16987: 49c577d1644782a7a8e1dfdf41c57436c825880a @ 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_16987/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx]  ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-17  0:36 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2) Patchwork
@ 2020-03-17 10:43   ` Lisovskiy, Stanislav
  2020-03-18  8:33     ` Arkadiusz Hiler
  0 siblings, 1 reply; 20+ messages in thread
From: Lisovskiy, Stanislav @ 2020-03-17 10:43 UTC (permalink / raw)
  To: intel-gfx, Peres, Martin, Hiler, Arkadiusz, Vudum, Lakshminarayana


[-- Attachment #1.1: Type: text/plain, Size: 1553 bytes --]

What is this weird DOC warning about? "Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c"

- wondering, how is that related to this patch.


Best Regards,

Lisovskiy Stanislav

Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
________________________________
From: Patchwork <patchwork@emeril.freedesktop.org>
Sent: Tuesday, March 17, 2020 2:36:10 AM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org
Subject: ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

== Series Details ==

Series: Consider DBuf bandwidth when calculating CDCLK (rev2)
URL   : https://patchwork.freedesktop.org/series/74739/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
Error: Cannot open file ./drivers/gpu/drm/i915/i915_gem_fence_reg.c
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -internal ./drivers/gpu/drm/i915/i915_gem_fence_reg.c' failed with return code 2
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -function fence register handling ./drivers/gpu/drm/i915/i915_gem_fence_reg.c' failed with return code 1
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -function tiling swizzling details ./drivers/gpu/drm/i915/i915_gem_fence_reg.c' failed with return code 1


[-- Attachment #1.2: Type: text/html, Size: 3191 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks
  2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks Stanislav Lisovskiy
  2020-03-16 23:36   ` Manasi Navare
@ 2020-03-17 13:21   ` Ville Syrjälä
  1 sibling, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2020-03-17 13:21 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

On Mon, Mar 16, 2020 at 01:37:42PM +0200, Stanislav Lisovskiy wrote:
> We need to calculate cdclk after watermarks/ddb has been calculated
> as with recent hw CDCLK needs to be adjusted accordingly to DBuf
> requirements, which is not possible with current code organization.
> 
> Setting CDCLK according to DBuf BW requirements and not just rejecting
> if it doesn't satisfy BW requirements, will allow us to save power when
> it is possible and gain additional bandwidth when it's needed - i.e
> boosting both our power management and perfomance capabilities.
> 
> This patch is preparation for that, first we now extract modeset
> calculation from modeset checks, in order to call it after wm/ddb
> has been calculated.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f23c4d51c33..cdff3054b344 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14542,6 +14542,14 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
>  			return ret;
>  	}
>  
> +	return 0;
> +}
> +
> +static int intel_modeset_cdclk(struct intel_atomic_state *state)
> +{

Misleading name here since you didn't extract just the cdclk part.
IMO just move intel_modeset_calc_cdclk() alone out from
intel_modeset_checks(), and keep the reordering minimal in that
patch. Ie. just call intel_modeset_calc_cdclk() right after
intel_modeset_checks().

Then in the next patch you can do the
intel_modeset_calc_cdclk()+intel_atomic_check_crtcs() vs. wm reorder.

The two things that currently need cdclk in intel_crtc_atomic_check()
would appear to be ips and linetime watermarks. The rest looks like
safe to reorder.

Though at least one thing that I think is totally misplaced is the
.crtc_compute_clock() call. That really should be done much earlier,
even earlier than where it is now. However since it doesn't
adjust .crtc_clock with the results of the computation doesn't really
matter for now. So looks like we can ignore this particular mess
for now.

>
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	int ret;
> +
>  	ret = intel_modeset_calc_cdclk(state);
>  	if (ret)
>  		return ret;
> @@ -14879,10 +14887,6 @@ static int intel_atomic_check(struct drm_device *dev,
>  			goto fail;
>  	}
>  
> -	ret = intel_atomic_check_crtcs(state);
> -	if (ret)
> -		goto fail;
> -
>  	intel_fbc_choose_crtc(dev_priv, state);
>  	ret = calc_watermark_data(state);
>  	if (ret)
> @@ -14892,6 +14896,16 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	if (any_ms) {
> +		ret = intel_modeset_cdclk(state);
> +		if (ret)
> +			goto fail;
> +	}
> +
> +	ret = intel_atomic_check_crtcs(state);
> +	if (ret)
> +		goto fail;
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (!needs_modeset(new_crtc_state) &&
> -- 
> 2.24.1.485.gad05a3d8e5

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
  2020-03-16 22:43   ` [Intel-gfx] [PATCH v2 " Stanislav Lisovskiy
@ 2020-03-17 13:46     ` Ville Syrjälä
  2020-03-17 14:40       ` Lisovskiy, Stanislav
  0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2020-03-17 13:46 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

On Tue, Mar 17, 2020 at 12:43:38AM +0200, Stanislav Lisovskiy wrote:
> According to BSpec max BW per slice is calculated using formula
> Max BW = CDCLK * 64. Currently when calculating min CDCLK we
> account only per plane requirements, however in order to avoid
> FIFO underruns we need to estimate accumulated BW consumed by
> all planes(ddb entries basically) residing on that particular
> DBuf slice. This will allow us to put CDCLK lower and save power
> when we don't need that much bandwidth or gain additional
> performance once plane consumption grows.
> 
> v2: - Fix long line warning
>     - Limited new DBuf bw checks to only gens >= 11
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c       | 46 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_bw.h       |  1 +
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 25 ++++++++++
>  drivers/gpu/drm/i915/display/intel_display.c  | 10 +++-
>  .../drm/i915/display/intel_display_power.h    |  1 +
>  drivers/gpu/drm/i915/intel_pm.c               | 34 +++++++++++++-
>  drivers/gpu/drm/i915/intel_pm.h               |  3 ++
>  7 files changed, 117 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 58b264bc318d..a85125110d7e 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -6,6 +6,7 @@
>  #include <drm/drm_atomic_state_helper.h>
>  
>  #include "intel_bw.h"
> +#include "intel_pm.h"
>  #include "intel_display_types.h"
>  #include "intel_sideband.h"
>  
> @@ -334,6 +335,51 @@ static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_
>  	return data_rate;
>  }
>  
> +int intel_bw_calc_min_cdclk(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	int max_bw_per_dbuf[DBUF_SLICE_MAX];
> +	int i = 0;
> +	enum plane_id plane_id;
> +	struct intel_crtc_state *crtc_state;
> +	struct intel_crtc *crtc;
> +	int max_bw = 0;
> +	int min_cdclk;
> +
> +	memset(max_bw_per_dbuf, 0, sizeof(max_bw_per_dbuf[0]) * DBUF_SLICE_MAX);
> +
> +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		for_each_plane_id_on_crtc(crtc, plane_id) {
> +			struct skl_ddb_entry *plane_alloc =
> +				&crtc_state->wm.skl.plane_ddb_y[plane_id];
> +			struct skl_ddb_entry *uv_plane_alloc =
> +				&crtc_state->wm.skl.plane_ddb_uv[plane_id];
> +			unsigned int data_rate = crtc_state->data_rate[plane_id];
> +			int slice_id = 0;
> +			u32 dbuf_mask = skl_ddb_dbuf_slice_mask(dev_priv, plane_alloc);
> +
> +			dbuf_mask |= skl_ddb_dbuf_slice_mask(dev_priv, uv_plane_alloc);
> +
> +			DRM_DEBUG_KMS("Got dbuf mask %x for pipe %c ddb %d-%d plane %d data rate %d\n",
> +				      dbuf_mask, pipe_name(crtc->pipe), plane_alloc->start,
> +				      plane_alloc->end, plane_id, data_rate);
> +
> +			while (dbuf_mask != 0) {
> +				if (dbuf_mask & 1) {
> +					max_bw_per_dbuf[slice_id] += data_rate;
> +					max_bw = max(max_bw, max_bw_per_dbuf[slice_id]);
> +				}
> +				slice_id++;
> +				dbuf_mask >>= 1;
> +			}
> +		}
> +	}

Something like?

for_each_plane_id() {
	for_each_dbuf_slice() {
		skl_ddb_entry_for_slices(BIT(slice), &ddb_slice);
		
		if (skl_ddb_entries_overlap(&ddb_slice, &ddb[plane_id])))
			bw[slice] += data_rate;
	}
}

But this seems to borked anyway since we only consider the crtcs in the
state, and there are those ugly FIXMEs below.

I have a feeling what we want is dbuf_state, and track the bw used for 
each slice therein. Should also allow us to flag the cdclk recalculation
only when things actually change in a way that needs more cdclk, instead 
of pessimising every plane enable/disable like you do below.

> +
> +	min_cdclk = max_bw / 64;
> +
> +	return min_cdclk;
> +}
> +
>  void intel_bw_crtc_update(struct intel_bw_state *bw_state,
>  			  const struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index a8aa7624c5aa..8a522b571c51 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -29,5 +29,6 @@ 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 intel_bw_calc_min_cdclk(struct intel_atomic_state *state);
>  
>  #endif /* __INTEL_BW_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 0741d643455b..9f2de613642e 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -25,6 +25,7 @@
>  #include "intel_cdclk.h"
>  #include "intel_display_types.h"
>  #include "intel_sideband.h"
> +#include "intel_bw.h"
>  
>  /**
>   * DOC: CDCLK / RAWCLK
> @@ -1979,11 +1980,19 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_atomic_state *state = NULL;
>  	int min_cdclk;
>  
>  	if (!crtc_state->hw.enable)
>  		return 0;
>  
> +	/*
> +	 * FIXME: Unfortunately when this gets called from intel_modeset_setup_hw_state
> +	 * there is no intel_atomic_state at all. So lets not then use it.
> +	 */
> +	if (crtc_state->uapi.state)
> +		state = to_intel_atomic_state(crtc_state->uapi.state);
> +
>  	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
>  
>  	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> @@ -2058,6 +2067,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	if (IS_TIGERLAKE(dev_priv))
>  		min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
>  
> +	/*
> +	 * Similar story as with skl_write_plane_wm and intel_enable_sagv
> +	 * - in some certain driver parts, we don't have any guarantee that
> +	 * parent exists. So we might be having a crtc_state without
> +	 * parent state.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 11) {
> +		if (state) {
> +			int dbuf_bw_cdclk = intel_bw_calc_min_cdclk(state);
> +
> +			DRM_DEBUG_KMS("DBuf bw min cdclk %d current min_cdclk %d\n",
> +				      dbuf_bw_cdclk, min_cdclk);
> +			min_cdclk = max(min_cdclk, dbuf_bw_cdclk);
> +		}
> +	}
> +
>  	if (min_cdclk > dev_priv->max_cdclk_freq) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index cdff3054b344..aae5521424c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14632,7 +14632,7 @@ static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
>  	/* See {hsw,vlv,ivb}_plane_ratio() */
>  	return IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv) ||
>  		IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> -		IS_IVYBRIDGE(dev_priv);
> +		IS_IVYBRIDGE(dev_priv) || (INTEL_GEN(dev_priv) >= 11);
>  }
>  
>  static int intel_atomic_check_planes(struct intel_atomic_state *state,
> @@ -14681,6 +14681,14 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state,
>  		if (hweight8(old_active_planes) == hweight8(new_active_planes))
>  			continue;
>  
> +		/*
> +		 * active_planes bitmask has been updated, whenever amount
> +		 * of active planes had changed we need to recalculate CDCLK
> +		 * as it depends on total bandwidth now, not only min_cdclk
> +		 * per plane.
> +		 */
> +		*need_cdclk_calc = true;
> +
>  		ret = intel_crtc_add_planes_to_state(state, crtc, new_active_planes);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index da64a5edae7a..3446c3ce6a4f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -311,6 +311,7 @@ intel_display_power_put_async(struct drm_i915_private *i915,
>  enum dbuf_slice {
>  	DBUF_S1,
>  	DBUF_S2,
> +	DBUF_SLICE_MAX
>  };
>  
>  #define with_intel_display_power(i915, domain, wf) \
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8375054ba27d..15300c44d9dc 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3844,10 +3844,9 @@ icl_get_first_dbuf_slice_offset(u32 dbuf_slice_mask,
>  	return offset;
>  }
>  
> -static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
> +u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
>  {
>  	u16 ddb_size = INTEL_INFO(dev_priv)->ddb_size;
> -
>  	drm_WARN_ON(&dev_priv->drm, ddb_size == 0);
>  
>  	if (INTEL_GEN(dev_priv) < 11)
> @@ -3856,6 +3855,37 @@ static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
>  	return ddb_size;
>  }
>  
> +u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
> +			    const struct skl_ddb_entry *entry)
> +{
> +	u32 slice_mask = 0;
> +	u16 ddb_size = intel_get_ddb_size(dev_priv);
> +	u16 num_supported_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
> +	u16 slice_size = ddb_size / num_supported_slices;
> +	u16 start_slice;
> +	u16 end_slice;
> +
> +	if (!skl_ddb_entry_size(entry))
> +		return 0;
> +
> +	start_slice = entry->start / slice_size;
> +	end_slice = (entry->end - 1) / slice_size;
> +
> +	DRM_DEBUG_KMS("ddb size %d slices %d slice size %d start slice %d end slice %d\n",
> +		      ddb_size, num_supported_slices, slice_size, start_slice, end_slice);
> +
> +	/*
> +	 * Per plane DDB entry can in a really worst case be on multiple slices
> +	 * but single entry is anyway contigious.
> +	 */
> +	while (start_slice <= end_slice) {
> +		slice_mask |= 1 << start_slice;
> +		start_slice++;
> +	}
> +
> +	return slice_mask;
> +}
> +
>  static u8 skl_compute_dbuf_slices(const struct intel_crtc_state *crtc_state,
>  				  u8 active_pipes);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index d60a85421c5a..a9e3835927a8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -37,6 +37,9 @@ void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
>  			       struct skl_ddb_entry *ddb_y,
>  			       struct skl_ddb_entry *ddb_uv);
>  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv);
> +u16 intel_get_ddb_size(struct drm_i915_private *dev_priv);
> +u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
> +			    const struct skl_ddb_entry *entry);
>  void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
>  			      struct skl_pipe_wm *out);
>  void g4x_wm_sanitize(struct drm_i915_private *dev_priv);
> -- 
> 2.24.1.485.gad05a3d8e5

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs
  2020-03-17 13:46     ` Ville Syrjälä
@ 2020-03-17 14:40       ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 20+ messages in thread
From: Lisovskiy, Stanislav @ 2020-03-17 14:40 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 15856 bytes --]

>Something like?

>for_each_plane_id() {
>       for_each_dbuf_slice() {
>               skl_ddb_entry_for_slices(BIT(slice), &ddb_slice);
>
>                if (skl_ddb_entries_overlap(&ddb_slice, &ddb[plane_id])))
>                       bw[slice] += data_rate;
>        }
>}


In fact even in your example this is not fully correct:


it should be then


for_each_new_crtc_in_state()   => because there are multiple crtcs

  for_each_plane_id() {
       for_each_dbuf_slice() {
               skl_ddb_entry_for_slices(BIT(slice), &ddb_slice);

                if (skl_ddb_entries_overlap(&ddb_slice, &ddb[plane_id])))
                       bw[slice] += data_rate;
        }
}


which would be in fact same complexity or even worse because in the patch

we get a mask of only used slices per plane ddb and then account for those

while here it would be iterating all slices everytime.

Slight difference but still.

I can of course make this function shorter, by implementing some helpers -

that's for sure.


>But this seems to borked anyway since we only consider the crtcs in the

>state, and there are those ugly FIXMEs below.


Those ugly FIXME's are there because of same issue that we don't

have a parent state for crtc_state in some situations.


Not this patch fault or subject in fact. We probably need some series

to somehow tackle this everywhere, so that those functions which

need intel_atomic_state can always find it.



So it is not those FIXME's in the patch which are ugly, but the code

which is calling this function, so that even though we have a crtc_state

we never now if we can have a parent atomic state, which is violating

OOP principles.


I.e it is called from intel_modeset_setup_hw_state _already_ in a hacky way.

>I have a feeling what we want is dbuf_state, and track the bw used for
>each slice therein. Should also allow us to flag the cdclk recalculation
>only when things actually change in a way that needs more cdclk, instead
>of pessimising every plane enable/disable like you do below.


I think CDCLK recalculation itself is pretty trivial - it is actually when we really

needs to be changed, that is what we don't want to often, right?

To estimate if it needs to be flagged or not you will need exatly same code, i.e

calculating BW used per slice, while determining which ddb entries are related

to which slice.


In fact there are already IGT results(which pass) and CDCLK doesn't change too

often at all - because we change it only when we really need it otherwise


intel_crtc_compute_min_cdclk will return same value as before and nothing changes.


If you really want so, we can start tracking it, once your dbuf_state patches land - currently

the main problem is that we need finally a proper way to estimate CDCLK

without keeping it bumped all the time to make 8K happy, at the same time

we don't want FIFO underruns again.


Best Regards,

Lisovskiy Stanislav
________________________________
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Sent: Tuesday, March 17, 2020 3:46:35 PM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; Roper, Matthew D
Subject: Re: [PATCH v2 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs

On Tue, Mar 17, 2020 at 12:43:38AM +0200, Stanislav Lisovskiy wrote:
> According to BSpec max BW per slice is calculated using formula
> Max BW = CDCLK * 64. Currently when calculating min CDCLK we
> account only per plane requirements, however in order to avoid
> FIFO underruns we need to estimate accumulated BW consumed by
> all planes(ddb entries basically) residing on that particular
> DBuf slice. This will allow us to put CDCLK lower and save power
> when we don't need that much bandwidth or gain additional
> performance once plane consumption grows.
>
> v2: - Fix long line warning
>     - Limited new DBuf bw checks to only gens >= 11
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c       | 46 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_bw.h       |  1 +
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 25 ++++++++++
>  drivers/gpu/drm/i915/display/intel_display.c  | 10 +++-
>  .../drm/i915/display/intel_display_power.h    |  1 +
>  drivers/gpu/drm/i915/intel_pm.c               | 34 +++++++++++++-
>  drivers/gpu/drm/i915/intel_pm.h               |  3 ++
>  7 files changed, 117 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 58b264bc318d..a85125110d7e 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -6,6 +6,7 @@
>  #include <drm/drm_atomic_state_helper.h>
>
>  #include "intel_bw.h"
> +#include "intel_pm.h"
>  #include "intel_display_types.h"
>  #include "intel_sideband.h"
>
> @@ -334,6 +335,51 @@ static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_
>        return data_rate;
>  }
>
> +int intel_bw_calc_min_cdclk(struct intel_atomic_state *state)
> +{
> +     struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +     int max_bw_per_dbuf[DBUF_SLICE_MAX];
> +     int i = 0;
> +     enum plane_id plane_id;
> +     struct intel_crtc_state *crtc_state;
> +     struct intel_crtc *crtc;
> +     int max_bw = 0;
> +     int min_cdclk;
> +
> +     memset(max_bw_per_dbuf, 0, sizeof(max_bw_per_dbuf[0]) * DBUF_SLICE_MAX);
> +
> +     for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +             for_each_plane_id_on_crtc(crtc, plane_id) {
> +                     struct skl_ddb_entry *plane_alloc =
> +                             &crtc_state->wm.skl.plane_ddb_y[plane_id];
> +                     struct skl_ddb_entry *uv_plane_alloc =
> +                             &crtc_state->wm.skl.plane_ddb_uv[plane_id];
> +                     unsigned int data_rate = crtc_state->data_rate[plane_id];
> +                     int slice_id = 0;
> +                     u32 dbuf_mask = skl_ddb_dbuf_slice_mask(dev_priv, plane_alloc);
> +
> +                     dbuf_mask |= skl_ddb_dbuf_slice_mask(dev_priv, uv_plane_alloc);
> +
> +                     DRM_DEBUG_KMS("Got dbuf mask %x for pipe %c ddb %d-%d plane %d data rate %d\n",
> +                                   dbuf_mask, pipe_name(crtc->pipe), plane_alloc->start,
> +                                   plane_alloc->end, plane_id, data_rate);
> +
> +                     while (dbuf_mask != 0) {
> +                             if (dbuf_mask & 1) {
> +                                     max_bw_per_dbuf[slice_id] += data_rate;
> +                                     max_bw = max(max_bw, max_bw_per_dbuf[slice_id]);
> +                             }
> +                             slice_id++;
> +                             dbuf_mask >>= 1;
> +                     }
> +             }
> +     }

Something like?

for_each_plane_id() {
        for_each_dbuf_slice() {
                skl_ddb_entry_for_slices(BIT(slice), &ddb_slice);

                if (skl_ddb_entries_overlap(&ddb_slice, &ddb[plane_id])))
                        bw[slice] += data_rate;
        }
}

But this seems to borked anyway since we only consider the crtcs in the
state, and there are those ugly FIXMEs below.

I have a feeling what we want is dbuf_state, and track the bw used for
each slice therein. Should also allow us to flag the cdclk recalculation
only when things actually change in a way that needs more cdclk, instead
of pessimising every plane enable/disable like you do below.

> +
> +     min_cdclk = max_bw / 64;
> +
> +     return min_cdclk;
> +}
> +
>  void intel_bw_crtc_update(struct intel_bw_state *bw_state,
>                          const struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index a8aa7624c5aa..8a522b571c51 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -29,5 +29,6 @@ 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 intel_bw_calc_min_cdclk(struct intel_atomic_state *state);
>
>  #endif /* __INTEL_BW_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 0741d643455b..9f2de613642e 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -25,6 +25,7 @@
>  #include "intel_cdclk.h"
>  #include "intel_display_types.h"
>  #include "intel_sideband.h"
> +#include "intel_bw.h"
>
>  /**
>   * DOC: CDCLK / RAWCLK
> @@ -1979,11 +1980,19 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  {
>        struct drm_i915_private *dev_priv =
>                to_i915(crtc_state->uapi.crtc->dev);
> +     struct intel_atomic_state *state = NULL;
>        int min_cdclk;
>
>        if (!crtc_state->hw.enable)
>                return 0;
>
> +     /*
> +      * FIXME: Unfortunately when this gets called from intel_modeset_setup_hw_state
> +      * there is no intel_atomic_state at all. So lets not then use it.
> +      */
> +     if (crtc_state->uapi.state)
> +             state = to_intel_atomic_state(crtc_state->uapi.state);
> +
>        min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
>
>        /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> @@ -2058,6 +2067,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>        if (IS_TIGERLAKE(dev_priv))
>                min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
>
> +     /*
> +      * Similar story as with skl_write_plane_wm and intel_enable_sagv
> +      * - in some certain driver parts, we don't have any guarantee that
> +      * parent exists. So we might be having a crtc_state without
> +      * parent state.
> +      */
> +     if (INTEL_GEN(dev_priv) >= 11) {
> +             if (state) {
> +                     int dbuf_bw_cdclk = intel_bw_calc_min_cdclk(state);
> +
> +                     DRM_DEBUG_KMS("DBuf bw min cdclk %d current min_cdclk %d\n",
> +                                   dbuf_bw_cdclk, min_cdclk);
> +                     min_cdclk = max(min_cdclk, dbuf_bw_cdclk);
> +             }
> +     }
> +
>        if (min_cdclk > dev_priv->max_cdclk_freq) {
>                drm_dbg_kms(&dev_priv->drm,
>                            "required cdclk (%d kHz) exceeds max (%d kHz)\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index cdff3054b344..aae5521424c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14632,7 +14632,7 @@ static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
>        /* See {hsw,vlv,ivb}_plane_ratio() */
>        return IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv) ||
>                IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> -             IS_IVYBRIDGE(dev_priv);
> +             IS_IVYBRIDGE(dev_priv) || (INTEL_GEN(dev_priv) >= 11);
>  }
>
>  static int intel_atomic_check_planes(struct intel_atomic_state *state,
> @@ -14681,6 +14681,14 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state,
>                if (hweight8(old_active_planes) == hweight8(new_active_planes))
>                        continue;
>
> +             /*
> +              * active_planes bitmask has been updated, whenever amount
> +              * of active planes had changed we need to recalculate CDCLK
> +              * as it depends on total bandwidth now, not only min_cdclk
> +              * per plane.
> +              */
> +             *need_cdclk_calc = true;
> +
>                ret = intel_crtc_add_planes_to_state(state, crtc, new_active_planes);
>                if (ret)
>                        return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index da64a5edae7a..3446c3ce6a4f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -311,6 +311,7 @@ intel_display_power_put_async(struct drm_i915_private *i915,
>  enum dbuf_slice {
>        DBUF_S1,
>        DBUF_S2,
> +     DBUF_SLICE_MAX
>  };
>
>  #define with_intel_display_power(i915, domain, wf) \
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8375054ba27d..15300c44d9dc 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3844,10 +3844,9 @@ icl_get_first_dbuf_slice_offset(u32 dbuf_slice_mask,
>        return offset;
>  }
>
> -static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
> +u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
>  {
>        u16 ddb_size = INTEL_INFO(dev_priv)->ddb_size;
> -
>        drm_WARN_ON(&dev_priv->drm, ddb_size == 0);
>
>        if (INTEL_GEN(dev_priv) < 11)
> @@ -3856,6 +3855,37 @@ static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
>        return ddb_size;
>  }
>
> +u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
> +                         const struct skl_ddb_entry *entry)
> +{
> +     u32 slice_mask = 0;
> +     u16 ddb_size = intel_get_ddb_size(dev_priv);
> +     u16 num_supported_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
> +     u16 slice_size = ddb_size / num_supported_slices;
> +     u16 start_slice;
> +     u16 end_slice;
> +
> +     if (!skl_ddb_entry_size(entry))
> +             return 0;
> +
> +     start_slice = entry->start / slice_size;
> +     end_slice = (entry->end - 1) / slice_size;
> +
> +     DRM_DEBUG_KMS("ddb size %d slices %d slice size %d start slice %d end slice %d\n",
> +                   ddb_size, num_supported_slices, slice_size, start_slice, end_slice);
> +
> +     /*
> +      * Per plane DDB entry can in a really worst case be on multiple slices
> +      * but single entry is anyway contigious.
> +      */
> +     while (start_slice <= end_slice) {
> +             slice_mask |= 1 << start_slice;
> +             start_slice++;
> +     }
> +
> +     return slice_mask;
> +}
> +
>  static u8 skl_compute_dbuf_slices(const struct intel_crtc_state *crtc_state,
>                                  u8 active_pipes);
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index d60a85421c5a..a9e3835927a8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -37,6 +37,9 @@ void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
>                               struct skl_ddb_entry *ddb_y,
>                               struct skl_ddb_entry *ddb_uv);
>  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv);
> +u16 intel_get_ddb_size(struct drm_i915_private *dev_priv);
> +u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
> +                         const struct skl_ddb_entry *entry);
>  void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
>                              struct skl_pipe_wm *out);
>  void g4x_wm_sanitize(struct drm_i915_private *dev_priv);
> --
> 2.24.1.485.gad05a3d8e5

--
Ville Syrjälä
Intel

[-- Attachment #1.2: Type: text/html, Size: 44007 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx]  ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-17 10:43   ` Lisovskiy, Stanislav
@ 2020-03-18  8:33     ` Arkadiusz Hiler
  2020-03-18  8:43       ` Lisovskiy, Stanislav
  0 siblings, 1 reply; 20+ messages in thread
From: Arkadiusz Hiler @ 2020-03-18  8:33 UTC (permalink / raw)
  To: Lisovskiy, Stanislav; +Cc: intel-gfx, Peres, Martin, Vudum, Lakshminarayana

On Tue, Mar 17, 2020 at 12:43:40PM +0200, Lisovskiy, Stanislav wrote:
> What is this weird DOC warning about? "Error: Cannot open file ./drivers/gpu/
> drm/i915/i915_gem_fence_reg.c"
> 
> - wondering, how is that related to this patch.

Looks like you were unlucky and your series got tested with this merged:
https://patchwork.freedesktop.org/series/74738/

but before the fix has landed:
https://patchwork.freedesktop.org/series/74778/

It's all clean now.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx]  ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-18  8:33     ` Arkadiusz Hiler
@ 2020-03-18  8:43       ` Lisovskiy, Stanislav
  2020-03-18  8:55         ` Arkadiusz Hiler
  0 siblings, 1 reply; 20+ messages in thread
From: Lisovskiy, Stanislav @ 2020-03-18  8:43 UTC (permalink / raw)
  To: Hiler, Arkadiusz; +Cc: intel-gfx, Peres, Martin, Vudum, Lakshminarayana


[-- Attachment #1.1: Type: text/plain, Size: 1131 bytes --]

Wonder, how we end up merging _stuff_ with failed IGT and loads of warnings..


https://patchwork.freedesktop.org/series/74738/


... while I get beaten for each and every single warning in my patches 😊


Best Regards,

Lisovskiy Stanislav

Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
________________________________
From: Hiler, Arkadiusz
Sent: Wednesday, March 18, 2020 10:33:07 AM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org; Peres, Martin; Vudum, Lakshminarayana
Subject: Re: ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

On Tue, Mar 17, 2020 at 12:43:40PM +0200, Lisovskiy, Stanislav wrote:
> What is this weird DOC warning about? "Error: Cannot open file ./drivers/gpu/
> drm/i915/i915_gem_fence_reg.c"
>
> - wondering, how is that related to this patch.

Looks like you were unlucky and your series got tested with this merged:
https://patchwork.freedesktop.org/series/74738/

but before the fix has landed:
https://patchwork.freedesktop.org/series/74778/

It's all clean now.

--
Cheers,
Arek

[-- Attachment #1.2: Type: text/html, Size: 2783 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx]  ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-18  8:43       ` Lisovskiy, Stanislav
@ 2020-03-18  8:55         ` Arkadiusz Hiler
  2020-03-18  9:00           ` Lisovskiy, Stanislav
  0 siblings, 1 reply; 20+ messages in thread
From: Arkadiusz Hiler @ 2020-03-18  8:55 UTC (permalink / raw)
  To: Lisovskiy, Stanislav; +Cc: intel-gfx, Peres, Martin, Vudum, Lakshminarayana

On Wed, Mar 18, 2020 at 10:43:29AM +0200, Lisovskiy, Stanislav wrote:
> Wonder, how we end up merging _stuff_ with failed IGT and loads of warnings..
> 
> https://patchwork.freedesktop.org/series/74738/
> 
> ... while I get beaten for each and every single warning in my patches 😊

True. This shouldn't get merged like this. Ask the authors and the
commiters why this got in without anyone looking at the docs issue and
without any explanation of the failures in Fi.CI.IGT.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx]  ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)
  2020-03-18  8:55         ` Arkadiusz Hiler
@ 2020-03-18  9:00           ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 20+ messages in thread
From: Lisovskiy, Stanislav @ 2020-03-18  9:00 UTC (permalink / raw)
  To: Hiler, Arkadiusz; +Cc: intel-gfx, Peres, Martin, Vudum, Lakshminarayana


[-- Attachment #1.1: Type: text/plain, Size: 1270 bytes --]

Remember when I have been working in Ericsson we had been using a gerrit review and you simply

would not be able to merge anything until it has +2(r-b) and no tests failing. Or you have to exclude

unstable tests at least until those are fixed.


It was simply automated and no exceptions, no matter how good person you are 😊


Best Regards,

Lisovskiy Stanislav

Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
________________________________
From: Hiler, Arkadiusz
Sent: Wednesday, March 18, 2020 10:55:19 AM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org; Peres, Martin; Vudum, Lakshminarayana
Subject: Re: ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

On Wed, Mar 18, 2020 at 10:43:29AM +0200, Lisovskiy, Stanislav wrote:
> Wonder, how we end up merging _stuff_ with failed IGT and loads of warnings..
>
> https://patchwork.freedesktop.org/series/74738/
>
> ... while I get beaten for each and every single warning in my patches 😊

True. This shouldn't get merged like this. Ask the authors and the
commiters why this got in without anyone looking at the docs issue and
without any explanation of the failures in Fi.CI.IGT.

--
Cheers,
Arek

[-- Attachment #1.2: Type: text/html, Size: 2576 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-03-18  9:00 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-16 11:37 [Intel-gfx] [PATCH v1 0/3] Consider DBuf bandwidth when calculating CDCLK Stanislav Lisovskiy
2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 1/3] drm/i915: Decouple cdclk calculation from modeset checks Stanislav Lisovskiy
2020-03-16 23:36   ` Manasi Navare
2020-03-17 13:21   ` Ville Syrjälä
2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 2/3] drm/i915: Adjust CDCLK accordingly to our DBuf bw needs Stanislav Lisovskiy
2020-03-16 22:43   ` [Intel-gfx] [PATCH v2 " Stanislav Lisovskiy
2020-03-17 13:46     ` Ville Syrjälä
2020-03-17 14:40       ` Lisovskiy, Stanislav
2020-03-16 11:37 ` [Intel-gfx] [PATCH v1 3/3] drm/i915: Remove unneeded hack now for CDCLK Stanislav Lisovskiy
2020-03-16 23:51   ` Manasi Navare
2020-03-16 20:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Consider DBuf bandwidth when calculating CDCLK Patchwork
2020-03-16 20:34 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-03-17  0:36 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2) Patchwork
2020-03-17 10:43   ` Lisovskiy, Stanislav
2020-03-18  8:33     ` Arkadiusz Hiler
2020-03-18  8:43       ` Lisovskiy, Stanislav
2020-03-18  8:55         ` Arkadiusz Hiler
2020-03-18  9:00           ` Lisovskiy, Stanislav
2020-03-17  0:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-03-17  9:26 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).