All of lore.kernel.org
 help / color / mirror / Atom feed
From: ville.syrjala@linux.intel.com
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH v2] drm/i915: Put back lane_count into intel_dp and add link_rate too
Date: Mon, 17 Aug 2015 18:05:12 +0300	[thread overview]
Message-ID: <1439823912-1813-1-git-send-email-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <1439813939-32568-1-git-send-email-ville.syrjala@linux.intel.com>

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

With MST there won't be a crtc assigned to the main link encoder, so
trying to dig up the pipe_config from there is a recipe for an oops.

Instead store the parameters (lane_count and link_rate) in the encoder,
and use those values during link training etc. Since those parameters
are now assigned only when the link is actually enabled,
.compute_config() won't clobber them as it did before.

Hardware state readout is still bonkers though as we don't transfer the
link parameters from pipe_config intel_dp. We should do that during
encoder sanitation. But since we don't even do a proper job of reading
out the main link encoder state for MST there's littel point in
worrying about this now.

Fixes a regression with MST caused by:
 commit 90a6b7b052b1aa17fbb98b049e9c8b7f729c35a7
 Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
 Date:   Mon Jul 6 16:39:15 2015 +0300

    drm/i915: Move intel_dp->lane_count into pipe_config

v2: Different apporoach that should keep intel_dp_check_mst_status()
    somewhat less oopsy

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    |  5 ++--
 drivers/gpu/drm/i915/intel_dp.c     | 53 +++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_dp_mst.c |  2 ++
 drivers/gpu/drm/i915/intel_drv.h    |  4 +++
 4 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 56d778f..5dff8b7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -728,11 +728,10 @@ void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct intel_digital_port *intel_dig_port =
 		enc_to_dig_port(&encoder->base);
-	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 
 	intel_dp->DP = intel_dig_port->saved_port_bits |
 		DDI_BUF_CTL_ENABLE | DDI_BUF_TRANS_SELECT(0);
-	intel_dp->DP |= DDI_PORT_WIDTH(crtc->config->lane_count);
+	intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
 }
 
 static struct intel_encoder *
@@ -2314,6 +2313,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+		intel_dp_set_link_params(intel_dp, crtc->config);
+
 		intel_ddi_init_dp_buf_reg(intel_encoder);
 
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b905c19..dcda86a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1584,6 +1584,13 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp)
 	udelay(500);
 }
 
+void intel_dp_set_link_params(struct intel_dp *intel_dp,
+			      const struct intel_crtc_state *pipe_config)
+{
+	intel_dp->link_rate = pipe_config->port_clock;
+	intel_dp->lane_count = pipe_config->lane_count;
+}
+
 static void intel_dp_prepare(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -1593,6 +1600,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 
+	intel_dp_set_link_params(intel_dp, crtc->config);
+
 	/*
 	 * There are four kinds of DP registers:
 	 *
@@ -3348,15 +3357,13 @@ static void
 intel_get_adjust_train(struct intel_dp *intel_dp,
 		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
-	struct intel_crtc *crtc =
-		to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
 	uint8_t v = 0;
 	uint8_t p = 0;
 	int lane;
 	uint8_t voltage_max;
 	uint8_t preemph_max;
 
-	for (lane = 0; lane < crtc->config->lane_count; lane++) {
+	for (lane = 0; lane < intel_dp->lane_count; lane++) {
 		uint8_t this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
 		uint8_t this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
 
@@ -3527,8 +3534,6 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv =
 		to_i915(intel_dig_port->base.base.dev);
-	struct intel_crtc *crtc =
-		to_intel_crtc(intel_dig_port->base.base.crtc);
 	uint8_t buf[sizeof(intel_dp->train_set) + 1];
 	int ret, len;
 
@@ -3544,8 +3549,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 		len = 1;
 	} else {
 		/* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET */
-		memcpy(buf + 1, intel_dp->train_set, crtc->config->lane_count);
-		len = crtc->config->lane_count + 1;
+		memcpy(buf + 1, intel_dp->train_set, intel_dp->lane_count);
+		len = intel_dp->lane_count + 1;
 	}
 
 	ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
@@ -3571,8 +3576,6 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv =
 		to_i915(intel_dig_port->base.base.dev);
-	struct intel_crtc *crtc =
-		to_intel_crtc(intel_dig_port->base.base.crtc);
 	int ret;
 
 	intel_get_adjust_train(intel_dp, link_status);
@@ -3582,9 +3585,9 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
 	POSTING_READ(intel_dp->output_reg);
 
 	ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
-				intel_dp->train_set, crtc->config->lane_count);
+				intel_dp->train_set, intel_dp->lane_count);
 
-	return ret == crtc->config->lane_count;
+	return ret == intel_dp->lane_count;
 }
 
 static void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
@@ -3623,8 +3626,6 @@ void
 intel_dp_start_link_train(struct intel_dp *intel_dp)
 {
 	struct drm_encoder *encoder = &dp_to_dig_port(intel_dp)->base.base;
-	struct intel_crtc *crtc =
-		to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
 	struct drm_device *dev = encoder->dev;
 	int i;
 	uint8_t voltage;
@@ -3636,12 +3637,12 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	if (HAS_DDI(dev))
 		intel_ddi_prepare_link_retrain(encoder);
 
-	intel_dp_compute_rate(intel_dp, crtc->config->port_clock,
+	intel_dp_compute_rate(intel_dp, intel_dp->link_rate,
 			      &link_bw, &rate_select);
 
 	/* Write the link configuration data */
 	link_config[0] = link_bw;
-	link_config[1] = crtc->config->lane_count;
+	link_config[1] = intel_dp->lane_count;
 	if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
 		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
 	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
@@ -3675,7 +3676,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 			break;
 		}
 
-		if (drm_dp_clock_recovery_ok(link_status, crtc->config->lane_count)) {
+		if (drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
 			DRM_DEBUG_KMS("clock recovery OK\n");
 			break;
 		}
@@ -3698,10 +3699,10 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 		}
 
 		/* Check to see if we've tried the max voltage */
-		for (i = 0; i < crtc->config->lane_count; i++)
+		for (i = 0; i < intel_dp->lane_count; i++)
 			if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0)
 				break;
-		if (i == crtc->config->lane_count) {
+		if (i == intel_dp->lane_count) {
 			++loop_tries;
 			if (loop_tries == 5) {
 				DRM_ERROR("too many full retries, give up\n");
@@ -3738,15 +3739,13 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 void
 intel_dp_complete_link_train(struct intel_dp *intel_dp)
 {
-	struct intel_crtc *crtc =
-		to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
 	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
 
 	/* Training Pattern 3 for HBR2 or 1.2 devices that support it*/
-	if (crtc->config->port_clock == 540000 || intel_dp->use_tps3)
+	if (intel_dp->link_rate == 540000 || intel_dp->use_tps3)
 		training_pattern = DP_TRAINING_PATTERN_3;
 
 	/* channel equalization */
@@ -3776,7 +3775,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 
 		/* Make sure clock is still ok */
 		if (!drm_dp_clock_recovery_ok(link_status,
-					      crtc->config->lane_count)) {
+					      intel_dp->lane_count)) {
 			intel_dp->train_set_valid = false;
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
@@ -3787,7 +3786,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		}
 
 		if (drm_dp_channel_eq_ok(link_status,
-					 crtc->config->lane_count)) {
+					 intel_dp->lane_count)) {
 			channel_eq = true;
 			break;
 		}
@@ -4285,8 +4284,6 @@ update_status:
 static int
 intel_dp_check_mst_status(struct intel_dp *intel_dp)
 {
-	struct intel_crtc *crtc =
-		to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
 	bool bret;
 
 	if (intel_dp->is_mst) {
@@ -4300,7 +4297,7 @@ go_again:
 
 			/* check link status - esi[10] = 0x200c */
 			if (intel_dp->active_mst_links &&
-			    !drm_dp_channel_eq_ok(&esi[10], crtc->config->lane_count)) {
+			    !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
 				DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
 				intel_dp_start_link_train(intel_dp);
 				intel_dp_complete_link_train(intel_dp);
@@ -4355,8 +4352,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
-	struct intel_crtc *crtc =
-		to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
@@ -4392,7 +4387,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	if (!drm_dp_channel_eq_ok(link_status, crtc->config->lane_count)) {
+	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
 		intel_dp_start_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 9ec5c20..ebf2054 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -165,6 +165,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 	if (intel_dp->active_mst_links == 0) {
 		enum port port = intel_ddi_get_encoder_port(encoder);
 
+		intel_dp_set_link_params(intel_dp, intel_crtc->config);
+
 		/* FIXME: add support for SKL */
 		if (INTEL_INFO(dev)->gen < 9)
 			I915_WRITE(PORT_CLK_SEL(port),
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 81b7d77..71a2e18 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -708,6 +708,8 @@ struct intel_dp {
 	uint32_t output_reg;
 	uint32_t aux_ch_ctl_reg;
 	uint32_t DP;
+	int link_rate;
+	uint8_t lane_count;
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool limited_color_range;
@@ -1161,6 +1163,8 @@ void assert_csr_loaded(struct drm_i915_private *dev_priv);
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
 bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 			     struct intel_connector *intel_connector);
+void intel_dp_set_link_params(struct intel_dp *intel_dp,
+			      const struct intel_crtc_state *pipe_config);
 void intel_dp_start_link_train(struct intel_dp *intel_dp);
 void intel_dp_complete_link_train(struct intel_dp *intel_dp);
 void intel_dp_stop_link_train(struct intel_dp *intel_dp);
-- 
2.4.6

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

  reply	other threads:[~2015-08-17 15:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-17 12:18 [PATCH] drm/i915: Pass pipe_config to DP link training functions ville.syrjala
2015-08-17 15:05 ` ville.syrjala [this message]
2015-08-18 11:56   ` [PATCH v2] drm/i915: Put back lane_count into intel_dp and add link_rate too Maarten Lankhorst
2015-08-26  7:58     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1439823912-1813-1-git-send-email-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.