[PATCH] drm/i915: simplify DP/DDI port width macros

[PATCH] drm/i915: simplify DP/DDI port width macros

[Daniel Vetter]

If we ever leak a non-DP compliant port width through here, we have a
pretty serious issue. So just rip out all these WARNs - if we need
them it's probably better to have them at a central place where we
compute the dp lane count.

Also use the new DDI width macro for FDI mode.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h  | 11 ++---------
 drivers/gpu/drm/i915/intel_ddi.c | 34 ++--------------------------------
 drivers/gpu/drm/i915/intel_dp.c  | 12 +-----------
 3 files changed, 5 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 49033f0..25ccbff 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2660,9 +2660,7 @@
 #define   DP_PRE_EMPHASIS_SHIFT		22
 
 /* How many wires to use. I guess 3 was too hard */
-#define   DP_PORT_WIDTH_1		(0 << 19)
-#define   DP_PORT_WIDTH_2		(1 << 19)
-#define   DP_PORT_WIDTH_4		(3 << 19)
+#define   DP_PORT_WIDTH(width)		(((width) - 1) << 19)
 #define   DP_PORT_WIDTH_MASK		(7 << 19)
 
 /* Mystic DPCD version 1.1 special mode */
@@ -4747,9 +4745,6 @@
 #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
 #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
 #define  TRANS_DDI_BFI_ENABLE		(1<<4)
-#define  TRANS_DDI_PORT_WIDTH_X1	(0<<1)
-#define  TRANS_DDI_PORT_WIDTH_X2	(1<<1)
-#define  TRANS_DDI_PORT_WIDTH_X4	(3<<1)
 
 /* DisplayPort Transport Control */
 #define DP_TP_CTL_A			0x64040
@@ -4793,9 +4788,7 @@
 #define  DDI_BUF_PORT_REVERSAL			(1<<16)
 #define  DDI_BUF_IS_IDLE			(1<<7)
 #define  DDI_A_4_LANES				(1<<4)
-#define  DDI_PORT_WIDTH_X1			(0<<1)
-#define  DDI_PORT_WIDTH_X2			(1<<1)
-#define  DDI_PORT_WIDTH_X4			(3<<1)
+#define  DDI_PORT_WIDTH(width)			(((width) - 1) << 1)
 #define  DDI_INIT_DISPLAY_DETECTED		(1<<0)
 
 /* DDI Buffer Translations */
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 96354a5..b552aba 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -687,22 +687,7 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
 
 		intel_dp->DP = intel_dig_port->port_reversal |
 			       DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
-		switch (intel_dp->lane_count) {
-		case 1:
-			intel_dp->DP |= DDI_PORT_WIDTH_X1;
-			break;
-		case 2:
-			intel_dp->DP |= DDI_PORT_WIDTH_X2;
-			break;
-		case 4:
-			intel_dp->DP |= DDI_PORT_WIDTH_X4;
-			break;
-		default:
-			intel_dp->DP |= DDI_PORT_WIDTH_X4;
-			WARN(1, "Unexpected DP lane count %d\n",
-			     intel_dp->lane_count);
-			break;
-		}
+		intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
 
 		if (intel_dp->has_audio) {
 			DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n",
@@ -1031,22 +1016,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 
 		temp |= TRANS_DDI_MODE_SELECT_DP_SST;
 
-		switch (intel_dp->lane_count) {
-		case 1:
-			temp |= TRANS_DDI_PORT_WIDTH_X1;
-			break;
-		case 2:
-			temp |= TRANS_DDI_PORT_WIDTH_X2;
-			break;
-		case 4:
-			temp |= TRANS_DDI_PORT_WIDTH_X4;
-			break;
-		default:
-			temp |= TRANS_DDI_PORT_WIDTH_X4;
-			WARN(1, "Unsupported lane count %d\n",
-			     intel_dp->lane_count);
-		}
-
+		intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
 	} else {
 		WARN(1, "Invalid encoder type %d for pipe %c\n",
 		     intel_encoder->type, pipe_name(pipe));
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 994a05d..6d71c01 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -888,18 +888,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 
 	/* Handle DP bits in common between all three register formats */
 	intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0;
+	intel_dp->DP |= DP_PORT_WIDTH(intel_dp->lane_count);
 
-	switch (intel_dp->lane_count) {
-	case 1:
-		intel_dp->DP |= DP_PORT_WIDTH_1;
-		break;
-	case 2:
-		intel_dp->DP |= DP_PORT_WIDTH_2;
-		break;
-	case 4:
-		intel_dp->DP |= DP_PORT_WIDTH_4;
-		break;
-	}
 	if (intel_dp->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
 				 pipe_name(intel_crtc->pipe));
-- 
1.7.11.7

Re: [PATCH] drm/i915: simplify DP/DDI port width macros

[Chris Wilson]

On Wed, Apr 24, 2013 at 03:46:04PM +0200, Daniel Vetter wrote:
> If we ever leak a non-DP compliant port width through here, we have a
> pretty serious issue. So just rip out all these WARNs - if we need
> them it's probably better to have them at a central place where we
> compute the dp lane count.
> 
> Also use the new DDI width macro for FDI mode.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Nice. Do we have the central warn though?
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: simplify DP/DDI port width macros

[Daniel Vetter]

On Wed, Apr 24, 2013 at 02:52:52PM +0100, Chris Wilson wrote:
> On Wed, Apr 24, 2013 at 03:46:04PM +0200, Daniel Vetter wrote:
> > If we ever leak a non-DP compliant port width through here, we have a
> > pretty serious issue. So just rip out all these WARNs - if we need
> > them it's probably better to have them at a central place where we
> > compute the dp lane count.
> > 
> > Also use the new DDI width macro for FDI mode.
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Nice. Do we have the central warn though?
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Well the current dp lane count compute uses shifts, so is pretty much
guaranteed to end up with a power of two value. I've figured smashing WARN
right below that looks silly. Compared to eg the pipe bpc stuff we also
don't have different limits on different platforms, so either it's broken
everywhere or nowhere.

The only case that's different is fdi mode on haswell, but the old code
didn't have a paranoid check either. So I've figured I'll let it be.
Especially since the current code lets invalid configs slip through (that
needs my fdi patches to work properly) ...

All in all I'm voting for the "wait for godzilla to smash things" approach
here ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch