[PATCH v3 00/18] drm/i915: VLV/CHV two-stage watermarks (v3)

[PATCH v3 00/18] drm/i915: VLV/CHV two-stage watermarks (v3)

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Everything is r-b'd already, but I wanted to do a few mostly
cosmetic changes so that things will look more similar to my
upcoming g4x two stage watermarks work.

The changes are:
 * rename the 'noninverted' watermarks to 'raw' since
   g4x doesn't have any inverted watermarks but I still
   want to have the same structure for storing these things
 * rename vlv_plane_wm_set() to vlv_raw_plane_wm_set() to
   make it clear that it operates on the "raw" watermarks
 * I also spotted one memset() missing a '&'. I fixed it up
   even though the entire memset() disappears in a later
   patch. It might avoid some problems if someone lands on
   it during bisection

So nothing should have changed functionally, but let's
see if CI agrees before I go and push this...

Ville Syrjälä (18):
  drm/i915: Track visible planes in a bitmask
  drm/i915: Track plane fifo sizes under intel_crtc
  drm/i915: Move vlv wms from crtc->wm_state to crtc->wm.active.vlv
  drm/i915: Plop vlv wm state into crtc_state
  drm/i915: Plop vlv/chv fifo sizes into crtc state
  drm/i915: Compute VLV/CHV FIFO sizes based on the PM2 watermarks
  drm/i915: Compute vlv/chv wms the atomic way
  drm/i915: Skip useless watermark/FIFO related work on VLV/CHV when not
    needed
  drm/i915: Compute proper intermediate wms for vlv/cvh
  drm/i915: Nuke crtc->wm.cxsr_allowed
  drm/i915: Only use update_wm_{pre,post} for pre-ilk platforms
  drm/i915: Sanitize VLV/CHV watermarks properly
  drm/i915: Workaround VLV/CHV sprite1->sprite0 enable underrun
  drm/i915: Kill level 0 wm hack for VLV/CHV
  drm/i915: Add plane update/disable tracepoints
  drm/i915: Add VLV/CHV watermark/FIFO programming tracepoints
  drm/i915: Add cxsr toggle tracepoint
  drm/i915: Add FIFO underrun tracepoints

 drivers/gpu/drm/i915/i915_drv.h            |   8 +
 drivers/gpu/drm/i915/i915_irq.c            |   3 +
 drivers/gpu/drm/i915/i915_trace.h          | 200 +++++++++
 drivers/gpu/drm/i915/intel_atomic.c        |   1 +
 drivers/gpu/drm/i915/intel_atomic_plane.c  |  17 +-
 drivers/gpu/drm/i915/intel_display.c       | 185 ++++----
 drivers/gpu/drm/i915/intel_drv.h           |  73 ++--
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  11 +-
 drivers/gpu/drm/i915/intel_pm.c            | 670 ++++++++++++++++++++---------
 9 files changed, 844 insertions(+), 324 deletions(-)

-- 
2.10.2

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

[PATCH 01/18] drm/i915: Track visible planes in a bitmask

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In a lot of place we wish to know which planes on the crtc are actually
visible, or how many of them there are. Let's start tracking that in a
bitmask in the crtc state.

We already track enabled planes (ie. ones with an fb and crtc specified by
the user) but that's not quite the same thing as enabled planes may
still end up being invisible due to clipping and whatnot.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  6 +++
 drivers/gpu/drm/i915/intel_display.c      | 74 +++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h          |  3 ++
 3 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 41fd94e62d3c..1eaf840cf9ff 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -189,6 +189,12 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
+	/* FIXME pre-g4x don't work like this */
+	if (intel_state->base.visible)
+		crtc_state->active_planes |= BIT(intel_plane->id);
+	else
+		crtc_state->active_planes &= ~BIT(intel_plane->id);
+
 	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92cf8862648b..6f4c024aaacc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2671,6 +2671,29 @@ update_state_fb(struct drm_plane *plane)
 }
 
 static void
+intel_set_plane_visible(struct intel_crtc_state *crtc_state,
+			struct intel_plane_state *plane_state,
+			bool visible)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+
+	plane_state->base.visible = visible;
+
+	/* FIXME pre-g4x don't work like this */
+	if (visible) {
+		crtc_state->base.plane_mask |= BIT(drm_plane_index(&plane->base));
+		crtc_state->active_planes |= BIT(plane->id);
+	} else {
+		crtc_state->base.plane_mask &= ~BIT(drm_plane_index(&plane->base));
+		crtc_state->active_planes &= ~BIT(plane->id);
+	}
+
+	DRM_DEBUG_KMS("%s active planes 0x%x\n",
+		      crtc_state->base.crtc->name,
+		      crtc_state->active_planes);
+}
+
+static void
 intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 			     struct intel_initial_plane_config *plane_config)
 {
@@ -2727,8 +2750,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	 * simplest solution is to just disable the primary plane now and
 	 * pretend the BIOS never had it enabled.
 	 */
-	plane_state->visible = false;
-	crtc_state->plane_mask &= ~(1 << drm_plane_index(primary));
+	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
+				to_intel_plane_state(plane_state),
+				false);
 	intel_pre_disable_primary_noatomic(&intel_crtc->base);
 	intel_plane->disable_plane(primary, &intel_crtc->base);
 
@@ -2768,7 +2792,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	drm_framebuffer_reference(fb);
 	primary->fb = primary->state->fb = fb;
 	primary->crtc = primary->state->crtc = &intel_crtc->base;
-	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
+
+	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
+				to_intel_plane_state(plane_state),
+				true);
+
 	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
 		  &obj->frontbuffer_bits);
 }
@@ -10759,11 +10787,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
 	struct drm_crtc *crtc = crtc_state->crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_plane *plane = plane_state->plane;
+	struct intel_plane *plane = to_intel_plane(plane_state->plane);
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane_state *old_plane_state =
-		to_intel_plane_state(plane->state);
+		to_intel_plane_state(plane->base.state);
 	bool mode_changed = needs_modeset(crtc_state);
 	bool was_crtc_enabled = crtc->state->active;
 	bool is_crtc_enabled = crtc_state->active;
@@ -10771,7 +10799,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	struct drm_framebuffer *fb = plane_state->fb;
 	int ret;
 
-	if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) {
+	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
 		ret = skl_update_scaler_plane(
 			to_intel_crtc_state(crtc_state),
 			to_intel_plane_state(plane_state));
@@ -10795,8 +10823,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	 * per-plane wm computation to the .check_plane() hook, and
 	 * only combine the results from all planes in the current place?
 	 */
-	if (!is_crtc_enabled)
+	if (!is_crtc_enabled) {
 		plane_state->visible = visible = false;
+		to_intel_crtc_state(crtc_state)->active_planes &= ~BIT(plane->id);
+	}
 
 	if (!was_visible && !visible)
 		return 0;
@@ -10808,13 +10838,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	turn_on = visible && (!was_visible || mode_changed);
 
 	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
-			 intel_crtc->base.base.id,
-			 intel_crtc->base.name,
-			 plane->base.id, plane->name,
+			 intel_crtc->base.base.id, intel_crtc->base.name,
+			 plane->base.base.id, plane->base.name,
 			 fb ? fb->base.id : -1);
 
 	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
-			 plane->base.id, plane->name,
+			 plane->base.base.id, plane->base.name,
 			 was_visible, visible,
 			 turn_off, turn_on, mode_changed);
 
@@ -10822,15 +10851,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		pipe_config->update_wm_pre = true;
 
 		/* must disable cxsr around plane enable/disable */
-		if (plane->type != DRM_PLANE_TYPE_CURSOR)
+		if (plane->id != PLANE_CURSOR)
 			pipe_config->disable_cxsr = true;
 	} else if (turn_off) {
 		pipe_config->update_wm_post = true;
 
 		/* must disable cxsr around plane enable/disable */
-		if (plane->type != DRM_PLANE_TYPE_CURSOR)
+		if (plane->id != PLANE_CURSOR)
 			pipe_config->disable_cxsr = true;
-	} else if (intel_wm_need_update(plane, plane_state)) {
+	} else if (intel_wm_need_update(&plane->base, plane_state)) {
 		/* FIXME bollocks */
 		pipe_config->update_wm_pre = true;
 		pipe_config->update_wm_post = true;
@@ -10842,7 +10871,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
 
 	if (visible || was_visible)
-		pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
+		pipe_config->fb_bits |= plane->frontbuffer_bit;
 
 	/*
 	 * WaCxSRDisabledForSpriteScaling:ivb
@@ -10850,7 +10879,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	 * cstate->update_wm was already set above, so this flag will
 	 * take effect when we commit and program watermarks.
 	 */
-	if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) &&
+	if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
 	    needs_scaling(to_intel_plane_state(plane_state)) &&
 	    !needs_scaling(old_plane_state))
 		pipe_config->disable_lp_wm = true;
@@ -15264,15 +15293,14 @@ static bool primary_get_hw_state(struct intel_plane *plane)
 /* FIXME read out full plane state for all planes */
 static void readout_plane_state(struct intel_crtc *crtc)
 {
-	struct drm_plane *primary = crtc->base.primary;
-	struct intel_plane_state *plane_state =
-		to_intel_plane_state(primary->state);
+	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
+	bool visible;
 
-	plane_state->base.visible = crtc->active &&
-		primary_get_hw_state(to_intel_plane(primary));
+	visible = crtc->active && primary_get_hw_state(primary);
 
-	if (plane_state->base.visible)
-		crtc->base.state->plane_mask |= 1 << drm_plane_index(primary);
+	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
+				to_intel_plane_state(primary->base.state),
+				visible);
 }
 
 static void intel_modeset_readout_hw_state(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e6fbbeecfde3..34ddda1f0697 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -695,6 +695,9 @@ struct intel_crtc_state {
 
 	/* Gamma mode programmed on the pipe */
 	uint32_t gamma_mode;
+
+	/* bitmask of visible planes (enum plane_id) */
+	u8 active_planes;
 };
 
 struct vlv_wm_state {
-- 
2.10.2

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

[PATCH v2 02/18] drm/i915: Track plane fifo sizes under intel_crtc

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Track the plane fifo sizes under intel_crtc instead of under each
intel_plane. Avoids looping over the planes in a bunch of places,
and later we'll move this tracking into the crtc state properly.

v2: Nuke intel_plane_wm_parameters (Maarten)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  32 ++---------
 drivers/gpu/drm/i915/intel_pm.c  | 115 ++++++++++++++++-----------------------
 2 files changed, 54 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 34ddda1f0697..1d9a28816046 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -709,6 +709,10 @@ struct vlv_wm_state {
 	bool cxsr;
 };
 
+struct vlv_fifo_state {
+	uint16_t plane[I915_MAX_PLANES];
+};
+
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
@@ -758,6 +762,8 @@ struct intel_crtc {
 
 		/* allow CxSR on this pipe */
 		bool cxsr_allowed;
+
+		struct vlv_fifo_state fifo_state;
 	} wm;
 
 	int scanline_offset;
@@ -775,25 +781,6 @@ struct intel_crtc {
 	struct vlv_wm_state wm_state;
 };
 
-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;
-};
-
 struct intel_plane {
 	struct drm_plane base;
 	u8 plane;
@@ -803,13 +790,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 b00c95e4a81f..df595f0a93ed 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -415,15 +415,14 @@ static const int pessimal_latency_ns = 5000;
 #define VLV_FIFO_START(dsparb, dsparb2, lo_shift, hi_shift) \
 	((((dsparb) >> (lo_shift)) & 0xff) | ((((dsparb2) >> (hi_shift)) & 0x1) << 8))
 
-static int vlv_get_fifo_size(struct intel_plane *plane)
+static void vlv_get_fifo_size(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-	int sprite0_start, sprite1_start, size;
-
-	if (plane->id == PLANE_CURSOR)
-		return 63;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	enum pipe pipe = crtc->pipe;
+	int sprite0_start, sprite1_start;
 
-	switch (plane->pipe) {
+	switch (pipe) {
 		uint32_t dsparb, dsparb2, dsparb3;
 	case PIPE_A:
 		dsparb = I915_READ(DSPARB);
@@ -444,26 +443,21 @@ static int vlv_get_fifo_size(struct intel_plane *plane)
 		sprite1_start = VLV_FIFO_START(dsparb3, dsparb2, 8, 20);
 		break;
 	default:
-		return 0;
-	}
-
-	switch (plane->id) {
-	case PLANE_PRIMARY:
-		size = sprite0_start;
-		break;
-	case PLANE_SPRITE0:
-		size = sprite1_start - sprite0_start;
-		break;
-	case PLANE_SPRITE1:
-		size = 512 - 1 - sprite1_start;
-		break;
-	default:
-		return 0;
+		MISSING_CASE(pipe);
+		return;
 	}
 
-	DRM_DEBUG_KMS("%s FIFO size: %d\n", plane->base.name, size);
+	fifo_state->plane[PLANE_PRIMARY] = sprite0_start;
+	fifo_state->plane[PLANE_SPRITE0] = sprite1_start - sprite0_start;
+	fifo_state->plane[PLANE_SPRITE1] = 511 - sprite1_start;
+	fifo_state->plane[PLANE_CURSOR] = 63;
 
-	return size;
+	DRM_DEBUG_KMS("Pipe %c FIFO size: %d/%d/%d/%d\n",
+		      pipe_name(pipe),
+		      fifo_state->plane[PLANE_PRIMARY],
+		      fifo_state->plane[PLANE_SPRITE0],
+		      fifo_state->plane[PLANE_SPRITE1],
+		      fifo_state->plane[PLANE_CURSOR]);
 }
 
 static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv, int plane)
@@ -1041,8 +1035,9 @@ static uint16_t vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
 
 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 vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
 	unsigned int total_rate = 0;
 	const int fifo_size = 512 - 1;
@@ -1052,7 +1047,7 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
 		struct intel_plane_state *state =
 			to_intel_plane_state(plane->base.state);
 
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
+		if (plane->id == PLANE_CURSOR)
 			continue;
 
 		if (state->base.visible) {
@@ -1066,19 +1061,19 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
 			to_intel_plane_state(plane->base.state);
 		unsigned int rate;
 
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
-			plane->wm.fifo_size = 63;
+		if (plane->id == PLANE_CURSOR) {
+			fifo_state->plane[plane->id] = 63;
 			continue;
 		}
 
 		if (!state->base.visible) {
-			plane->wm.fifo_size = 0;
+			fifo_state->plane[plane->id] = 0;
 			continue;
 		}
 
 		rate = state->base.fb->format->cpp[0];
-		plane->wm.fifo_size = fifo_size * rate / total_rate;
-		fifo_left -= plane->wm.fifo_size;
+		fifo_state->plane[plane->id] = fifo_size * rate / total_rate;
+		fifo_left -= fifo_state->plane[plane->id];
 	}
 
 	fifo_extra = DIV_ROUND_UP(fifo_left, wm_state->num_active_planes ?: 1);
@@ -1090,16 +1085,16 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
 		if (fifo_left == 0)
 			break;
 
-		if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
+		if (plane->id == PLANE_CURSOR)
 			continue;
 
 		/* give it all to the first plane if none are active */
-		if (plane->wm.fifo_size == 0 &&
+		if (fifo_state->plane[plane->id] == 0 &&
 		    wm_state->num_active_planes)
 			continue;
 
 		plane_extra = min(fifo_extra, fifo_left);
-		plane->wm.fifo_size += plane_extra;
+		fifo_state->plane[plane->id] += plane_extra;
 		fifo_left -= plane_extra;
 	}
 
@@ -1121,9 +1116,10 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 
 	for (level = 0; level < wm_state->num_levels; level++) {
 		struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+		const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 		const int sr_fifo_size =
 			INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
-		struct intel_plane *plane;
+		enum plane_id plane_id;
 
 		wm_state->sr[level].plane =
 			vlv_invert_wm_value(wm_state->sr[level].plane,
@@ -1132,10 +1128,10 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 			vlv_invert_wm_value(wm_state->sr[level].cursor,
 					    63);
 
-		for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
-			wm_state->wm[level].plane[plane->id] =
-				vlv_invert_wm_value(wm_state->wm[level].plane[plane->id],
-						    plane->wm.fifo_size);
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			wm_state->wm[level].plane[plane_id] =
+				vlv_invert_wm_value(wm_state->wm[level].plane[plane_id],
+						    fifo_state->plane[plane_id]);
 		}
 	}
 }
@@ -1144,6 +1140,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 	struct intel_plane *plane;
 	int level;
 
@@ -1170,7 +1167,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 		/* normal watermarks */
 		for (level = 0; level < wm_state->num_levels; level++) {
 			int wm = vlv_compute_wm_level(crtc->config, state, level);
-			int max_wm = plane->wm.fifo_size;
+			int max_wm = fifo_state->plane[plane->id];
 
 			/* hack */
 			if (WARN_ON(level == 0 && wm > max_wm))
@@ -1214,32 +1211,16 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 
 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;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	int sprite0_start, sprite1_start, fifo_size;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		switch (plane->id) {
-		case PLANE_PRIMARY:
-			sprite0_start = plane->wm.fifo_size;
-			break;
-		case PLANE_SPRITE0:
-			sprite1_start = sprite0_start + plane->wm.fifo_size;
-			break;
-		case PLANE_SPRITE1:
-			fifo_size = sprite1_start + plane->wm.fifo_size;
-			break;
-		case PLANE_CURSOR:
-			WARN_ON(plane->wm.fifo_size != 63);
-			break;
-		default:
-			MISSING_CASE(plane->id);
-			break;
-		}
-	}
+	sprite0_start = fifo_state->plane[PLANE_PRIMARY];
+	sprite1_start = fifo_state->plane[PLANE_SPRITE0] + sprite0_start;
+	fifo_size = fifo_state->plane[PLANE_SPRITE1] + sprite1_start;
 
-	WARN_ON(fifo_size != 512 - 1);
+	WARN_ON(fifo_state->plane[PLANE_CURSOR] != 63);
+	WARN_ON(fifo_size != 511);
 
 	DRM_DEBUG_KMS("Pipe %c FIFO split %d / %d / %d\n",
 		      pipe_name(crtc->pipe), sprite0_start,
@@ -4531,14 +4512,14 @@ 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_plane *plane;
+	struct intel_crtc *crtc;
 	enum pipe pipe;
 	u32 val;
 
 	vlv_read_wm_values(dev_priv, wm);
 
-	for_each_intel_plane(dev, plane)
-		plane->wm.fifo_size = vlv_get_fifo_size(plane);
+	for_each_intel_crtc(dev, crtc)
+		vlv_get_fifo_size(crtc);
 
 	wm->cxsr = I915_READ(FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
 	wm->level = VLV_WM_LEVEL_PM2;
-- 
2.10.2

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

[PATCH 03/18] drm/i915: Move vlv wms from crtc->wm_state to crtc->wm.active.vlv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In an effort to make the vlv/chv wm code look and behave more like the
ilk+ code, let's move the current active wms next to the
corresponding ilk wms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  3 +--
 drivers/gpu/drm/i915/intel_pm.c  | 10 +++++-----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1d9a28816046..0d8ea6461248 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -758,6 +758,7 @@ struct intel_crtc {
 		/* watermarks currently being used  */
 		union {
 			struct intel_pipe_wm ilk;
+			struct vlv_wm_state vlv;
 		} active;
 
 		/* allow CxSR on this pipe */
@@ -777,8 +778,6 @@ struct intel_crtc {
 
 	/* scalers available on this crtc */
 	int num_scalers;
-
-	struct vlv_wm_state wm_state;
 };
 
 struct intel_plane {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index df595f0a93ed..54e4d35808e9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1035,7 +1035,7 @@ static uint16_t vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
 
 static void vlv_compute_fifo(struct intel_crtc *crtc)
 {
-	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 	struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
@@ -1111,7 +1111,7 @@ static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
 
 static void vlv_invert_wms(struct intel_crtc *crtc)
 {
-	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 	int level;
 
 	for (level = 0; level < wm_state->num_levels; level++) {
@@ -1139,7 +1139,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 static void vlv_compute_wm(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct vlv_wm_state *wm_state = &crtc->wm_state;
+	struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 	const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 	struct intel_plane *plane;
 	int level;
@@ -1302,7 +1302,7 @@ static void vlv_merge_wm(struct drm_i915_private *dev_priv,
 	wm->cxsr = true;
 
 	for_each_intel_crtc(&dev_priv->drm, 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;
@@ -1321,7 +1321,7 @@ static void vlv_merge_wm(struct drm_i915_private *dev_priv,
 		wm->level = VLV_WM_LEVEL_PM2;
 
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
-		struct vlv_wm_state *wm_state = &crtc->wm_state;
+		const struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 		enum pipe pipe = crtc->pipe;
 
 		if (!crtc->active)
-- 
2.10.2

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

[PATCH 04/18] drm/i915: Plop vlv wm state into crtc_state

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Relocate the vlv/chv wm state to live under intel_crtc_state. Note
that for now this just behaves as a temporary storage. But it'll be
easier to conver the thing over to properly pre-computing the state
when it's already in the right place.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 30 +++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_pm.c  | 32 ++++++++++++++++----------------
 2 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0d8ea6461248..24f71812f72b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -492,6 +492,22 @@ struct skl_pipe_wm {
 	uint32_t linetime;
 };
 
+enum vlv_wm_level {
+	VLV_WM_LEVEL_PM2,
+	VLV_WM_LEVEL_PM5,
+	VLV_WM_LEVEL_DDR_DVFS,
+	NUM_VLV_WM_LEVELS,
+};
+
+struct vlv_wm_state {
+	struct vlv_pipe_wm wm[NUM_VLV_WM_LEVELS];
+	struct vlv_sr_wm sr[NUM_VLV_WM_LEVELS];
+	uint8_t num_active_planes;
+	uint8_t num_levels;
+	uint8_t level;
+	bool cxsr;
+};
+
 struct intel_crtc_wm_state {
 	union {
 		struct {
@@ -516,6 +532,11 @@ struct intel_crtc_wm_state {
 			struct skl_pipe_wm optimal;
 			struct skl_ddb_entry ddb;
 		} skl;
+
+		struct {
+			/* optimal watermarks (inverted) */
+			struct vlv_wm_state optimal;
+		} vlv;
 	};
 
 	/*
@@ -700,15 +721,6 @@ struct intel_crtc_state {
 	u8 active_planes;
 };
 
-struct vlv_wm_state {
-	struct vlv_pipe_wm wm[3];
-	struct vlv_sr_wm sr[3];
-	uint8_t num_active_planes;
-	uint8_t num_levels;
-	uint8_t level;
-	bool cxsr;
-};
-
 struct vlv_fifo_state {
 	uint16_t plane[I915_MAX_PLANES];
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 54e4d35808e9..1b5261840ac3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -957,12 +957,6 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 
 #undef FW_WM_VLV
 
-enum vlv_wm_level {
-	VLV_WM_LEVEL_PM2,
-	VLV_WM_LEVEL_PM5,
-	VLV_WM_LEVEL_DDR_DVFS,
-};
-
 /* latency must be in 0.1us units. */
 static unsigned int vlv_wm_method2(unsigned int pixel_rate,
 				   unsigned int pipe_htotal,
@@ -1033,9 +1027,10 @@ static uint16_t vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
 	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 *crtc_state)
 {
-	struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
 	struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
@@ -1109,9 +1104,10 @@ static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
 		return fifo_size - wm;
 }
 
-static void vlv_invert_wms(struct intel_crtc *crtc)
+static void vlv_invert_wms(struct intel_crtc_state *crtc_state)
 {
-	struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
 	int level;
 
 	for (level = 0; level < wm_state->num_levels; level++) {
@@ -1136,10 +1132,11 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
 	}
 }
 
-static void vlv_compute_wm(struct intel_crtc *crtc)
+static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
+	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
 	const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 	struct intel_plane *plane;
 	int level;
@@ -1151,7 +1148,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 
 	wm_state->num_active_planes = 0;
 
-	vlv_compute_fifo(crtc);
+	vlv_compute_fifo(crtc_state);
 
 	if (wm_state->num_active_planes != 1)
 		wm_state->cxsr = false;
@@ -1166,7 +1163,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
 
 		/* normal watermarks */
 		for (level = 0; level < wm_state->num_levels; level++) {
-			int wm = vlv_compute_wm_level(crtc->config, state, level);
+			int wm = vlv_compute_wm_level(crtc_state, state, level);
 			int max_wm = fifo_state->plane[plane->id];
 
 			/* hack */
@@ -1203,7 +1200,7 @@ 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_state);
 }
 
 #define VLV_FIFO(plane, value) \
@@ -1351,11 +1348,14 @@ static bool is_enabling(int old, int new, int threshold)
 static void vlv_update_wm(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_crtc_state *crtc_state =
+		to_intel_crtc_state(crtc->base.state);
 	enum pipe pipe = crtc->pipe;
 	struct vlv_wm_values *old_wm = &dev_priv->wm.vlv;
 	struct vlv_wm_values new_wm = {};
 
-	vlv_compute_wm(crtc);
+	vlv_compute_wm(crtc_state);
+	crtc->wm.active.vlv = crtc_state->wm.vlv.optimal;
 	vlv_merge_wm(dev_priv, &new_wm);
 
 	if (memcmp(old_wm, &new_wm, sizeof(new_wm)) == 0) {
-- 
2.10.2

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

[PATCH 05/18] drm/i915: Plop vlv/chv fifo sizes into crtc state

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move the vlv/chv FIFO size tracking into the crtc_state. As with the wms
for now this just acts as temporary storage.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 12 ++++++------
 drivers/gpu/drm/i915/intel_pm.c  | 26 +++++++++++++++-----------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 24f71812f72b..920f8557072a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -508,6 +508,10 @@ struct vlv_wm_state {
 	bool cxsr;
 };
 
+struct vlv_fifo_state {
+	u16 plane[I915_MAX_PLANES];
+};
+
 struct intel_crtc_wm_state {
 	union {
 		struct {
@@ -536,6 +540,8 @@ struct intel_crtc_wm_state {
 		struct {
 			/* optimal watermarks (inverted) */
 			struct vlv_wm_state optimal;
+			/* display FIFO split */
+			struct vlv_fifo_state fifo_state;
 		} vlv;
 	};
 
@@ -721,10 +727,6 @@ struct intel_crtc_state {
 	u8 active_planes;
 };
 
-struct vlv_fifo_state {
-	uint16_t plane[I915_MAX_PLANES];
-};
-
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
@@ -775,8 +777,6 @@ struct intel_crtc {
 
 		/* allow CxSR on this pipe */
 		bool cxsr_allowed;
-
-		struct vlv_fifo_state fifo_state;
 	} wm;
 
 	int scanline_offset;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1b5261840ac3..ea509b222336 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -415,10 +415,11 @@ static const int pessimal_latency_ns = 5000;
 #define VLV_FIFO_START(dsparb, dsparb2, lo_shift, hi_shift) \
 	((((dsparb) >> (lo_shift)) & 0xff) | ((((dsparb2) >> (hi_shift)) & 0x1) << 8))
 
-static void vlv_get_fifo_size(struct intel_crtc *crtc)
+static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	struct vlv_fifo_state *fifo_state = &crtc_state->wm.vlv.fifo_state;
 	enum pipe pipe = crtc->pipe;
 	int sprite0_start, sprite1_start;
 
@@ -1031,7 +1032,7 @@ static void vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
-	struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	struct vlv_fifo_state *fifo_state = &crtc_state->wm.vlv.fifo_state;
 	struct drm_device *dev = crtc->base.dev;
 	struct intel_plane *plane;
 	unsigned int total_rate = 0;
@@ -1108,11 +1109,12 @@ static void vlv_invert_wms(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
+	const struct vlv_fifo_state *fifo_state =
+		&crtc_state->wm.vlv.fifo_state;
 	int level;
 
 	for (level = 0; level < wm_state->num_levels; level++) {
 		struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-		const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
 		const int sr_fifo_size =
 			INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
 		enum plane_id plane_id;
@@ -1137,7 +1139,8 @@ static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
-	const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	const struct vlv_fifo_state *fifo_state =
+		&crtc_state->wm.vlv.fifo_state;
 	struct intel_plane *plane;
 	int level;
 
@@ -1206,10 +1209,12 @@ static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
 #define VLV_FIFO(plane, value) \
 	(((value) << DSPARB_ ## plane ## _SHIFT_VLV) & DSPARB_ ## plane ## _MASK_VLV)
 
-static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc)
+static void vlv_pipe_set_fifo_size(const struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	const struct vlv_fifo_state *fifo_state = &crtc->wm.fifo_state;
+	const struct vlv_fifo_state *fifo_state =
+		&crtc_state->wm.vlv.fifo_state;
 	int sprite0_start, sprite1_start, fifo_size;
 
 	sprite0_start = fifo_state->plane[PLANE_PRIMARY];
@@ -1360,8 +1365,7 @@ static void vlv_update_wm(struct intel_crtc *crtc)
 
 	if (memcmp(old_wm, &new_wm, sizeof(new_wm)) == 0) {
 		/* FIXME should be part of crtc atomic commit */
-		vlv_pipe_set_fifo_size(crtc);
-
+		vlv_pipe_set_fifo_size(crtc_state);
 		return;
 	}
 
@@ -1375,7 +1379,7 @@ static void vlv_update_wm(struct intel_crtc *crtc)
 		_intel_set_memory_cxsr(dev_priv, false);
 
 	/* FIXME should be part of crtc atomic commit */
-	vlv_pipe_set_fifo_size(crtc);
+	vlv_pipe_set_fifo_size(crtc_state);
 
 	vlv_write_wm_values(dev_priv, &new_wm);
 
@@ -4519,7 +4523,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 	vlv_read_wm_values(dev_priv, wm);
 
 	for_each_intel_crtc(dev, crtc)
-		vlv_get_fifo_size(crtc);
+		vlv_get_fifo_size(to_intel_crtc_state(crtc->base.state));
 
 	wm->cxsr = I915_READ(FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
 	wm->level = VLV_WM_LEVEL_PM2;
-- 
2.10.2

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

[PATCH v2 06/18] drm/i915: Compute VLV/CHV FIFO sizes based on the PM2 watermarks

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's compute the watermarks first and the FIFO size second. This way we
can make sure the FIFO split is the most accommodating to the watermarks.
Previously we could have potentially computed a FIFO split that couldn't
accommodate the PM2 watermarks simply due to a bad split even if the
total FIFO size would have been sufficient.

It'll also allow us to avoid recomputing the wms for all planes whenever
the FIFO split would change. Thus we don't have to add any extra planes
to the state when the FIFO needs to be repartitioned.

To help with this we'll keep around copies of the non-inverted
watermarks in the crtc state. For now that doesn't help too much, but
once we start to do the watermark computation only for the planes
that change we'll need the non-inverted values around for the other
planes.

v2: s/noninverted/raw/ for consistency with other platforms
    Fix the memset() of the "raw" watermarks

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 drivers/gpu/drm/i915/intel_pm.c  | 111 ++++++++++++++++++---------------------
 2 files changed, 53 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 920f8557072a..28ebb2e5801e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -538,6 +538,8 @@ struct intel_crtc_wm_state {
 		} skl;
 
 		struct {
+			/* "raw" watermarks (not inverted) */
+			struct vlv_pipe_wm raw[NUM_VLV_WM_LEVELS];
 			/* optimal watermarks (inverted) */
 			struct vlv_wm_state optimal;
 			/* display FIFO split */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ea509b222336..c4c7cf8f384e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1028,73 +1028,70 @@ static uint16_t vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
 	return min_t(int, wm, USHRT_MAX);
 }
 
-static void vlv_compute_fifo(struct intel_crtc_state *crtc_state)
+static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
+	const struct vlv_pipe_wm *raw =
+		&crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2];
 	struct vlv_fifo_state *fifo_state = &crtc_state->wm.vlv.fifo_state;
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_plane *plane;
-	unsigned int total_rate = 0;
-	const int fifo_size = 512 - 1;
+	unsigned int active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR);
+	int num_active_planes = hweight32(active_planes);
+	const int fifo_size = 511;
 	int fifo_extra, fifo_left = fifo_size;
+	unsigned int total_rate;
+	enum plane_id plane_id;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+	total_rate = raw->plane[PLANE_PRIMARY] +
+		raw->plane[PLANE_SPRITE0] +
+		raw->plane[PLANE_SPRITE1];
 
-		if (plane->id == PLANE_CURSOR)
-			continue;
+	if (total_rate > fifo_size)
+		return -EINVAL;
 
-		if (state->base.visible) {
-			wm_state->num_active_planes++;
-			total_rate += state->base.fb->format->cpp[0];
-		}
-	}
+	if (total_rate == 0)
+		total_rate = 1;
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+	for_each_plane_id_on_crtc(crtc, plane_id) {
 		unsigned int rate;
 
-		if (plane->id == PLANE_CURSOR) {
-			fifo_state->plane[plane->id] = 63;
-			continue;
-		}
-
-		if (!state->base.visible) {
-			fifo_state->plane[plane->id] = 0;
+		if ((active_planes & BIT(plane_id)) == 0) {
+			fifo_state->plane[plane_id] = 0;
 			continue;
 		}
 
-		rate = state->base.fb->format->cpp[0];
-		fifo_state->plane[plane->id] = fifo_size * rate / total_rate;
-		fifo_left -= fifo_state->plane[plane->id];
+		rate = raw->plane[plane_id];
+		fifo_state->plane[plane_id] = fifo_size * rate / total_rate;
+		fifo_left -= fifo_state->plane[plane_id];
 	}
 
-	fifo_extra = DIV_ROUND_UP(fifo_left, wm_state->num_active_planes ?: 1);
+	fifo_state->plane[PLANE_CURSOR] = 63;
+
+	fifo_extra = DIV_ROUND_UP(fifo_left, num_active_planes ?: 1);
 
 	/* spread the remainder evenly */
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+	for_each_plane_id_on_crtc(crtc, plane_id) {
 		int plane_extra;
 
 		if (fifo_left == 0)
 			break;
 
-		if (plane->id == PLANE_CURSOR)
-			continue;
-
-		/* give it all to the first plane if none are active */
-		if (fifo_state->plane[plane->id] == 0 &&
-		    wm_state->num_active_planes)
+		if ((active_planes & BIT(plane_id)) == 0)
 			continue;
 
 		plane_extra = min(fifo_extra, fifo_left);
-		fifo_state->plane[plane->id] += plane_extra;
+		fifo_state->plane[plane_id] += plane_extra;
 		fifo_left -= plane_extra;
 	}
 
-	WARN_ON(fifo_left != 0);
+	WARN_ON(active_planes != 0 && fifo_left != 0);
+
+	/* give it all to the first plane if none are active */
+	if (active_planes == 0) {
+		WARN_ON(fifo_left != fifo_size);
+		fifo_state->plane[PLANE_PRIMARY] = fifo_left;
+	}
+
+	return 0;
 }
 
 static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
@@ -1139,35 +1136,31 @@ static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
-	const struct vlv_fifo_state *fifo_state =
-		&crtc_state->wm.vlv.fifo_state;
 	struct intel_plane *plane;
 	int level;
 
 	memset(wm_state, 0, sizeof(*wm_state));
+	memset(&crtc_state->wm.vlv.raw, 0, sizeof(crtc_state->wm.vlv.raw));
 
 	wm_state->cxsr = crtc->pipe != PIPE_C && crtc->wm.cxsr_allowed;
 	wm_state->num_levels = dev_priv->wm.max_level + 1;
 
 	wm_state->num_active_planes = 0;
 
-	vlv_compute_fifo(crtc_state);
-
 	if (wm_state->num_active_planes != 1)
 		wm_state->cxsr = false;
 
 	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
 		struct intel_plane_state *state =
 			to_intel_plane_state(plane->base.state);
-		int level;
 
 		if (!state->base.visible)
 			continue;
 
-		/* normal watermarks */
 		for (level = 0; level < wm_state->num_levels; level++) {
+			struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
 			int wm = vlv_compute_wm_level(crtc_state, state, level);
-			int max_wm = fifo_state->plane[plane->id];
+			int max_wm = plane->id == PLANE_CURSOR ? 63 : 511;
 
 			/* hack */
 			if (WARN_ON(level == 0 && wm > max_wm))
@@ -1176,25 +1169,23 @@ static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
 			if (wm > max_wm)
 				break;
 
-			wm_state->wm[level].plane[plane->id] = wm;
+			raw->plane[plane->id] = wm;
 		}
 
 		wm_state->num_levels = level;
+	}
 
-		if (!wm_state->cxsr)
-			continue;
+	vlv_compute_fifo(crtc_state);
 
-		/* maxfifo watermarks */
-		if (plane->id == PLANE_CURSOR) {
-			for (level = 0; level < wm_state->num_levels; level++)
-				wm_state->sr[level].cursor =
-					wm_state->wm[level].plane[PLANE_CURSOR];
-		} else {
-			for (level = 0; level < wm_state->num_levels; level++)
-				wm_state->sr[level].plane =
-					max(wm_state->sr[level].plane,
-					    wm_state->wm[level].plane[plane->id]);
-		}
+	for (level = 0; level < wm_state->num_levels; level++) {
+		struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
+
+		wm_state->wm[level] = *raw;
+
+		wm_state->sr[level].plane = max3(raw->plane[PLANE_PRIMARY],
+						 raw->plane[PLANE_SPRITE0],
+						 raw->plane[PLANE_SPRITE1]);
+		wm_state->sr[level].cursor = raw->plane[PLANE_CURSOR];
 	}
 
 	/* clear any (partially) filled invalid levels */
-- 
2.10.2

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

[PATCH v2 07/18] drm/i915: Compute vlv/chv wms the atomic way

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Start computing the vlv/chv watermarks the atomic way, from the
.compute_pipe_wm() hook. We'll recompute the actual watermarks
for only planes that are part of the state, the other planes will
keep their watermark from the last time it was computed.

And the actual watermark programming will happen from the
.initial_watermarks() hook. For now we'll just compute the
optimal watermarks, and we'll hook up the intermediate
watermarks properly later.

The DSPARB registers responsible for the FIFO paritioning are
double buffered, so they will be programming from
intel_begin_crtc_commit().

v2: s/noninverted/raw/ for consistency with other platforms
    s/vlv_plane_wm_set/vlv_raw_plane_wm_set/ for clarity

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   8 +
 drivers/gpu/drm/i915/intel_display.c |  21 ++-
 drivers/gpu/drm/i915/intel_drv.h     |   2 -
 drivers/gpu/drm/i915/intel_pm.c      | 302 +++++++++++++++++++++++------------
 4 files changed, 228 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 26f36b004d2f..0e8fd78d33e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -526,6 +526,14 @@ struct i915_hotplug {
 	for_each_power_well_rev(__dev_priv, __power_well)		        \
 		for_each_if ((__power_well)->domains & (__domain_mask))
 
+#define for_each_intel_plane_in_state(__state, plane, plane_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
+		     ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \
+		      (plane_state) = to_intel_plane_state((__state)->base.planes[__i].state), 1); \
+	     (__i)++) \
+		for_each_if (plane_state)
+
 struct drm_i915_private;
 struct i915_mm_struct;
 struct i915_mmu_object;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6f4c024aaacc..7ca7d3e5d18e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5684,6 +5684,8 @@ static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
 static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 				   struct drm_atomic_state *old_state)
 {
+	struct intel_atomic_state *old_intel_state =
+		to_intel_atomic_state(old_state);
 	struct drm_crtc *crtc = pipe_config->base.crtc;
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -5728,7 +5730,8 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_color_load_luts(&pipe_config->base);
 
-	intel_update_watermarks(intel_crtc);
+	dev_priv->display.initial_watermarks(old_intel_state,
+					     pipe_config);
 	intel_enable_pipe(intel_crtc);
 
 	assert_vblank_disabled(crtc);
@@ -5845,6 +5848,9 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	if (!IS_GEN2(dev_priv))
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
+	if (!dev_priv->display.initial_watermarks)
+		intel_update_watermarks(intel_crtc);
 }
 
 static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
@@ -11268,10 +11274,13 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 static void
 clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 {
+	struct drm_i915_private *dev_priv =
+		to_i915(crtc_state->base.crtc->dev);
 	struct drm_crtc_state tmp_state;
 	struct intel_crtc_scaler_state scaler_state;
 	struct intel_dpll_hw_state dpll_hw_state;
 	struct intel_shared_dpll *shared_dpll;
+	struct intel_crtc_wm_state wm_state;
 	bool force_thru;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
@@ -11284,6 +11293,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	shared_dpll = crtc_state->shared_dpll;
 	dpll_hw_state = crtc_state->dpll_hw_state;
 	force_thru = crtc_state->pch_pfit.force_thru;
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		wm_state = crtc_state->wm;
 
 	memset(crtc_state, 0, sizeof *crtc_state);
 
@@ -11292,6 +11303,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	crtc_state->shared_dpll = shared_dpll;
 	crtc_state->dpll_hw_state = dpll_hw_state;
 	crtc_state->pch_pfit.force_thru = force_thru;
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		crtc_state->wm = wm_state;
 }
 
 static int
@@ -12804,12 +12817,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 				/*
 				 * Make sure we don't call initial_watermarks
 				 * for ILK-style watermark updates.
+				 *
+				 * No clue what this is supposed to achieve.
 				 */
-				if (dev_priv->display.atomic_update_watermarks)
+				if (INTEL_GEN(dev_priv) >= 9)
 					dev_priv->display.initial_watermarks(intel_state,
 									     to_intel_crtc_state(crtc->state));
-				else
-					intel_update_watermarks(intel_crtc);
 			}
 		}
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 28ebb2e5801e..351a14c98205 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -502,9 +502,7 @@ enum vlv_wm_level {
 struct vlv_wm_state {
 	struct vlv_pipe_wm wm[NUM_VLV_WM_LEVELS];
 	struct vlv_sr_wm sr[NUM_VLV_WM_LEVELS];
-	uint8_t num_active_planes;
 	uint8_t num_levels;
-	uint8_t level;
 	bool cxsr;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c4c7cf8f384e..f7cc013b3a78 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1094,6 +1094,28 @@ static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+static int vlv_num_wm_levels(struct drm_i915_private *dev_priv)
+{
+	return dev_priv->wm.max_level + 1;
+}
+
+/* mark all levels starting from 'level' as invalid */
+static void vlv_invalidate_wms(struct intel_crtc *crtc,
+			       struct vlv_wm_state *wm_state, int level)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	for (; level < vlv_num_wm_levels(dev_priv); level++) {
+		enum plane_id plane_id;
+
+		for_each_plane_id_on_crtc(crtc, plane_id)
+			wm_state->wm[level].plane[plane_id] = USHRT_MAX;
+
+		wm_state->sr[level].cursor = USHRT_MAX;
+		wm_state->sr[level].plane = USHRT_MAX;
+	}
+}
+
 static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
 {
 	if (wm > fifo_size)
@@ -1102,105 +1124,162 @@ static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
 		return fifo_size - wm;
 }
 
-static void vlv_invert_wms(struct intel_crtc_state *crtc_state)
+/*
+ * Starting from 'level' set all higher
+ * levels to 'value' in the "raw" watermarks.
+ */
+static void vlv_raw_plane_wm_set(struct intel_crtc_state *crtc_state,
+				 int level, enum plane_id plane_id, u16 value)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
-	const struct vlv_fifo_state *fifo_state =
-		&crtc_state->wm.vlv.fifo_state;
-	int level;
-
-	for (level = 0; level < wm_state->num_levels; level++) {
-		struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-		const int sr_fifo_size =
-			INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
-		enum plane_id plane_id;
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	int num_levels = vlv_num_wm_levels(dev_priv);
 
-		wm_state->sr[level].plane =
-			vlv_invert_wm_value(wm_state->sr[level].plane,
-					    sr_fifo_size);
-		wm_state->sr[level].cursor =
-			vlv_invert_wm_value(wm_state->sr[level].cursor,
-					    63);
+	for (; level < num_levels; level++) {
+		struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
 
-		for_each_plane_id_on_crtc(crtc, plane_id) {
-			wm_state->wm[level].plane[plane_id] =
-				vlv_invert_wm_value(wm_state->wm[level].plane[plane_id],
-						    fifo_state->plane[plane_id]);
-		}
+		raw->plane[plane_id] = value;
 	}
 }
 
-static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
+static void vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
+				 const struct intel_plane_state *plane_state)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
-	struct intel_plane *plane;
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	enum plane_id plane_id = plane->id;
+	int num_levels = vlv_num_wm_levels(to_i915(plane->base.dev));
 	int level;
 
-	memset(wm_state, 0, sizeof(*wm_state));
-	memset(&crtc_state->wm.vlv.raw, 0, sizeof(crtc_state->wm.vlv.raw));
+	if (!plane_state->base.visible) {
+		vlv_raw_plane_wm_set(crtc_state, 0, plane_id, 0);
+		return;
+	}
 
-	wm_state->cxsr = crtc->pipe != PIPE_C && crtc->wm.cxsr_allowed;
-	wm_state->num_levels = dev_priv->wm.max_level + 1;
+	for (level = 0; level < num_levels; level++) {
+		struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
+		int wm = vlv_compute_wm_level(crtc_state, plane_state, level);
+		int max_wm = plane_id == PLANE_CURSOR ? 63 : 511;
 
-	wm_state->num_active_planes = 0;
+		/* FIXME just bail */
+		if (WARN_ON(level == 0 && wm > max_wm))
+			wm = max_wm;
 
-	if (wm_state->num_active_planes != 1)
-		wm_state->cxsr = false;
+		if (wm > max_wm)
+			break;
 
-	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
-		struct intel_plane_state *state =
-			to_intel_plane_state(plane->base.state);
+		raw->plane[plane_id] = wm;
+	}
 
-		if (!state->base.visible)
-			continue;
+	/* mark all higher levels as invalid */
+	vlv_raw_plane_wm_set(crtc_state, level, plane_id, USHRT_MAX);
 
-		for (level = 0; level < wm_state->num_levels; level++) {
-			struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
-			int wm = vlv_compute_wm_level(crtc_state, state, level);
-			int max_wm = plane->id == PLANE_CURSOR ? 63 : 511;
+	DRM_DEBUG_KMS("%s wms: [0]=%d,[1]=%d,[2]=%d\n",
+		      plane->base.name,
+		      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2].plane[plane_id],
+		      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM5].plane[plane_id],
+		      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_DDR_DVFS].plane[plane_id]);
+}
 
-			/* hack */
-			if (WARN_ON(level == 0 && wm > max_wm))
-				wm = max_wm;
+static bool vlv_plane_wm_is_valid(const struct intel_crtc_state *crtc_state,
+				  enum plane_id plane_id, int level)
+{
+	const struct vlv_pipe_wm *raw =
+		&crtc_state->wm.vlv.raw[level];
+	const struct vlv_fifo_state *fifo_state =
+		&crtc_state->wm.vlv.fifo_state;
 
-			if (wm > max_wm)
-				break;
+	return raw->plane[plane_id] <= fifo_state->plane[plane_id];
+}
 
-			raw->plane[plane->id] = wm;
-		}
+static bool vlv_crtc_wm_is_valid(const struct intel_crtc_state *crtc_state, int level)
+{
+	return vlv_plane_wm_is_valid(crtc_state, PLANE_PRIMARY, level) &&
+		vlv_plane_wm_is_valid(crtc_state, PLANE_SPRITE0, level) &&
+		vlv_plane_wm_is_valid(crtc_state, PLANE_SPRITE1, level) &&
+		vlv_plane_wm_is_valid(crtc_state, PLANE_CURSOR, level);
+}
+
+static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(crtc_state->base.state);
+	struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
+	const struct vlv_fifo_state *fifo_state =
+		&crtc_state->wm.vlv.fifo_state;
+	int num_active_planes = hweight32(crtc_state->active_planes &
+					  ~BIT(PLANE_CURSOR));
+	struct intel_plane_state *plane_state;
+	struct intel_plane *plane;
+	enum plane_id plane_id;
+	int level, ret, i;
+
+	for_each_intel_plane_in_state(state, plane, plane_state, i) {
+		const struct intel_plane_state *old_plane_state =
+			to_intel_plane_state(plane->base.state);
+
+		if (plane_state->base.crtc != &crtc->base &&
+		    old_plane_state->base.crtc != &crtc->base)
+			continue;
 
-		wm_state->num_levels = level;
+		vlv_plane_wm_compute(crtc_state, plane_state);
 	}
 
-	vlv_compute_fifo(crtc_state);
+	/* initially allow all levels */
+	wm_state->num_levels = vlv_num_wm_levels(dev_priv);
+	/*
+	 * Note that enabling cxsr with no primary/sprite planes
+	 * enabled can wedge the pipe. Hence we only allow cxsr
+	 * with exactly one enabled primary/sprite plane.
+	 */
+	wm_state->cxsr = crtc->pipe != PIPE_C &&
+		crtc->wm.cxsr_allowed && num_active_planes == 1;
+
+	ret = vlv_compute_fifo(crtc_state);
+	if (ret)
+		return ret;
 
 	for (level = 0; level < wm_state->num_levels; level++) {
-		struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
+		const struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
+		const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
 
-		wm_state->wm[level] = *raw;
+		if (!vlv_crtc_wm_is_valid(crtc_state, level))
+			break;
 
-		wm_state->sr[level].plane = max3(raw->plane[PLANE_PRIMARY],
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			wm_state->wm[level].plane[plane_id] =
+				vlv_invert_wm_value(raw->plane[plane_id],
+						    fifo_state->plane[plane_id]);
+		}
+
+		wm_state->sr[level].plane =
+			vlv_invert_wm_value(max3(raw->plane[PLANE_PRIMARY],
 						 raw->plane[PLANE_SPRITE0],
-						 raw->plane[PLANE_SPRITE1]);
-		wm_state->sr[level].cursor = raw->plane[PLANE_CURSOR];
-	}
+						 raw->plane[PLANE_SPRITE1]),
+					    sr_fifo_size);
 
-	/* clear any (partially) filled invalid levels */
-	for (level = wm_state->num_levels; level < dev_priv->wm.max_level + 1; level++) {
-		memset(&wm_state->wm[level], 0, sizeof(wm_state->wm[level]));
-		memset(&wm_state->sr[level], 0, sizeof(wm_state->sr[level]));
+		wm_state->sr[level].cursor =
+			vlv_invert_wm_value(raw->plane[PLANE_CURSOR],
+					    63);
 	}
 
-	vlv_invert_wms(crtc_state);
+	if (level == 0)
+		return -EINVAL;
+
+	/* limit to only levels we can actually handle */
+	wm_state->num_levels = level;
+
+	/* invalidate the higher levels */
+	vlv_invalidate_wms(crtc, wm_state, level);
+
+	return 0;
 }
 
 #define VLV_FIFO(plane, value) \
 	(((value) << DSPARB_ ## plane ## _SHIFT_VLV) & DSPARB_ ## plane ## _MASK_VLV)
 
-static void vlv_pipe_set_fifo_size(const struct intel_crtc_state *crtc_state)
+static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
+				   struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1215,10 +1294,6 @@ static void vlv_pipe_set_fifo_size(const struct intel_crtc_state *crtc_state)
 	WARN_ON(fifo_state->plane[PLANE_CURSOR] != 63);
 	WARN_ON(fifo_size != 511);
 
-	DRM_DEBUG_KMS("Pipe %c FIFO split %d / %d / %d\n",
-		      pipe_name(crtc->pipe), sprite0_start,
-		      sprite1_start, fifo_size);
-
 	spin_lock(&dev_priv->wm.dsparb_lock);
 
 	switch (crtc->pipe) {
@@ -1317,11 +1392,8 @@ static void vlv_merge_wm(struct drm_i915_private *dev_priv,
 		const struct vlv_wm_state *wm_state = &crtc->wm.active.vlv;
 		enum pipe pipe = crtc->pipe;
 
-		if (!crtc->active)
-			continue;
-
 		wm->pipe[pipe] = wm_state->wm[wm->level];
-		if (wm->cxsr)
+		if (crtc->active && wm->cxsr)
 			wm->sr = wm_state->sr[wm->level];
 
 		wm->ddl[pipe].plane[PLANE_PRIMARY] = DDL_PRECISION_HIGH | 2;
@@ -1341,24 +1413,15 @@ static bool is_enabling(int old, int new, int threshold)
 	return old < threshold && new >= threshold;
 }
 
-static void vlv_update_wm(struct intel_crtc *crtc)
+static void vlv_program_watermarks(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_crtc_state *crtc_state =
-		to_intel_crtc_state(crtc->base.state);
-	enum pipe pipe = crtc->pipe;
 	struct vlv_wm_values *old_wm = &dev_priv->wm.vlv;
 	struct vlv_wm_values new_wm = {};
 
-	vlv_compute_wm(crtc_state);
-	crtc->wm.active.vlv = crtc_state->wm.vlv.optimal;
 	vlv_merge_wm(dev_priv, &new_wm);
 
-	if (memcmp(old_wm, &new_wm, sizeof(new_wm)) == 0) {
-		/* FIXME should be part of crtc atomic commit */
-		vlv_pipe_set_fifo_size(crtc_state);
+	if (memcmp(old_wm, &new_wm, sizeof(new_wm)) == 0)
 		return;
-	}
 
 	if (is_disabling(old_wm->level, new_wm.level, VLV_WM_LEVEL_DDR_DVFS))
 		chv_set_memory_dvfs(dev_priv, false);
@@ -1369,17 +1432,8 @@ static void vlv_update_wm(struct intel_crtc *crtc)
 	if (is_disabling(old_wm->cxsr, new_wm.cxsr, true))
 		_intel_set_memory_cxsr(dev_priv, false);
 
-	/* FIXME should be part of crtc atomic commit */
-	vlv_pipe_set_fifo_size(crtc_state);
-
 	vlv_write_wm_values(dev_priv, &new_wm);
 
-	DRM_DEBUG_KMS("Setting FIFO watermarks - %c: plane=%d, cursor=%d, "
-		      "sprite0=%d, sprite1=%d, SR: plane=%d, cursor=%d level=%d cxsr=%d\n",
-		      pipe_name(pipe), new_wm.pipe[pipe].plane[PLANE_PRIMARY], new_wm.pipe[pipe].plane[PLANE_CURSOR],
-		      new_wm.pipe[pipe].plane[PLANE_SPRITE0], new_wm.pipe[pipe].plane[PLANE_SPRITE1],
-		      new_wm.sr.plane, new_wm.sr.cursor, new_wm.level, new_wm.cxsr);
-
 	if (is_enabling(old_wm->cxsr, new_wm.cxsr, true))
 		_intel_set_memory_cxsr(dev_priv, true);
 
@@ -1392,6 +1446,18 @@ static void vlv_update_wm(struct intel_crtc *crtc)
 	*old_wm = new_wm;
 }
 
+static void vlv_initial_watermarks(struct intel_atomic_state *state,
+				   struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+
+	mutex_lock(&dev_priv->wm.wm_mutex);
+	crtc->wm.active.vlv = crtc_state->wm.vlv.optimal;
+	vlv_program_watermarks(dev_priv);
+	mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
 #define single_plane_enabled(mask) is_power_of_2(mask)
 
 static void g4x_update_wm(struct intel_crtc *crtc)
@@ -4508,14 +4574,10 @@ 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;
-	enum pipe pipe;
 	u32 val;
 
 	vlv_read_wm_values(dev_priv, wm);
 
-	for_each_intel_crtc(dev, crtc)
-		vlv_get_fifo_size(to_intel_crtc_state(crtc->base.state));
-
 	wm->cxsr = I915_READ(FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
 	wm->level = VLV_WM_LEVEL_PM2;
 
@@ -4553,13 +4615,53 @@ 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) {
+		struct intel_crtc_state *crtc_state =
+			to_intel_crtc_state(crtc->base.state);
+		struct vlv_wm_state *active = &crtc->wm.active.vlv;
+		const struct vlv_fifo_state *fifo_state =
+			&crtc_state->wm.vlv.fifo_state;
+		enum pipe pipe = crtc->pipe;
+		enum plane_id plane_id;
+		int level;
+
+		vlv_get_fifo_size(crtc_state);
+
+		active->num_levels = wm->level + 1;
+		active->cxsr = wm->cxsr;
+
+		/* FIXME sanitize things more */
+		for (level = 0; level < active->num_levels; level++) {
+			struct vlv_pipe_wm *raw =
+				&crtc_state->wm.vlv.raw[level];
+
+			active->sr[level].plane = wm->sr.plane;
+			active->sr[level].cursor = wm->sr.cursor;
+
+			for_each_plane_id_on_crtc(crtc, plane_id) {
+				active->wm[level].plane[plane_id] =
+					wm->pipe[pipe].plane[plane_id];
+
+				raw->plane[plane_id] =
+					vlv_invert_wm_value(active->wm[level].plane[plane_id],
+							    fifo_state->plane[plane_id]);
+			}
+		}
+
+		for_each_plane_id_on_crtc(crtc, plane_id)
+			vlv_raw_plane_wm_set(crtc_state, level,
+					     plane_id, USHRT_MAX);
+		vlv_invalidate_wms(crtc, active, level);
+
+		crtc_state->wm.vlv.optimal = *active;
+
 		DRM_DEBUG_KMS("Initial watermarks: pipe %c, plane=%d, cursor=%d, sprite0=%d, sprite1=%d\n",
 			      pipe_name(pipe),
 			      wm->pipe[pipe].plane[PLANE_PRIMARY],
 			      wm->pipe[pipe].plane[PLANE_CURSOR],
 			      wm->pipe[pipe].plane[PLANE_SPRITE0],
 			      wm->pipe[pipe].plane[PLANE_SPRITE1]);
+	}
 
 	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);
@@ -7717,7 +7819,9 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
 		}
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		vlv_setup_wm_latency(dev_priv);
-		dev_priv->display.update_wm = vlv_update_wm;
+		dev_priv->display.compute_pipe_wm = vlv_compute_pipe_wm;
+		dev_priv->display.initial_watermarks = vlv_initial_watermarks;
+		dev_priv->display.atomic_update_watermarks = vlv_atomic_update_fifo;
 	} else if (IS_PINEVIEW(dev_priv)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
 					    dev_priv->is_ddr3,
-- 
2.10.2

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

[PATCH v2 08/18] drm/i915: Skip useless watermark/FIFO related work on VLV/CHV when not needed

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Check whether anything relevant has actually change when we compute new
watermarks for each plane in the state. If the watermarks for no
primary/sprite planes changed we don't have to recompute the FIFO split
or reprogram the DSBARB registers. And even the cursor watermarks didn't
change we can skip the merge+invert step between all the planes on
the pipe as well.

v2: s/noninverted/raw/ for consistency with other platforms

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c |  1 +
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_pm.c     | 73 +++++++++++++++++++++++++++++--------
 3 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index e65818e5d2ab..50fb1f76cc5f 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -99,6 +99,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	crtc_state->update_wm_pre = false;
 	crtc_state->update_wm_post = false;
 	crtc_state->fb_changed = false;
+	crtc_state->fifo_changed = false;
 	crtc_state->wm.need_postvbl_update = false;
 	crtc_state->fb_bits = 0;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 351a14c98205..04943732255e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -573,6 +573,7 @@ struct intel_crtc_state {
 	bool disable_cxsr;
 	bool update_wm_pre, update_wm_post; /* watermarks are updated */
 	bool fb_changed; /* fb on any of the planes is changed */
+	bool fifo_changed; /* FIFO split is changed */
 
 	/* Pipe source size (ie. panel fitter input size)
 	 * All planes will be positioned inside this space,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f7cc013b3a78..2566c88d8c89 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1128,30 +1128,35 @@ static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
  * Starting from 'level' set all higher
  * levels to 'value' in the "raw" watermarks.
  */
-static void vlv_raw_plane_wm_set(struct intel_crtc_state *crtc_state,
+static bool vlv_raw_plane_wm_set(struct intel_crtc_state *crtc_state,
 				 int level, enum plane_id plane_id, u16 value)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	int num_levels = vlv_num_wm_levels(dev_priv);
+	bool dirty = false;
 
 	for (; level < num_levels; level++) {
 		struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
 
+		dirty |= raw->plane[plane_id] != value;
 		raw->plane[plane_id] = value;
 	}
+
+	return dirty;
 }
 
-static void vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
+static bool vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
 				 const struct intel_plane_state *plane_state)
 {
 	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
 	enum plane_id plane_id = plane->id;
 	int num_levels = vlv_num_wm_levels(to_i915(plane->base.dev));
 	int level;
+	bool dirty = false;
 
 	if (!plane_state->base.visible) {
-		vlv_raw_plane_wm_set(crtc_state, 0, plane_id, 0);
-		return;
+		dirty |= vlv_raw_plane_wm_set(crtc_state, 0, plane_id, 0);
+		goto out;
 	}
 
 	for (level = 0; level < num_levels; level++) {
@@ -1166,17 +1171,22 @@ static void vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
 		if (wm > max_wm)
 			break;
 
+		dirty |= raw->plane[plane_id] != wm;
 		raw->plane[plane_id] = wm;
 	}
 
 	/* mark all higher levels as invalid */
-	vlv_raw_plane_wm_set(crtc_state, level, plane_id, USHRT_MAX);
+	dirty |= vlv_raw_plane_wm_set(crtc_state, level, plane_id, USHRT_MAX);
 
-	DRM_DEBUG_KMS("%s wms: [0]=%d,[1]=%d,[2]=%d\n",
-		      plane->base.name,
-		      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2].plane[plane_id],
-		      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM5].plane[plane_id],
-		      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_DDR_DVFS].plane[plane_id]);
+out:
+	if (dirty)
+		DRM_DEBUG_KMS("%s wms: [0]=%d,[1]=%d,[2]=%d\n",
+			      plane->base.name,
+			      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2].plane[plane_id],
+			      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM5].plane[plane_id],
+			      crtc_state->wm.vlv.raw[VLV_WM_LEVEL_DDR_DVFS].plane[plane_id]);
+
+	return dirty;
 }
 
 static bool vlv_plane_wm_is_valid(const struct intel_crtc_state *crtc_state,
@@ -1209,10 +1219,12 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 		&crtc_state->wm.vlv.fifo_state;
 	int num_active_planes = hweight32(crtc_state->active_planes &
 					  ~BIT(PLANE_CURSOR));
+	bool needs_modeset = drm_atomic_crtc_needs_modeset(&crtc_state->base);
 	struct intel_plane_state *plane_state;
 	struct intel_plane *plane;
 	enum plane_id plane_id;
 	int level, ret, i;
+	unsigned int dirty = 0;
 
 	for_each_intel_plane_in_state(state, plane, plane_state, i) {
 		const struct intel_plane_state *old_plane_state =
@@ -1222,7 +1234,37 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 		    old_plane_state->base.crtc != &crtc->base)
 			continue;
 
-		vlv_plane_wm_compute(crtc_state, plane_state);
+		if (vlv_plane_wm_compute(crtc_state, plane_state))
+			dirty |= BIT(plane->id);
+	}
+
+	/*
+	 * DSPARB registers may have been reset due to the
+	 * power well being turned off. Make sure we restore
+	 * them to a consistent state even if no primary/sprite
+	 * planes are initially active.
+	 */
+	if (needs_modeset)
+		crtc_state->fifo_changed = true;
+
+	if (!dirty)
+		return 0;
+
+	/* cursor changes don't warrant a FIFO recompute */
+	if (dirty & ~BIT(PLANE_CURSOR)) {
+		const struct intel_crtc_state *old_crtc_state =
+			to_intel_crtc_state(crtc->base.state);
+		const struct vlv_fifo_state *old_fifo_state =
+			&old_crtc_state->wm.vlv.fifo_state;
+
+		ret = vlv_compute_fifo(crtc_state);
+		if (ret)
+			return ret;
+
+		if (needs_modeset ||
+		    memcmp(old_fifo_state, fifo_state,
+			   sizeof(*fifo_state)) != 0)
+			crtc_state->fifo_changed = true;
 	}
 
 	/* initially allow all levels */
@@ -1235,10 +1277,6 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	wm_state->cxsr = crtc->pipe != PIPE_C &&
 		crtc->wm.cxsr_allowed && num_active_planes == 1;
 
-	ret = vlv_compute_fifo(crtc_state);
-	if (ret)
-		return ret;
-
 	for (level = 0; level < wm_state->num_levels; level++) {
 		const struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
 		const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
@@ -1287,6 +1325,9 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		&crtc_state->wm.vlv.fifo_state;
 	int sprite0_start, sprite1_start, fifo_size;
 
+	if (!crtc_state->fifo_changed)
+		return;
+
 	sprite0_start = fifo_state->plane[PLANE_PRIMARY];
 	sprite1_start = fifo_state->plane[PLANE_SPRITE0] + sprite0_start;
 	fifo_size = fifo_state->plane[PLANE_SPRITE1] + sprite1_start;
@@ -4630,6 +4671,8 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 		active->num_levels = wm->level + 1;
 		active->cxsr = wm->cxsr;
 
+		vlv_get_fifo_size(crtc_state);
+
 		/* FIXME sanitize things more */
 		for (level = 0; level < active->num_levels; level++) {
 			struct vlv_pipe_wm *raw =
-- 
2.10.2

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

[PATCH 09/18] drm/i915: Compute proper intermediate wms for vlv/cvh

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since the watermark registers arent double buffered on VLV/CHV, we'll
need to play around with intermediate watermarks same was as we do on
ILK-BDW.

The watermark registers on VLV/CHV contain inverted values, so to find
the intermediate watermark value we just take the minimum of the
active and optimal values. This also means that, unlike ILK-BDW,
there's no chance that we'd fail to find a working intermediate
watermarks. As long as both the active and optimal watermarks are valid
the intermediate watermarks will come out valid as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c  | 59 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 04943732255e..a72b699f45fe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -538,6 +538,8 @@ struct intel_crtc_wm_state {
 		struct {
 			/* "raw" watermarks (not inverted) */
 			struct vlv_pipe_wm raw[NUM_VLV_WM_LEVELS];
+			/* intermediate watermarks (inverted) */
+			struct vlv_wm_state intermediate;
 			/* optimal watermarks (inverted) */
 			struct vlv_wm_state optimal;
 			/* display FIFO split */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2566c88d8c89..07e5c9b67ca5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1401,6 +1401,45 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 #undef VLV_FIFO
 
+static int vlv_compute_intermediate_wm(struct drm_device *dev,
+				       struct intel_crtc *crtc,
+				       struct intel_crtc_state *crtc_state)
+{
+	struct vlv_wm_state *intermediate = &crtc_state->wm.vlv.intermediate;
+	const struct vlv_wm_state *optimal = &crtc_state->wm.vlv.optimal;
+	const struct vlv_wm_state *active = &crtc->wm.active.vlv;
+	int level;
+
+	intermediate->num_levels = min(optimal->num_levels, active->num_levels);
+	intermediate->cxsr = optimal->cxsr & active->cxsr;
+
+	for (level = 0; level < intermediate->num_levels; level++) {
+		enum plane_id plane_id;
+
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			intermediate->wm[level].plane[plane_id] =
+				min(optimal->wm[level].plane[plane_id],
+				    active->wm[level].plane[plane_id]);
+		}
+
+		intermediate->sr[level].plane = min(optimal->sr[level].plane,
+						    active->sr[level].plane);
+		intermediate->sr[level].cursor = min(optimal->sr[level].cursor,
+						     active->sr[level].cursor);
+	}
+
+	vlv_invalidate_wms(crtc, intermediate, level);
+
+	/*
+	 * 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(intermediate, optimal, sizeof(*intermediate)) == 0)
+		crtc_state->wm.need_postvbl_update = false;
+
+	return 0;
+}
+
 static void vlv_merge_wm(struct drm_i915_private *dev_priv,
 			 struct vlv_wm_values *wm)
 {
@@ -1494,7 +1533,22 @@ static void vlv_initial_watermarks(struct intel_atomic_state *state,
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 
 	mutex_lock(&dev_priv->wm.wm_mutex);
-	crtc->wm.active.vlv = crtc_state->wm.vlv.optimal;
+	crtc->wm.active.vlv = crtc_state->wm.vlv.intermediate;
+	vlv_program_watermarks(dev_priv);
+	mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
+static void vlv_optimize_watermarks(struct intel_atomic_state *state,
+				    struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+
+	if (!crtc_state->wm.need_postvbl_update)
+		return;
+
+	mutex_lock(&dev_priv->wm.wm_mutex);
+	intel_crtc->wm.active.vlv = crtc_state->wm.vlv.optimal;
 	vlv_program_watermarks(dev_priv);
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
@@ -4697,6 +4751,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 		vlv_invalidate_wms(crtc, active, level);
 
 		crtc_state->wm.vlv.optimal = *active;
+		crtc_state->wm.vlv.intermediate = *active;
 
 		DRM_DEBUG_KMS("Initial watermarks: pipe %c, plane=%d, cursor=%d, sprite0=%d, sprite1=%d\n",
 			      pipe_name(pipe),
@@ -7863,7 +7918,9 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		vlv_setup_wm_latency(dev_priv);
 		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;
 		dev_priv->display.atomic_update_watermarks = vlv_atomic_update_fifo;
 	} else if (IS_PINEVIEW(dev_priv)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
-- 
2.10.2

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

[PATCH 10/18] drm/i915: Nuke crtc->wm.cxsr_allowed

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Remove crtc->wm.cxsr_allowed and just rely on crtc_state->disable_cxsr
instead. This was used only by vlv/chv to indicate whether to enable
cxsr in the wm computation. That doesn't really work anymore, and as far
as the optimal watermarks go we'll just consider the number of planes
and the current pipe, and for the intermediate watermarks we'll also
start to consider disable_cxsr which is set appropriately when planes
are being enabled/disabled.

We'll also flip over the crtc_state->wm.need_postvbl_update setup so
that it's the wm code that will set it. Previously the generic code set
it up, and then the wm code cleared it again if it thought it's not
needed after all.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++-------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ---
 drivers/gpu/drm/i915/intel_pm.c      | 14 ++++++------
 3 files changed, 20 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7ca7d3e5d18e..48743950a7c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5000,8 +5000,6 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 
 	intel_frontbuffer_flip(to_i915(crtc->base.dev), pipe_config->fb_bits);
 
-	crtc->wm.cxsr_allowed = true;
-
 	if (pipe_config->update_wm_post && pipe_config->base.active)
 		intel_update_watermarks(crtc);
 
@@ -5048,22 +5046,18 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
 			intel_pre_disable_primary(&crtc->base);
 	}
 
-	if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev_priv)) {
-		crtc->wm.cxsr_allowed = false;
-
-		/*
-		 * Vblank time updates from the shadow to live plane control register
-		 * are blocked if the memory self-refresh mode is active at that
-		 * moment. So to make sure the plane gets truly disabled, disable
-		 * first the self-refresh mode. The self-refresh enable bit in turn
-		 * will be checked/applied by the HW only at the next frame start
-		 * event which is after the vblank start event, so we need to have a
-		 * wait-for-vblank between disabling the plane and the pipe.
-		 */
-		if (old_crtc_state->base.active &&
-		    intel_set_memory_cxsr(dev_priv, false))
-			intel_wait_for_vblank(dev_priv, crtc->pipe);
-	}
+	/*
+	 * Vblank time updates from the shadow to live plane control register
+	 * are blocked if the memory self-refresh mode is active at that
+	 * moment. So to make sure the plane gets truly disabled, disable
+	 * first the self-refresh mode. The self-refresh enable bit in turn
+	 * will be checked/applied by the HW only at the next frame start
+	 * event which is after the vblank start event, so we need to have a
+	 * wait-for-vblank between disabling the plane and the pipe.
+	 */
+	if (HAS_GMCH_DISPLAY(dev_priv) && old_crtc_state->base.active &&
+	    pipe_config->disable_cxsr && intel_set_memory_cxsr(dev_priv, false))
+		intel_wait_for_vblank(dev_priv, crtc->pipe);
 
 	/*
 	 * IVB workaround: must disable low power watermarks for at least
@@ -10871,11 +10865,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		pipe_config->update_wm_post = true;
 	}
 
-	/* Pre-gen9 platforms need two-step watermark updates */
-	if ((pipe_config->update_wm_pre || pipe_config->update_wm_post) &&
-	    INTEL_GEN(dev_priv) < 9 && dev_priv->display.optimize_watermarks)
-		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
-
 	if (visible || was_visible)
 		pipe_config->fb_bits |= plane->frontbuffer_bit;
 
@@ -12619,12 +12608,7 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
 	if (crtc_state->update_wm_post)
 		return true;
 
-	/*
-	 * cxsr is re-enabled after vblank.
-	 * This is already handled by crtc_state->update_wm_post,
-	 * but added for clarity.
-	 */
-	if (crtc_state->disable_cxsr)
+	if (crtc_state->wm.need_postvbl_update)
 		return true;
 
 	return false;
@@ -13893,8 +13877,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	intel_crtc->cursor_cntl = ~0;
 	intel_crtc->cursor_size = ~0;
 
-	intel_crtc->wm.cxsr_allowed = true;
-
 	/* initialize shared scalers */
 	intel_crtc_init_scalers(intel_crtc, crtc_state);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a72b699f45fe..2a809d03e747 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -777,9 +777,6 @@ struct intel_crtc {
 			struct intel_pipe_wm ilk;
 			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 07e5c9b67ca5..7a86d277ca9e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1274,8 +1274,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	 * enabled can wedge the pipe. Hence we only allow cxsr
 	 * with exactly one enabled primary/sprite plane.
 	 */
-	wm_state->cxsr = crtc->pipe != PIPE_C &&
-		crtc->wm.cxsr_allowed && num_active_planes == 1;
+	wm_state->cxsr = crtc->pipe != PIPE_C && num_active_planes == 1;
 
 	for (level = 0; level < wm_state->num_levels; level++) {
 		const struct vlv_pipe_wm *raw = &crtc_state->wm.vlv.raw[level];
@@ -1411,7 +1410,8 @@ static int vlv_compute_intermediate_wm(struct drm_device *dev,
 	int level;
 
 	intermediate->num_levels = min(optimal->num_levels, active->num_levels);
-	intermediate->cxsr = optimal->cxsr & active->cxsr;
+	intermediate->cxsr = optimal->cxsr && active->cxsr &&
+		!crtc_state->disable_cxsr;
 
 	for (level = 0; level < intermediate->num_levels; level++) {
 		enum plane_id plane_id;
@@ -1434,8 +1434,8 @@ static int vlv_compute_intermediate_wm(struct drm_device *dev,
 	 * 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(intermediate, optimal, sizeof(*intermediate)) == 0)
-		crtc_state->wm.need_postvbl_update = false;
+	if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
+		crtc_state->wm.need_postvbl_update = true;
 
 	return 0;
 }
@@ -2628,8 +2628,8 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
 	 * 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;
+	if (memcmp(a, &newstate->wm.ilk.optimal, sizeof(*a)) != 0)
+		newstate->wm.need_postvbl_update = true;
 
 	return 0;
 }
-- 
2.10.2

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

[PATCH 11/18] drm/i915: Only use update_wm_{pre, post} for pre-ilk platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that vlv/chv have more proper wm programming support, let's reduce
the the update_wm_{pre,post} flags to only cover the pre-ilk platforms.
When we finally convert those as well we can drop these flags entirely.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48743950a7c9..e408e251125f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10848,21 +10848,25 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 			 turn_off, turn_on, mode_changed);
 
 	if (turn_on) {
-		pipe_config->update_wm_pre = true;
+		if (INTEL_GEN(dev_priv) < 5)
+			pipe_config->update_wm_pre = true;
 
 		/* must disable cxsr around plane enable/disable */
 		if (plane->id != PLANE_CURSOR)
 			pipe_config->disable_cxsr = true;
 	} else if (turn_off) {
-		pipe_config->update_wm_post = true;
+		if (INTEL_GEN(dev_priv) < 5)
+			pipe_config->update_wm_post = true;
 
 		/* must disable cxsr around plane enable/disable */
 		if (plane->id != PLANE_CURSOR)
 			pipe_config->disable_cxsr = true;
 	} else if (intel_wm_need_update(&plane->base, plane_state)) {
-		/* FIXME bollocks */
-		pipe_config->update_wm_pre = true;
-		pipe_config->update_wm_post = true;
+		if (INTEL_GEN(dev_priv) < 5) {
+			/* FIXME bollocks */
+			pipe_config->update_wm_pre = true;
+			pipe_config->update_wm_post = true;
+		}
 	}
 
 	if (visible || was_visible)
-- 
2.10.2

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

[PATCH v2 12/18] drm/i915: Sanitize VLV/CHV watermarks properly

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clear out the watermark for all disabled planes to 0. This is required
to avoid falsely thinking that the inherited watermarks are bogus in
case the watermark is actually higher than the FIFO size.

v2: s/noninverted/raw/ for consistency with other platforms

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 50 +++++++++++++++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e408e251125f..8ea8290a808d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3479,7 +3479,8 @@ __intel_display_resume(struct drm_device *dev,
 	}
 
 	/* ignore any reset values/BIOS leftovers in the WM registers */
-	to_intel_atomic_state(state)->skip_intermediate_wm = true;
+	if (!HAS_GMCH_DISPLAY(to_i915(dev)))
+		to_intel_atomic_state(state)->skip_intermediate_wm = true;
 
 	ret = drm_atomic_helper_commit_duplicated_state(state, ctx);
 
@@ -14880,7 +14881,8 @@ static void sanitize_watermarks(struct drm_device *dev)
 	 * intermediate watermarks (since we don't trust the current
 	 * watermarks).
 	 */
-	intel_state->skip_intermediate_wm = true;
+	if (!HAS_GMCH_DISPLAY(dev_priv))
+		intel_state->skip_intermediate_wm = true;
 
 	ret = intel_atomic_check(dev, state);
 	if (ret) {
@@ -15044,7 +15046,8 @@ int intel_modeset_init(struct drm_device *dev)
 	 * Note that we need to do this after reconstructing the BIOS fb's
 	 * since the watermark calculation done here will use pstate->fb.
 	 */
-	sanitize_watermarks(dev);
+	if (!HAS_GMCH_DISPLAY(dev_priv))
+		sanitize_watermarks(dev);
 
 	return 0;
 }
@@ -15515,12 +15518,14 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
 		pll->on = false;
 	}
 
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		vlv_wm_get_hw_state(dev);
-	else if (IS_GEN9(dev_priv))
+		vlv_wm_sanitize(dev_priv);
+	} else if (IS_GEN9(dev_priv)) {
 		skl_wm_get_hw_state(dev);
-	else if (HAS_PCH_SPLIT(dev_priv))
+	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		ilk_wm_get_hw_state(dev);
+	}
 
 	for_each_intel_crtc(dev, crtc) {
 		u64 put_domains;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2a809d03e747..98dac197fd76 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1824,6 +1824,7 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */);
 void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
 			      struct skl_pipe_wm *out);
+void vlv_wm_sanitize(struct drm_i915_private *dev_priv);
 bool intel_can_enable_sagv(struct drm_atomic_state *state);
 int intel_enable_sagv(struct drm_i915_private *dev_priv);
 int intel_disable_sagv(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7a86d277ca9e..9c69a5404bcf 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4727,7 +4727,6 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 
 		vlv_get_fifo_size(crtc_state);
 
-		/* FIXME sanitize things more */
 		for (level = 0; level < active->num_levels; level++) {
 			struct vlv_pipe_wm *raw =
 				&crtc_state->wm.vlv.raw[level];
@@ -4765,6 +4764,55 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 		      wm->sr.plane, wm->sr.cursor, wm->level, wm->cxsr);
 }
 
+void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
+{
+	struct intel_plane *plane;
+	struct intel_crtc *crtc;
+
+	mutex_lock(&dev_priv->wm.wm_mutex);
+
+	for_each_intel_plane(&dev_priv->drm, plane) {
+		struct intel_crtc *crtc =
+			intel_get_crtc_for_pipe(dev_priv, plane->pipe);
+		struct intel_crtc_state *crtc_state =
+			to_intel_crtc_state(crtc->base.state);
+		struct intel_plane_state *plane_state =
+			to_intel_plane_state(plane->base.state);
+		struct vlv_wm_state *wm_state = &crtc_state->wm.vlv.optimal;
+		const struct vlv_fifo_state *fifo_state =
+			&crtc_state->wm.vlv.fifo_state;
+		enum plane_id plane_id = plane->id;
+		int level;
+
+		if (plane_state->base.visible)
+			continue;
+
+		for (level = 0; level < wm_state->num_levels; level++) {
+			struct vlv_pipe_wm *raw =
+				&crtc_state->wm.vlv.raw[level];
+
+			raw->plane[plane_id] = 0;
+
+			wm_state->wm[level].plane[plane_id] =
+				vlv_invert_wm_value(raw->plane[plane_id],
+						    fifo_state->plane[plane_id]);
+		}
+	}
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_crtc_state *crtc_state =
+			to_intel_crtc_state(crtc->base.state);
+
+		crtc_state->wm.vlv.intermediate =
+			crtc_state->wm.vlv.optimal;
+		crtc->wm.active.vlv = crtc_state->wm.vlv.optimal;
+	}
+
+	vlv_program_watermarks(dev_priv);
+
+	mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
 void ilk_wm_get_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-- 
2.10.2

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

[PATCH v2 13/18] drm/i915: Workaround VLV/CHV sprite1->sprite0 enable underrun

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On VLV/CHV enabling sprite0 when sprite1 has already been enabled may
lead to an underrun. This only happens when sprite0 FIFO size is zero
prior to enabling it. Hence an effective workaround is to always
allocate at least one cacheline for sprite0 when sprite1 is active.

I've not observed this sort of failure during any other type of plane
enable/disable sequence.

v2: s/noninverted/raw/ for consistency with other platforms

Testcase: igt/kms_plane_blinker
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9c69a5404bcf..967798b0e9ce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1028,6 +1028,12 @@ static uint16_t vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
 	return min_t(int, wm, USHRT_MAX);
 }
 
+static bool vlv_need_sprite0_fifo_workaround(unsigned int active_planes)
+{
+	return (active_planes & (BIT(PLANE_SPRITE0) |
+				 BIT(PLANE_SPRITE1))) == BIT(PLANE_SPRITE1);
+}
+
 static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -1038,12 +1044,25 @@ static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 	int num_active_planes = hweight32(active_planes);
 	const int fifo_size = 511;
 	int fifo_extra, fifo_left = fifo_size;
+	int sprite0_fifo_extra = 0;
 	unsigned int total_rate;
 	enum plane_id plane_id;
 
+	/*
+	 * When enabling sprite0 after sprite1 has already been enabled
+	 * we tend to get an underrun unless sprite0 already has some
+	 * FIFO space allcoated. Hence we always allocate at least one
+	 * cacheline for sprite0 whenever sprite1 is enabled.
+	 *
+	 * All other plane enable sequences appear immune to this problem.
+	 */
+	if (vlv_need_sprite0_fifo_workaround(active_planes))
+		sprite0_fifo_extra = 1;
+
 	total_rate = raw->plane[PLANE_PRIMARY] +
 		raw->plane[PLANE_SPRITE0] +
-		raw->plane[PLANE_SPRITE1];
+		raw->plane[PLANE_SPRITE1] +
+		sprite0_fifo_extra;
 
 	if (total_rate > fifo_size)
 		return -EINVAL;
@@ -1064,6 +1083,9 @@ static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 		fifo_left -= fifo_state->plane[plane_id];
 	}
 
+	fifo_state->plane[PLANE_SPRITE0] += sprite0_fifo_extra;
+	fifo_left -= sprite0_fifo_extra;
+
 	fifo_state->plane[PLANE_CURSOR] = 63;
 
 	fifo_extra = DIV_ROUND_UP(fifo_left, num_active_planes ?: 1);
-- 
2.10.2

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

[PATCH 14/18] drm/i915: Kill level 0 wm hack for VLV/CHV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We now compute the watermarks correctly, so just return an error if we
can't support the configuration.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 967798b0e9ce..aff67de030df 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1186,10 +1186,6 @@ static bool vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
 		int wm = vlv_compute_wm_level(crtc_state, plane_state, level);
 		int max_wm = plane_id == PLANE_CURSOR ? 63 : 511;
 
-		/* FIXME just bail */
-		if (WARN_ON(level == 0 && wm > max_wm))
-			wm = max_wm;
-
 		if (wm > max_wm)
 			break;
 
-- 
2.10.2

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

[PATCH v2 15/18] drm/i915: Add plane update/disable tracepoints

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add tracepoints for plane programming. The tracepoints will dump
the frame and scanline counters, so this can be used to verify eg. that
the plane gets reprogrammed at the right time with respect to watermark
programming (if we have appropriate tracepoints for that as well).

v2: Rebase due to legacy cursor changes

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c           |  3 ++
 drivers/gpu/drm/i915/i915_trace.h         | 56 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_atomic_plane.c | 11 ++++--
 drivers/gpu/drm/i915/intel_display.c      | 15 +++++++--
 4 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ca8c7b22748e..1918673873d6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -783,6 +783,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 	enum pipe pipe = crtc->pipe;
 	int position, vtotal;
 
+	if (!crtc->active)
+		return -1;
+
 	vtotal = mode->crtc_vtotal;
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		vtotal /= 2;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 2dbef3147a60..6cb03bd40aef 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -14,6 +14,62 @@
 #define TRACE_SYSTEM i915
 #define TRACE_INCLUDE_FILE i915_trace
 
+/* plane updates */
+
+TRACE_EVENT(intel_update_plane,
+	    TP_PROTO(struct drm_plane *plane, struct intel_crtc *crtc),
+	    TP_ARGS(plane, crtc),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(const char *, name)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     __array(int, src, 4)
+			     __array(int, dst, 4)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = crtc->pipe;
+			   __entry->name = plane->name;
+			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+										       crtc->pipe);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   memcpy(__entry->src, &plane->state->src, sizeof(__entry->src));
+			   memcpy(__entry->dst, &plane->state->dst, sizeof(__entry->dst));
+			   ),
+
+	    TP_printk("pipe %c, plane %s, frame=%u, scanline=%u, " DRM_RECT_FP_FMT " -> " DRM_RECT_FMT,
+		      pipe_name(__entry->pipe), __entry->name,
+		      __entry->frame, __entry->scanline,
+		      DRM_RECT_FP_ARG((const struct drm_rect *)__entry->src),
+		      DRM_RECT_ARG((const struct drm_rect *)__entry->dst))
+);
+
+TRACE_EVENT(intel_disable_plane,
+	    TP_PROTO(struct drm_plane *plane, struct intel_crtc *crtc),
+	    TP_ARGS(plane, crtc),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(const char *, name)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = crtc->pipe;
+			   __entry->name = plane->name;
+			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+										       crtc->pipe);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   ),
+
+	    TP_printk("pipe %c, plane %s, frame=%u, scanline=%u",
+		      pipe_name(__entry->pipe), __entry->name,
+		      __entry->frame, __entry->scanline)
+);
+
 /* pipe updates */
 
 TRACE_EVENT(i915_pipe_update_start,
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 1eaf840cf9ff..cfb47293fd53 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -231,12 +231,19 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 		to_intel_plane_state(plane->state);
 	struct drm_crtc *crtc = plane->state->crtc ?: old_state->crtc;
 
-	if (intel_state->base.visible)
+	if (intel_state->base.visible) {
+		trace_intel_update_plane(plane,
+					 to_intel_crtc(crtc));
+
 		intel_plane->update_plane(plane,
 					  to_intel_crtc_state(crtc->state),
 					  intel_state);
-	else
+	} else {
+		trace_intel_disable_plane(plane,
+					  to_intel_crtc(crtc));
+
 		intel_plane->disable_plane(plane, crtc);
+	}
 }
 
 const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8ea8290a808d..2d7d687a8002 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2754,6 +2754,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 				to_intel_plane_state(plane_state),
 				false);
 	intel_pre_disable_primary_noatomic(&intel_crtc->base);
+	trace_intel_disable_plane(primary, intel_crtc);
 	intel_plane->disable_plane(primary, &intel_crtc->base);
 
 	return;
@@ -3447,10 +3448,14 @@ static void intel_update_primary_planes(struct drm_device *dev)
 		struct intel_plane_state *plane_state =
 			to_intel_plane_state(plane->base.state);
 
-		if (plane_state->base.visible)
+		if (plane_state->base.visible) {
+			trace_intel_update_plane(&plane->base,
+						 to_intel_crtc(crtc));
+
 			plane->update_plane(&plane->base,
 					    to_intel_crtc_state(crtc->state),
 					    plane_state);
+		}
 	}
 }
 
@@ -13494,12 +13499,15 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	new_plane_state->fb = old_fb;
 	to_intel_plane_state(new_plane_state)->vma = old_vma;
 
-	if (plane->state->visible)
+	if (plane->state->visible) {
+		trace_intel_update_plane(plane, to_intel_crtc(crtc));
 		intel_plane->update_plane(plane,
 					  to_intel_crtc_state(crtc->state),
 					  to_intel_plane_state(plane->state));
-	else
+	} else {
+		trace_intel_disable_plane(plane, to_intel_crtc(crtc));
 		intel_plane->disable_plane(plane, crtc);
+	}
 
 	intel_cleanup_plane_fb(plane, new_plane_state);
 
@@ -15149,6 +15157,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 			if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
 				continue;
 
+			trace_intel_disable_plane(&plane->base, crtc);
 			plane->disable_plane(&plane->base, &crtc->base);
 		}
 	}
-- 
2.10.2

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

[PATCH 16/18] drm/i915: Add VLV/CHV watermark/FIFO programming tracepoints

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add tracepoints for observing the WM/FIFO programming on VLV/CHV. When
compared with the plane and pipe update tracepoints this can be used
to verify that everything is performed in the right sequence.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h | 71 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c   |  4 +++
 2 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 6cb03bd40aef..ca8198785311 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -14,6 +14,77 @@
 #define TRACE_SYSTEM i915
 #define TRACE_INCLUDE_FILE i915_trace
 
+/* watermark/fifo updates */
+
+TRACE_EVENT(vlv_wm,
+	    TP_PROTO(struct intel_crtc *crtc, const struct vlv_wm_values *wm),
+	    TP_ARGS(crtc, wm),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     __field(u32, level)
+			     __field(u32, cxsr)
+			     __field(u32, primary)
+			     __field(u32, sprite0)
+			     __field(u32, sprite1)
+			     __field(u32, cursor)
+			     __field(u32, sr_plane)
+			     __field(u32, sr_cursor)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = crtc->pipe;
+			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+										       crtc->pipe);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->level = wm->level;
+			   __entry->cxsr = wm->cxsr;
+			   __entry->primary = wm->pipe[crtc->pipe].plane[PLANE_PRIMARY];
+			   __entry->sprite0 = wm->pipe[crtc->pipe].plane[PLANE_SPRITE0];
+			   __entry->sprite1 = wm->pipe[crtc->pipe].plane[PLANE_SPRITE1];
+			   __entry->cursor = wm->pipe[crtc->pipe].plane[PLANE_CURSOR];
+			   __entry->sr_plane = wm->sr.plane;
+			   __entry->sr_cursor = wm->sr.cursor;
+			   ),
+
+	    TP_printk("pipe %c, frame=%u, scanline=%u, level=%d, cxsr=%d, wm %d/%d/%d/%d, sr %d/%d",
+		      pipe_name(__entry->pipe), __entry->frame,
+		      __entry->scanline, __entry->level, __entry->cxsr,
+		      __entry->primary, __entry->sprite0, __entry->sprite1, __entry->cursor,
+		      __entry->sr_plane, __entry->sr_cursor)
+);
+
+TRACE_EVENT(vlv_fifo_size,
+	    TP_PROTO(struct intel_crtc *crtc, u32 sprite0_start, u32 sprite1_start, u32 fifo_size),
+	    TP_ARGS(crtc, sprite0_start, sprite1_start, fifo_size),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     __field(u32, sprite0_start)
+			     __field(u32, sprite1_start)
+			     __field(u32, fifo_size)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = crtc->pipe;
+			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+										       crtc->pipe);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->sprite0_start = sprite0_start;
+			   __entry->sprite1_start = sprite1_start;
+			   __entry->fifo_size = fifo_size;
+			   ),
+
+	    TP_printk("pipe %c, frame=%u, scanline=%u, %d/%d/%d",
+		      pipe_name(__entry->pipe), __entry->frame,
+		      __entry->scanline, __entry->sprite0_start,
+		      __entry->sprite1_start, __entry->fifo_size)
+);
+
 /* plane updates */
 
 TRACE_EVENT(intel_update_plane,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aff67de030df..b6ac7f1574b7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -888,6 +888,8 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 	enum pipe pipe;
 
 	for_each_pipe(dev_priv, pipe) {
+		trace_vlv_wm(intel_get_crtc_for_pipe(dev_priv, pipe), wm);
+
 		I915_WRITE(VLV_DDL(pipe),
 			   (wm->ddl[pipe].plane[PLANE_CURSOR] << DDL_CURSOR_SHIFT) |
 			   (wm->ddl[pipe].plane[PLANE_SPRITE1] << DDL_SPRITE_SHIFT(1)) |
@@ -1352,6 +1354,8 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 	WARN_ON(fifo_state->plane[PLANE_CURSOR] != 63);
 	WARN_ON(fifo_size != 511);
 
+	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
+
 	spin_lock(&dev_priv->wm.dsparb_lock);
 
 	switch (crtc->pipe) {
-- 
2.10.2

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

[PATCH 17/18] drm/i915: Add cxsr toggle tracepoint

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a tracepoint for observing changes in the cxsr state. The tracepoint
will dump out the frame and scanline counters for each pipe so that the
information can be compared with eg. plane update tracepoints.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c   |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index ca8198785311..2537a03555cc 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -16,6 +16,36 @@
 
 /* watermark/fifo updates */
 
+TRACE_EVENT(intel_memory_cxsr,
+	    TP_PROTO(struct drm_i915_private *dev_priv, bool old, bool new),
+	    TP_ARGS(dev_priv, old, new),
+
+	    TP_STRUCT__entry(
+			     __array(u32, frame, 3)
+			     __array(u32, scanline, 3)
+			     __field(bool, old)
+			     __field(bool, new)
+			     ),
+
+	    TP_fast_assign(
+			   enum pipe pipe;
+			   for_each_pipe(dev_priv, pipe) {
+				   __entry->frame[pipe] =
+					   dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
+				   __entry->scanline[pipe] =
+					   intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   }
+			   __entry->old = old;
+			   __entry->new = new;
+			   ),
+
+	    TP_printk("%s->%s, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u",
+		      onoff(__entry->old), onoff(__entry->new),
+		      __entry->frame[PIPE_A], __entry->scanline[PIPE_A],
+		      __entry->frame[PIPE_B], __entry->scanline[PIPE_B],
+		      __entry->frame[PIPE_C], __entry->scanline[PIPE_C])
+);
+
 TRACE_EVENT(vlv_wm,
 	    TP_PROTO(struct intel_crtc *crtc, const struct vlv_wm_values *wm),
 	    TP_ARGS(crtc, wm),
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b6ac7f1574b7..0b8956acae4c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -377,6 +377,8 @@ static bool _intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enabl
 		return false;
 	}
 
+	trace_intel_memory_cxsr(dev_priv, was_enabled, enable);
+
 	DRM_DEBUG_KMS("memory self-refresh is %s (was %s)\n",
 		      enableddisabled(enable),
 		      enableddisabled(was_enabled));
-- 
2.10.2

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

[PATCH v2 18/18] drm/i915: Add FIFO underrun tracepoints

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add tracepoints for display FIFO underruns. Makes it more convenient to
correlate the underruns with other display tracepoints.

v2: s/i915/intel/ in the tracepoint name

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_trace.h          | 43 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 11 ++++++--
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 2537a03555cc..5503f5ab1e98 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -16,6 +16,49 @@
 
 /* watermark/fifo updates */
 
+TRACE_EVENT(intel_cpu_fifo_underrun,
+	    TP_PROTO(struct drm_i915_private *dev_priv, enum pipe pipe),
+	    TP_ARGS(dev_priv, pipe),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = pipe;
+			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
+			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   ),
+
+	    TP_printk("pipe %c, frame=%u, scanline=%u",
+		      pipe_name(__entry->pipe),
+		      __entry->frame, __entry->scanline)
+);
+
+TRACE_EVENT(intel_pch_fifo_underrun,
+	    TP_PROTO(struct drm_i915_private *dev_priv, enum transcoder pch_transcoder),
+	    TP_ARGS(dev_priv, pch_transcoder),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     ),
+
+	    TP_fast_assign(
+			   enum pipe pipe = (enum pipe)pch_transcoder;
+			   __entry->pipe = pipe;
+			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
+			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   ),
+
+	    TP_printk("pch transcoder %c, frame=%u, scanline=%u",
+		      pipe_name(__entry->pipe),
+		      __entry->frame, __entry->scanline)
+);
+
 TRACE_EVENT(intel_memory_cxsr,
 	    TP_PROTO(struct drm_i915_private *dev_priv, bool old, bool new),
 	    TP_ARGS(dev_priv, old, new),
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index e660d8b4bbc3..924f41f5da0f 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -98,6 +98,7 @@ static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
 	I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
 	POSTING_READ(reg);
 
+	trace_intel_cpu_fifo_underrun(dev_priv, crtc->pipe);
 	DRM_ERROR("pipe %c underrun\n", pipe_name(crtc->pipe));
 }
 
@@ -147,6 +148,7 @@ static void ivybridge_check_fifo_underruns(struct intel_crtc *crtc)
 	I915_WRITE(GEN7_ERR_INT, ERR_INT_FIFO_UNDERRUN(pipe));
 	POSTING_READ(GEN7_ERR_INT);
 
+	trace_intel_cpu_fifo_underrun(dev_priv, pipe);
 	DRM_ERROR("fifo underrun on pipe %c\n", pipe_name(pipe));
 }
 
@@ -212,6 +214,7 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc)
 	I915_WRITE(SERR_INT, SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
 	POSTING_READ(SERR_INT);
 
+	trace_intel_pch_fifo_underrun(dev_priv, pch_transcoder);
 	DRM_ERROR("pch fifo underrun on pch transcoder %s\n",
 		  transcoder_name(pch_transcoder));
 }
@@ -368,9 +371,11 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 	    crtc->cpu_fifo_underrun_disabled)
 		return;
 
-	if (intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false))
+	if (intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false)) {
+		trace_intel_cpu_fifo_underrun(dev_priv, pipe);
 		DRM_ERROR("CPU pipe %c FIFO underrun\n",
 			  pipe_name(pipe));
+	}
 
 	intel_fbc_handle_fifo_underrun_irq(dev_priv);
 }
@@ -388,9 +393,11 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 					 enum transcoder pch_transcoder)
 {
 	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
-						  false))
+						  false)) {
+		trace_intel_pch_fifo_underrun(dev_priv, pch_transcoder);
 		DRM_ERROR("PCH transcoder %s FIFO underrun\n",
 			  transcoder_name(pch_transcoder));
+	}
 }
 
 /**
-- 
2.10.2

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

✓ Fi.CI.BAT: success for drm/i915: VLV/CHV two-stage watermarks (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: VLV/CHV two-stage watermarks (rev3)
URL   : https://patchwork.freedesktop.org/series/16712/
State : success

== Summary ==

Series 16712v3 drm/i915: VLV/CHV two-stage watermarks
https://patchwork.freedesktop.org/api/1.0/series/16712/revisions/3/mbox/

Test gem_busy:
        Subgroup basic-hang-default:
                fail       -> PASS       (fi-hsw-4770r)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (fi-skl-6700hq) fdo#99739

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:260  dwarn:0   dfail:0   fail:1   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

06dbfb1f6e3af2f332189d566a9f2f1227acda76 drm-tip: 2017y-03m-02d-18h-25m-10s UTC integration manifest
9d3394e drm/i915: Add FIFO underrun tracepoints
595085d drm/i915: Add cxsr toggle tracepoint
dd479dd drm/i915: Add VLV/CHV watermark/FIFO programming tracepoints
2d00424 drm/i915: Add plane update/disable tracepoints
a490a15 drm/i915: Kill level 0 wm hack for VLV/CHV
25905df drm/i915: Workaround VLV/CHV sprite1->sprite0 enable underrun
3be6248 drm/i915: Sanitize VLV/CHV watermarks properly
8de9633 drm/i915: Only use update_wm_{pre, post} for pre-ilk platforms
7a15fc5 drm/i915: Nuke crtc->wm.cxsr_allowed
0fcca4d drm/i915: Compute proper intermediate wms for vlv/cvh
0fadad1 drm/i915: Skip useless watermark/FIFO related work on VLV/CHV when not needed
99a13e8 drm/i915: Compute vlv/chv wms the atomic way
640003c drm/i915: Compute VLV/CHV FIFO sizes based on the PM2 watermarks
f678587 drm/i915: Plop vlv/chv fifo sizes into crtc state
3699fc1 drm/i915: Plop vlv wm state into crtc_state
d8dbbf8 drm/i915: Move vlv wms from crtc->wm_state to crtc->wm.active.vlv
63727ba drm/i915: Track plane fifo sizes under intel_crtc
56c42a9 drm/i915: Track visible planes in a bitmask

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4042/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 08/18] drm/i915: Skip useless watermark/FIFO related work on VLV/CHV when not needed

[Ville Syrjälä]

On Thu, Mar 02, 2017 at 07:14:58PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check whether anything relevant has actually change when we compute new
> watermarks for each plane in the state. If the watermarks for no
> primary/sprite planes changed we don't have to recompute the FIFO split
> or reprogram the DSBARB registers. And even the cursor watermarks didn't
> change we can skip the merge+invert step between all the planes on
> the pipe as well.
> 
> v2: s/noninverted/raw/ for consistency with other platforms
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c |  1 +
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_pm.c     | 73 +++++++++++++++++++++++++++++--------
>  3 files changed, 60 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index e65818e5d2ab..50fb1f76cc5f 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -99,6 +99,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  	crtc_state->update_wm_pre = false;
>  	crtc_state->update_wm_post = false;
>  	crtc_state->fb_changed = false;
> +	crtc_state->fifo_changed = false;
>  	crtc_state->wm.need_postvbl_update = false;
>  	crtc_state->fb_bits = 0;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 351a14c98205..04943732255e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -573,6 +573,7 @@ struct intel_crtc_state {
>  	bool disable_cxsr;
>  	bool update_wm_pre, update_wm_post; /* watermarks are updated */
>  	bool fb_changed; /* fb on any of the planes is changed */
> +	bool fifo_changed; /* FIFO split is changed */
>  
>  	/* Pipe source size (ie. panel fitter input size)
>  	 * All planes will be positioned inside this space,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f7cc013b3a78..2566c88d8c89 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
<snip>
> @@ -4630,6 +4671,8 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
>  		active->num_levels = wm->level + 1;
>  		active->cxsr = wm->cxsr;
>  
> +		vlv_get_fifo_size(crtc_state);
> +

Entire series pushed to dinq. While doing that I spotted this spurious
vlv_get_fifo_size() call being added here. There's an identical call
just a few lines earlier in the code, so I just dropped this extra call
while applying the patch.

Thanks for the review.

>  		/* FIXME sanitize things more */
>  		for (level = 0; level < active->num_levels; level++) {
>  			struct vlv_pipe_wm *raw =
> -- 
> 2.10.2

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