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

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

[ville.syrjala]

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

A rebased version of the two stage watermark support for VLV/CHV. Mostly
just cosmetic changes due to churn. I did clear out that unused
intel_plane_wm_parameters structure Maarten pointed out.

The sticking point in the last series was the active_planes bitmask, but
I've decided to leave that as is because it was the most convenient way
to do it for VLV/CHV, and I think it should work quite well for g4x as
well (which is something I want to tackle after landing this stuff).
I might still change my mind when I get down to gen2-gen4 territory
but until I really start on that I don't want to needlessly change
directions.

Oh one new thing I added here is cleaned up versions of some the
tracepoints I used in debugging this stuff. I think those would
be usedful to have upstream so I've included them in the series.

Entire series is available here:
git://github.com/vsyrjala/linux.git vlv_atomic_wm_6

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       | 179 +++++---
 drivers/gpu/drm/i915/intel_drv.h           |  71 ++-
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  11 +-
 drivers/gpu/drm/i915/intel_pm.c            | 668 ++++++++++++++++++++---------
 9 files changed, 836 insertions(+), 322 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>
---
 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 0d46a8db5c6f..0ccc04607b8a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2674,6 +2674,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)
 {
@@ -2730,8 +2753,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);
 
@@ -2771,7 +2795,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);
 }
@@ -10853,11 +10881,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;
@@ -10865,7 +10893,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));
@@ -10889,8 +10917,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;
@@ -10902,13 +10932,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);
 
@@ -10916,15 +10945,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;
@@ -10936,7 +10965,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
@@ -10944,7 +10973,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;
@@ -15336,15 +15365,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 4467f4e773e5..e1f28dd61821 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -691,6 +691,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 e1f28dd61821..a6ee05b0404a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -705,6 +705,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;
@@ -754,6 +758,8 @@ struct intel_crtc {
 
 		/* allow CxSR on this pipe */
 		bool cxsr_allowed;
+
+		struct vlv_fifo_state fifo_state;
 	} wm;
 
 	int scanline_offset;
@@ -771,25 +777,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;
@@ -799,13 +786,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 fe243c65de1a..06826945576e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -405,15 +405,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;
+	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;
 
-	if (plane->id == PLANE_CURSOR)
-		return 63;
-
-	switch (plane->pipe) {
+	switch (pipe) {
 		uint32_t dsparb, dsparb2, dsparb3;
 	case PIPE_A:
 		dsparb = I915_READ(DSPARB);
@@ -434,26 +433,21 @@ static int vlv_get_fifo_size(struct intel_plane *plane)
 		sprite1_start = VLV_FIFO_START(dsparb3, dsparb2, 8, 20);
 		break;
 	default:
-		return 0;
+		MISSING_CASE(pipe);
+		return;
 	}
 
-	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;
-	}
-
-	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)
@@ -1031,8 +1025,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;
@@ -1042,7 +1037,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) {
@@ -1056,19 +1051,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);
@@ -1080,16 +1075,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;
 	}
 
@@ -1111,9 +1106,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,
@@ -1122,10 +1118,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]);
 		}
 	}
 }
@@ -1134,6 +1130,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;
 
@@ -1160,7 +1157,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))
@@ -1204,32 +1201,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,
@@ -4517,14 +4498,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 a6ee05b0404a..f4ba686e3e22 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -754,6 +754,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 */
@@ -773,8 +774,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 06826945576e..b29b05a195d8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1025,7 +1025,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;
@@ -1101,7 +1101,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++) {
@@ -1129,7 +1129,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;
@@ -1292,7 +1292,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;
@@ -1311,7 +1311,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 f4ba686e3e22..82caf4f4a02b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -488,6 +488,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 {
@@ -512,6 +528,11 @@ struct intel_crtc_wm_state {
 			struct skl_pipe_wm optimal;
 			struct skl_ddb_entry ddb;
 		} skl;
+
+		struct {
+			/* inverted optimal watermarks */
+			struct vlv_wm_state optimal;
+		} vlv;
 	};
 
 	/*
@@ -696,15 +717,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 b29b05a195d8..507e49cd6e4a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -947,12 +947,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,
@@ -1023,9 +1017,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;
@@ -1099,9 +1094,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++) {
@@ -1126,10 +1122,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;
@@ -1141,7 +1138,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;
@@ -1156,7 +1153,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 */
@@ -1193,7 +1190,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) \
@@ -1341,11 +1338,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>
---
 drivers/gpu/drm/i915/intel_drv.h | 11 +++++------
 drivers/gpu/drm/i915/intel_pm.c  | 26 +++++++++++++++-----------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 82caf4f4a02b..493a37b089fb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -504,6 +504,10 @@ struct vlv_wm_state {
 	bool cxsr;
 };
 
+struct vlv_fifo_state {
+	u16 plane[I915_MAX_PLANES];
+};
+
 struct intel_crtc_wm_state {
 	union {
 		struct {
@@ -530,6 +534,7 @@ struct intel_crtc_wm_state {
 		} skl;
 
 		struct {
+			struct vlv_fifo_state fifo_state;
 			/* inverted optimal watermarks */
 			struct vlv_wm_state optimal;
 		} vlv;
@@ -717,10 +722,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;
@@ -771,8 +772,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 507e49cd6e4a..9371b9645c9d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -405,10 +405,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;
 
@@ -1021,7 +1022,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;
@@ -1098,11 +1099,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;
@@ -1127,7 +1129,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;
 
@@ -1196,10 +1199,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];
@@ -1350,8 +1355,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;
 	}
 
@@ -1365,7 +1369,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);
 
@@ -4505,7 +4509,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 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.

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  | 116 +++++++++++++++++++--------------------
 2 files changed, 57 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 493a37b089fb..ada5fd3fdeab 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -534,6 +534,8 @@ struct intel_crtc_wm_state {
 		} skl;
 
 		struct {
+			/* non-inverted optimal watermarks */
+			struct vlv_pipe_wm noninverted[NUM_VLV_WM_LEVELS];
 			struct vlv_fifo_state fifo_state;
 			/* inverted optimal watermarks */
 			struct vlv_wm_state optimal;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9371b9645c9d..ad02edb11ca2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1018,73 +1018,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 *noninverted =
+		&crtc_state->wm.vlv.noninverted[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 = noninverted->plane[PLANE_PRIMARY] +
+		noninverted->plane[PLANE_SPRITE0] +
+		noninverted->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;
+		if ((active_planes & BIT(plane_id)) == 0) {
+			fifo_state->plane[plane_id] = 0;
 			continue;
 		}
 
-		if (!state->base.visible) {
-			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 = noninverted->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)
@@ -1129,35 +1126,33 @@ 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.noninverted, 0,
+	       sizeof(crtc_state->wm.vlv.noninverted));
 
 	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 *noninverted =
+				&crtc_state->wm.vlv.noninverted[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))
@@ -1166,25 +1161,24 @@ static void vlv_compute_wm(struct intel_crtc_state *crtc_state)
 			if (wm > max_wm)
 				break;
 
-			wm_state->wm[level].plane[plane->id] = wm;
+			noninverted->plane[plane->id] = wm;
 		}
 
 		wm_state->num_levels = level;
+	}
 
-		if (!wm_state->cxsr)
-			continue;
-
-		/* 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]);
-		}
+	vlv_compute_fifo(crtc_state);
+
+	for (level = 0; level < wm_state->num_levels; level++) {
+		struct vlv_pipe_wm *noninverted =
+			&crtc_state->wm.vlv.noninverted[level];
+
+		wm_state->wm[level] = *noninverted;
+
+		wm_state->sr[level].plane = max3(noninverted->plane[PLANE_PRIMARY],
+						 noninverted->plane[PLANE_SPRITE0],
+						 noninverted->plane[PLANE_SPRITE1]);
+		wm_state->sr[level].cursor = noninverted->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 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().

Signed-off-by: Ville Syrjälä <ville.syrjala@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      | 327 +++++++++++++++++++++++------------
 4 files changed, 238 insertions(+), 120 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7495a16b869a..e7317827cf0b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -500,6 +500,14 @@ struct i915_hotplug {
 	for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)	\
 		for_each_if (BIT_ULL(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 0ccc04607b8a..77160eaa2294 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5763,6 +5763,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);
@@ -5807,7 +5809,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);
@@ -5924,6 +5927,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)
@@ -11362,10 +11368,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
@@ -11378,6 +11387,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);
 
@@ -11386,6 +11397,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
@@ -12898,12 +12911,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 ada5fd3fdeab..9db2548ccf6c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -498,9 +498,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 ad02edb11ca2..cb40e5826a71 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1084,6 +1084,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)
@@ -1092,108 +1114,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' */
+static void vlv_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);
+
+	for (; level < num_levels; level++) {
+		struct vlv_pipe_wm *noninverted =
+			&crtc_state->wm.vlv.noninverted[level];
+
+		noninverted->plane[plane_id] = value;
+	}
+}
+
+static void 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;
+
+	if (!plane_state->base.visible) {
+		vlv_plane_wm_set(crtc_state, 0, plane_id, 0);
+		return;
+	}
+
+	for (level = 0; level < num_levels; level++) {
+		struct vlv_pipe_wm *noninverted =
+			&crtc_state->wm.vlv.noninverted[level];
+		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;
+
+		noninverted->plane[plane_id] = wm;
+	}
+
+	/* mark all higher levels as invalid */
+	vlv_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.noninverted[VLV_WM_LEVEL_PM2].plane[plane_id],
+		      crtc_state->wm.vlv.noninverted[VLV_WM_LEVEL_PM5].plane[plane_id],
+		      crtc_state->wm.vlv.noninverted[VLV_WM_LEVEL_DDR_DVFS].plane[plane_id]);
+}
+
+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 *noninverted =
+		&crtc_state->wm.vlv.noninverted[level];
+	const struct vlv_fifo_state *fifo_state =
+		&crtc_state->wm.vlv.fifo_state;
+
+	return noninverted->plane[plane_id] <= fifo_state->plane[plane_id];
+}
+
+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 level;
+	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;
+
+		vlv_plane_wm_compute(crtc_state, plane_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 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;
+		const struct vlv_pipe_wm *noninverted =
+			&crtc_state->wm.vlv.noninverted[level];
+		const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
+
+		if (!vlv_crtc_wm_is_valid(crtc_state, level))
+			break;
+
+		for_each_plane_id_on_crtc(crtc, plane_id) {
+			wm_state->wm[level].plane[plane_id] =
+				vlv_invert_wm_value(noninverted->plane[plane_id],
+						    fifo_state->plane[plane_id]);
+		}
 
 		wm_state->sr[level].plane =
-			vlv_invert_wm_value(wm_state->sr[level].plane,
+			vlv_invert_wm_value(max3(noninverted->plane[PLANE_PRIMARY],
+						 noninverted->plane[PLANE_SPRITE0],
+						 noninverted->plane[PLANE_SPRITE1]),
 					    sr_fifo_size);
+
 		wm_state->sr[level].cursor =
-			vlv_invert_wm_value(wm_state->sr[level].cursor,
+			vlv_invert_wm_value(noninverted->plane[PLANE_CURSOR],
 					    63);
-
-		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]);
-		}
-	}
-}
-
-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;
-	struct intel_plane *plane;
-	int level;
-
-	memset(wm_state, 0, sizeof(*wm_state));
-	memset(crtc_state->wm.vlv.noninverted, 0,
-	       sizeof(crtc_state->wm.vlv.noninverted));
-
-	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;
-
-	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);
-
-		if (!state->base.visible)
-			continue;
-
-		for (level = 0; level < wm_state->num_levels; level++) {
-			struct vlv_pipe_wm *noninverted =
-				&crtc_state->wm.vlv.noninverted[level];
-			int wm = vlv_compute_wm_level(crtc_state, state, level);
-			int max_wm = plane->id == PLANE_CURSOR ? 63 : 511;
-
-			/* hack */
-			if (WARN_ON(level == 0 && wm > max_wm))
-				wm = max_wm;
-
-			if (wm > max_wm)
-				break;
-
-			noninverted->plane[plane->id] = wm;
-		}
-
-		wm_state->num_levels = level;
 	}
 
-	vlv_compute_fifo(crtc_state);
+	if (level == 0)
+		return -EINVAL;
 
-	for (level = 0; level < wm_state->num_levels; level++) {
-		struct vlv_pipe_wm *noninverted =
-			&crtc_state->wm.vlv.noninverted[level];
+	/* limit to only levels we can actually handle */
+	wm_state->num_levels = level;
 
-		wm_state->wm[level] = *noninverted;
-
-		wm_state->sr[level].plane = max3(noninverted->plane[PLANE_PRIMARY],
-						 noninverted->plane[PLANE_SPRITE0],
-						 noninverted->plane[PLANE_SPRITE1]);
-		wm_state->sr[level].cursor = noninverted->plane[PLANE_CURSOR];
-	}
-
-	/* 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]));
-	}
+	/* invalidate the higher levels */
+	vlv_invalidate_wms(crtc, wm_state, level);
 
-	vlv_invert_wms(crtc_state);
+	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);
@@ -1208,10 +1284,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) {
@@ -1310,11 +1382,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;
@@ -1334,24 +1403,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);
@@ -1362,17 +1422,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);
 
@@ -1385,6 +1436,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)
@@ -4497,14 +4560,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;
 
@@ -4542,13 +4601,51 @@ 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 *noninverted =
+				&crtc_state->wm.vlv.noninverted[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];
+
+				noninverted->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_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);
@@ -7700,7 +7797,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 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.

Signed-off-by: Ville Syrjälä <ville.syrjala@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 aa9160e7f1d8..c5d62aa24d4c 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 9db2548ccf6c..d7b9ddf22b3e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -568,6 +568,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 cb40e5826a71..6ecad9f949c7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1115,31 +1115,36 @@ static u16 vlv_invert_wm_value(u16 wm, u16 fifo_size)
 }
 
 /* starting from 'level' set all higher levels to 'value' */
-static void vlv_plane_wm_set(struct intel_crtc_state *crtc_state,
+static bool vlv_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 *noninverted =
 			&crtc_state->wm.vlv.noninverted[level];
 
+		dirty |= noninverted->plane[plane_id] != value;
 		noninverted->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_plane_wm_set(crtc_state, 0, plane_id, 0);
-		return;
+		dirty |= vlv_plane_wm_set(crtc_state, 0, plane_id, 0);
+		goto out;
 	}
 
 	for (level = 0; level < num_levels; level++) {
@@ -1155,17 +1160,22 @@ static void vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
 		if (wm > max_wm)
 			break;
 
+		dirty |= noninverted->plane[plane_id] != wm;
 		noninverted->plane[plane_id] = wm;
 	}
 
 	/* mark all higher levels as invalid */
-	vlv_plane_wm_set(crtc_state, level, plane_id, USHRT_MAX);
+	dirty |= vlv_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.noninverted[VLV_WM_LEVEL_PM2].plane[plane_id],
-		      crtc_state->wm.vlv.noninverted[VLV_WM_LEVEL_PM5].plane[plane_id],
-		      crtc_state->wm.vlv.noninverted[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.noninverted[VLV_WM_LEVEL_PM2].plane[plane_id],
+			      crtc_state->wm.vlv.noninverted[VLV_WM_LEVEL_PM5].plane[plane_id],
+			      crtc_state->wm.vlv.noninverted[VLV_WM_LEVEL_DDR_DVFS].plane[plane_id]);
+
+	return dirty;
 }
 
 static bool vlv_plane_wm_is_valid(const struct intel_crtc_state *crtc_state,
@@ -1198,10 +1208,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 =
@@ -1211,7 +1223,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 */
@@ -1224,10 +1266,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 *noninverted =
 			&crtc_state->wm.vlv.noninverted[level];
@@ -1277,6 +1315,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;
@@ -4616,6 +4657,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 *noninverted =
-- 
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>
---
 drivers/gpu/drm/i915/intel_drv.h |  3 +-
 drivers/gpu/drm/i915/intel_pm.c  | 59 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d7b9ddf22b3e..1c31100482ef 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -535,7 +535,8 @@ struct intel_crtc_wm_state {
 			/* non-inverted optimal watermarks */
 			struct vlv_pipe_wm noninverted[NUM_VLV_WM_LEVELS];
 			struct vlv_fifo_state fifo_state;
-			/* inverted optimal watermarks */
+			/* inverted intermediate and optimal watermarks */
+			struct vlv_wm_state intermediate;
 			struct vlv_wm_state optimal;
 		} vlv;
 	};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6ecad9f949c7..ec384d559aae 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1391,6 +1391,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)
 {
@@ -1484,7 +1523,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);
 }
@@ -4681,6 +4735,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),
@@ -7841,7 +7896,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>
---
 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 77160eaa2294..82135101faae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5002,8 +5002,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);
 
@@ -5050,22 +5048,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
@@ -10965,11 +10959,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;
 
@@ -12713,12 +12702,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;
@@ -13970,8 +13954,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 1c31100482ef..d197617e7eeb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -771,9 +771,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 ec384d559aae..5beb2d2db701 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1263,8 +1263,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 *noninverted =
@@ -1401,7 +1400,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;
@@ -1424,8 +1424,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;
 }
@@ -2614,8 +2614,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>
---
 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 82135101faae..a8f28e776879 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10942,21 +10942,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 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.

Signed-off-by: Ville Syrjälä <ville.syrjala@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 a8f28e776879..9fef24944c8e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3481,7 +3481,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);
 
@@ -14952,7 +14953,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) {
@@ -15116,7 +15118,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;
 }
@@ -15567,12 +15570,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 d197617e7eeb..251794a9e5f4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1807,6 +1807,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 5beb2d2db701..558a4e5b42b0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4713,7 +4713,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 *noninverted =
 				&crtc_state->wm.vlv.noninverted[level];
@@ -4749,6 +4748,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 *noninverted =
+				&crtc_state->wm.vlv.noninverted[level];
+
+			noninverted->plane[plane_id] = 0;
+
+			wm_state->wm[level].plane[plane_id] =
+				vlv_invert_wm_value(noninverted->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 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.

Testcase: igt/kms_plane_blinker
Signed-off-by: Ville Syrjälä <ville.syrjala@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 558a4e5b42b0..23600ad1dfcf 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1018,6 +1018,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);
@@ -1028,12 +1034,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 = noninverted->plane[PLANE_PRIMARY] +
 		noninverted->plane[PLANE_SPRITE0] +
-		noninverted->plane[PLANE_SPRITE1];
+		noninverted->plane[PLANE_SPRITE1] +
+		sprite0_fifo_extra;
 
 	if (total_rate > fifo_size)
 		return -EINVAL;
@@ -1054,6 +1073,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 23600ad1dfcf..fbd9cfba720a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1175,10 +1175,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 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).

Signed-off-by: Ville Syrjälä <ville.syrjala@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      |  9 ++++-
 4 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 91be31617e78..5a4d483062b9 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 7a547cdfc381..6354b71fc6a7 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 9fef24944c8e..044c606be1a9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2757,6 +2757,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;
@@ -3449,10 +3450,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);
+		}
 	}
 }
 
@@ -13572,6 +13577,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	new_plane_state->fb = old_fb;
 	to_intel_plane_state(new_plane_state)->vma = old_vma;
 
+	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));
@@ -15221,6 +15227,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>
---
 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 6354b71fc6a7..865084fd8455 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 fbd9cfba720a..69b52da19f6d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -878,6 +878,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)) |
@@ -1342,6 +1344,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>
---
 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 865084fd8455..bca46406030d 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 69b52da19f6d..0cba41d94863 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -367,6 +367,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 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.

Signed-off-by: Ville Syrjälä <ville.syrjala@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 bca46406030d..b0eff4d6d5fa 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(i915_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(i915_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..9c2eebaa5253 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_i915_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_i915_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_i915_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_i915_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_i915_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 (rev2)

[Patchwork]

== Series Details ==

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

== Summary ==

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

fi-bdw-5557u     total:252  pass:241  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:202  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:235  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:224  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:223  dwarn:0   dfail:0   fail:0   skip:29 

c2033e7aa383d062000e024c5fac5f46560327cd drm-tip: 2017y-02m-16d-20h-42m-04s UTC integration manifest
3075e09 drm/i915: Add FIFO underrun tracepoints
3f7fef8 drm/i915: Add cxsr toggle tracepoint
ef01cfb drm/i915: Add VLV/CHV watermark/FIFO programming tracepoints
027dcd2 drm/i915: Add plane update/disable tracepoints
540bda6 drm/i915: Kill level 0 wm hack for VLV/CHV
e888f80 drm/i915: Workaround VLV/CHV sprite1->sprite0 enable underrun
53dd443 drm/i915: Sanitize VLV/CHV watermarks properly
1fa9864 drm/i915: Only use update_wm_{pre, post} for pre-ilk platforms
a5f675e drm/i915: Nuke crtc->wm.cxsr_allowed
58db269 drm/i915: Compute proper intermediate wms for vlv/cvh
3fb8e38 drm/i915: Skip useless watermark/FIFO related work on VLV/CHV when not needed
53ed9ee drm/i915: Compute vlv/chv wms the atomic way
f7a92c6 drm/i915: Compute VLV/CHV FIFO sizes based on the PM2 watermarks
d76aa3c drm/i915: Plop vlv/chv fifo sizes into crtc state
5ce068a drm/i915: Plop vlv wm state into crtc_state
262b888 drm/i915: Move vlv wms from crtc->wm_state to crtc->wm.active.vlv
fe0354c drm/i915: Track plane fifo sizes under intel_crtc
56e8c1b drm/i915: Track visible planes in a bitmask

== Logs ==

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

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

[Maarten Lankhorst]

Op 16-02-17 om 19:07 schreef ville.syrjala@linux.intel.com:
> 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.
I'm still not sold on how crtc_state->visible_planes deviates from crtc_state->planes_mask and other *_mask's.

Should probably rename it to visible_plane_id_mask to make it clear it's a mask not using drm_plane_index.

Otherwise looks good,

so for the whole series:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx