All of lore.kernel.org
 help / color / mirror / Atom feed
* Atomic and two-level watermark support for VLV and CHV
@ 2016-06-01  7:10 chix.ding
  2016-06-01  7:10 ` [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters chix.ding
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: chix.ding @ 2016-06-01  7:10 UTC (permalink / raw)
  To: intel-gfx

[RFC 1/5] Remove unused parameters from intel_plane_wm_parameters
[RFC 2/5] Rename skl_plane_id to wm_plane_id
[RFC 3/5] Move fifo_size from intel_plane_wm_parameters to
[RFC 4/5] Add optimal field in intel_crtc_wm_state for VLV 
[RFC 5/5] Add intermediate field in intel_crtc_wm_state and handlers
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters
  2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
@ 2016-06-01  7:10 ` chix.ding
  2016-06-03 22:58   ` Matt Roper
  2016-06-01  7:10 ` [RFC 2/5] Rename skl_plane_id to wm_plane_id chix.ding
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: chix.ding @ 2016-06-01  7:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: yetundex.adebisi, Chi Ding

From: Chi Ding <chix.ding@intel.com>

Everything except fifo_size is unused and therefore removed

This is the first patch of two-level watermark for VLV/CHV

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.

Signed-off-by: Chi Ding <chix.ding@intel.com>

cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
cc: matthew.d.roper@intel.com
cc: yetundex.adebisi@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 9b5f663..b973b86 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -697,21 +697,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

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

* [RFC 2/5] Rename skl_plane_id to wm_plane_id
  2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
  2016-06-01  7:10 ` [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters chix.ding
@ 2016-06-01  7:10 ` chix.ding
  2016-06-03 22:58   ` Matt Roper
  2016-06-01  7:10 ` [RFC 3/5] Move fifo_size from intel_plane_wm_parameters to vlv_wm_state chix.ding
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: chix.ding @ 2016-06-01  7:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: yetundex.adebisi, Chi Ding

From: Chi Ding <chix.ding@intel.com>

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

Signed-off-by: Chi Ding <chix.ding@intel.com>

cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
cc: matthew.d.roper@intel.com
cc: yetundex.adebisi@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 b6dfd02..a3942df 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

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

* [RFC 3/5] Move fifo_size from intel_plane_wm_parameters to vlv_wm_state
  2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
  2016-06-01  7:10 ` [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters chix.ding
  2016-06-01  7:10 ` [RFC 2/5] Rename skl_plane_id to wm_plane_id chix.ding
@ 2016-06-01  7:10 ` chix.ding
  2016-06-03 23:03   ` Matt Roper
  2016-06-01  7:10 ` [RFC 4/5] Add optimal field in intel_crtc_wm_state for VLV chix.ding
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: chix.ding @ 2016-06-01  7:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: yetundex.adebisi, Chi Ding

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

This patch doesn't change the code to use two-level watermark yet,
With this patch the watermarks are saved for each plane and the wm
state, instead of previously only for the plane

The patch is based on Maarten Lankhorst's work and created by Chi Ding

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

cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
cc: matthew.d.roper@intel.com
cc: yetundex.adebisi@intel.com
---
 drivers/gpu/drm/i915/intel_drv.h |  12 +---
 drivers/gpu/drm/i915/intel_pm.c  | 117 +++++++++++++++++++++------------------
 2 files changed, 65 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b973b86..31118e1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -624,6 +624,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;
@@ -696,10 +697,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;
@@ -708,13 +705,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 a3942df..5515328 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -55,7 +55,7 @@
 #define INTEL_RC6pp_ENABLE			(1<<2)
 
 /*
- * Return the index of a plane in the DDB and wm result arrays.  Primary
+ * Return the index of a plane in the 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.
@@ -983,14 +983,17 @@ 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 vlv_wm_state *wm_state)
 {
+	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;
 	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,58 +1004,55 @@ 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;
 	}
 
 	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++) {
@@ -1064,19 +1064,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,8 +1089,9 @@ 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_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;
@@ -1099,7 +1105,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 
 	wm_state->num_active_planes = 0;
 
-	vlv_compute_fifo(crtc);
+	vlv_compute_fifo(cstate, wm_state);
 
 	if (wm_state->num_active_planes != 1)
 		wm_state->cxsr = false;
@@ -1123,11 +1129,16 @@ static void 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",
+				      to_intel_crtc(cstate->base.crtc)->pipe,
+				      drm_plane_index(&plane->base), wm, max_wm);
+				return -EINVAL;
+			}
 
-			if (wm > plane->wm.fifo_size)
+			if (wm > wm_state->fifo_size[wm_plane_id(plane)])
 				break;
 
 			switch (plane->base.type) {
@@ -1180,7 +1191,9 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 		memset(&wm_state->sr[level], 0, sizeof(wm_state->sr[level]));
 	}
 
-	vlv_invert_wms(crtc);
+	vlv_invert_wms(crtc, wm_state);
+
+	return 0;
 }
 
 #define VLV_FIFO(plane, value) \
@@ -1190,24 +1203,18 @@ 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(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];
 
-	WARN_ON(fifo_size != 512 - 1);
+	WARN(fifo_size != 512 - 1, "Pipe %c FIFO split %d / %d / %d\n",
+		      pipe_name(crtc->pipe), sprite0_start,
+		      sprite1_start, fifo_size);
 
 	DRM_DEBUG_KMS("Pipe %c FIFO split %d / %d / %d\n",
 		      pipe_name(crtc->pipe), sprite0_start,
@@ -1327,7 +1334,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) {
@@ -4216,6 +4223,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 +4231,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

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

* [RFC 4/5] Add optimal field in intel_crtc_wm_state for VLV
  2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
                   ` (2 preceding siblings ...)
  2016-06-01  7:10 ` [RFC 3/5] Move fifo_size from intel_plane_wm_parameters to vlv_wm_state chix.ding
@ 2016-06-01  7:10 ` chix.ding
  2016-06-01  7:10 ` [RFC 5/5] Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark for VLV/CHV chix.ding
  2016-06-01  7:44 ` ✗ Ro.CI.BAT: warning for series starting with [RFC,1/5] Remove unused parameters from intel_plane_wm_parameters Patchwork
  5 siblings, 0 replies; 10+ messages in thread
From: chix.ding @ 2016-06-01  7:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: yetundex.adebisi, Chi Ding

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

This patch prepares for two-level watermark support in the next commit

Change the code to use the optimal field

This patch adds optimal field, but doesn't change the code to use
two-level watermark yet

The patch is based on Maarten Lankhorst's code and created by Chi Ding

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

cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
cc: matthew.d.roper@intel.com
cc: yetundex.adebisi@intel.com
---
 drivers/gpu/drm/i915/intel_drv.h | 23 ++++++++++++++---------
 drivers/gpu/drm/i915/intel_pm.c  | 11 +++++++++--
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 31118e1..6d2e628 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -420,6 +420,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 {
@@ -440,6 +450,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;
 
@@ -621,15 +635,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 5515328..89eb139 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -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;
@@ -1335,6 +1335,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 	struct vlv_wm_values wm = {};
 
 	vlv_compute_wm(intel_crtc->config);
+	intel_crtc->wm_state = intel_crtc->config->wm.vlv.optimal;
 	vlv_merge_wm(dev, &wm);
 
 	if (memcmp(&dev_priv->wm.vlv, &wm, sizeof(wm)) == 0) {
@@ -1377,6 +1378,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 		chv_set_memory_dvfs(dev_priv, true);
 
 	dev_priv->wm.vlv = wm;
+
 }
 
 #define single_plane_enabled(mask) is_power_of_2(mask)
@@ -4286,10 +4288,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

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

* [RFC 5/5] Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark for VLV/CHV
  2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
                   ` (3 preceding siblings ...)
  2016-06-01  7:10 ` [RFC 4/5] Add optimal field in intel_crtc_wm_state for VLV chix.ding
@ 2016-06-01  7:10 ` chix.ding
  2016-06-01  7:44 ` ✗ Ro.CI.BAT: warning for series starting with [RFC,1/5] Remove unused parameters from intel_plane_wm_parameters Patchwork
  5 siblings, 0 replies; 10+ messages in thread
From: chix.ding @ 2016-06-01  7:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: yetundex.adebisi, 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.

The patch is based on Maarten Lankhorst's code and created by Chi Ding.

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


cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
cc: matthew.d.roper@intel.com
cc: yetundex.adebisi@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      | 134 ++++++++++++++++++++++++++++++-----
 3 files changed, 118 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e775d08..66b8629 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4599,8 +4599,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);
 
@@ -4646,7 +4644,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
@@ -6173,7 +6170,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);
@@ -14486,8 +14483,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 6d2e628..65c07b1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -450,6 +450,7 @@ struct intel_crtc_wm_state {
 		} ilk;
 
 		struct {
+			struct vlv_wm_state intermediate;
 			struct vlv_wm_state optimal;
 		} vlv;
 
@@ -683,8 +684,6 @@ struct intel_crtc {
 			struct skl_pipe_wm skl;
 		} 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 89eb139..54a88c8 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,10 @@ 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;
 
@@ -995,9 +996,13 @@ static void vlv_compute_fifo(struct intel_crtc_state *cstate,
 	int rate[I915_MAX_PLANES] = {};
 	int i;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	for_each_intel_plane_mask(dev, plane, cstate->base.plane_mask) {
 		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+			intel_atomic_get_existing_plane_state(
+			cstate->base.state, plane);
+
+		if (!state)
+			state = to_intel_plane_state(plane->base.state);
 
 		if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
 			continue;
@@ -1089,7 +1094,7 @@ 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;
@@ -1100,7 +1105,7 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 
 	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,16 +1122,27 @@ static int vlv_compute_wm(struct intel_crtc_state *cstate)
 		}
 	}
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	for_each_intel_plane_mask(dev, plane, cstate->base.plane_mask) {
 		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+			intel_atomic_get_existing_plane_state(cstate->base.state,
+						plane);
+
+	/*
+	* Because we hold the crtc mutex planes that are part of
+	* plane_mask cannot be removed from this crtc while we hold
+	* the lock. This will allow us to inspect plane->state
+	* when the plane's not part of the state without the state
+	* being	updated.
+	*/
+	if (!state)
+		state = to_intel_plane_state(plane->base.state);
 
 		if (!state->visible)
 			continue;
 
 		/* 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) {
@@ -1196,6 +1212,63 @@ 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_state;
+	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;
+
+	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)
 
@@ -1334,8 +1407,6 @@ 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);
-	intel_crtc->wm_state = intel_crtc->config->wm.vlv.optimal;
 	vlv_merge_wm(dev, &wm);
 
 	if (memcmp(&dev_priv->wm.vlv, &wm, sizeof(wm)) == 0) {
@@ -1381,6 +1452,29 @@ static void vlv_update_wm(struct drm_crtc *crtc)
 
 }
 
+
+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_state = 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_state = 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)
@@ -7409,12 +7503,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

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

* ✗ Ro.CI.BAT: warning for series starting with [RFC,1/5] Remove unused parameters from intel_plane_wm_parameters
  2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
                   ` (4 preceding siblings ...)
  2016-06-01  7:10 ` [RFC 5/5] Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark for VLV/CHV chix.ding
@ 2016-06-01  7:44 ` Patchwork
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-06-01  7:44 UTC (permalink / raw)
  To: chix.ding; +Cc: intel-gfx

== Series Details ==

Series: series starting with [RFC,1/5] Remove unused parameters from intel_plane_wm_parameters
URL   : https://patchwork.freedesktop.org/series/8078/
State : warning

== Summary ==

Series 8078v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8078/revisions/1/mbox

Test core_auth:
        Subgroup basic-auth:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (fi-byt-n2820)
        Subgroup basic-uc-rw-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-wb-set-default:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (ro-byt-n2820)
                pass       -> DMESG-WARN (fi-byt-n2820)
Test gem_mmap_gtt:
        Subgroup basic-copy:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-write:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_pread:
        Subgroup basic:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_storedw_loop:
        Subgroup basic-render:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-vebox:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_addfb_basic:
        Subgroup addfb25-framebuffer-vs-set-tiling:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup addfb25-modifier-no-flag:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup addfb25-y-tiled-small:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup bad-pitch-1024:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup bad-pitch-32:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup bad-pitch-65536:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup size-max:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup unused-pitches:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_flip:
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (ro-byt-n2820)
                pass       -> DMESG-WARN (fi-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-byt-n2820)
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (ro-byt-n2820)
                pass       -> DMESG-WARN (fi-byt-n2820)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (ro-byt-n2820)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-byt-n2820)

fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
fi-bsw-n3050     total:209  pass:164  dwarn:3   dfail:0   fail:2   skip:40 
fi-byt-n2820     total:209  pass:164  dwarn:5   dfail:0   fail:2   skip:38 
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:209  pass:198  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-byt-n2820     total:209  pass:164  dwarn:5   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0   skip:19 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-skl-i7-6700hq total:204  pass:170  dwarn:13  dfail:0   fail:0   skip:21 
ro-bdw-i7-5557U failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot
ro-ilk-i7-620lm failed to connect after reboot
ro-ivb2-i7-3770 failed to connect after reboot
ro-snb-i7-2620M failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1072/

d8eea9f drm-intel-nightly: 2016y-05m-31d-23h-16m-57s UTC integration manifest
861b026 Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark for VLV/CHV
6c9588c Add optimal field in intel_crtc_wm_state for VLV
0d4b26a Move fifo_size from intel_plane_wm_parameters to vlv_wm_state
f01250f Rename skl_plane_id to wm_plane_id
6a0321a Remove unused parameters from intel_plane_wm_parameters

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

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

* Re: [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters
  2016-06-01  7:10 ` [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters chix.ding
@ 2016-06-03 22:58   ` Matt Roper
  0 siblings, 0 replies; 10+ messages in thread
From: Matt Roper @ 2016-06-03 22:58 UTC (permalink / raw)
  To: chix.ding; +Cc: yetundex.adebisi, intel-gfx

General comment (that applies to the whole series); when you write your
commit message, the first line (which becomes the email subject above)
should be prefixed by "drm/i915:"  If your patch is only modifying the
code for a specific platform, you can include that too when appropriate
(e.g., "drm/i915/skl:").

On Wed, Jun 01, 2016 at 08:10:17AM +0100, chix.ding@intel.com wrote:
> From: Chi Ding <chix.ding@intel.com>
> 
> Everything except fifo_size is unused and therefore removed
> 
> This is the first patch of two-level watermark for VLV/CHV

I think you can leave this line out of your description.  Killing off
dead code / unused fields is worthwhile on its own and doesn't really
need two-level watermarks as justification.

With an updated commit message,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> 
> 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.
> 
> Signed-off-by: Chi Ding <chix.ding@intel.com>
> 
> cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> cc: matthew.d.roper@intel.com
> cc: yetundex.adebisi@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 9b5f663..b973b86 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -697,21 +697,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
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC 2/5] Rename skl_plane_id to wm_plane_id
  2016-06-01  7:10 ` [RFC 2/5] Rename skl_plane_id to wm_plane_id chix.ding
@ 2016-06-03 22:58   ` Matt Roper
  0 siblings, 0 replies; 10+ messages in thread
From: Matt Roper @ 2016-06-03 22:58 UTC (permalink / raw)
  To: chix.ding; +Cc: yetundex.adebisi, intel-gfx

On Wed, Jun 01, 2016 at 08:10:18AM +0100, chix.ding@intel.com wrote:
> From: Chi Ding <chix.ding@intel.com>
> 
> This function will be used not only by SKL but also VLV/CHV.
> Therefore it's renamed.
> 
> Signed-off-by: Chi Ding <chix.ding@intel.com>

As with the first patch, you should update the patch headline.  Other
than that,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> 
> cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> cc: matthew.d.roper@intel.com
> cc: yetundex.adebisi@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 b6dfd02..a3942df 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
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC 3/5] Move fifo_size from intel_plane_wm_parameters to vlv_wm_state
  2016-06-01  7:10 ` [RFC 3/5] Move fifo_size from intel_plane_wm_parameters to vlv_wm_state chix.ding
@ 2016-06-03 23:03   ` Matt Roper
  0 siblings, 0 replies; 10+ messages in thread
From: Matt Roper @ 2016-06-03 23:03 UTC (permalink / raw)
  To: chix.ding; +Cc: yetundex.adebisi, intel-gfx

On Wed, Jun 01, 2016 at 08:10:19AM +0100, chix.ding@intel.com wrote:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> This patch doesn't change the code to use two-level watermark yet,

Just as a general note, people may look back on this commit in the
future (once it's been accepted and merged upstream) and won't have the
context of the rest of the series in front of them, so notes like this
will just be confusing (and they're probably confusing even to people
looking at the whole series now who aren't familiar with the motivation
and challenges of two-stage watermarks for atomic).

In general it's good to write some high-level overview in the cover
letter email to give the background and overall goal of the series.
Then it's a little bit easier to write individual commit messages that
just focus on the specific changes that patch is making (sometimes they
might make general comments like "in the future we'll want to do <foo>
so make <bar> changes now in preparation").

> With this patch the watermarks are saved for each plane and the wm
> state, instead of previously only for the plane

It would be good to explain a little bit more the motivation for the
switch from a plane-based structure to a CRTC-based structure.

> 
> The patch is based on Maarten Lankhorst's work and created by Chi Ding

This can be seen from the "From:" line above, along with the
Signed-off-by lines, so I don't think you need to include it.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Chi Ding <chix.ding@intel.com>
> 
> cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> cc: matthew.d.roper@intel.com
> cc: yetundex.adebisi@intel.com
> ---
>  drivers/gpu/drm/i915/intel_drv.h |  12 +---
>  drivers/gpu/drm/i915/intel_pm.c  | 117 +++++++++++++++++++++------------------
>  2 files changed, 65 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b973b86..31118e1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -624,6 +624,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;
> @@ -696,10 +697,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;
> @@ -708,13 +705,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 a3942df..5515328 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -55,7 +55,7 @@
>  #define INTEL_RC6pp_ENABLE			(1<<2)
>  
>  /*
> - * Return the index of a plane in the DDB and wm result arrays.  Primary
> + * Return the index of a plane in the wm result arrays.  Primary

This change seems unrelated to this patch.  Also, the code the comment
applies to is still used on SKL/BXT where DDB is important.


>   * 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.
> @@ -983,14 +983,17 @@ 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 vlv_wm_state *wm_state)
>  {

There's some logic changes in this function (and some of the subsequent
functions ) that seem unrelated to the migration of fifo_size into
vlv_wm_state.  I'd break out those logic changes into a separate patch
that has its own commit message describing how/why the code is changing.

E.g., changing some of these functions to operate on state rather than
the base CRTC objects is an important step in transitioning to atomic,
but the changes aren't what this patch was advertising in the commit
message, so they kind of slip under the radar.


Matt


> +	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;
>  	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,58 +1004,55 @@ 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;
>  	}
>  
>  	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++) {
> @@ -1064,19 +1064,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,8 +1089,9 @@ 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_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;
> @@ -1099,7 +1105,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
>  
>  	wm_state->num_active_planes = 0;
>  
> -	vlv_compute_fifo(crtc);
> +	vlv_compute_fifo(cstate, wm_state);
>  
>  	if (wm_state->num_active_planes != 1)
>  		wm_state->cxsr = false;
> @@ -1123,11 +1129,16 @@ static void 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",
> +				      to_intel_crtc(cstate->base.crtc)->pipe,
> +				      drm_plane_index(&plane->base), wm, max_wm);
> +				return -EINVAL;
> +			}
>  
> -			if (wm > plane->wm.fifo_size)
> +			if (wm > wm_state->fifo_size[wm_plane_id(plane)])
>  				break;
>  
>  			switch (plane->base.type) {
> @@ -1180,7 +1191,9 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
>  		memset(&wm_state->sr[level], 0, sizeof(wm_state->sr[level]));
>  	}
>  
> -	vlv_invert_wms(crtc);
> +	vlv_invert_wms(crtc, wm_state);
> +
> +	return 0;
>  }
>  
>  #define VLV_FIFO(plane, value) \
> @@ -1190,24 +1203,18 @@ 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(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];
>  
> -	WARN_ON(fifo_size != 512 - 1);
> +	WARN(fifo_size != 512 - 1, "Pipe %c FIFO split %d / %d / %d\n",
> +		      pipe_name(crtc->pipe), sprite0_start,
> +		      sprite1_start, fifo_size);
>  
>  	DRM_DEBUG_KMS("Pipe %c FIFO split %d / %d / %d\n",
>  		      pipe_name(crtc->pipe), sprite0_start,
> @@ -1327,7 +1334,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) {
> @@ -4216,6 +4223,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 +4231,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
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-06-03 23:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01  7:10 Atomic and two-level watermark support for VLV and CHV chix.ding
2016-06-01  7:10 ` [RFC 1/5] Remove unused parameters from intel_plane_wm_parameters chix.ding
2016-06-03 22:58   ` Matt Roper
2016-06-01  7:10 ` [RFC 2/5] Rename skl_plane_id to wm_plane_id chix.ding
2016-06-03 22:58   ` Matt Roper
2016-06-01  7:10 ` [RFC 3/5] Move fifo_size from intel_plane_wm_parameters to vlv_wm_state chix.ding
2016-06-03 23:03   ` Matt Roper
2016-06-01  7:10 ` [RFC 4/5] Add optimal field in intel_crtc_wm_state for VLV chix.ding
2016-06-01  7:10 ` [RFC 5/5] Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark for VLV/CHV chix.ding
2016-06-01  7:44 ` ✗ Ro.CI.BAT: warning for series starting with [RFC,1/5] Remove unused parameters from intel_plane_wm_parameters Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.