[RFC 0/8] Add two-stage watermark programming for VLV/CHV (v4)

[RFC 0/8] Add two-stage watermark programming for VLV/CHV (v4)

[Chi Ding]

From: root <root@localhost.localdomain>

In addition to calculating final watermarks, we also pre-calculate 
a set of intermediate watermark values at atomic check time. These 
intermediate watermarks are a combination of the watermarks for the 
old state and the new state; they should satisfy the requirements of 
both states which means they can be programmed immediately when we 
commit the atomic state (without waiting for a vblank).  Once the 
vblank does happen, we can then re-program watermarks to the more 
optimal final value.

The DSPARB Display Arbitration Control register is double buffered. 
The FIFO repartitioning happens atomically with plane updates but 
how the register double buffering works isn't clear at the moment.
It needs to be figured out to fix the watermark updates.

v2: Split the first patch of v1 into the following patches
- Remove unused parameters from intel_plane_wm_parameters.
- Rename skl_plane_id to wm_plane_id.
- Move fifo_size from intel_plane_wm_parameters to vlv_wm_state.

v3: Split the 3rd patch of v2 into the following two patches
- Move fifo_size from intel_plane_wm_parameters to vlv_wm_state
- Change to use intel_crtc_state instead of base CRTC object
- The patch headlines are also changed to fit the requirement.

v4: 
- Split the 3rd patch of v3 "Move fifo_size from intel_plane_wm_parameters 
to vlv_wm_state" to add a new one "return EINVAL when computed 
watermark exceeds system limitation"
- Add a new patch "Move active watermarks into intel_crtc->wm.active.vlv"
to be consistent with what we do on other platforms
- Change the patch "Add intermediate field in intel_crtc_wm_state and handlers
for two-level watermark" to use macro drm_atomic_crtc_state_for_each_plane_state
to simplify the code
- Change the patch "Add optimal field in intel_crtc_wm_state" to 
use mutex in vlv_update_wm to make assigning currently active wm_state
and merging multiple wm_state become one atomic operation
  

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>



Maarten Lankhorst (8):
  drm/i915: Remove unused parameters from intel_plane_wm_parameters
  drm/i915: Rename skl_plane_id to wm_plane_id
  drm/i915/vlv: Move fifo_size from intel_plane_wm_parameters to
    vlv_wm_state
  drm/i915/vlv: return EINVAL when computed watermark exceeds system    
    limitation
  drm/i915/vlv: Change to use intel_crtc_state instead of base CRTC
    object
  drm/i915/vlv: Add optimal field in intel_crtc_wm_state
  drm/i915/vlv: Move active watermarks into intel_crtc->wm.active.vlv
  drm/i915/vlv: Add intermediate field in intel_crtc_wm_state and
    handlers     for two-level watermark

 drivers/gpu/drm/i915/intel_display.c |   7 +-
 drivers/gpu/drm/i915/intel_drv.h     |  52 ++----
 drivers/gpu/drm/i915/intel_pm.c      | 328 +++++++++++++++++++++++------------
 3 files changed, 233 insertions(+), 154 deletions(-)

-- 
1.8.0.1

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

[RFC 1/8] drm/i915: Remove unused parameters from intel_plane_wm_parameters

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Everything except fifo_size is unused and therefore removed

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 97de5e0..ec34d649 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -694,21 +694,6 @@ struct intel_crtc {
 };
 
 struct intel_plane_wm_parameters {
-	uint32_t horiz_pixels;
-	uint32_t vert_pixels;
-	/*
-	 *   For packed pixel formats:
-	 *     bytes_per_pixel - holds bytes per pixel
-	 *   For planar pixel formats:
-	 *     bytes_per_pixel - holds bytes per pixel for uv-plane
-	 *     y_bytes_per_pixel - holds bytes per pixel for y-plane
-	 */
-	uint8_t bytes_per_pixel;
-	uint8_t y_bytes_per_pixel;
-	bool enabled;
-	bool scaled;
-	u64 tiling;
-	unsigned int rotation;
 	uint16_t fifo_size;
 };
 
-- 
1.8.0.1

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

[RFC 2/8] drm/i915: Rename skl_plane_id to wm_plane_id

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This function will be used not only by SKL but also VLV/CHV.
Therefore it's renamed

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 59 +++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 29bdd79..adbd438 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -54,6 +54,28 @@
 #define INTEL_RC6p_ENABLE			(1<<1)
 #define INTEL_RC6pp_ENABLE			(1<<2)
 
+/*
+ * Return the index of a plane in the DDB and wm result arrays.  Primary
+ * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
+ * other universal planes are in indices 1..n.  Note that this may leave unused
+ * indices between the top "sprite" plane and the cursor.
+ */
+static int
+wm_plane_id(const struct intel_plane *plane)
+{
+	switch (plane->base.type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+		return 0;
+	case DRM_PLANE_TYPE_CURSOR:
+		return PLANE_CURSOR;
+	case DRM_PLANE_TYPE_OVERLAY:
+		return plane->plane + 1;
+	default:
+		MISSING_CASE(plane->base.type);
+		return plane->plane;
+	}
+}
+
 static void bxt_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2828,27 +2850,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
 #define SKL_DDB_SIZE		896	/* in blocks */
 #define BXT_DDB_SIZE		512
 
-/*
- * Return the index of a plane in the SKL DDB and wm result arrays.  Primary
- * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
- * other universal planes are in indices 1..n.  Note that this may leave unused
- * indices between the top "sprite" plane and the cursor.
- */
-static int
-skl_wm_plane_id(const struct intel_plane *plane)
-{
-	switch (plane->base.type) {
-	case DRM_PLANE_TYPE_PRIMARY:
-		return 0;
-	case DRM_PLANE_TYPE_CURSOR:
-		return PLANE_CURSOR;
-	case DRM_PLANE_TYPE_OVERLAY:
-		return plane->plane + 1;
-	default:
-		MISSING_CASE(plane->base.type);
-		return plane->plane;
-	}
-}
 
 static void
 skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
@@ -3011,7 +3012,7 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate)
 
 	/* Calculate and cache data rate for each plane */
 	for_each_plane_in_state(state, plane, pstate, i) {
-		id = skl_wm_plane_id(to_intel_plane(plane));
+		id = wm_plane_id(to_intel_plane(plane));
 		intel_plane = to_intel_plane(plane);
 
 		if (intel_plane->pipe != intel_crtc->pipe)
@@ -3030,7 +3031,7 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate)
 
 	/* Calculate CRTC's total data rate from cached values */
 	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
-		int id = skl_wm_plane_id(intel_plane);
+		int id = wm_plane_id(intel_plane);
 
 		/* packed/uv */
 		total_data_rate += intel_cstate->wm.skl.plane_data_rate[id];
@@ -3088,7 +3089,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	/* 1. Allocate the mininum required blocks for each active plane */
 	for_each_plane_in_state(state, plane, pstate, i) {
 		intel_plane = to_intel_plane(plane);
-		id = skl_wm_plane_id(intel_plane);
+		id = wm_plane_id(intel_plane);
 
 		if (intel_plane->pipe != pipe)
 			continue;
@@ -3130,7 +3131,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
 		unsigned int data_rate, y_data_rate;
 		uint16_t plane_blocks, y_plane_blocks = 0;
-		int id = skl_wm_plane_id(intel_plane);
+		int id = wm_plane_id(intel_plane);
 
 		data_rate = cstate->wm.skl.plane_data_rate[id];
 
@@ -3321,7 +3322,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 			DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
 			DRM_DEBUG_KMS("Plane %d.%d: blocks required = %u/%u, lines required = %u/31\n",
 				      to_intel_crtc(cstate->base.crtc)->pipe,
-				      skl_wm_plane_id(to_intel_plane(pstate->plane)),
+				      wm_plane_id(to_intel_plane(pstate->plane)),
 				      res_blocks, ddb_allocation, res_lines);
 
 			return -EINVAL;
@@ -3359,7 +3360,7 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv,
 	memset(result, 0, sizeof(*result));
 
 	for_each_intel_plane_mask(dev, intel_plane, cstate->base.plane_mask) {
-		int i = skl_wm_plane_id(intel_plane);
+		int i = wm_plane_id(intel_plane);
 
 		plane = &intel_plane->base;
 		intel_pstate = NULL;
@@ -3428,7 +3429,7 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
 
 	/* Until we know more, just disable transition WMs */
 	for_each_intel_plane_on_crtc(crtc->dev, intel_crtc, intel_plane) {
-		int i = skl_wm_plane_id(intel_plane);
+		int i = wm_plane_id(intel_plane);
 
 		trans_wm->plane_en[i] = false;
 	}
@@ -4068,7 +4069,7 @@ void skl_wm_get_hw_state(struct drm_device *dev)
 		for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
 			const struct drm_plane_state *pstate =
 				intel_plane->base.state;
-			int id = skl_wm_plane_id(intel_plane);
+			int id = wm_plane_id(intel_plane);
 
 			cstate->wm.skl.plane_data_rate[id] =
 				skl_plane_relative_data_rate(cstate, pstate, 0);
-- 
1.8.0.1

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

[RFC 3/8] drm/i915/vlv: Move fifo_size from intel_plane_wm_parameters to vlv_wm_state

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This commit saves watermark for each plane in vlv_wm_state to prepare
for two-level watermark because we'll compute and save intermediate and
optimal watermark and fifo size for each plane.

v2:
- remove redundant debug statements in vlv_pipe_set_fifo_size()
- reverse passing vlv_wm_state by parameter because it's unrelated to the
patch
- reverse the change of returning EINVAL when wm > max_wm in vlv_compute_wm
because it's unrelated to the patch

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 12 +-----
 drivers/gpu/drm/i915/intel_pm.c  | 90 ++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ec34d649..9d83494 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -621,6 +621,7 @@ struct intel_crtc_state {
 struct vlv_wm_state {
 	struct vlv_pipe_wm wm[3];
 	struct vlv_sr_wm sr[3];
+	uint16_t fifo_size[I915_MAX_PLANES];
 	uint8_t num_active_planes;
 	uint8_t num_levels;
 	uint8_t level;
@@ -693,10 +694,6 @@ struct intel_crtc {
 	struct vlv_wm_state wm_state;
 };
 
-struct intel_plane_wm_parameters {
-	uint16_t fifo_size;
-};
-
 struct intel_plane {
 	struct drm_plane base;
 	int plane;
@@ -705,13 +702,6 @@ struct intel_plane {
 	int max_downscale;
 	uint32_t frontbuffer_bit;
 
-	/* Since we need to change the watermarks before/after
-	 * enabling/disabling the planes, we need to store the parameters here
-	 * as the other pieces of the struct may not reflect the values we want
-	 * for the watermark calculations. Currently only Haswell uses this.
-	 */
-	struct intel_plane_wm_parameters wm;
-
 	/*
 	 * NOTE: Do not place new plane state fields here (e.g., when adding
 	 * new plane properties).  New runtime state should now be placed in
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index adbd438..a8aba2d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -985,12 +985,14 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 
 static void vlv_compute_fifo(struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->base.dev;
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
 	unsigned int total_rate = 0;
 	const int fifo_size = 512 - 1;
 	int fifo_extra, fifo_left = fifo_size;
+	int rate[I915_MAX_PLANES] = {};
+	int i;
 
 	for_each_intel_plane_on_crtc(dev, crtc, plane) {
 		struct intel_plane_state *state =
@@ -1001,49 +1003,46 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
 
 		if (state->visible) {
 			wm_state->num_active_planes++;
-			total_rate += drm_format_plane_cpp(state->base.fb->pixel_format, 0);
+			rate[wm_plane_id(plane)] =
+			drm_format_plane_cpp(state->base.fb->pixel_format, 0);
+			total_rate += rate[wm_plane_id(plane)];
 		}
 	}
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
-		unsigned int rate;
-
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
-			plane->wm.fifo_size = 63;
+	for (i = 0; i < I915_MAX_PLANES; i++) {
+		if (i == PLANE_CURSOR) {
+			wm_state->fifo_size[i] = 63;
 			continue;
 		}
 
-		if (!state->visible) {
-			plane->wm.fifo_size = 0;
+		if (!rate[i]) {
+			wm_state->fifo_size[i] = 0;
 			continue;
 		}
 
-		rate = drm_format_plane_cpp(state->base.fb->pixel_format, 0);
-		plane->wm.fifo_size = fifo_size * rate / total_rate;
-		fifo_left -= plane->wm.fifo_size;
+		wm_state->fifo_size[i] = fifo_size * rate[i] / total_rate;
+		fifo_left -= wm_state->fifo_size[i];
 	}
 
 	fifo_extra = DIV_ROUND_UP(fifo_left, wm_state->num_active_planes ?: 1);
 
 	/* spread the remainder evenly */
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	for (i = 0; i < I915_MAX_PLANES; i++) {
 		int plane_extra;
 
 		if (fifo_left == 0)
 			break;
 
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
+		if (i == PLANE_CURSOR)
 			continue;
 
 		/* give it all to the first plane if none are active */
-		if (plane->wm.fifo_size == 0 &&
+		if (!wm_state->fifo_size[i] &&
 		    wm_state->num_active_planes)
 			continue;
 
 		plane_extra = min(fifo_extra, fifo_left);
-		plane->wm.fifo_size += plane_extra;
+		wm_state->fifo_size[i] += plane_extra;
 		fifo_left -= plane_extra;
 	}
 
@@ -1064,19 +1063,24 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 		wm_state->sr[level].cursor = 63 - wm_state->sr[level].cursor;
 
 		for_each_intel_plane_on_crtc(dev, crtc, plane) {
+			int i = wm_plane_id(plane);
+
 			switch (plane->base.type) {
 				int sprite;
 			case DRM_PLANE_TYPE_CURSOR:
-				wm_state->wm[level].cursor = plane->wm.fifo_size -
+				wm_state->wm[level].cursor =
+					wm_state->fifo_size[i] -
 					wm_state->wm[level].cursor;
 				break;
 			case DRM_PLANE_TYPE_PRIMARY:
-				wm_state->wm[level].primary = plane->wm.fifo_size -
+				wm_state->wm[level].primary =
+					wm_state->fifo_size[i] -
 					wm_state->wm[level].primary;
 				break;
 			case DRM_PLANE_TYPE_OVERLAY:
 				sprite = plane->plane;
-				wm_state->wm[level].sprite[sprite] = plane->wm.fifo_size -
+				wm_state->wm[level].sprite[sprite] =
+					wm_state->fifo_size[i] -
 					wm_state->wm[level].sprite[sprite];
 				break;
 			}
@@ -1084,7 +1088,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 	}
 }
 
-static void vlv_compute_wm(struct intel_crtc *crtc)
+static int vlv_compute_wm(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
@@ -1127,7 +1131,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 			if (WARN_ON(level == 0 && wm > max_wm))
 				wm = max_wm;
 
-			if (wm > plane->wm.fifo_size)
+			if (wm > wm_state->fifo_size[wm_plane_id(plane)])
 				break;
 
 			switch (plane->base.type) {
@@ -1181,6 +1185,8 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 	}
 
 	vlv_invert_wms(crtc);
+
+	return 0;
 }
 
 #define VLV_FIFO(plane, value) \
@@ -1190,26 +1196,16 @@ static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *plane;
 	int sprite0_start = 0, sprite1_start = 0, fifo_size = 0;
+	const struct vlv_wm_state *wm_state = &crtc->wm_state;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
-			WARN_ON(plane->wm.fifo_size != 63);
-			continue;
-		}
-
-		if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
-			sprite0_start = plane->wm.fifo_size;
-		else if (plane->plane == 0)
-			sprite1_start = sprite0_start + plane->wm.fifo_size;
-		else
-			fifo_size = sprite1_start + plane->wm.fifo_size;
-	}
 
-	WARN_ON(fifo_size != 512 - 1);
+	WARN_ON(wm_state->fifo_size[PLANE_CURSOR] != 63);
+	sprite0_start = wm_state->fifo_size[0];
+	sprite1_start = sprite0_start + wm_state->fifo_size[1];
+	fifo_size = sprite1_start + wm_state->fifo_size[2];
 
-	DRM_DEBUG_KMS("Pipe %c FIFO split %d / %d / %d\n",
+	WARN(fifo_size != 512 - 1, "Pipe %c FIFO split %d / %d / %d\n",
 		      pipe_name(crtc->pipe), sprite0_start,
 		      sprite1_start, fifo_size);
 
@@ -4216,6 +4212,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct vlv_wm_values *wm = &dev_priv->wm.vlv;
+	struct intel_crtc *crtc;
 	struct intel_plane *plane;
 	enum pipe pipe;
 	u32 val;
@@ -4223,17 +4220,20 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 	vlv_read_wm_values(dev_priv, wm);
 
 	for_each_intel_plane(dev, plane) {
+		struct vlv_wm_state *wm_state;
+		int i = wm_plane_id(plane);
+
+		crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, plane->pipe));
+		wm_state = &crtc->wm_state;
+
 		switch (plane->base.type) {
-			int sprite;
 		case DRM_PLANE_TYPE_CURSOR:
-			plane->wm.fifo_size = 63;
+			wm_state->fifo_size[i] = 63;
 			break;
 		case DRM_PLANE_TYPE_PRIMARY:
-			plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, 0);
-			break;
 		case DRM_PLANE_TYPE_OVERLAY:
-			sprite = plane->plane;
-			plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, sprite + 1);
+			wm_state->fifo_size[i] =
+				vlv_get_fifo_size(dev, plane->pipe, i);
 			break;
 		}
 	}
-- 
1.8.0.1

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

[RFC 4/8] drm/i915/vlv: return EINVAL when computed watermark exceeds system limitation

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

When computing normal watermarks in vlv_compute_wm(), if the value
is bigger than system limitation, return EINVAL

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a8aba2d..3dbe160 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1127,9 +1127,14 @@ static int vlv_compute_wm(struct intel_crtc *crtc)
 			int wm = vlv_compute_wm_level(plane, crtc, state, level);
 			int max_wm = plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511;
 
-			/* hack */
-			if (WARN_ON(level == 0 && wm > max_wm))
-				wm = max_wm;
+			if (level == 0 && wm > max_wm) {
+				DRM_DEBUG_KMS("Requested display configuration "
+				"exceeds system watermark limitations\n");
+				DRM_DEBUG_KMS("Plane %d.%d: blocks required = %u/%u\n",
+					crtc->pipe,
+					drm_plane_index(&plane->base), wm, max_wm);
+				return -EINVAL;
+			}
 
 			if (wm > wm_state->fifo_size[wm_plane_id(plane)])
 				break;
-- 
1.8.0.1

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

[RFC 5/8] drm/i915/vlv: Change to use intel_crtc_state instead of base CRTC object

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This commit changs some functions to operate on intel_crtc_state rather
than the base CRTC objects in order to transit to atomic. The reason we
want to do this is to allow future patches to move the computation steps
into the atomic 'check' phase where they'll be operating on in-flight CRTC
states rather than already-committed states.

v2: make the change for vlv_compute_wm_level() which is forgotten in v1

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3dbe160..03cd139 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -947,7 +947,7 @@ static void vlv_setup_wm_latency(struct drm_device *dev)
 }
 
 static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
-				     struct intel_crtc *crtc,
+					const struct intel_crtc_state *cstate,
 				     const struct intel_plane_state *state,
 				     int level)
 {
@@ -961,9 +961,9 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 		return 0;
 
 	cpp = drm_format_plane_cpp(state->base.fb->pixel_format, 0);
-	clock = crtc->config->base.adjusted_mode.crtc_clock;
-	htotal = crtc->config->base.adjusted_mode.crtc_htotal;
-	width = crtc->config->pipe_src_w;
+	clock = cstate->base.adjusted_mode.crtc_clock;
+	htotal = cstate->base.adjusted_mode.crtc_htotal;
+	width = cstate->pipe_src_w;
 	if (WARN_ON(htotal == 0))
 		htotal = 1;
 
@@ -983,8 +983,9 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 	return min_t(int, wm, USHRT_MAX);
 }
 
-static void vlv_compute_fifo(struct intel_crtc *crtc)
+static void vlv_compute_fifo(struct intel_crtc_state *cstate)
 {
+	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
 	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
@@ -1088,8 +1089,9 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 	}
 }
 
-static int vlv_compute_wm(struct intel_crtc *crtc)
+static int vlv_compute_wm(struct intel_crtc_state *cstate)
 {
+	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
 	struct intel_plane *plane;
@@ -1103,7 +1105,7 @@ static int vlv_compute_wm(struct intel_crtc *crtc)
 
 	wm_state->num_active_planes = 0;
 
-	vlv_compute_fifo(crtc);
+	vlv_compute_fifo(cstate);
 
 	if (wm_state->num_active_planes != 1)
 		wm_state->cxsr = false;
@@ -1124,7 +1126,7 @@ static int vlv_compute_wm(struct intel_crtc *crtc)
 
 		/* normal watermarks */
 		for (level = 0; level < wm_state->num_levels; level++) {
-			int wm = vlv_compute_wm_level(plane, crtc, state, level);
+			int wm = vlv_compute_wm_level(plane, cstate, state, level);
 			int max_wm = plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511;
 
 			if (level == 0 && wm > max_wm) {
@@ -1328,7 +1330,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 	enum pipe pipe = intel_crtc->pipe;
 	struct vlv_wm_values wm = {};
 
-	vlv_compute_wm(intel_crtc);
+	vlv_compute_wm(intel_crtc->config);
 	vlv_merge_wm(dev, &wm);
 
 	if (memcmp(&dev_priv->wm.vlv, &wm, sizeof(wm)) == 0) {
-- 
1.8.0.1

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

[RFC 6/8] drm/i915/vlv: Add optimal field in intel_crtc_wm_state

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

For two-stage watermark programming, we need to calculate optimal
watermark which is set after vblank and intermediate watermark which
can be set without waiting for vblank.

This commit adds optimal watermark field and changes the code to use it
in vlv_compute_wm(), vlv_update_wm() and vlv_wm_get_hw_state()

v2:
- use mutex in vlv_update_wm to make assigning currently active wm_state
and merging multiple wm_state become one atomic operation
- change vlv_compute_fifo and vlv_invert_wms to pass wm_state as parameter

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 23 ++++++++++++++---------
 drivers/gpu/drm/i915/intel_pm.c  | 24 ++++++++++++++++--------
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9d83494..6d616b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -417,6 +417,16 @@ struct skl_pipe_wm {
 	uint32_t linetime;
 };
 
+struct vlv_wm_state {
+	struct vlv_pipe_wm wm[3];
+	struct vlv_sr_wm sr[3];
+	uint16_t fifo_size[I915_MAX_PLANES];
+	uint8_t num_active_planes;
+	uint8_t num_levels;
+	uint8_t level;
+	bool cxsr;
+};
+
 struct intel_crtc_wm_state {
 	union {
 		struct {
@@ -437,6 +447,10 @@ struct intel_crtc_wm_state {
 		} ilk;
 
 		struct {
+			struct vlv_wm_state optimal;
+		} vlv;
+
+		struct {
 			/* gen9+ only needs 1-step wm programming */
 			struct skl_pipe_wm optimal;
 
@@ -618,15 +632,6 @@ struct intel_crtc_state {
 	uint32_t gamma_mode;
 };
 
-struct vlv_wm_state {
-	struct vlv_pipe_wm wm[3];
-	struct vlv_sr_wm sr[3];
-	uint16_t fifo_size[I915_MAX_PLANES];
-	uint8_t num_active_planes;
-	uint8_t num_levels;
-	uint8_t level;
-	bool cxsr;
-};
 
 struct intel_crtc {
 	struct drm_crtc base;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 03cd139..0076716 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -983,10 +983,10 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 	return min_t(int, wm, USHRT_MAX);
 }
 
-static void vlv_compute_fifo(struct intel_crtc_state *cstate)
+static void vlv_compute_fifo(struct intel_crtc_state *cstate,
+				struct vlv_wm_state *wm_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
-	struct vlv_wm_state *wm_state = &crtc->wm_state;
 	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
 	unsigned int total_rate = 0;
@@ -1050,9 +1050,9 @@ static void vlv_compute_fifo(struct intel_crtc_state *cstate)
 	WARN_ON(fifo_left != 0);
 }
 
-static void vlv_invert_wms(struct intel_crtc *crtc)
+static void vlv_invert_wms(struct intel_crtc *crtc,
+			struct vlv_wm_state *wm_state)
 {
-	struct vlv_wm_state *wm_state = &crtc->wm_state;
 	int level;
 
 	for (level = 0; level < wm_state->num_levels; level++) {
@@ -1093,7 +1093,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 {
 	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
-	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	struct vlv_wm_state *wm_state = &cstate->wm.vlv.optimal;
 	struct intel_plane *plane;
 	int sr_fifo_size = INTEL_INFO(dev)->num_pipes * 512 - 1;
 	int level;
@@ -1105,7 +1105,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 
 	wm_state->num_active_planes = 0;
 
-	vlv_compute_fifo(cstate);
+	vlv_compute_fifo(cstate, wm_state);
 
 	if (wm_state->num_active_planes != 1)
 		wm_state->cxsr = false;
@@ -1191,7 +1191,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 		memset(&wm_state->sr[level], 0, sizeof(wm_state->sr[level]));
 	}
 
-	vlv_invert_wms(crtc);
+	vlv_invert_wms(crtc, wm_state);
 
 	return 0;
 }
@@ -1331,7 +1331,10 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 	struct vlv_wm_values wm = {};
 
 	vlv_compute_wm(intel_crtc->config);
+	mutex_lock(&dev_priv->wm.wm_mutex);
+	intel_crtc->wm_state = intel_crtc->config->wm.vlv.optimal;
 	vlv_merge_wm(dev, &wm);
+	mutex_unlock(&dev_priv->wm.wm_mutex);
 
 	if (memcmp(&dev_priv->wm.vlv, &wm, sizeof(wm)) == 0) {
 		/* FIXME should be part of crtc atomic commit */
@@ -4282,10 +4285,15 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 		mutex_unlock(&dev_priv->rps.hw_lock);
 	}
 
-	for_each_pipe(dev_priv, pipe)
+	for_each_intel_crtc(dev, crtc) {
+		pipe = crtc->pipe;
+		to_intel_crtc_state(crtc->base.state)->wm.vlv.optimal
+			= crtc->wm_state;
+
 		DRM_DEBUG_KMS("Initial watermarks: pipe %c, plane=%d, cursor=%d, sprite0=%d, sprite1=%d\n",
 			      pipe_name(pipe), wm->pipe[pipe].primary, wm->pipe[pipe].cursor,
 			      wm->pipe[pipe].sprite[0], wm->pipe[pipe].sprite[1]);
+	}
 
 	DRM_DEBUG_KMS("Initial watermarks: SR plane=%d, SR cursor=%d level=%d cxsr=%d\n",
 		      wm->sr.plane, wm->sr.cursor, wm->level, wm->cxsr);
-- 
1.8.0.1

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

[RFC 7/8] drm/i915/vlv: Move active watermarks into intel_crtc->wm.active.vlv

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Previously the active watermarks is saved in intel_crtc->wm_state
This commit adds a new field "vlv" into intel_crtc->wm.active and save
the active watermarks in it to be consistent with what we do on other
platforms.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6d616b6..2d3dfec 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -678,6 +678,7 @@ struct intel_crtc {
 		union {
 			struct intel_pipe_wm ilk;
 			struct skl_pipe_wm skl;
+			struct vlv_wm_state vlv;
 		} active;
 
 		/* allow CxSR on this pipe */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0076716..ceded5b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1204,7 +1204,7 @@ static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc)
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int sprite0_start = 0, sprite1_start = 0, fifo_size = 0;
-	const struct vlv_wm_state *wm_state = &crtc->wm_state;
+	const struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 
 
 	WARN_ON(wm_state->fifo_size[PLANE_CURSOR] != 63);
@@ -1286,7 +1286,7 @@ static void vlv_merge_wm(struct drm_device *dev,
 	wm->cxsr = true;
 
 	for_each_intel_crtc(dev, crtc) {
-		const struct vlv_wm_state *wm_state = &crtc->wm_state;
+		const struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 
 		if (!crtc->active)
 			continue;
@@ -1305,7 +1305,7 @@ static void vlv_merge_wm(struct drm_device *dev,
 		wm->level = VLV_WM_LEVEL_PM2;
 
 	for_each_intel_crtc(dev, crtc) {
-		struct vlv_wm_state *wm_state = &crtc->wm_state;
+		struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 		enum pipe pipe = crtc->pipe;
 
 		if (!crtc->active)
@@ -1332,7 +1332,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 
 	vlv_compute_wm(intel_crtc->config);
 	mutex_lock(&dev_priv->wm.wm_mutex);
-	intel_crtc->wm_state = intel_crtc->config->wm.vlv.optimal;
+	intel_crtc->wm.active.vlv = intel_crtc->config->wm.vlv.optimal;
 	vlv_merge_wm(dev, &wm);
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 
@@ -4234,7 +4234,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 		int i = wm_plane_id(plane);
 
 		crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, plane->pipe));
-		wm_state = &crtc->wm_state;
+		wm_state = &crtc->wm.active.vlv;
 
 		switch (plane->base.type) {
 		case DRM_PLANE_TYPE_CURSOR:
@@ -4288,7 +4288,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 	for_each_intel_crtc(dev, crtc) {
 		pipe = crtc->pipe;
 		to_intel_crtc_state(crtc->base.state)->wm.vlv.optimal
-			= crtc->wm_state;
+			= crtc->wm.active.vlv;
 
 		DRM_DEBUG_KMS("Initial watermarks: pipe %c, plane=%d, cursor=%d, sprite0=%d, sprite1=%d\n",
 			      pipe_name(pipe), wm->pipe[pipe].primary, wm->pipe[pipe].cursor,
-- 
1.8.0.1

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

[RFC 8/8] drm/i915/vlv: Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Rename vlv_compute_wm to vlv_compute_pipe_wm to compute optimal watermark
Add vlv_compute_intermediate_wm to computer intermediate watermark
Add vlv_initial_watermarks to write intermediate watermark into hardware
Add vlv_optimize_watermarks to write optimal watermark into hardware
Change valleyview_crtc_enable to call .initial_watermarks handler

This patch adds the handlers for two-level atomic watermark for VLV/CHV.
It makes use of the optimal and intermediate watermark fields added in
the previous commits to calculate the optimal and intermediate state.
It sets the intermediate watermark which is the safer value of the
currently active and the optimal watermark pre-vblank. Then it sets the
optimal watermark after-vblank.

v2:
- use macro drm_atomic_crtc_state_for_each_plane_state
- remove redundant debug statements in vlv_pipe_set_fifo_size

v3:
- use macro drm_atomic_crtc_state_for_each_plane_state to simplify the code
- check !new_state->active || modeset in computing intermediate watermark

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   7 +-
 drivers/gpu/drm/i915/intel_drv.h     |   3 +-
 drivers/gpu/drm/i915/intel_pm.c      | 150 +++++++++++++++++++++++++++--------
 3 files changed, 121 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3f8987b..431d082 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4598,8 +4598,6 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 
 	intel_frontbuffer_flip(dev, pipe_config->fb_bits);
 
-	crtc->wm.cxsr_allowed = true;
-
 	if (pipe_config->update_wm_post && pipe_config->base.active)
 		intel_update_watermarks(&crtc->base);
 
@@ -4645,7 +4643,6 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
 	}
 
 	if (pipe_config->disable_cxsr) {
-		crtc->wm.cxsr_allowed = false;
 
 		/*
 		 * Vblank time updates from the shadow to live plane control register
@@ -6092,7 +6089,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	intel_color_load_luts(&pipe_config->base);
 
-	intel_update_watermarks(crtc);
+	dev_priv->display.initial_watermarks(pipe_config);
 	intel_enable_pipe(intel_crtc);
 
 	assert_vblank_disabled(crtc);
@@ -14310,8 +14307,6 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	intel_crtc->cursor_cntl = ~0;
 	intel_crtc->cursor_size = ~0;
 
-	intel_crtc->wm.cxsr_allowed = true;
-
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2d3dfec..c207148 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -447,6 +447,7 @@ struct intel_crtc_wm_state {
 		} ilk;
 
 		struct {
+			struct vlv_wm_state intermediate;
 			struct vlv_wm_state optimal;
 		} vlv;
 
@@ -681,8 +682,6 @@ struct intel_crtc {
 			struct vlv_wm_state vlv;
 		} active;
 
-		/* allow CxSR on this pipe */
-		bool cxsr_allowed;
 	} wm;
 
 	int scanline_offset;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ceded5b..d9d1ecb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -30,6 +30,7 @@
 #include "intel_drv.h"
 #include "../../../platform/x86/intel_ips.h"
 #include <linux/module.h>
+#include <drm/drm_atomic_helper.h>
 
 /**
  * DOC: RC6
@@ -964,6 +965,7 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 	clock = cstate->base.adjusted_mode.crtc_clock;
 	htotal = cstate->base.adjusted_mode.crtc_htotal;
 	width = cstate->pipe_src_w;
+
 	if (WARN_ON(htotal == 0))
 		htotal = 1;
 
@@ -986,27 +988,28 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 static void vlv_compute_fifo(struct intel_crtc_state *cstate,
 				struct vlv_wm_state *wm_state)
 {
-	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_plane *plane;
 	unsigned int total_rate = 0;
 	const int fifo_size = 512 - 1;
 	int fifo_extra, fifo_left = fifo_size;
 	int rate[I915_MAX_PLANES] = {};
 	int i;
+	const struct drm_plane_state *pstate;
+	struct drm_plane *plane;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) {
 		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+			to_intel_plane_state(pstate);
+		struct intel_plane *intel_plane =
+			to_intel_plane(plane);
 
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
+		if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
 			continue;
 
 		if (state->visible) {
 			wm_state->num_active_planes++;
-			rate[wm_plane_id(plane)] =
+			rate[wm_plane_id(intel_plane)] =
 			drm_format_plane_cpp(state->base.fb->pixel_format, 0);
-			total_rate += rate[wm_plane_id(plane)];
+			total_rate += rate[wm_plane_id(intel_plane)];
 		}
 	}
 
@@ -1089,18 +1092,19 @@ static void vlv_invert_wms(struct intel_crtc *crtc,
 	}
 }
 
-static int vlv_compute_wm(struct intel_crtc_state *cstate)
+static int vlv_compute_pipe_wm(struct intel_crtc_state *cstate)
 {
 	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	struct vlv_wm_state *wm_state = &cstate->wm.vlv.optimal;
-	struct intel_plane *plane;
+	struct drm_plane *plane;
 	int sr_fifo_size = INTEL_INFO(dev)->num_pipes * 512 - 1;
 	int level;
+	const struct drm_plane_state *pstate;
 
 	memset(wm_state, 0, sizeof(*wm_state));
 
-	wm_state->cxsr = crtc->pipe != PIPE_C && crtc->wm.cxsr_allowed;
+	wm_state->cxsr = crtc->pipe != PIPE_C;
 	wm_state->num_levels = to_i915(dev)->wm.max_level + 1;
 
 	wm_state->num_active_planes = 0;
@@ -1117,31 +1121,33 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 		}
 	}
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) {
 		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+			to_intel_plane_state(pstate);
+		struct intel_plane *intel_plane =
+			to_intel_plane(plane);
 
 		if (!state->visible)
 			continue;
 
 		/* normal watermarks */
 		for (level = 0; level < wm_state->num_levels; level++) {
-			int wm = vlv_compute_wm_level(plane, cstate, state, level);
-			int max_wm = plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511;
+			int wm = vlv_compute_wm_level(intel_plane, cstate, state, level);
+			int max_wm = intel_plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511;
 
 			if (level == 0 && wm > max_wm) {
 				DRM_DEBUG_KMS("Requested display configuration "
 				"exceeds system watermark limitations\n");
 				DRM_DEBUG_KMS("Plane %d.%d: blocks required = %u/%u\n",
-					crtc->pipe,
-					drm_plane_index(&plane->base), wm, max_wm);
+				      crtc->pipe,
+				      drm_plane_index(&intel_plane->base), wm, max_wm);
 				return -EINVAL;
 			}
 
-			if (wm > wm_state->fifo_size[wm_plane_id(plane)])
+			if (wm > wm_state->fifo_size[wm_plane_id(intel_plane)])
 				break;
 
-			switch (plane->base.type) {
+			switch (intel_plane->base.type) {
 				int sprite;
 			case DRM_PLANE_TYPE_CURSOR:
 				wm_state->wm[level].cursor = wm;
@@ -1150,7 +1156,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 				wm_state->wm[level].primary = wm;
 				break;
 			case DRM_PLANE_TYPE_OVERLAY:
-				sprite = plane->plane;
+				sprite = intel_plane->plane;
 				wm_state->wm[level].sprite[sprite] = wm;
 				break;
 			}
@@ -1162,7 +1168,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 			continue;
 
 		/* maxfifo watermarks */
-		switch (plane->base.type) {
+		switch (intel_plane->base.type) {
 			int sprite, level;
 		case DRM_PLANE_TYPE_CURSOR:
 			for (level = 0; level < wm_state->num_levels; level++)
@@ -1176,7 +1182,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 					    wm_state->wm[level].primary);
 			break;
 		case DRM_PLANE_TYPE_OVERLAY:
-			sprite = plane->plane;
+			sprite = intel_plane->plane;
 			for (level = 0; level < wm_state->num_levels; level++)
 				wm_state->sr[level].plane =
 					min(wm_state->sr[level].plane,
@@ -1196,6 +1202,65 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 	return 0;
 }
 
+
+static int vlv_compute_intermediate_wm(struct drm_device *dev,
+				       struct intel_crtc *intel_crtc,
+				       struct intel_crtc_state *newstate)
+{
+	struct vlv_wm_state *a = &newstate->wm.vlv.intermediate;
+	struct vlv_wm_state *b = &intel_crtc->wm.active.vlv;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int level, max_level = dev_priv->wm.max_level;
+
+	/*
+	 * Start with the final, target watermarks, then combine with the
+	 * currently active watermarks to get values that are safe both before
+	 * and after the vblank.
+	 */
+	*a = newstate->wm.vlv.optimal;
+
+	if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base))
+		return 0;
+
+	a->num_levels = min(a->num_levels, b->num_levels);
+	a->cxsr &= b->cxsr;
+
+	for (level = 0; level < a->num_levels; level++) {
+		struct vlv_pipe_wm *a_wm = &a->wm[level];
+		const struct vlv_pipe_wm *b_wm = &b->wm[level];
+		struct vlv_sr_wm *a_sr = &a->sr[level];
+		const struct vlv_sr_wm *b_sr = &b->sr[level];
+
+		a_wm->primary = min(a_wm->primary, b_wm->primary);
+		a_wm->sprite[0] = min(a_wm->sprite[0], b_wm->sprite[0]);
+		a_wm->sprite[1] = min(a_wm->sprite[1], b_wm->sprite[1]);
+		a_wm->cursor = min(a_wm->cursor, b_wm->cursor);
+
+		if (a->cxsr) {
+			a_sr->plane = min(a_sr->plane, b_sr->plane);
+			a_sr->cursor = min(a_sr->cursor, b_sr->cursor);
+		} else {
+			a_sr->plane = b_sr->plane;
+			a_sr->cursor = b_sr->cursor;
+		}
+	}
+
+	/* reset the invalid levels */
+	for (level = a->num_levels; level <= max_level; level++) {
+		memset(&a->wm[level], 0, sizeof(struct vlv_pipe_wm));
+		memset(&a->sr[level], 0, sizeof(struct vlv_sr_wm));
+	}
+
+	/*
+	 * If our intermediate WM are identical to the final WM, then we can
+	 * omit the post-vblank programming; only update if it's different.
+	 */
+	if (memcmp(a, &newstate->wm.ilk.optimal, sizeof(*a)) == 0)
+		newstate->wm.need_postvbl_update = false;
+
+	return 0;
+}
+
 #define VLV_FIFO(plane, value) \
 	(((value) << DSPARB_ ## plane ## _SHIFT_VLV) & DSPARB_ ## plane ## _MASK_VLV)
 
@@ -1330,11 +1395,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 	enum pipe pipe = intel_crtc->pipe;
 	struct vlv_wm_values wm = {};
 
-	vlv_compute_wm(intel_crtc->config);
-	mutex_lock(&dev_priv->wm.wm_mutex);
-	intel_crtc->wm.active.vlv = intel_crtc->config->wm.vlv.optimal;
 	vlv_merge_wm(dev, &wm);
-	mutex_unlock(&dev_priv->wm.wm_mutex);
 
 	if (memcmp(&dev_priv->wm.vlv, &wm, sizeof(wm)) == 0) {
 		/* FIXME should be part of crtc atomic commit */
@@ -1378,6 +1439,29 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 	dev_priv->wm.vlv = wm;
 }
 
+
+static void vlv_initial_watermarks(struct intel_crtc_state *cstate)
+{
+	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+
+	mutex_lock(&dev_priv->wm.wm_mutex);
+	intel_crtc->wm.active.vlv = cstate->wm.vlv.intermediate;
+	vlv_update_wm(cstate->base.crtc);
+	mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
+static void vlv_optimize_watermarks(struct intel_crtc_state *cstate)
+{
+	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+
+	mutex_lock(&dev_priv->wm.wm_mutex);
+	intel_crtc->wm.active.vlv = cstate->wm.vlv.optimal;
+	vlv_update_wm(cstate->base.crtc);
+	mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
 #define single_plane_enabled(mask) is_power_of_2(mask)
 
 static void g4x_update_wm(struct drm_crtc *crtc)
@@ -7406,12 +7490,16 @@ void intel_init_pm(struct drm_device *dev)
 			DRM_DEBUG_KMS("Failed to read display plane latency. "
 				      "Disable CxSR\n");
 		}
-	} else if (IS_CHERRYVIEW(dev)) {
-		vlv_setup_wm_latency(dev);
-		dev_priv->display.update_wm = vlv_update_wm;
-	} else if (IS_VALLEYVIEW(dev)) {
+	} else if (IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev)) {
 		vlv_setup_wm_latency(dev);
-		dev_priv->display.update_wm = vlv_update_wm;
+		dev_priv->display.compute_pipe_wm =
+			vlv_compute_pipe_wm;
+		dev_priv->display.compute_intermediate_wm =
+			vlv_compute_intermediate_wm;
+		dev_priv->display.initial_watermarks =
+			vlv_initial_watermarks;
+		dev_priv->display.optimize_watermarks =
+			vlv_optimize_watermarks;
 	} else if (IS_PINEVIEW(dev)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
 					    dev_priv->is_ddr3,
-- 
1.8.0.1

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

✗ Ro.CI.BAT: failure for Add two-stage watermark programming for VLV/CHV (v4)

[Patchwork]

== Series Details ==

Series: Add two-stage watermark programming for VLV/CHV (v4)
URL   : https://patchwork.freedesktop.org/series/9067/
State : failure

== Summary ==

Applying: drm/i915: Remove unused parameters from intel_plane_wm_parameters
Applying: drm/i915: Rename skl_plane_id to wm_plane_id
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_pm.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_pm.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_pm.c
error: Failed to merge in the changes.
Patch failed at 0002 drm/i915: Rename skl_plane_id to wm_plane_id
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

[RFC 3/8] drm/i915/vlv: Move fifo_size from intel_plane_wm_parameters to vlv_wm_state

[Chi Ding]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This commit saves watermark for each plane in vlv_wm_state to prepare
for two-level watermark because we'll compute and save intermediate and
optimal watermark and fifo size for each plane.

v2:
- remove redundant debug statements in vlv_pipe_set_fifo_size()
- reverse passing vlv_wm_state by parameter because it's unrelated to the
patch
- reverse the change of returning EINVAL when wm > max_wm in vlv_compute_wm
because it's unrelated to the patch

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chi Ding <chix.ding@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 12 +-----
 drivers/gpu/drm/i915/intel_pm.c  | 90 ++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e6dea69..ae3e73e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -623,6 +623,7 @@ struct intel_crtc_state {
 struct vlv_wm_state {
 	struct vlv_pipe_wm wm[3];
 	struct vlv_sr_wm sr[3];
+	uint16_t fifo_size[I915_MAX_PLANES];
 	uint8_t num_active_planes;
 	uint8_t num_levels;
 	uint8_t level;
@@ -695,10 +696,6 @@ struct intel_crtc {
 	struct vlv_wm_state wm_state;
 };
 
-struct intel_plane_wm_parameters {
-	uint16_t fifo_size;
-};
-
 struct intel_plane {
 	struct drm_plane base;
 	int plane;
@@ -707,13 +704,6 @@ struct intel_plane {
 	int max_downscale;
 	uint32_t frontbuffer_bit;
 
-	/* Since we need to change the watermarks before/after
-	 * enabling/disabling the planes, we need to store the parameters here
-	 * as the other pieces of the struct may not reflect the values we want
-	 * for the watermark calculations. Currently only Haswell uses this.
-	 */
-	struct intel_plane_wm_parameters wm;
-
 	/*
 	 * NOTE: Do not place new plane state fields here (e.g., when adding
 	 * new plane properties).  New runtime state should now be placed in
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 29820e2..3e239af 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1010,12 +1010,14 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
 
 static void vlv_compute_fifo(struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->base.dev;
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
 	unsigned int total_rate = 0;
 	const int fifo_size = 512 - 1;
 	int fifo_extra, fifo_left = fifo_size;
+	int rate[I915_MAX_PLANES] = {};
+	int i;
 
 	for_each_intel_plane_on_crtc(dev, crtc, plane) {
 		struct intel_plane_state *state =
@@ -1026,49 +1028,46 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
 
 		if (state->visible) {
 			wm_state->num_active_planes++;
-			total_rate += drm_format_plane_cpp(state->base.fb->pixel_format, 0);
+			rate[wm_plane_id(plane)] =
+			drm_format_plane_cpp(state->base.fb->pixel_format, 0);
+			total_rate += rate[wm_plane_id(plane)];
 		}
 	}
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
-		unsigned int rate;
-
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
-			plane->wm.fifo_size = 63;
+	for (i = 0; i < I915_MAX_PLANES; i++) {
+		if (i == PLANE_CURSOR) {
+			wm_state->fifo_size[i] = 63;
 			continue;
 		}
 
-		if (!state->visible) {
-			plane->wm.fifo_size = 0;
+		if (!rate[i]) {
+			wm_state->fifo_size[i] = 0;
 			continue;
 		}
 
-		rate = drm_format_plane_cpp(state->base.fb->pixel_format, 0);
-		plane->wm.fifo_size = fifo_size * rate / total_rate;
-		fifo_left -= plane->wm.fifo_size;
+		wm_state->fifo_size[i] = fifo_size * rate[i] / total_rate;
+		fifo_left -= wm_state->fifo_size[i];
 	}
 
 	fifo_extra = DIV_ROUND_UP(fifo_left, wm_state->num_active_planes ?: 1);
 
 	/* spread the remainder evenly */
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	for (i = 0; i < I915_MAX_PLANES; i++) {
 		int plane_extra;
 
 		if (fifo_left == 0)
 			break;
 
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
+		if (i == PLANE_CURSOR)
 			continue;
 
 		/* give it all to the first plane if none are active */
-		if (plane->wm.fifo_size == 0 &&
+		if (!wm_state->fifo_size[i] &&
 		    wm_state->num_active_planes)
 			continue;
 
 		plane_extra = min(fifo_extra, fifo_left);
-		plane->wm.fifo_size += plane_extra;
+		wm_state->fifo_size[i] += plane_extra;
 		fifo_left -= plane_extra;
 	}
 
@@ -1089,19 +1088,24 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 		wm_state->sr[level].cursor = 63 - wm_state->sr[level].cursor;
 
 		for_each_intel_plane_on_crtc(dev, crtc, plane) {
+			int i = wm_plane_id(plane);
+
 			switch (plane->base.type) {
 				int sprite;
 			case DRM_PLANE_TYPE_CURSOR:
-				wm_state->wm[level].cursor = plane->wm.fifo_size -
+				wm_state->wm[level].cursor =
+					wm_state->fifo_size[i] -
 					wm_state->wm[level].cursor;
 				break;
 			case DRM_PLANE_TYPE_PRIMARY:
-				wm_state->wm[level].primary = plane->wm.fifo_size -
+				wm_state->wm[level].primary =
+					wm_state->fifo_size[i] -
 					wm_state->wm[level].primary;
 				break;
 			case DRM_PLANE_TYPE_OVERLAY:
 				sprite = plane->plane;
-				wm_state->wm[level].sprite[sprite] = plane->wm.fifo_size -
+				wm_state->wm[level].sprite[sprite] =
+					wm_state->fifo_size[i] -
 					wm_state->wm[level].sprite[sprite];
 				break;
 			}
@@ -1109,7 +1113,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 	}
 }
 
-static void vlv_compute_wm(struct intel_crtc *crtc)
+static int vlv_compute_wm(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
@@ -1152,7 +1156,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 			if (WARN_ON(level == 0 && wm > max_wm))
 				wm = max_wm;
 
-			if (wm > plane->wm.fifo_size)
+			if (wm > wm_state->fifo_size[wm_plane_id(plane)])
 				break;
 
 			switch (plane->base.type) {
@@ -1206,6 +1210,8 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 	}
 
 	vlv_invert_wms(crtc);
+
+	return 0;
 }
 
 #define VLV_FIFO(plane, value) \
@@ -1215,26 +1221,16 @@ static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *plane;
 	int sprite0_start = 0, sprite1_start = 0, fifo_size = 0;
+	const struct vlv_wm_state *wm_state = &crtc->wm_state;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
-			WARN_ON(plane->wm.fifo_size != 63);
-			continue;
-		}
-
-		if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
-			sprite0_start = plane->wm.fifo_size;
-		else if (plane->plane == 0)
-			sprite1_start = sprite0_start + plane->wm.fifo_size;
-		else
-			fifo_size = sprite1_start + plane->wm.fifo_size;
-	}
 
-	WARN_ON(fifo_size != 512 - 1);
+	WARN_ON(wm_state->fifo_size[PLANE_CURSOR] != 63);
+	sprite0_start = wm_state->fifo_size[0];
+	sprite1_start = sprite0_start + wm_state->fifo_size[1];
+	fifo_size = sprite1_start + wm_state->fifo_size[2];
 
-	DRM_DEBUG_KMS("Pipe %c FIFO split %d / %d / %d\n",
+	WARN(fifo_size != 512 - 1, "Pipe %c FIFO split %d / %d / %d\n",
 		      pipe_name(crtc->pipe), sprite0_start,
 		      sprite1_start, fifo_size);
 
@@ -4354,6 +4350,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct vlv_wm_values *wm = &dev_priv->wm.vlv;
+	struct intel_crtc *crtc;
 	struct intel_plane *plane;
 	enum pipe pipe;
 	u32 val;
@@ -4361,17 +4358,20 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 	vlv_read_wm_values(dev_priv, wm);
 
 	for_each_intel_plane(dev, plane) {
+		struct vlv_wm_state *wm_state;
+		int i = wm_plane_id(plane);
+
+		crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, plane->pipe));
+		wm_state = &crtc->wm_state;
+
 		switch (plane->base.type) {
-			int sprite;
 		case DRM_PLANE_TYPE_CURSOR:
-			plane->wm.fifo_size = 63;
+			wm_state->fifo_size[i] = 63;
 			break;
 		case DRM_PLANE_TYPE_PRIMARY:
-			plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, 0);
-			break;
 		case DRM_PLANE_TYPE_OVERLAY:
-			sprite = plane->plane;
-			plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, sprite + 1);
+			wm_state->fifo_size[i] =
+				vlv_get_fifo_size(dev, plane->pipe, i);
 			break;
 		}
 	}
-- 
1.8.0.1

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