All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Solve IVB FDI bifurcation problems using atomic
@ 2015-11-24 12:59 ville.syrjala
  0 siblings, 0 replies; only message in thread
From: ville.syrjala @ 2015-11-24 12:59 UTC (permalink / raw)
  To: intel-gfx

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

Now that we have atomic modesetting, we can use it to solve the IVB
pipe B/C FDI lane sharing issue. That is, we can force a modeset on
pipe B if pipe C requires the FDI lanes currently used by pipe B.

To achieve this we do the following:

First stage:
- first compute the initial pipe config for each modeset crtc
- initial max FDI estimate will be 4 for pipe B, 2 for pipe C
- also need to run through the encoder .compute_config()
  hooks once to update has_pch_encoder
- and we need to compute an initial number of required FDI lanes.
  Note that we only need something that makes fdi_lanes > 0 speak
  the truth, so there's no need to check against the max here

Second stage:
- Check if pipe C requires any FDI lanes, and based on that
  update the max_fdi_lanes for both pipe B and pipe C
- Check if pipe B conflicts with the new requirements,
  and if so, add it to the state forcing a modeset (in case
  it already wasn't included)

Third stage:
- run through the rest of the pipe config computation, incuding
  recomputing the FDI lane requirement and checking it against the
  max
- this will signal that we should recompute things with a lower
  bpp by returning -EAGAIN
- we keep doing this in a loop as long -EAGAIN is returned
  If some other error occurs we bail out as usual

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 353 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 2 files changed, 222 insertions(+), 132 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 68fb449ded77..082323c2aa8c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6492,87 +6492,120 @@ static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
-static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
-				     struct intel_crtc_state *pipe_config)
+static void
+intel_modeset_pipe_config_start(struct intel_crtc *crtc,
+				struct intel_crtc_state *pipe_config);
+static int
+intel_modeset_pipe_config_continue(struct intel_crtc *crtc,
+				   struct intel_crtc_state *pipe_config);
+
+static int intel_add_crtc_modeset_to_state(struct drm_atomic_state *state,
+					   struct intel_crtc *crtc)
 {
-	struct drm_atomic_state *state = pipe_config->base.state;
-	struct intel_crtc *other_crtc;
-	struct intel_crtc_state *other_crtc_state;
+	struct intel_crtc_state *crtc_state;
+	int ret;
 
-	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
-		      pipe_name(pipe), pipe_config->fdi_lanes);
-	if (pipe_config->fdi_lanes > 4) {
-		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
-			      pipe_name(pipe), pipe_config->fdi_lanes);
-		return -EINVAL;
-	}
+	crtc_state = intel_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
 
-	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
-		if (pipe_config->fdi_lanes > 2) {
-			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
-				      pipe_config->fdi_lanes);
-			return -EINVAL;
-		} else {
-			return 0;
-		}
-	}
+	crtc_state->base.mode_changed = true;
 
-	if (INTEL_INFO(dev)->num_pipes == 2)
-		return 0;
+	ret = drm_atomic_add_affected_connectors(state, &crtc->base);
+	if (ret)
+		return ret;
 
-	/* Ivybridge 3 pipe is really complicated */
-	switch (pipe) {
-	case PIPE_A:
-		return 0;
-	case PIPE_B:
-		if (pipe_config->fdi_lanes <= 2)
-			return 0;
+	intel_modeset_pipe_config_start(crtc, crtc_state);
 
-		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
-		other_crtc_state =
-			intel_atomic_get_crtc_state(state, other_crtc);
-		if (IS_ERR(other_crtc_state))
-			return PTR_ERR(other_crtc_state);
+	/*
+	 * Must run through this at least once for every new
+	 * pipe to make sure has_pch_encoder and fdi_lanes are
+	 * set up to something half decent.
+	 */
+	ret = intel_modeset_pipe_config_continue(crtc, crtc_state);
+	if (ret)
+		return ret;
 
-		if (pipe_required_fdi_lanes(other_crtc_state) > 0) {
-			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return -EINVAL;
-		}
-		return 0;
-	case PIPE_C:
-		if (pipe_config->fdi_lanes > 2) {
-			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
-				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return -EINVAL;
-		}
-
-		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
-		other_crtc_state =
-			intel_atomic_get_crtc_state(state, other_crtc);
-		if (IS_ERR(other_crtc_state))
-			return PTR_ERR(other_crtc_state);
-
-		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
-			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return -EINVAL;
-		}
-		return 0;
-	default:
-		BUG();
-	}
+	return 0;
 }
 
-#define RETRY 1
-static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
-				       struct intel_crtc_state *pipe_config)
+static bool intel_crtc_needs_modeset(struct drm_atomic_state *state,
+				     struct intel_crtc *crtc)
 {
-	struct drm_device *dev = intel_crtc->base.dev;
-	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	int lane, link_bw, fdi_dotclock, ret;
-	bool needs_recompute = false;
+	struct drm_crtc_state *crtc_state =
+		drm_atomic_get_existing_crtc_state(state, &crtc->base);
+
+	return crtc_state && needs_modeset(crtc_state);
+}
+
+static int ivybridge_compute_fdi_bc_bifurcation(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	struct intel_crtc *crtc_b, *crtc_c;
+	struct intel_crtc_state *crtc_state_b, *crtc_state_c;
+	int ret = 0;
+
+	crtc_b = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
+	crtc_c = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
+
+	/* Nothing to do if we're not modesetting pipe B or C */
+	if (!intel_crtc_needs_modeset(state, crtc_b) &&
+	    !intel_crtc_needs_modeset(state, crtc_c))
+		return 0;
+
+	crtc_state_b = intel_atomic_get_crtc_state(state, crtc_b);
+	if (IS_ERR(crtc_state_b))
+		return PTR_ERR(crtc_state_b);
+
+	crtc_state_c = intel_atomic_get_crtc_state(state, crtc_c);
+	if (IS_ERR(crtc_state_c))
+		return PTR_ERR(crtc_state_c);
+
+	if (pipe_required_fdi_lanes(crtc_state_c) > 0) {
+		DRM_DEBUG_KMS("pipe C requires FDI lanes\n");
+
+		/* need to modeset pipe B to make room for pipe C */
+		if (pipe_required_fdi_lanes(crtc_state_b) > 2) {
+			DRM_DEBUG_KMS("forcing a modeset in pipe B to free up FDI lanes\n");
+			ret = intel_add_crtc_modeset_to_state(state, crtc_b);
+		}
+
+		/*
+		 * must do this _after_ intel_add_crtc_modeset_to_state() since
+		 * otherwise we end up with the defaults
+		 */
+		crtc_state_b->max_fdi_lanes = 2;
+		crtc_state_c->max_fdi_lanes = 2;
+	} else {
+		DRM_DEBUG_KMS("pipe C doesn't require FDI lanes\n");
+
+		/* pipe C no longer needs FDI lanes, can give them to pipe B */
+		if (pipe_required_fdi_lanes(crtc_state_b) > 0 &&
+		    pipe_required_fdi_lanes(crtc_state_b) <= 2 &&
+		    crtc_state_b->bw_constrained) {
+			DRM_DEBUG_KMS("forcing a modeset in pipe B to use up FDI lanes\n");
+			ret = intel_add_crtc_modeset_to_state(state, crtc_b);
+		}
+
+		/*
+		 * must do this _after_ intel_add_crtc_modeset_to_state() since
+		 * otherwise we end up with the defaults
+		 */
+		crtc_state_b->max_fdi_lanes = 4;
+		crtc_state_c->max_fdi_lanes = 0;
+	}
+
+	return ret;
+}
+
+static void ironlake_fdi_compute_config(struct intel_crtc *crtc,
+					struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	const struct drm_display_mode *adjusted_mode =
+		&pipe_config->base.adjusted_mode;
+	int link_bw, fdi_dotclock;
 
-retry:
 	/* FDI is a binary signal running at ~2.7GHz, encoding
 	 * each output octet as 10 bits. The actual frequency
 	 * is stored as a divider into a 100MHz clock, and the
@@ -6584,30 +6617,36 @@ retry:
 
 	fdi_dotclock = adjusted_mode->crtc_clock;
 
-	lane = ironlake_get_lanes_required(fdi_dotclock, link_bw,
-					   pipe_config->pipe_bpp);
+	pipe_config->fdi_lanes =
+		ironlake_get_lanes_required(fdi_dotclock, link_bw,
+					    pipe_config->pipe_bpp);
 
-	pipe_config->fdi_lanes = lane;
+	intel_link_compute_m_n(pipe_config->pipe_bpp, pipe_config->fdi_lanes,
+			       fdi_dotclock, link_bw, &pipe_config->fdi_m_n);
+}
 
-	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
-			       link_bw, &pipe_config->fdi_m_n);
+static int ironlake_fdi_check_config(struct intel_crtc *crtc,
+				     struct intel_crtc_state *pipe_config)
+{
+	DRM_DEBUG_KMS("fdi lane config on pipe %c: %i lanes (max: %i)\n",
+		      pipe_name(crtc->pipe), pipe_config->fdi_lanes,
+		      pipe_config->max_fdi_lanes);
 
-	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
-				       intel_crtc->pipe, pipe_config);
-	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
+	if (pipe_config->fdi_lanes <= pipe_config->max_fdi_lanes)
+		return 0;
+
+	DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes (max: %i)\n",
+		      pipe_name(crtc->pipe), pipe_config->fdi_lanes,
+		      pipe_config->max_fdi_lanes);
+
+	if (pipe_config->pipe_bpp > 6*3) {
 		pipe_config->pipe_bpp -= 2*3;
-		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
-			      pipe_config->pipe_bpp);
-		needs_recompute = true;
 		pipe_config->bw_constrained = true;
 
-		goto retry;
+		return -EAGAIN;
 	}
 
-	if (needs_recompute)
-		return RETRY;
-
-	return ret;
+	return -EINVAL;
 }
 
 static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
@@ -6701,7 +6740,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		hsw_compute_ips_config(crtc, pipe_config);
 
 	if (pipe_config->has_pch_encoder)
-		return ironlake_fdi_compute_config(crtc, pipe_config);
+		return ironlake_fdi_check_config(crtc, pipe_config);
 
 	return 0;
 }
@@ -11986,7 +12025,7 @@ connected_sink_compute_bpp(struct intel_connector *connector,
 	}
 }
 
-static int
+static void
 compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 			  struct intel_crtc_state *pipe_config)
 {
@@ -12016,8 +12055,6 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 		connected_sink_compute_bpp(to_intel_connector(connector),
 					   pipe_config);
 	}
-
-	return bpp;
 }
 
 static void intel_dump_crtc_timings(const struct drm_display_mode *mode)
@@ -12047,9 +12084,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("cpu_transcoder: %c\n", transcoder_name(pipe_config->cpu_transcoder));
 	DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
 		      pipe_config->pipe_bpp, pipe_config->dither);
-	DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
+	DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, max_lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
 		      pipe_config->has_pch_encoder,
 		      pipe_config->fdi_lanes,
+		      pipe_config->max_fdi_lanes,
 		      pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
 		      pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
 		      pipe_config->fdi_m_n.tu);
@@ -12245,22 +12283,14 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	crtc_state->pch_pfit.force_thru = force_thru;
 }
 
-static int
-intel_modeset_pipe_config(struct drm_crtc *crtc,
-			  struct intel_crtc_state *pipe_config)
+static void
+intel_modeset_pipe_config_start(struct intel_crtc *crtc,
+				struct intel_crtc_state *pipe_config)
 {
-	struct drm_atomic_state *state = pipe_config->base.state;
-	struct intel_encoder *encoder;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	int base_bpp, ret = -EINVAL;
-	int i;
-	bool retry = true;
-
 	clear_intel_crtc_state(pipe_config);
 
 	pipe_config->cpu_transcoder =
-		(enum transcoder) to_intel_crtc(crtc)->pipe;
+		(enum transcoder) crtc->pipe;
 
 	/*
 	 * Sanitize sync polarity flags based on requested ones. If neither
@@ -12275,10 +12305,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	      (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
 		pipe_config->base.adjusted_mode.flags |= DRM_MODE_FLAG_NVSYNC;
 
-	base_bpp = compute_baseline_pipe_bpp(to_intel_crtc(crtc),
-					     pipe_config);
-	if (base_bpp < 0)
-		goto fail;
+	compute_baseline_pipe_bpp(crtc, pipe_config);
 
 	/*
 	 * Determine the real pipe dimensions. Note that stereo modes can
@@ -12292,7 +12319,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 			       &pipe_config->pipe_src_w,
 			       &pipe_config->pipe_src_h);
 
-encoder_retry:
+	/* initial assumption is no shared lanes */
+	if (HAS_DDI(crtc->base.dev) || crtc->pipe == PIPE_C)
+		pipe_config->max_fdi_lanes = 2;
+	else
+		pipe_config->max_fdi_lanes = 4;
+}
+
+static int
+intel_modeset_pipe_config_continue(struct intel_crtc *crtc,
+				   struct intel_crtc_state *pipe_config)
+{
+	struct drm_atomic_state *state = pipe_config->base.state;
+	struct intel_encoder *encoder;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	int i;
+
 	/* Ensure the port clock defaults are reset when retrying. */
 	pipe_config->port_clock = 0;
 	pipe_config->pixel_multiplier = 1;
@@ -12306,14 +12349,14 @@ encoder_retry:
 	 * a chance to reject the mode entirely.
 	 */
 	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != crtc)
+		if (connector_state->crtc != &crtc->base)
 			continue;
 
 		encoder = to_intel_encoder(connector_state->best_encoder);
 
 		if (!(encoder->compute_config(encoder, pipe_config))) {
 			DRM_DEBUG_KMS("Encoder config failure\n");
-			goto fail;
+			return -EINVAL;
 		}
 	}
 
@@ -12323,31 +12366,37 @@ encoder_retry:
 		pipe_config->port_clock = pipe_config->base.adjusted_mode.crtc_clock
 			* pipe_config->pixel_multiplier;
 
-	ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config);
-	if (ret < 0) {
-		DRM_DEBUG_KMS("CRTC fixup failed\n");
-		goto fail;
+	if (pipe_config->has_pch_encoder)
+		ironlake_fdi_compute_config(crtc, pipe_config);
+
+	return 0;
+}
+
+static int
+intel_modeset_pipe_config_finish(struct intel_crtc *crtc,
+				 struct intel_crtc_state *pipe_config)
+{
+	int ret;
+
+	ret = intel_crtc_compute_config(crtc, pipe_config);
+
+	if (ret == -EAGAIN) {
+		DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
+		return ret;
 	}
 
-	if (ret == RETRY) {
-		if (WARN(!retry, "loop in pipe configuration computation\n")) {
-			ret = -EINVAL;
-			goto fail;
-		}
-
-		DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
-		retry = false;
-		goto encoder_retry;
+	if (ret) {
+		DRM_DEBUG_KMS("CRTC fixup failed\n");
+		return ret;
 	}
 
 	/* Dithering seems to not pass-through bits correctly when it should, so
 	 * only enable it on 6bpc panels. */
 	pipe_config->dither = pipe_config->pipe_bpp == 6*3;
-	DRM_DEBUG_KMS("hw max bpp: %i, pipe bpp: %i, dithering: %i\n",
-		      base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
+	DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
+		      pipe_config->pipe_bpp, pipe_config->dither);
 
-fail:
-	return ret;
+	return 0;
 }
 
 static void
@@ -13194,15 +13243,13 @@ static int intel_atomic_check(struct drm_device *dev,
 	struct drm_crtc_state *crtc_state;
 	int ret, i;
 	bool any_ms = false;
+	bool retry;
 
 	ret = drm_atomic_helper_check_modeset(dev, state);
 	if (ret)
 		return ret;
 
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		struct intel_crtc_state *pipe_config =
-			to_intel_crtc_state(crtc_state);
-
 		memset(&to_intel_crtc(crtc)->atomic, 0,
 		       sizeof(struct intel_crtc_atomic_commit));
 
@@ -13222,13 +13269,55 @@ static int intel_atomic_check(struct drm_device *dev,
 		/* FIXME: For only active_changed we shouldn't need to do any
 		 * state recomputation at all. */
 
-		ret = drm_atomic_add_affected_connectors(state, crtc);
+		ret = intel_add_crtc_modeset_to_state(state,
+						      to_intel_crtc(crtc));
 		if (ret)
 			return ret;
+	}
 
-		ret = intel_modeset_pipe_config(crtc, pipe_config);
+	if (IS_IVYBRIDGE(dev)) {
+		ret = ivybridge_compute_fdi_bc_bifurcation(state);
 		if (ret)
 			return ret;
+	}
+
+	/*
+	 * Keep at it until we run out of ways
+	 * to reduce FDI bandwidth utilization.
+	 */
+	do {
+		retry = false;
+
+		for_each_crtc_in_state(state, crtc, crtc_state, i) {
+			struct intel_crtc_state *pipe_config =
+				to_intel_crtc_state(crtc_state);
+
+			if (!crtc_state->enable) {
+				if (needs_modeset(crtc_state))
+					any_ms = true;
+				continue;
+			}
+
+			if (!needs_modeset(crtc_state))
+				continue;
+
+			ret = intel_modeset_pipe_config_continue(to_intel_crtc(crtc),
+								 pipe_config);
+			if (ret)
+				return ret;
+
+			ret = intel_modeset_pipe_config_finish(to_intel_crtc(crtc),
+							       pipe_config);
+			if (ret == -EAGAIN)
+				retry = true;
+			else if (ret)
+				return ret;
+		}
+	} while (retry);
+
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		struct intel_crtc_state *pipe_config =
+			to_intel_crtc_state(crtc_state);
 
 		if (i915.fastboot &&
 		    intel_pipe_config_compare(state->dev,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ab5c147fa9e9..631a9be89fdf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -471,6 +471,7 @@ struct intel_crtc_state {
 	} pch_pfit;
 
 	/* FDI configuration, only valid if has_pch_encoder is set. */
+	int max_fdi_lanes;
 	int fdi_lanes;
 	struct intel_link_m_n fdi_m_n;
 
-- 
2.4.10

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

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2015-11-24 12:59 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-24 12:59 [PATCH] drm/i915: Solve IVB FDI bifurcation problems using atomic ville.syrjala

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