[03/12] drm/i915: Clarfify the DP code platform checks
diff mbox

Message ID 1430835458-11187-4-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show

Commit Message

Ville Syrjälä May 5, 2015, 2:17 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_dp.c is a mess with all the checks for different
platform/PCH variants and ports. Try to clean it up by recognizing
the following facts:
- IVB port A, and CPT port B/C/D are always the special cases
- VLV/CHV don't have port A
- Using the same kind of logic everywhere makes things much easier to
  parse

So let's move the IVB port A and PCH port B/C/D checks to be done first,
and let the other cases fall through, and always check for these things
using the same logic.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

Comments

Jesse Barnes May 21, 2015, 7:10 p.m. UTC | #1
On 05/05/2015 07:17 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_dp.c is a mess with all the checks for different
> platform/PCH variants and ports. Try to clean it up by recognizing
> the following facts:
> - IVB port A, and CPT port B/C/D are always the special cases
> - VLV/CHV don't have port A
> - Using the same kind of logic everywhere makes things much easier to
>   parse
> 
> So let's move the IVB port A and PCH port B/C/D checks to be done first,
> and let the other cases fall through, and always check for these things
> using the same logic.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 46 ++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e9426dd..0971a5d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1561,7 +1561,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
>  
>  	/* Split out the IBX/CPU vs CPT settings */
>  
> -	if (port == PORT_A && IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) {
> +	if (IS_GEN7(dev) && port == PORT_A) {
>  		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
>  			intel_dp->DP |= DP_SYNC_HS_HIGH;
>  		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> @@ -1572,7 +1572,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
>  			intel_dp->DP |= DP_ENHANCED_FRAMING;
>  
>  		intel_dp->DP |= crtc->pipe << 29;
> -	} else if (!HAS_PCH_CPT(dev) || port == PORT_A) {
> +	} else if (HAS_PCH_CPT(dev) && port != PORT_A) {
> +		intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
> +	} else {
>  		if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
>  			intel_dp->DP |= intel_dp->color_range;
>  
> @@ -1585,14 +1587,10 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
>  		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
>  			intel_dp->DP |= DP_ENHANCED_FRAMING;
>  
> -		if (!IS_CHERRYVIEW(dev)) {
> -			if (crtc->pipe == 1)
> -				intel_dp->DP |= DP_PIPEB_SELECT;
> -		} else {
> +		if (IS_CHERRYVIEW(dev))
>  			intel_dp->DP |= DP_PIPE_SELECT_CHV(crtc->pipe);
> -		}
> -	} else {
> -		intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
> +		else if (crtc->pipe == PIPE_B)
> +			intel_dp->DP |= DP_PIPEB_SELECT;
>  	}
>  }
>  
> @@ -2176,13 +2174,9 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
>  	if (!(tmp & DP_PORT_EN))
>  		return false;
>  
> -	if (port == PORT_A && IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) {
> +	if (IS_GEN7(dev) && port == PORT_A) {
>  		*pipe = PORT_TO_PIPE_CPT(tmp);
> -	} else if (IS_CHERRYVIEW(dev)) {
> -		*pipe = DP_PORT_TO_PIPE_CHV(tmp);
> -	} else if (!HAS_PCH_CPT(dev) || port == PORT_A) {
> -		*pipe = PORT_TO_PIPE(tmp);
> -	} else {
> +	} else if (HAS_PCH_CPT(dev) && port != PORT_A) {
>  		u32 trans_sel;
>  		u32 trans_dp;
>  		int i;
> @@ -2211,6 +2205,10 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
>  
>  		DRM_DEBUG_KMS("No pipe for dp port 0x%x found\n",
>  			      intel_dp->output_reg);
> +	} else if (IS_CHERRYVIEW(dev)) {
> +		*pipe = DP_PORT_TO_PIPE_CHV(tmp);
> +	} else {
> +		*pipe = PORT_TO_PIPE(tmp);
>  	}
>  
>  	return true;
> @@ -2231,24 +2229,24 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
>  		pipe_config->has_audio = true;
>  
> -	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
> -		if (tmp & DP_SYNC_HS_HIGH)
> +	if (HAS_PCH_CPT(dev) && port != PORT_A) {
> +		tmp = I915_READ(TRANS_DP_CTL(crtc->pipe));
> +		if (tmp & TRANS_DP_HSYNC_ACTIVE_HIGH)
>  			flags |= DRM_MODE_FLAG_PHSYNC;
>  		else
>  			flags |= DRM_MODE_FLAG_NHSYNC;
>  
> -		if (tmp & DP_SYNC_VS_HIGH)
> +		if (tmp & TRANS_DP_VSYNC_ACTIVE_HIGH)
>  			flags |= DRM_MODE_FLAG_PVSYNC;
>  		else
>  			flags |= DRM_MODE_FLAG_NVSYNC;
>  	} else {
> -		tmp = I915_READ(TRANS_DP_CTL(crtc->pipe));
> -		if (tmp & TRANS_DP_HSYNC_ACTIVE_HIGH)
> +		if (tmp & DP_SYNC_HS_HIGH)
>  			flags |= DRM_MODE_FLAG_PHSYNC;
>  		else
>  			flags |= DRM_MODE_FLAG_NHSYNC;
>  
> -		if (tmp & TRANS_DP_VSYNC_ACTIVE_HIGH)
> +		if (tmp & DP_SYNC_VS_HIGH)
>  			flags |= DRM_MODE_FLAG_PVSYNC;
>  		else
>  			flags |= DRM_MODE_FLAG_NVSYNC;
> @@ -2415,7 +2413,8 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
>  		}
>  		I915_WRITE(DP_TP_CTL(port), temp);
>  
> -	} else if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || port != PORT_A)) {
> +	} else if ((IS_GEN7(dev) && port == PORT_A) ||
> +		   (HAS_PCH_CPT(dev) && port != PORT_A)) {
>  		*DP &= ~DP_LINK_TRAIN_MASK_CPT;
>  
>  		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
> @@ -3842,7 +3841,8 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>  
>  	DRM_DEBUG_KMS("\n");
>  
> -	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || port != PORT_A)) {
> +	if ((IS_GEN7(dev) && port == PORT_A) ||
> +	    (HAS_PCH_CPT(dev) && port != PORT_A)) {
>  		DP &= ~DP_LINK_TRAIN_MASK_CPT;
>  		I915_WRITE(intel_dp->output_reg, DP | DP_LINK_TRAIN_PAT_IDLE_CPT);
>  	} else {
> 

Yeah a bit clearer.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e9426dd..0971a5d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1561,7 +1561,7 @@  static void intel_dp_prepare(struct intel_encoder *encoder)
 
 	/* Split out the IBX/CPU vs CPT settings */
 
-	if (port == PORT_A && IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) {
+	if (IS_GEN7(dev) && port == PORT_A) {
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
 			intel_dp->DP |= DP_SYNC_HS_HIGH;
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
@@ -1572,7 +1572,9 @@  static void intel_dp_prepare(struct intel_encoder *encoder)
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
 		intel_dp->DP |= crtc->pipe << 29;
-	} else if (!HAS_PCH_CPT(dev) || port == PORT_A) {
+	} else if (HAS_PCH_CPT(dev) && port != PORT_A) {
+		intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
+	} else {
 		if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
 			intel_dp->DP |= intel_dp->color_range;
 
@@ -1585,14 +1587,10 @@  static void intel_dp_prepare(struct intel_encoder *encoder)
 		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
-		if (!IS_CHERRYVIEW(dev)) {
-			if (crtc->pipe == 1)
-				intel_dp->DP |= DP_PIPEB_SELECT;
-		} else {
+		if (IS_CHERRYVIEW(dev))
 			intel_dp->DP |= DP_PIPE_SELECT_CHV(crtc->pipe);
-		}
-	} else {
-		intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
+		else if (crtc->pipe == PIPE_B)
+			intel_dp->DP |= DP_PIPEB_SELECT;
 	}
 }
 
@@ -2176,13 +2174,9 @@  static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	if (!(tmp & DP_PORT_EN))
 		return false;
 
-	if (port == PORT_A && IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) {
+	if (IS_GEN7(dev) && port == PORT_A) {
 		*pipe = PORT_TO_PIPE_CPT(tmp);
-	} else if (IS_CHERRYVIEW(dev)) {
-		*pipe = DP_PORT_TO_PIPE_CHV(tmp);
-	} else if (!HAS_PCH_CPT(dev) || port == PORT_A) {
-		*pipe = PORT_TO_PIPE(tmp);
-	} else {
+	} else if (HAS_PCH_CPT(dev) && port != PORT_A) {
 		u32 trans_sel;
 		u32 trans_dp;
 		int i;
@@ -2211,6 +2205,10 @@  static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 
 		DRM_DEBUG_KMS("No pipe for dp port 0x%x found\n",
 			      intel_dp->output_reg);
+	} else if (IS_CHERRYVIEW(dev)) {
+		*pipe = DP_PORT_TO_PIPE_CHV(tmp);
+	} else {
+		*pipe = PORT_TO_PIPE(tmp);
 	}
 
 	return true;
@@ -2231,24 +2229,24 @@  static void intel_dp_get_config(struct intel_encoder *encoder,
 	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
 		pipe_config->has_audio = true;
 
-	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
-		if (tmp & DP_SYNC_HS_HIGH)
+	if (HAS_PCH_CPT(dev) && port != PORT_A) {
+		tmp = I915_READ(TRANS_DP_CTL(crtc->pipe));
+		if (tmp & TRANS_DP_HSYNC_ACTIVE_HIGH)
 			flags |= DRM_MODE_FLAG_PHSYNC;
 		else
 			flags |= DRM_MODE_FLAG_NHSYNC;
 
-		if (tmp & DP_SYNC_VS_HIGH)
+		if (tmp & TRANS_DP_VSYNC_ACTIVE_HIGH)
 			flags |= DRM_MODE_FLAG_PVSYNC;
 		else
 			flags |= DRM_MODE_FLAG_NVSYNC;
 	} else {
-		tmp = I915_READ(TRANS_DP_CTL(crtc->pipe));
-		if (tmp & TRANS_DP_HSYNC_ACTIVE_HIGH)
+		if (tmp & DP_SYNC_HS_HIGH)
 			flags |= DRM_MODE_FLAG_PHSYNC;
 		else
 			flags |= DRM_MODE_FLAG_NHSYNC;
 
-		if (tmp & TRANS_DP_VSYNC_ACTIVE_HIGH)
+		if (tmp & DP_SYNC_VS_HIGH)
 			flags |= DRM_MODE_FLAG_PVSYNC;
 		else
 			flags |= DRM_MODE_FLAG_NVSYNC;
@@ -2415,7 +2413,8 @@  _intel_dp_set_link_train(struct intel_dp *intel_dp,
 		}
 		I915_WRITE(DP_TP_CTL(port), temp);
 
-	} else if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || port != PORT_A)) {
+	} else if ((IS_GEN7(dev) && port == PORT_A) ||
+		   (HAS_PCH_CPT(dev) && port != PORT_A)) {
 		*DP &= ~DP_LINK_TRAIN_MASK_CPT;
 
 		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
@@ -3842,7 +3841,8 @@  intel_dp_link_down(struct intel_dp *intel_dp)
 
 	DRM_DEBUG_KMS("\n");
 
-	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || port != PORT_A)) {
+	if ((IS_GEN7(dev) && port == PORT_A) ||
+	    (HAS_PCH_CPT(dev) && port != PORT_A)) {
 		DP &= ~DP_LINK_TRAIN_MASK_CPT;
 		I915_WRITE(intel_dp->output_reg, DP | DP_LINK_TRAIN_PAT_IDLE_CPT);
 	} else {