Re: [PATCH 01/22] drm/i915: Don't pass *DP around to link training functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Thanks for making the changes suggested :)

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@xxxxxxxxx>

On 10/23/2015 3:31 PM, Ander Conselvan de Oliveira wrote:

It just makes the code more confusing, so just reference intel_dp_>DP
directly.

Note that this also fix a bug where the value of intel_dp->DP could be
different than the last value written to the hw, due to an early return
that would skip the 'intel_dp->DP = DP' line.

v2: Don't preserve old DP value on failure. (Sivakumar)
   - Don't call drm_dp_clock_recovery_ok() twice. (Sivakumar)
   - Keep return type of clock recovery and channel equalization
     functions as void. (Ander)

v3: Remove DP parameter from intel_dp_set_signal_levels(). (Sivakumar)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_dp.c | 47 ++++++++++++++++++-----------------------
  1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8287df4..648300e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3552,7 +3552,7 @@ gen7_edp_signal_levels(uint8_t train_set)
/* Properly updates "DP" with the correct signal levels. */ 
  static void
-intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
+intel_dp_set_signal_levels(struct intel_dp *intel_dp)
  {
  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
  	enum port port = intel_dig_port->port;
@@ -3591,12 +3591,11 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
  		(train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
  			DP_TRAIN_PRE_EMPHASIS_SHIFT);
- *DP = (*DP & ~mask) | signal_levels; 
+	intel_dp->DP = (intel_dp->DP & ~mask) | signal_levels;
  }
static bool 
  intel_dp_set_link_train(struct intel_dp *intel_dp,
-			uint32_t *DP,
  			uint8_t dp_train_pat)
  {
  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -3605,9 +3604,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
  	uint8_t buf[sizeof(intel_dp->train_set) + 1];
  	int ret, len;
- _intel_dp_set_link_train(intel_dp, DP, dp_train_pat); 
+	_intel_dp_set_link_train(intel_dp, &intel_dp->DP, dp_train_pat);
- I915_WRITE(intel_dp->output_reg, *DP); 
+	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
  	POSTING_READ(intel_dp->output_reg);
buf[0] = dp_train_pat; 
@@ -3628,17 +3627,17 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
  }
static bool 
-intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
+intel_dp_reset_link_train(struct intel_dp *intel_dp,
  			uint8_t dp_train_pat)
  {
  	if (!intel_dp->train_set_valid)
  		memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
-	intel_dp_set_signal_levels(intel_dp, DP);
-	return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
+	intel_dp_set_signal_levels(intel_dp);
+	return intel_dp_set_link_train(intel_dp, dp_train_pat);
  }
static bool 
-intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
+intel_dp_update_link_train(struct intel_dp *intel_dp,
  			   const uint8_t link_status[DP_LINK_STATUS_SIZE])
  {
  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -3647,9 +3646,9 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
  	int ret;
intel_get_adjust_train(intel_dp, link_status); 
-	intel_dp_set_signal_levels(intel_dp, DP);
+	intel_dp_set_signal_levels(intel_dp);
- I915_WRITE(intel_dp->output_reg, *DP); 
+	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
  	POSTING_READ(intel_dp->output_reg);
ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, 
@@ -3698,7 +3697,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
  	int i;
  	uint8_t voltage;
  	int voltage_tries, loop_tries;
-	uint32_t DP = intel_dp->DP;
  	uint8_t link_config[2];
  	uint8_t link_bw, rate_select;
@@ -3722,10 +3720,10 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp) 
  	link_config[1] = DP_SET_ANSI_8B10B;
  	drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
- DP |= DP_PORT_EN; 
+	intel_dp->DP |= DP_PORT_EN;
/* clock recovery */ 
-	if (!intel_dp_reset_link_train(intel_dp, &DP,
+	if (!intel_dp_reset_link_train(intel_dp,
  				       DP_TRAINING_PATTERN_1 |
  				       DP_LINK_SCRAMBLING_DISABLE)) {
  		DRM_ERROR("failed to enable link training\n");
@@ -3757,7 +3755,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
  			DRM_DEBUG_KMS("clock recovery not ok, reset");
  			/* clear the flag as we are not reusing train set */
  			intel_dp->train_set_valid = false;
-			if (!intel_dp_reset_link_train(intel_dp, &DP,
+			if (!intel_dp_reset_link_train(intel_dp,
  						       DP_TRAINING_PATTERN_1 |
  						       DP_LINK_SCRAMBLING_DISABLE)) {
  				DRM_ERROR("failed to enable link training\n");
@@ -3776,7 +3774,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
  				DRM_ERROR("too many full retries, give up\n");
  				break;
  			}
-			intel_dp_reset_link_train(intel_dp, &DP,
+			intel_dp_reset_link_train(intel_dp,
  						  DP_TRAINING_PATTERN_1 |
  						  DP_LINK_SCRAMBLING_DISABLE);
  			voltage_tries = 0;
@@ -3795,13 +3793,11 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
  		voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
/* Update training set as requested by target */ 
-		if (!intel_dp_update_link_train(intel_dp, &DP, link_status)) {
+		if (!intel_dp_update_link_train(intel_dp, link_status)) {
  			DRM_ERROR("failed to update link training\n");
  			break;
  		}
  	}
-
-	intel_dp->DP = DP;
  }
static void 
@@ -3811,7 +3807,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  	struct drm_device *dev = dig_port->base.base.dev;
  	bool channel_eq = false;
  	int tries, cr_tries;
-	uint32_t DP = intel_dp->DP;
  	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
/* 
@@ -3830,7 +3825,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  		DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
/* channel equalization */ 
-	if (!intel_dp_set_link_train(intel_dp, &DP,
+	if (!intel_dp_set_link_train(intel_dp,
  				     training_pattern |
  				     DP_LINK_SCRAMBLING_DISABLE)) {
  		DRM_ERROR("failed to start channel equalization\n");
@@ -3859,7 +3854,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  					      intel_dp->lane_count)) {
  			intel_dp->train_set_valid = false;
  			intel_dp_link_training_clock_recovery(intel_dp);
-			intel_dp_set_link_train(intel_dp, &DP,
+			intel_dp_set_link_train(intel_dp,
  						training_pattern |
  						DP_LINK_SCRAMBLING_DISABLE);
  			cr_tries++;
@@ -3876,7 +3871,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  		if (tries > 5) {
  			intel_dp->train_set_valid = false;
  			intel_dp_link_training_clock_recovery(intel_dp);
-			intel_dp_set_link_train(intel_dp, &DP,
+			intel_dp_set_link_train(intel_dp,
  						training_pattern |
  						DP_LINK_SCRAMBLING_DISABLE);
  			tries = 0;
@@ -3885,7 +3880,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  		}
/* Update training set as requested by target */ 
-		if (!intel_dp_update_link_train(intel_dp, &DP, link_status)) {
+		if (!intel_dp_update_link_train(intel_dp, link_status)) {
  			DRM_ERROR("failed to update link training\n");
  			break;
  		}
@@ -3894,8 +3889,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
intel_dp_set_idle_link_train(intel_dp); - intel_dp->DP = DP; 
-
  	if (channel_eq) {
  		intel_dp->train_set_valid = true;
  		DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n");
@@ -3904,7 +3897,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
void intel_dp_stop_link_train(struct intel_dp *intel_dp) 
  {
-	intel_dp_set_link_train(intel_dp, &intel_dp->DP,
+	intel_dp_set_link_train(intel_dp,
  				DP_TRAINING_PATTERN_DISABLE);
  }


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux