[PATCH 00/15] high-bpp fixes and fdi auto dithering

[PATCH 00/15] high-bpp fixes and fdi auto dithering

[Daniel Vetter]

Hi all,

This fixes all the bugs I've found in my various systems when using non-24bpp
modes as the first part of the series.

And with working non-standard bpp support I've figured we can go fancy and
implemented auto-dithering if we hit an fdi bw limit. Which means that you can
now use 3-pipe pch configurations on ivb on pretty much everywhere. The only
restriction is that you need to fire up pipe C first, since without atomic
modeset pipe B will otherwise too much bw.

One big thing here is that this will break Paulo's hsw eDP machine, specifically
the patch called "drm/i915: force bpp for eDP panels". But apparently without
that my machine here is broken ... Ideas highly welcome about how we could quirk
ourselves out of this mess.

Cheers, Daniel

Daniel Vetter (15):
  drm/i915: fixup 12bpc hdmi dotclock handling
  drm/i915: Disable high-bpc on pre-1.4 EDID screens
  drm/i915: force bpp for eDP panels
  drm/i915: drop adjusted_mode from *_set_pipeconf functions
  drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv
  drm/i915: allow high-bpc modes on DP
  drm/i915: Fixup non-24bpp support for VGA screens on Haswell
  drm/i915: move intel_crtc->fdi_lanes to pipe_config
  drm/i915: hw state readout support for pipe_config->fdi_lanes
  drm/i915: split up fdi_set_m_n into computation and hw setup
  drm/i915: compute fdi lane config earlier
  drm/i915: Split up ironlake_check_fdi_lanes
  drm/i915: move fdi lane configuration checks ahead
  drm/i915: don't count cpu ports for fdi B/C lane sharing
  drm/i915: implement fdi auto-dithering

 drivers/gpu/drm/i915/intel_crt.c     |   4 +
 drivers/gpu/drm/i915/intel_ddi.c     |   7 +-
 drivers/gpu/drm/i915/intel_display.c | 349 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      |  12 +-
 drivers/gpu/drm/i915/intel_drv.h     |  12 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |  31 +++-
 drivers/gpu/drm/i915/intel_lvds.c    |   4 +-
 7 files changed, 275 insertions(+), 144 deletions(-)

-- 
1.7.11.7

[PATCH 01/15] drm/i915: fixup 12bpc hdmi dotclock handling

[Daniel Vetter]

We need to multiply the hdmi port dotclock by 1.5x since it's not
really a dotclock, but the 10/8 encoding bitclock divided by 10.

Also add correct limit checks for the dotclock and reject modes which
don't fit. HDMI 1.4 would allow more, but our hw doesn't support that
unfortunately :(

Somehow I suspect 12bpc hdmi output never really worked - we really
need an i-g-t testcase to check all the different pixel modes and
outputs.

v2: Fixup the adjusted port clock handling - we need to make sure that
the fdi link code still gets the real pixelclock.

v3: g4x/vlv don't support 12bpc hdmi output so drop the bogus comment.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 53ce8a5..a8273c7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -783,6 +783,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
 
 	if (intel_hdmi->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -802,16 +803,28 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	/*
 	 * HDMI is either 12 or 8, so if the display lets 10bpc sneak
 	 * through, clamp it down. Note that g4x/vlv don't support 12bpc hdmi
-	 * outputs.
+	 * outputs. We also need to check that the higher clock still fits
+	 * within limits.
 	 */
-	if (pipe_config->pipe_bpp > 8*3 && HAS_PCH_SPLIT(dev)) {
+	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
+	    && HAS_PCH_SPLIT(dev)) {
 		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
 		pipe_config->pipe_bpp = 12*3;
+
+		/* Need to adjust the port link by 1.5x for 12bpc. */
+		adjusted_mode->clock = clock_12bpc;
+		pipe_config->pixel_target_clock =
+			pipe_config->requested_mode.clock;
 	} else {
 		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
 		pipe_config->pipe_bpp = 8*3;
 	}
 
+	if (adjusted_mode->clock > 225000) {
+		DRM_DEBUG_KMS("too high HDMI clock, rejecting mode\n");
+		return false;
+	}
+
 	return true;
 }
 
-- 
1.7.11.7

[PATCH 02/15] drm/i915: Disable high-bpc on pre-1.4 EDID screens

[Daniel Vetter]

Prevents black screens when using 30bpp framebuffers on my
HDMI screens here. The DP input on the same screen though reports a
1.4 EDID with the correct 8bpc limit set.

v2: Actually check for the right thing!

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6e265b0..1e6efab 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7612,6 +7612,13 @@ pipe_config_set_bpp(struct drm_crtc *crtc,
 				      bpp, connector->display_info.bpc*3);
 			pipe_config->pipe_bpp = connector->display_info.bpc*3;
 		}
+
+		/* Clamp bpp to 8 on screens without EDID 1.4 */
+		if (connector->display_info.bpc == 0 && bpp > 24) {
+			DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of 24\n",
+				      bpp);
+			pipe_config->pipe_bpp = 24;
+		}
 	}
 
 	return bpp;
-- 
1.7.11.7

[PATCH 03/15] drm/i915: force bpp for eDP panels

[Daniel Vetter]

We've had our fair share of woes already which showed that we can't
rely on the bpc limits in the EDID for eDP panels without risking
black screens. So now we limit the depth by what the BIOS recommends
in the VBT:

commit 2f4f649a69a9eb51f6e98130e19dd90a260a4145
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Nov 12 14:33:44 2012 +0200

    drm/i915: do not ignore eDP bpc settings from vbt

But that's not enough, since at least the panel on my ASUS Zenbook
Prime here is also unhappy if the bpc is too low. Hence just take the
firmware value and dither to get what flimsy panels want.

Like before we ensure that we don't change the bpp if the firmware
doesn't provide a value, see

commit 9a30a61f3516871c5c638fd7c025fbaa11ddf7fe
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Nov 12 14:33:45 2012 +0200

    drm/i915: do not default to 18 bpp for eDP if missing from VBT

FIXME: This conflicts with the bit of duct-tape added to make Paulo's
Haswell machine work again!!!!

Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b1a3a64..1ac7235 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -745,6 +745,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
 	 * bpc in between. */
 	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
+
+	/* eDP panels are really fickle, try to enfore the bpp the firmware
+	 * recomments. This means we'll up-dither 16bpp framebuffers on
+	 * high-depth panels. */
+	if (is_edp(intel_dp) && dev_priv->edp.bpp) {
+		DRM_DEBUG_KMS("forcing bpp for eDP panel to BIOS-provided %i\n",
+			      dev_priv->edp.bpp);
+		bpp = dev_priv->edp.bpp;
+	}
+
 	for (; bpp >= 6*3; bpp -= 2*3) {
 		mode_rate = intel_dp_link_required(target_clock, bpp);
 
-- 
1.7.11.7

[PATCH 04/15] drm/i915: drop adjusted_mode from *_set_pipeconf functions

[Daniel Vetter]

They can get at the adjusted mode through intel_crtc->config.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1e6efab..8c36376 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5151,8 +5151,7 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
 	return 120000;
 }
 
-static void ironlake_set_pipeconf(struct drm_crtc *crtc,
-				  struct drm_display_mode *adjusted_mode)
+static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -5185,7 +5184,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
 		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
 
 	val &= ~PIPECONF_INTERLACE_MASK;
-	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
+	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
 		val |= PIPECONF_INTERLACED_ILK;
 	else
 		val |= PIPECONF_PROGRESSIVE;
@@ -5263,8 +5262,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
 	}
 }
 
-static void haswell_set_pipeconf(struct drm_crtc *crtc,
-				 struct drm_display_mode *adjusted_mode)
+static void haswell_set_pipeconf(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -5278,7 +5276,7 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
 		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
 
 	val &= ~PIPECONF_INTERLACE_MASK_HSW;
-	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
+	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
 		val |= PIPECONF_INTERLACED_ILK;
 	else
 		val |= PIPECONF_PROGRESSIVE;
@@ -5737,7 +5735,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
-	ironlake_set_pipeconf(crtc, adjusted_mode);
+	ironlake_set_pipeconf(crtc);
 
 	/* Set up the display plane register */
 	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
@@ -5862,7 +5860,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	if (intel_crtc->config.has_pch_encoder)
 		ironlake_fdi_set_m_n(crtc);
 
-	haswell_set_pipeconf(crtc, adjusted_mode);
+	haswell_set_pipeconf(crtc);
 
 	intel_set_pipe_csc(crtc);
 
-- 
1.7.11.7

[PATCH 05/15] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

The current code is rather ... ugly. The only thing it managed to pull
off is getting 6bpc on DP working on g4x. Then someone added another
custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
implementing the PIPECONF-based dither/bpc controls on g4x/vlv.

Note that compared to pch based platforms g4x/vlv don't support 12bpc
modes. g4x is already caught, extend the check for vlv.

The other fixup is to restrict the lvds-specific dithering to early
gen4 devices - g4x should use the pipeconf dither controls. Note that
on gen2/3 the dither control is in the panel fitter even.

v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":

"Programming note: Dithering should only be enabled for 8 bpc or 6
bpc."

v3: Actually drop the old ugly dither code.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8c36376..c2579c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4605,22 +4605,27 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
 	}
 
-	/* default to 8bpc */
-	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
-	if (intel_crtc->config.has_dp_encoder) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-				    PIPECONF_DITHER_EN |
+	/* only g4x and later have fancy bpc/dither controls */
+	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
+		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
+		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
+			pipeconf |= PIPECONF_DITHER_EN |
 				    PIPECONF_DITHER_TYPE_SP;
-		}
-	}
 
-	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
-						      INTEL_OUTPUT_EDP)) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-					PIPECONF_ENABLE |
-					I965_PIPECONF_ACTIVE;
+		pipeconf &= ~PIPECONF_BPC_MASK;
+		switch (intel_crtc->config.pipe_bpp) {
+		case 18:
+			pipeconf |= PIPECONF_6BPC;
+			break;
+		case 24:
+			pipeconf |= PIPECONF_8BPC;
+			break;
+		case 30:
+			pipeconf |= PIPECONF_10BPC;
+			break;
+		default:
+			/* Case prevented by intel_choose_pipe_bpp_dither. */
+			BUG();
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 58a98ff..094f3c5 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
 	/* Set the dithering flag on LVDS as needed, note that there is no
 	 * special lvds dither control bit on pch-split platforms, dithering is
 	 * only controlled through the PIPECONF reg. */
-	if (INTEL_INFO(dev)->gen == 4) {
+	if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) {
 		if (intel_crtc->config.dither)
 			temp |= LVDS_ENABLE_DITHER;
 		else
-- 
1.7.11.7

[PATCH 06/15] drm/i915: allow high-bpc modes on DP

[Daniel Vetter]

Totally untested due to lack of screens supporting more than 8bpc. But
now we should have closed all holes in our bpp handling, so this
should be safe. The last missing piece was 10bpc support for g4x/vlv,
since we directly use the pipe bpp to feed the display link (and
anyway, only the cpt has any means to have a pipe bpp != the display
link bpp).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1ac7235..50a9d9f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -744,7 +744,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
 	 * bpc in between. */
-	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
+	bpp = pipe_config->pipe_bpp;
 
 	/* eDP panels are really fickle, try to enfore the bpp the firmware
 	 * recomments. This means we'll up-dither 16bpp framebuffers on
-- 
1.7.11.7

[PATCH 07/15] drm/i915: Fixup non-24bpp support for VGA screens on Haswell

[Daniel Vetter]

The LPT PCH only supports 8bpc, so we need to force the pipe bpp
to the right value.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_crt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 58b4a53..1b9ebf4 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -207,6 +207,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
 	if (HAS_PCH_SPLIT(dev))
 		pipe_config->has_pch_encoder = true;
 
+	/* LPT FDI RX only supports 8bpc. */
+	if (HAS_PCH_LPT(dev))
+		pipe_config->pipe_bpp = 24;
+
 	return true;
 }
 
-- 
1.7.11.7

[PATCH 08/15] drm/i915: move intel_crtc->fdi_lanes to pipe_config

[Daniel Vetter]

We need this for two reasons:
- Correct handling of shared fdi lanes on ivb with fastboot.
- Handling fdi link bw limits when we only have two fdi lanes by
  dithering down a bit.

Just search&replace in this patch, no functional change at all.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_ddi.c     |  7 ++++---
 drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
 3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e14fe5f..2e12ee7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -181,7 +181,8 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 
 	/* Enable the PCH Receiver FDI PLL */
 	rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
-		     FDI_RX_PLL_ENABLE | ((intel_crtc->fdi_lanes - 1) << 19);
+		     FDI_RX_PLL_ENABLE |
+		     ((intel_crtc->config.fdi_lanes - 1) << 19);
 	I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
 	POSTING_READ(_FDI_RXA_CTL);
 	udelay(220);
@@ -209,7 +210,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 		 * port reversal bit */
 		I915_WRITE(DDI_BUF_CTL(PORT_E),
 			   DDI_BUF_CTL_ENABLE |
-			   ((intel_crtc->fdi_lanes - 1) << 1) |
+			   ((intel_crtc->config.fdi_lanes - 1) << 1) |
 			   hsw_ddi_buf_ctl_values[i / 2]);
 		POSTING_READ(DDI_BUF_CTL(PORT_E));
 
@@ -1022,7 +1023,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 
 	} else if (type == INTEL_OUTPUT_ANALOG) {
 		temp |= TRANS_DDI_MODE_SELECT_FDI;
-		temp |= (intel_crtc->fdi_lanes - 1) << 1;
+		temp |= (intel_crtc->config.fdi_lanes - 1) << 1;
 
 	} else if (type == INTEL_OUTPUT_DISPLAYPORT ||
 		   type == INTEL_OUTPUT_EDP) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c2579c0..7cb1abf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2432,7 +2432,7 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~(7 << 19);
-	temp |= (intel_crtc->fdi_lanes - 1) << 19;
+	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
 	temp &= ~FDI_LINK_TRAIN_NONE;
 	temp |= FDI_LINK_TRAIN_PATTERN_1;
 	I915_WRITE(reg, temp | FDI_TX_ENABLE);
@@ -2530,7 +2530,7 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~(7 << 19);
-	temp |= (intel_crtc->fdi_lanes - 1) << 19;
+	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
 	temp &= ~FDI_LINK_TRAIN_NONE;
 	temp |= FDI_LINK_TRAIN_PATTERN_1;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
@@ -2665,7 +2665,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~(7 << 19);
-	temp |= (intel_crtc->fdi_lanes - 1) << 19;
+	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
 	temp &= ~(FDI_LINK_TRAIN_AUTO | FDI_LINK_TRAIN_NONE_IVB);
 	temp |= FDI_LINK_TRAIN_PATTERN_1_IVB;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
@@ -2767,7 +2767,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~((0x7 << 19) | (0x7 << 16));
-	temp |= (intel_crtc->fdi_lanes - 1) << 19;
+	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
 	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
 	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
 
@@ -5379,12 +5379,12 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
 		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
 
 	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
-		      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
-	if (intel_crtc->fdi_lanes > 4) {
+		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
+	if (intel_crtc->config.fdi_lanes > 4) {
 		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
-			      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
+			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
 		/* Clamp lanes to avoid programming the hw with bogus values. */
-		intel_crtc->fdi_lanes = 4;
+		intel_crtc->config.fdi_lanes = 4;
 
 		return false;
 	}
@@ -5397,28 +5397,28 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
 		return true;
 	case PIPE_B:
 		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
-		    intel_crtc->fdi_lanes > 2) {
+		    intel_crtc->config.fdi_lanes > 2) {
 			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-				      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
+				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
 			/* Clamp lanes to avoid programming the hw with bogus values. */
-			intel_crtc->fdi_lanes = 2;
+			intel_crtc->config.fdi_lanes = 2;
 
 			return false;
 		}
 
-		if (intel_crtc->fdi_lanes > 2)
+		if (intel_crtc->config.fdi_lanes > 2)
 			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
 		else
 			cpt_enable_fdi_bc_bifurcation(dev);
 
 		return true;
 	case PIPE_C:
-		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->fdi_lanes <= 2) {
-			if (intel_crtc->fdi_lanes > 2) {
+		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
+			if (intel_crtc->config.fdi_lanes > 2) {
 				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-					      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
+					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
 				/* Clamp lanes to avoid programming the hw with bogus values. */
-				intel_crtc->fdi_lanes = 2;
+				intel_crtc->config.fdi_lanes = 2;
 
 				return false;
 			}
@@ -5506,7 +5506,7 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
 	lane = ironlake_get_lanes_required(target_clock, link_bw,
 					   intel_crtc->config.pipe_bpp);
 
-	intel_crtc->fdi_lanes = lane;
+	intel_crtc->config.fdi_lanes = lane;
 
 	if (intel_crtc->config.pixel_multiplier > 1)
 		link_bw *= intel_crtc->config.pixel_multiplier;
@@ -5734,7 +5734,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
 	 * ironlake_check_fdi_lanes. */
-	intel_crtc->fdi_lanes = 0;
+	intel_crtc->config.fdi_lanes = 0;
 	if (intel_crtc->config.has_pch_encoder)
 		ironlake_fdi_set_m_n(crtc);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e3ca7e7..179b42b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -229,6 +229,9 @@ struct intel_crtc_config {
 	int pixel_target_clock;
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
+
+	/* FDI lanes used, only valid if has_pch_encoder is set. */
+	int fdi_lanes;
 };
 
 struct intel_crtc {
@@ -247,7 +250,6 @@ struct intel_crtc {
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
 	struct intel_unpin_work *unpin_work;
-	int fdi_lanes;
 
 	atomic_t unpin_work_count;
 
-- 
1.7.11.7

[PATCH 09/15] drm/i915: hw state readout support for pipe_config->fdi_lanes

[Daniel Vetter]

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7cb1abf..b7774c1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5766,9 +5766,13 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
-	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
+	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		pipe_config->has_pch_encoder = true;
 
+		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
+		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;
+	}
+
 	return true;
 }
 
@@ -5905,9 +5909,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	 */
 	tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
-	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
+	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE) {
 		pipe_config->has_pch_encoder = true;
 
+		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
+		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;
+	}
+
 	return true;
 }
 
@@ -7869,6 +7877,14 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 		return false;
 	}
 
+	if (current_config->fdi_lanes != pipe_config->fdi_lanes) {
+		DRM_ERROR("mismatch in fdi_lanes "
+			  "(expected %i, found %i)\n",
+			  current_config->fdi_lanes,
+			  pipe_config->fdi_lanes);
+		return false;
+	}
+
 	return true;
 }
 
-- 
1.7.11.7

[PATCH 10/15] drm/i915: split up fdi_set_m_n into computation and hw setup

[Daniel Vetter]

And also move the computed m_n values into the pipe_config. This is a
prep step to move the fdi state computation completely into the
prepare phase of the modeset sequence. Which will allow us to handle
fdi link bw constraints in a better way.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b7774c1..7abcd60 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5480,13 +5480,11 @@ void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 	}
 }
 
-static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
+static void ironlake_fdi_compute_config(struct intel_crtc *intel_crtc)
 {
-	struct drm_device *dev = crtc->dev;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
-	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
 
 	/* FDI is a binary signal running at ~2.7GHz, encoding
@@ -5511,9 +5509,7 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
 	if (intel_crtc->config.pixel_multiplier > 1)
 		link_bw *= intel_crtc->config.pixel_multiplier;
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
-			       link_bw, &m_n);
-
-	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
+			       link_bw, &intel_crtc->config.fdi_m_n);
 }
 
 static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
@@ -5735,8 +5731,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
 	 * ironlake_check_fdi_lanes. */
 	intel_crtc->config.fdi_lanes = 0;
-	if (intel_crtc->config.has_pch_encoder)
-		ironlake_fdi_set_m_n(crtc);
+	if (intel_crtc->config.has_pch_encoder) {
+		ironlake_fdi_compute_config(intel_crtc);
+
+		intel_cpu_transcoder_set_m_n(intel_crtc,
+					     &intel_crtc->config.fdi_m_n);
+	}
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
@@ -5866,8 +5866,12 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	if (intel_crtc->config.has_pch_encoder)
-		ironlake_fdi_set_m_n(crtc);
+	if (intel_crtc->config.has_pch_encoder) {
+		ironlake_fdi_compute_config(intel_crtc);
+
+		intel_cpu_transcoder_set_m_n(intel_crtc,
+					     &intel_crtc->config.fdi_m_n);
+	}
 
 	haswell_set_pipeconf(crtc);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 179b42b..f40b43f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -230,8 +230,9 @@ struct intel_crtc_config {
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
 
-	/* FDI lanes used, only valid if has_pch_encoder is set. */
+	/* FDI configuration, only valid if has_pch_encoder is set. */
 	int fdi_lanes;
+	struct intel_link_m_n fdi_m_n;
 };
 
 struct intel_crtc {
-- 
1.7.11.7

[PATCH 11/15] drm/i915: compute fdi lane config earlier

[Daniel Vetter]

Now that it's split up, we can easily move it around and precompute
the fdi lane configuration.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 71 +++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7abcd60..8e10e1b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3955,6 +3955,37 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
+static void ironlake_fdi_compute_config(struct drm_device *dev,
+					struct intel_crtc_config *pipe_config)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	int target_clock, lane, link_bw;
+
+	/* 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
+	 * mode pixel clock is stored in units of 1KHz.
+	 * Hence the bw of each lane in terms of the mode signal
+	 * is:
+	 */
+	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
+
+	if (pipe_config->pixel_target_clock)
+		target_clock = pipe_config->pixel_target_clock;
+	else
+		target_clock = adjusted_mode->clock;
+
+	lane = ironlake_get_lanes_required(target_clock, link_bw,
+					   pipe_config->pipe_bpp);
+
+	pipe_config->fdi_lanes = lane;
+
+	if (pipe_config->pixel_multiplier > 1)
+		link_bw *= pipe_config->pixel_multiplier;
+	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
+			       link_bw, &pipe_config->fdi_m_n);
+}
+
 static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 				      struct intel_crtc_config *pipe_config)
 {
@@ -3989,6 +4020,9 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 		pipe_config->pipe_bpp = 8*3;
 	}
 
+	if (pipe_config->has_pch_encoder)
+		ironlake_fdi_compute_config(dev, pipe_config);
+
 	return true;
 }
 
@@ -5480,38 +5514,6 @@ void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 	}
 }
 
-static void ironlake_fdi_compute_config(struct intel_crtc *intel_crtc)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
-	int target_clock, lane, link_bw;
-
-	/* 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
-	 * mode pixel clock is stored in units of 1KHz.
-	 * Hence the bw of each lane in terms of the mode signal
-	 * is:
-	 */
-	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
-
-	if (intel_crtc->config.pixel_target_clock)
-		target_clock = intel_crtc->config.pixel_target_clock;
-	else
-		target_clock = adjusted_mode->clock;
-
-	lane = ironlake_get_lanes_required(target_clock, link_bw,
-					   intel_crtc->config.pipe_bpp);
-
-	intel_crtc->config.fdi_lanes = lane;
-
-	if (intel_crtc->config.pixel_multiplier > 1)
-		link_bw *= intel_crtc->config.pixel_multiplier;
-	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
-			       link_bw, &intel_crtc->config.fdi_m_n);
-}
-
 static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
 {
 	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
@@ -5730,10 +5732,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
 	 * ironlake_check_fdi_lanes. */
-	intel_crtc->config.fdi_lanes = 0;
 	if (intel_crtc->config.has_pch_encoder) {
-		ironlake_fdi_compute_config(intel_crtc);
-
 		intel_cpu_transcoder_set_m_n(intel_crtc,
 					     &intel_crtc->config.fdi_m_n);
 	}
@@ -5867,8 +5866,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
 	if (intel_crtc->config.has_pch_encoder) {
-		ironlake_fdi_compute_config(intel_crtc);
-
 		intel_cpu_transcoder_set_m_n(intel_crtc,
 					     &intel_crtc->config.fdi_m_n);
 	}
-- 
1.7.11.7

[PATCH 12/15] drm/i915: Split up ironlake_check_fdi_lanes

[Daniel Vetter]

Again in preparation to move the configuration checks into the
pipe_config computation stage of the modeset sequence.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8e10e1b..c9a2f0b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5440,11 +5440,6 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
 			return false;
 		}
 
-		if (intel_crtc->config.fdi_lanes > 2)
-			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
-		else
-			cpt_enable_fdi_bc_bifurcation(dev);
-
 		return true;
 	case PIPE_C:
 		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
@@ -5461,9 +5456,31 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
 			return false;
 		}
 
+		return true;
+	default:
+		BUG();
+	}
+}
+
+static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	switch (intel_crtc->pipe) {
+	case PIPE_A:
+		break;
+	case PIPE_B:
+		if (intel_crtc->config.fdi_lanes > 2)
+			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
+		else
+			cpt_enable_fdi_bc_bifurcation(dev);
+
+		break;
+	case PIPE_C:
 		cpt_enable_fdi_bc_bifurcation(dev);
 
-		return true;
+		break;
 	default:
 		BUG();
 	}
@@ -5738,6 +5755,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	}
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
+	if (IS_IVYBRIDGE(dev))
+		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
 
 	ironlake_set_pipeconf(crtc);
 
-- 
1.7.11.7

[PATCH 13/15] drm/i915: move fdi lane configuration checks ahead

[Daniel Vetter]

This nicely allows us to drop some hacks which have only been used
to work around modeset failures due to lack of fdi lanes.

v2: Implement proper checking for Haswell platforms - the fdi link to
the LPT PCH has only 2 lanes. Note that we already filter out
impossible modes in intel_crt_mode_valid. Unfortunately LPT does not
support 6bpc on the fdi rx, so we can't pull clever tricks to squeeze
in a few more modes.

v2: Rebased on top of Ben Widawsky's num_pipes reorg.

v3: Rebase on top of Ville's pipe debug output ocd rampage.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 129 ++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c9a2f0b..6bcb196 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3955,9 +3955,68 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static void ironlake_fdi_compute_config(struct drm_device *dev,
+static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *pipe_B_crtc =
+		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
+
+	DRM_DEBUG_KMS("checking fdi config on pipe %i, 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 %i: %i lanes\n",
+			      pipe_name(pipe), pipe_config->fdi_lanes);
+		return false;
+	}
+
+	if (IS_HASWELL(dev)) {
+		if (pipe_config->fdi_lanes > 2) {
+			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
+				      pipe_config->fdi_lanes);
+			return false;
+		} else {
+			return true;
+		}
+	}
+
+	if (INTEL_INFO(dev)->num_pipes)
+		return true;
+
+	/* Ivybridge 3 pipe is really complicated */
+	switch (pipe) {
+	case PIPE_A:
+		return true;
+	case PIPE_B:
+		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
+		    pipe_config->fdi_lanes > 2) {
+			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n",
+				      pipe_name(pipe), pipe_config->fdi_lanes);
+			return false;
+		}
+		return true;
+	case PIPE_C:
+		if (!pipe_B_crtc->base.enabled ||
+		    pipe_B_crtc->config.fdi_lanes <= 2) {
+			if (pipe_config->fdi_lanes > 2) {
+				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n",
+					      pipe_name(pipe), pipe_config->fdi_lanes);
+				return false;
+			}
+		} else {
+			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
+			return false;
+		}
+		return true;
+	default:
+		BUG();
+	}
+}
+
+static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 					struct intel_crtc_config *pipe_config)
 {
+	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	int target_clock, lane, link_bw;
 
@@ -3984,6 +4043,9 @@ static void ironlake_fdi_compute_config(struct drm_device *dev,
 		link_bw *= pipe_config->pixel_multiplier;
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
 			       link_bw, &pipe_config->fdi_m_n);
+
+	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
+					intel_crtc->pipe, pipe_config);
 }
 
 static bool intel_crtc_compute_config(struct drm_crtc *crtc,
@@ -4021,7 +4083,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 	}
 
 	if (pipe_config->has_pch_encoder)
-		ironlake_fdi_compute_config(dev, pipe_config);
+		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
 
 	return true;
 }
@@ -5405,63 +5467,6 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
 	POSTING_READ(SOUTH_CHICKEN1);
 }
 
-static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *pipe_B_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
-
-	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
-		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-	if (intel_crtc->config.fdi_lanes > 4) {
-		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
-			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-		/* Clamp lanes to avoid programming the hw with bogus values. */
-		intel_crtc->config.fdi_lanes = 4;
-
-		return false;
-	}
-
-	if (INTEL_INFO(dev)->num_pipes == 2)
-		return true;
-
-	switch (intel_crtc->pipe) {
-	case PIPE_A:
-		return true;
-	case PIPE_B:
-		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
-		    intel_crtc->config.fdi_lanes > 2) {
-			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-			/* Clamp lanes to avoid programming the hw with bogus values. */
-			intel_crtc->config.fdi_lanes = 2;
-
-			return false;
-		}
-
-		return true;
-	case PIPE_C:
-		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
-			if (intel_crtc->config.fdi_lanes > 2) {
-				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-				/* Clamp lanes to avoid programming the hw with bogus values. */
-				intel_crtc->config.fdi_lanes = 2;
-
-				return false;
-			}
-		} else {
-			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return false;
-		}
-
-		return true;
-	default:
-		BUG();
-	}
-}
-
 static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -5653,7 +5658,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	bool is_lvds = false;
 	struct intel_encoder *encoder;
 	int ret;
-	bool fdi_config_ok;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		switch (encoder->type) {
@@ -5747,14 +5751,11 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
-	 * ironlake_check_fdi_lanes. */
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
 					     &intel_crtc->config.fdi_m_n);
 	}
 
-	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 	if (IS_IVYBRIDGE(dev))
 		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
 
@@ -5770,7 +5771,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
 
-	return fdi_config_ok ? ret : -EINVAL;
+	return ret;
 }
 
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
-- 
1.7.11.7

[PATCH 14/15] drm/i915: don't count cpu ports for fdi B/C lane sharing

[Daniel Vetter]

This allows us to use all 4 fdi lanes on fdi B when the cpu eDP is
running on pipe C. Yay!

v2: Encapsulate test into a little helper function, as suggested by
Chris Wilson.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6bcb196..c25dbdd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2381,6 +2381,11 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
 			   FDI_FE_ERRC_ENABLE);
 }
 
+static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
+{
+	return intel_crtc->base.enabled && intel_crtc->config.has_pch_encoder;
+}
+
 static void ivb_modeset_global_resources(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2390,10 +2395,13 @@ static void ivb_modeset_global_resources(struct drm_device *dev)
 		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
 	uint32_t temp;
 
-	/* When everything is off disable fdi C so that we could enable fdi B
-	 * with all lanes. XXX: This misses the case where a pipe is not using
-	 * any pch resources and so doesn't need any fdi lanes. */
-	if (!pipe_B_crtc->base.enabled && !pipe_C_crtc->base.enabled) {
+	/*
+	 * When everything is off disable fdi C so that we could enable fdi B
+	 * with all lanes. Note that we don't care about enabled pipes without
+	 * an enabled pch encoder.
+	 */
+	if (!pipe_has_enabled_pch(pipe_B_crtc) &&
+	    !pipe_has_enabled_pch(pipe_C_crtc)) {
 		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
 		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
 
@@ -3996,7 +4004,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 		}
 		return true;
 	case PIPE_C:
-		if (!pipe_B_crtc->base.enabled ||
+		if (!pipe_has_enabled_pch(pipe_B_crtc) ||
 		    pipe_B_crtc->config.fdi_lanes <= 2) {
 			if (pipe_config->fdi_lanes > 2) {
 				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n",
-- 
1.7.11.7

[PATCH 15/15] drm/i915: implement fdi auto-dithering

[Daniel Vetter]

So on a bunch of setups we only have 2 fdi lanes available, e.g. hsw
VGA or 3 pipes on ivb. And seemingly a lot of modes don't quite fit
into this, among them the default 1080p mode.

The solution is to dither down the pipe a bit so that everything fits,
which this patch implements.

But ports compute their state under the assumption that the bpp they
pick will be the one selected, e.g. the display port bw computations
won't work otherwise. Now we could adjust our code to again up-dither
to the computed DP link parameters, but that's pointless.

So instead when the pipe needs to adjust parameters we need to retry
the pipe_config computation at the encoder stage. Furthermore we need
to inform encoders that they should not increase bandwidth
requirements if possible. This is required for the hdmi code, which
prefers the pipe to up-dither to either of the two possible hdmi bpc
values.

LVDS has a similar requirement, although that's probably only
theoretical in nature: It's unlikely that we'll ever see an 8bpc
high-res lvds panel (which is required to hit the 2 fdi lane limit).

v2: Rebased on top of a bikeshed from Paulo.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 56 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
 drivers/gpu/drm/i915/intel_hdmi.c    | 14 ++++++---
 drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
 4 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c25dbdd..6d35ccd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4021,13 +4021,16 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 	}
 }
 
-static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
-					struct intel_crtc_config *pipe_config)
+#define RETRY 1
+static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
+				       struct intel_crtc_config *pipe_config)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	int target_clock, lane, link_bw;
+	bool setup_ok, needs_recompute = false;
 
+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
@@ -4052,12 +4055,26 @@ static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
 			       link_bw, &pipe_config->fdi_m_n);
 
-	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
-					intel_crtc->pipe, pipe_config);
+	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
+					    intel_crtc->pipe, pipe_config);
+	if (!setup_ok && 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;
+	}
+
+	if (needs_recompute)
+		return RETRY;
+
+	return setup_ok ? 0 : -EINVAL;
 }
 
-static bool intel_crtc_compute_config(struct drm_crtc *crtc,
-				      struct intel_crtc_config *pipe_config)
+static int intel_crtc_compute_config(struct drm_crtc *crtc,
+				     struct intel_crtc_config *pipe_config)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
@@ -4066,7 +4083,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 		/* FDI link clock is fixed at 2.7G */
 		if (pipe_config->requested_mode.clock * 3
 		    > IRONLAKE_FDI_FREQ * 4)
-			return false;
+			return -EINVAL;
 	}
 
 	/* All interlaced capable intel hw wants timings in frames. Note though
@@ -4080,7 +4097,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 	 */
 	if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
 		adjusted_mode->hsync_start == adjusted_mode->hdisplay)
-		return false;
+		return -EINVAL;
 
 	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) && pipe_config->pipe_bpp > 10*3) {
 		pipe_config->pipe_bpp = 10*3; /* 12bpc is gen5+ */
@@ -4093,7 +4110,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 	if (pipe_config->has_pch_encoder)
 		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
 
-	return true;
+	return 0;
 }
 
 static int valleyview_get_display_clock_speed(struct drm_device *dev)
@@ -7673,7 +7690,8 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	struct drm_encoder_helper_funcs *encoder_funcs;
 	struct intel_encoder *encoder;
 	struct intel_crtc_config *pipe_config;
-	int plane_bpp;
+	int plane_bpp, ret = -EINVAL;
+	bool retry = true;
 
 	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
 	if (!pipe_config)
@@ -7686,6 +7704,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	if (plane_bpp < 0)
 		goto fail;
 
+encoder_retry:
 	/* Pass our mode to the connectors and the CRTC to give them a chance to
 	 * adjust it according to limitations or connector properties, and also
 	 * a chance to reject the mode entirely.
@@ -7714,10 +7733,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		}
 	}
 
-	if (!(intel_crtc_compute_config(crtc, pipe_config))) {
+	ret = intel_crtc_compute_config(crtc, pipe_config);
+	if (ret < 0) {
 		DRM_DEBUG_KMS("CRTC fixup failed\n");
 		goto fail;
 	}
+
+	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;
+	}
+
 	DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
 
 	pipe_config->dither = pipe_config->pipe_bpp != plane_bpp;
@@ -7727,7 +7759,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	return pipe_config;
 fail:
 	kfree(pipe_config);
-	return ERR_PTR(-EINVAL);
+	return ERR_PTR(ret);
 }
 
 /* Computes which crtcs are affected and sets the relevant bits in the mask. For
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f40b43f..c14afc6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -211,6 +211,13 @@ struct intel_crtc_config {
 	/* Controls for the clock computation, to override various stages. */
 	bool clock_set;
 
+	/*
+	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
+	 * required. This is set in the 2nd loop of calling encoder's
+	 * ->compute_config if the first pick doesn't work out.
+	 */
+	bool bw_constrained;
+
 	/* Settings for the intel dpll used on pretty much everything but
 	 * haswell. */
 	struct dpll {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a8273c7..3942041 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -784,6 +784,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
+	int desired_bpp;
 
 	if (intel_hdmi->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -808,16 +809,21 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 */
 	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
 	    && HAS_PCH_SPLIT(dev)) {
-		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
-		pipe_config->pipe_bpp = 12*3;
+		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
+		desired_bpp = 12*3;
 
 		/* Need to adjust the port link by 1.5x for 12bpc. */
 		adjusted_mode->clock = clock_12bpc;
 		pipe_config->pixel_target_clock =
 			pipe_config->requested_mode.clock;
 	} else {
-		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
-		pipe_config->pipe_bpp = 8*3;
+		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
+		desired_bpp = 8*3;
+	}
+
+	if (!pipe_config->bw_constrained) {
+		DRM_DEBUG_KMS("forcing pipe bpc to %i for HDMI\n", desired_bpp);
+		pipe_config->pipe_bpp = desired_bpp;
 	}
 
 	if (adjusted_mode->clock > 225000) {
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 094f3c5..c426581 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -331,7 +331,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	else
 		lvds_bpp = 6*3;
 
-	if (lvds_bpp != pipe_config->pipe_bpp) {
+	if (lvds_bpp != pipe_config->pipe_bpp && !pipe_config->bw_constrained) {
 		DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
 			      pipe_config->pipe_bpp, lvds_bpp);
 		pipe_config->pipe_bpp = lvds_bpp;
-- 
1.7.11.7

Re: [PATCH 00/15] high-bpp fixes and fdi auto dithering

[Chris Wilson]

On Fri, Apr 19, 2013 at 11:24:32AM +0200, Daniel Vetter wrote:
> Hi all,
> 
> This fixes all the bugs I've found in my various systems when using non-24bpp
> modes as the first part of the series.
> 
> And with working non-standard bpp support I've figured we can go fancy and
> implemented auto-dithering if we hit an fdi bw limit. Which means that you can
> now use 3-pipe pch configurations on ivb on pretty much everywhere. The only
> restriction is that you need to fire up pipe C first, since without atomic
> modeset pipe B will otherwise too much bw.
> 
> One big thing here is that this will break Paulo's hsw eDP machine, specifically
> the patch called "drm/i915: force bpp for eDP panels". But apparently without
> that my machine here is broken ... Ideas highly welcome about how we could quirk
> ourselves out of this mess.

Just tested (danvet/fdi-dither 93b459d0a80024041daea5195ff6f8a576fa89bc)
using depths 24 and 30 with Ironlake DP, still works.

Still has the wait_for_pipe_off WARN though.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 05/15] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Jesse Barnes]

On Fri, 19 Apr 2013 11:24:37 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> +		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> +		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)

I think the magic != 30 check needs a comment, or we should never
set .dither for 30bpp modes in the various compute_configs.

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

The current code is rather ... ugly. The only thing it managed to pull
off is getting 6bpc on DP working on g4x. Then someone added another
custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
implementing the PIPECONF-based dither/bpc controls on g4x/vlv.

Note that compared to pch based platforms g4x/vlv don't support 12bpc
modes. g4x is already caught, extend the check for vlv.

The other fixup is to restrict the lvds-specific dithering to early
gen4 devices - g4x should use the pipeconf dither controls. Note that
on gen2/3 the dither control is in the panel fitter even.

v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":

"Programming note: Dithering should only be enabled for 8 bpc or 6
bpc."

v3: Actually drop the old ugly dither code.

v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
pipes (Jesse).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8c36376..7f1ab8b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
 	}
 
-	/* default to 8bpc */
-	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
-	if (intel_crtc->config.has_dp_encoder) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-				    PIPECONF_DITHER_EN |
+	/* only g4x and later have fancy bpc/dither controls */
+	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
+		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
+
+		/* Bspec claims that we can't use dithering for 30bpp pipes. */
+		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
+			pipeconf |= PIPECONF_DITHER_EN |
 				    PIPECONF_DITHER_TYPE_SP;
-		}
-	}
 
-	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
-						      INTEL_OUTPUT_EDP)) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-					PIPECONF_ENABLE |
-					I965_PIPECONF_ACTIVE;
+		pipeconf &= ~PIPECONF_BPC_MASK;
+		switch (intel_crtc->config.pipe_bpp) {
+		case 18:
+			pipeconf |= PIPECONF_6BPC;
+			break;
+		case 24:
+			pipeconf |= PIPECONF_8BPC;
+			break;
+		case 30:
+			pipeconf |= PIPECONF_10BPC;
+			break;
+		default:
+			/* Case prevented by intel_choose_pipe_bpp_dither. */
+			BUG();
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 58a98ff..094f3c5 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
 	/* Set the dithering flag on LVDS as needed, note that there is no
 	 * special lvds dither control bit on pch-split platforms, dithering is
 	 * only controlled through the PIPECONF reg. */
-	if (INTEL_INFO(dev)->gen == 4) {
+	if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) {
 		if (intel_crtc->config.dither)
 			temp |= LVDS_ENABLE_DITHER;
 		else
-- 
1.7.11.7

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Jesse Barnes]

On Fri, 19 Apr 2013 20:17:10 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8c36376..7f1ab8b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
>  	}
>  
> -	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (intel_crtc->config.has_dp_encoder) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -				    PIPECONF_DITHER_EN |
> +	/* only g4x and later have fancy bpc/dither controls */
> +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> +		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> +
> +		/* Bspec claims that we can't use dithering for 30bpp pipes. */
> +		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
> +			pipeconf |= PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
> -		}
> -	}
>  
> -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
> -						      INTEL_OUTPUT_EDP)) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -					PIPECONF_ENABLE |
> -					I965_PIPECONF_ACTIVE;
> +		pipeconf &= ~PIPECONF_BPC_MASK;
> +		switch (intel_crtc->config.pipe_bpp) {
> +		case 18:
> +			pipeconf |= PIPECONF_6BPC;
> +			break;
> +		case 24:
> +			pipeconf |= PIPECONF_8BPC;
> +			break;
> +		case 30:
> +			pipeconf |= PIPECONF_10BPC;
> +			break;
> +		default:
> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
> +			BUG();

Am I misreading here?  It looks like we may set the 10bpc pipeconf bit,
but never enable it.  Should there be another G4x check somewhere?  Or
should the 10bpc case be dropped from the switch?

>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 58a98ff..094f3c5 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
>  	/* Set the dithering flag on LVDS as needed, note that there is no
>  	 * special lvds dither control bit on pch-split platforms, dithering is
>  	 * only controlled through the PIPECONF reg. */
> -	if (INTEL_INFO(dev)->gen == 4) {
> +	if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) {
>  		if (intel_crtc->config.dither)
>  			temp |= LVDS_ENABLE_DITHER;
>  		else

Looks ok otherwise, though I'd still like to see LVDS dither compared
with pipe dither.

And what happens now if a 30bpp config gets chosen for an LVDS or eDP
panel?  Do we just output it anyway and get junk on the screen?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

On Fri, Apr 19, 2013 at 8:39 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Fri, 19 Apr 2013 20:17:10 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 8c36376..7f1ab8b 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>>                       pipeconf &= ~PIPECONF_DOUBLE_WIDE;
>>       }
>>
>> -     /* default to 8bpc */
>> -     pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
>> -     if (intel_crtc->config.has_dp_encoder) {
>> -             if (intel_crtc->config.dither) {
>> -                     pipeconf |= PIPECONF_6BPC |
>> -                                 PIPECONF_DITHER_EN |
>> +     /* only g4x and later have fancy bpc/dither controls */
>> +     if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
>> +             pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
>> +
>> +             /* Bspec claims that we can't use dithering for 30bpp pipes. */
>> +             if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
>> +                     pipeconf |= PIPECONF_DITHER_EN |
>>                                   PIPECONF_DITHER_TYPE_SP;
>> -             }
>> -     }
>>
>> -     if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
>> -                                                   INTEL_OUTPUT_EDP)) {
>> -             if (intel_crtc->config.dither) {
>> -                     pipeconf |= PIPECONF_6BPC |
>> -                                     PIPECONF_ENABLE |
>> -                                     I965_PIPECONF_ACTIVE;
>> +             pipeconf &= ~PIPECONF_BPC_MASK;
>> +             switch (intel_crtc->config.pipe_bpp) {
>> +             case 18:
>> +                     pipeconf |= PIPECONF_6BPC;
>> +                     break;
>> +             case 24:
>> +                     pipeconf |= PIPECONF_8BPC;
>> +                     break;
>> +             case 30:
>> +                     pipeconf |= PIPECONF_10BPC;
>> +                     break;
>> +             default:
>> +                     /* Case prevented by intel_choose_pipe_bpp_dither. */
>> +                     BUG();
>
> Am I misreading here?  It looks like we may set the 10bpc pipeconf bit,
> but never enable it.  Should there be another G4x check somewhere?  Or
> should the 10bpc case be dropped from the switch?

Previous patches fixed up the pipe_bpp computation for vlv, and other
patches in this series allow us to actually come up with a 10bpp
config (e.g. dp unconditionally goes with a 24bpp limit thus far ...).
So I don't see exactly where you see an issue.


>>               }
>>       }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>> index 58a98ff..094f3c5 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
>>       /* Set the dithering flag on LVDS as needed, note that there is no
>>        * special lvds dither control bit on pch-split platforms, dithering is
>>        * only controlled through the PIPECONF reg. */
>> -     if (INTEL_INFO(dev)->gen == 4) {
>> +     if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) {
>>               if (intel_crtc->config.dither)
>>                       temp |= LVDS_ENABLE_DITHER;
>>               else
>
> Looks ok otherwise, though I'd still like to see LVDS dither compared
> with pipe dither.

I've figued since we want dither for DP (e.g. cheap vga dongles which
only work with 6bpc on high-res modes) we should switch to pipe
dithering unconditionally.

> And what happens now if a 30bpp config gets chosen for an LVDS or eDP
> panel?  Do we just output it anyway and get junk on the screen?

encoder->compute_config will save the day. If you read up to the fdi
auto-dither code you can watch a pretty elaborate negotiation going on
between encoders and the pipe about which bpp things should run at ;-)

So should all just magically work. In the end (once everything
interesting is converted over) the mode_set/enable functions just grab
the parameters from pipe_conf and smash it into hw registers. All
interesting stuff is done up-front in the pipe/encoder compute_config
stage. Of course that means for a given hw feature you need to check
individual encoders and the pipe to figure out what exactly pops out
at the end.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: force bpp for eDP panels

[Daniel Vetter]

We've had our fair share of woes already which showed that we can't
rely on the bpc limits in the EDID for eDP panels without risking
black screens. So now we limit the depth by what the BIOS recommends
in the VBT:

commit 2f4f649a69a9eb51f6e98130e19dd90a260a4145
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Nov 12 14:33:44 2012 +0200

    drm/i915: do not ignore eDP bpc settings from vbt

But that's not enough, since at least the panel on my ASUS Zenbook
Prime here is also unhappy if the bpc is too low. Hence just take the
firmware value and dither to get what flimsy panels want.

Like before we ensure that we don't change the bpp if the firmware
doesn't provide a value, see

commit 9a30a61f3516871c5c638fd7c025fbaa11ddf7fe
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Nov 12 14:33:45 2012 +0200

    drm/i915: do not default to 18 bpp for eDP if missing from VBT

v2: Apparently there are some horribly broken eDP panels around which
only work if the DP link is set up as if we want to driver a 24bpp
mode, but still only work if the data is feed at 18bpp. See

commit 57c219633275c7e7413f8bc7be250dc092887458
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Apr 4 17:19:37 2013 +0200

    drm/i915: revert eDP bpp clamping code changes

for the gory details.

Adjust the patch accordingly and update all the relevant comments.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Paulo Zanoni <przanoni@gmail.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b1a3a64..d602886 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -745,6 +745,25 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
 	 * bpc in between. */
 	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
+
+	/*
+	 * eDP panels are really fickle, try to enfore the bpp the firmware
+	 * recomments. This means we'll up-dither 16bpp framebuffers on
+	 * high-depth panels.
+	 */
+	if (is_edp(intel_dp) && dev_priv->edp.bpp) {
+		DRM_DEBUG_KMS("forcing bpp for eDP panel to BIOS-provided %i\n",
+			      dev_priv->edp.bpp);
+		/*
+		 * But for added hilarity some of the 18bpp panels only work if
+		 * we set up the pipe with enough bw for a 24bpp mode, since
+		 * that's what the bios apparently does. So start the bw
+		 * calculation with at least 24 bpp (down-dither is still
+		 * possible).
+		 */
+		bpp = max_t(int, dev_priv->edp.bpp, 24);
+	}
+
 	for (; bpp >= 6*3; bpp -= 2*3) {
 		mode_rate = intel_dp_link_required(target_clock, bpp);
 
@@ -795,11 +814,8 @@ found:
 			       &pipe_config->dp_m_n);
 
 	/*
-	 * XXX: We have a strange regression where using the vbt edp bpp value
-	 * for the link bw computation results in black screens, the panel only
-	 * works when we do the computation at the usual 24bpp (but still
-	 * requires us to use 18bpp). Until that's fully debugged, stay
-	 * bug-for-bug compatible with the old code.
+	 * Since we might have picked a too high bpp for the bw calculation,
+	 * clamp it down again to make even the most fickle eDP panel happy.
 	 */
 	if (is_edp(intel_dp) && dev_priv->edp.bpp) {
 		DRM_DEBUG_KMS("clamping display bpc (was %d) to eDP (%d)\n",
-- 
1.7.11.7

Re: [PATCH 13/15] drm/i915: move fdi lane configuration checks ahead

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:45AM +0200, Daniel Vetter wrote:
> This nicely allows us to drop some hacks which have only been used
> to work around modeset failures due to lack of fdi lanes.
> 
> v2: Implement proper checking for Haswell platforms - the fdi link to
> the LPT PCH has only 2 lanes. Note that we already filter out
> impossible modes in intel_crt_mode_valid. Unfortunately LPT does not
> support 6bpc on the fdi rx, so we can't pull clever tricks to squeeze
> in a few more modes.
> 
> v2: Rebased on top of Ben Widawsky's num_pipes reorg.
> 
> v3: Rebase on top of Ville's pipe debug output ocd rampage.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 129 ++++++++++++++++++-----------------
>  1 file changed, 65 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c9a2f0b..6bcb196 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3955,9 +3955,68 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> -static void ironlake_fdi_compute_config(struct drm_device *dev,
> +static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> +				     struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *pipe_B_crtc =
> +		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> +
> +	DRM_DEBUG_KMS("checking fdi config on pipe %i, lanes %i\n",
                                                   ^^

%c

More of the same below.

> +		      pipe_name(pipe), pipe_config->fdi_lanes);
> +	if (pipe_config->fdi_lanes > 4) {
> +		DRM_DEBUG_KMS("invalid fdi lane config on pipe %i: %i lanes\n",
> +			      pipe_name(pipe), pipe_config->fdi_lanes);
> +		return false;
> +	}
> +
> +	if (IS_HASWELL(dev)) {
> +		if (pipe_config->fdi_lanes > 2) {
> +			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
> +				      pipe_config->fdi_lanes);
> +			return false;
> +		} else {
> +			return true;
> +		}
> +	}
> +
> +	if (INTEL_INFO(dev)->num_pipes)
> +		return true;
> +
> +	/* Ivybridge 3 pipe is really complicated */
> +	switch (pipe) {
> +	case PIPE_A:
> +		return true;
> +	case PIPE_B:
> +		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> +		    pipe_config->fdi_lanes > 2) {
> +			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n",
> +				      pipe_name(pipe), pipe_config->fdi_lanes);
> +			return false;
> +		}
> +		return true;
> +	case PIPE_C:
> +		if (!pipe_B_crtc->base.enabled ||
> +		    pipe_B_crtc->config.fdi_lanes <= 2) {
> +			if (pipe_config->fdi_lanes > 2) {
> +				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n",
> +					      pipe_name(pipe), pipe_config->fdi_lanes);
> +				return false;
> +			}
> +		} else {
> +			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> +			return false;
> +		}
> +		return true;
> +	default:
> +		BUG();
> +	}
> +}
> +
> +static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  					struct intel_crtc_config *pipe_config)
>  {
> +	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
>  	int target_clock, lane, link_bw;
>  
> @@ -3984,6 +4043,9 @@ static void ironlake_fdi_compute_config(struct drm_device *dev,
>  		link_bw *= pipe_config->pixel_multiplier;
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
>  			       link_bw, &pipe_config->fdi_m_n);
> +
> +	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
> +					intel_crtc->pipe, pipe_config);
>  }
>  
>  static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> @@ -4021,7 +4083,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  	}
>  
>  	if (pipe_config->has_pch_encoder)
> -		ironlake_fdi_compute_config(dev, pipe_config);
> +		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
>  
>  	return true;
>  }
> @@ -5405,63 +5467,6 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
>  	POSTING_READ(SOUTH_CHICKEN1);
>  }
>  
> -static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
> -{
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *pipe_B_crtc =
> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> -
> -	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
> -		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -	if (intel_crtc->config.fdi_lanes > 4) {
> -		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
> -			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -		/* Clamp lanes to avoid programming the hw with bogus values. */
> -		intel_crtc->config.fdi_lanes = 4;
> -
> -		return false;
> -	}
> -
> -	if (INTEL_INFO(dev)->num_pipes == 2)
> -		return true;
> -
> -	switch (intel_crtc->pipe) {
> -	case PIPE_A:
> -		return true;
> -	case PIPE_B:
> -		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> -		    intel_crtc->config.fdi_lanes > 2) {
> -			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> -				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -			/* Clamp lanes to avoid programming the hw with bogus values. */
> -			intel_crtc->config.fdi_lanes = 2;
> -
> -			return false;
> -		}
> -
> -		return true;
> -	case PIPE_C:
> -		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
> -			if (intel_crtc->config.fdi_lanes > 2) {
> -				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> -					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -				/* Clamp lanes to avoid programming the hw with bogus values. */
> -				intel_crtc->config.fdi_lanes = 2;
> -
> -				return false;
> -			}
> -		} else {
> -			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> -			return false;
> -		}
> -
> -		return true;
> -	default:
> -		BUG();
> -	}
> -}
> -
>  static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
> @@ -5653,7 +5658,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	bool is_lvds = false;
>  	struct intel_encoder *encoder;
>  	int ret;
> -	bool fdi_config_ok;
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder) {
>  		switch (encoder->type) {
> @@ -5747,14 +5751,11 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
> -	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
> -	 * ironlake_check_fdi_lanes. */
>  	if (intel_crtc->config.has_pch_encoder) {
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
>  					     &intel_crtc->config.fdi_m_n);
>  	}
>  
> -	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>  	if (IS_IVYBRIDGE(dev))
>  		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
>  
> @@ -5770,7 +5771,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
>  
> -	return fdi_config_ok ? ret : -EINVAL;
> +	return ret;
>  }
>  
>  static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: move fdi lane configuration checks ahead

[Daniel Vetter]

This nicely allows us to drop some hacks which have only been used
to work around modeset failures due to lack of fdi lanes.

v2: Implement proper checking for Haswell platforms - the fdi link to
the LPT PCH has only 2 lanes. Note that we already filter out
impossible modes in intel_crt_mode_valid. Unfortunately LPT does not
support 6bpc on the fdi rx, so we can't pull clever tricks to squeeze
in a few more modes.

v2: Rebased on top of Ben Widawsky's num_pipes reorg.

v3: Rebase on top of Ville's pipe debug output ocd rampage.

v4: Fixup rebase fail spotted by Ville.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 129 ++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9ff3cf6..53d6e2f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3943,9 +3943,68 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static void ironlake_fdi_compute_config(struct drm_device *dev,
+static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *pipe_B_crtc =
+		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
+
+	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 false;
+	}
+
+	if (IS_HASWELL(dev)) {
+		if (pipe_config->fdi_lanes > 2) {
+			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
+				      pipe_config->fdi_lanes);
+			return false;
+		} else {
+			return true;
+		}
+	}
+
+	if (INTEL_INFO(dev)->num_pipes)
+		return true;
+
+	/* Ivybridge 3 pipe is really complicated */
+	switch (pipe) {
+	case PIPE_A:
+		return true;
+	case PIPE_B:
+		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
+		    pipe_config->fdi_lanes > 2) {
+			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
+				      pipe_name(pipe), pipe_config->fdi_lanes);
+			return false;
+		}
+		return true;
+	case PIPE_C:
+		if (!pipe_B_crtc->base.enabled ||
+		    pipe_B_crtc->config.fdi_lanes <= 2) {
+			if (pipe_config->fdi_lanes > 2) {
+				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
+					      pipe_name(pipe), pipe_config->fdi_lanes);
+				return false;
+			}
+		} else {
+			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
+			return false;
+		}
+		return true;
+	default:
+		BUG();
+	}
+}
+
+static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 					struct intel_crtc_config *pipe_config)
 {
+	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	int target_clock, lane, link_bw;
 
@@ -3972,6 +4031,9 @@ static void ironlake_fdi_compute_config(struct drm_device *dev,
 		link_bw *= pipe_config->pixel_multiplier;
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
 			       link_bw, &pipe_config->fdi_m_n);
+
+	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
+					intel_crtc->pipe, pipe_config);
 }
 
 static bool intel_crtc_compute_config(struct drm_crtc *crtc,
@@ -4009,7 +4071,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 	}
 
 	if (pipe_config->has_pch_encoder)
-		ironlake_fdi_compute_config(dev, pipe_config);
+		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
 
 	return true;
 }
@@ -5393,63 +5455,6 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
 	POSTING_READ(SOUTH_CHICKEN1);
 }
 
-static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *pipe_B_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
-
-	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
-		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-	if (intel_crtc->config.fdi_lanes > 4) {
-		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
-			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-		/* Clamp lanes to avoid programming the hw with bogus values. */
-		intel_crtc->config.fdi_lanes = 4;
-
-		return false;
-	}
-
-	if (INTEL_INFO(dev)->num_pipes == 2)
-		return true;
-
-	switch (intel_crtc->pipe) {
-	case PIPE_A:
-		return true;
-	case PIPE_B:
-		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
-		    intel_crtc->config.fdi_lanes > 2) {
-			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-			/* Clamp lanes to avoid programming the hw with bogus values. */
-			intel_crtc->config.fdi_lanes = 2;
-
-			return false;
-		}
-
-		return true;
-	case PIPE_C:
-		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
-			if (intel_crtc->config.fdi_lanes > 2) {
-				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-				/* Clamp lanes to avoid programming the hw with bogus values. */
-				intel_crtc->config.fdi_lanes = 2;
-
-				return false;
-			}
-		} else {
-			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return false;
-		}
-
-		return true;
-	default:
-		BUG();
-	}
-}
-
 static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -5641,7 +5646,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	bool is_lvds = false;
 	struct intel_encoder *encoder;
 	int ret;
-	bool fdi_config_ok;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		switch (encoder->type) {
@@ -5734,14 +5738,11 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
-	 * ironlake_check_fdi_lanes. */
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
 					     &intel_crtc->config.fdi_m_n);
 	}
 
-	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 	if (IS_IVYBRIDGE(dev))
 		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
 
@@ -5757,7 +5758,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
 
-	return fdi_config_ok ? ret : -EINVAL;
+	return ret;
 }
 
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
-- 
1.7.11.7

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

Re: [PATCH 01/15] drm/i915: fixup 12bpc hdmi dotclock handling

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:33AM +0200, Daniel Vetter wrote:
> We need to multiply the hdmi port dotclock by 1.5x since it's not
> really a dotclock, but the 10/8 encoding bitclock divided by 10.
> 
> Also add correct limit checks for the dotclock and reject modes which
> don't fit. HDMI 1.4 would allow more, but our hw doesn't support that
> unfortunately :(
> 
> Somehow I suspect 12bpc hdmi output never really worked - we really
> need an i-g-t testcase to check all the different pixel modes and
> outputs.
> 
> v2: Fixup the adjusted port clock handling - we need to make sure that
> the fdi link code still gets the real pixelclock.
> 
> v3: g4x/vlv don't support 12bpc hdmi output so drop the bogus comment.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Seems to match what the spec says. Although I'm getting confused about
all the clocks we have. Somehow I'd prefer to store the pixel clock in
the mode, and move the link clock out, but perhaps that makes things
more messy elsewhere...

Also I'm confused about the 225 MHz limit. That seems to be specific to
PCH platforms. My g4x docs say that HDMI limit is 165 MHz. Also
intel_limits_g4x_hdmi() has dot.max of 400 MHz which seems to contradict
the docs (400 MHz is correct for the DAC apparently). But then again the
docs also say that the p1_slow/p2_fast switchover point is 165 MHz, so
if the max is 165 MHz too, there would never be any need to use p2_fast
with HDMI. The more I read the docs, the more confused I get.

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 53ce8a5..a8273c7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -783,6 +783,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> +	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
>  
>  	if (intel_hdmi->color_range_auto) {
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> @@ -802,16 +803,28 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	/*
>  	 * HDMI is either 12 or 8, so if the display lets 10bpc sneak
>  	 * through, clamp it down. Note that g4x/vlv don't support 12bpc hdmi
> -	 * outputs.
> +	 * outputs. We also need to check that the higher clock still fits
> +	 * within limits.
>  	 */
> -	if (pipe_config->pipe_bpp > 8*3 && HAS_PCH_SPLIT(dev)) {
> +	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
> +	    && HAS_PCH_SPLIT(dev)) {
>  		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
>  		pipe_config->pipe_bpp = 12*3;
> +
> +		/* Need to adjust the port link by 1.5x for 12bpc. */
> +		adjusted_mode->clock = clock_12bpc;
> +		pipe_config->pixel_target_clock =
> +			pipe_config->requested_mode.clock;
>  	} else {
>  		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
>  		pipe_config->pipe_bpp = 8*3;
>  	}
>  
> +	if (adjusted_mode->clock > 225000) {
> +		DRM_DEBUG_KMS("too high HDMI clock, rejecting mode\n");
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 02/15] drm/i915: Disable high-bpc on pre-1.4 EDID screens

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:34AM +0200, Daniel Vetter wrote:
> Prevents black screens when using 30bpp framebuffers on my
> HDMI screens here. The DP input on the same screen though reports a
> 1.4 EDID with the correct 8bpc limit set.
> 
> v2: Actually check for the right thing!
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6e265b0..1e6efab 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7612,6 +7612,13 @@ pipe_config_set_bpp(struct drm_crtc *crtc,
>  				      bpp, connector->display_info.bpc*3);
>  			pipe_config->pipe_bpp = connector->display_info.bpc*3;
>  		}
> +
> +		/* Clamp bpp to 8 on screens without EDID 1.4 */
> +		if (connector->display_info.bpc == 0 && bpp > 24) {
> +			DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of 24\n",
> +				      bpp);
> +			pipe_config->pipe_bpp = 24;
> +		}
>  	}
>  
>  	return bpp;
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 04/15] drm/i915: drop adjusted_mode from *_set_pipeconf functions

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:36AM +0200, Daniel Vetter wrote:
> They can get at the adjusted mode through intel_crtc->config.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1e6efab..8c36376 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5151,8 +5151,7 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
>  	return 120000;
>  }
>  
> -static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> -				  struct drm_display_mode *adjusted_mode)
> +static void ironlake_set_pipeconf(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -5185,7 +5184,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>  		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>  
>  	val &= ~PIPECONF_INTERLACE_MASK;
> -	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> +	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		val |= PIPECONF_INTERLACED_ILK;
>  	else
>  		val |= PIPECONF_PROGRESSIVE;
> @@ -5263,8 +5262,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static void haswell_set_pipeconf(struct drm_crtc *crtc,
> -				 struct drm_display_mode *adjusted_mode)
> +static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -5278,7 +5276,7 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
>  		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>  
>  	val &= ~PIPECONF_INTERLACE_MASK_HSW;
> -	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> +	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		val |= PIPECONF_INTERLACED_ILK;
>  	else
>  		val |= PIPECONF_PROGRESSIVE;
> @@ -5737,7 +5735,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>  
> -	ironlake_set_pipeconf(crtc, adjusted_mode);
> +	ironlake_set_pipeconf(crtc);
>  
>  	/* Set up the display plane register */
>  	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
> @@ -5862,7 +5860,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	if (intel_crtc->config.has_pch_encoder)
>  		ironlake_fdi_set_m_n(crtc);
>  
> -	haswell_set_pipeconf(crtc, adjusted_mode);
> +	haswell_set_pipeconf(crtc);
>  
>  	intel_set_pipe_csc(crtc);
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 08:17:10PM +0200, Daniel Vetter wrote:
> The current code is rather ... ugly. The only thing it managed to pull
> off is getting 6bpc on DP working on g4x. Then someone added another
> custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
> implementing the PIPECONF-based dither/bpc controls on g4x/vlv.
> 
> Note that compared to pch based platforms g4x/vlv don't support 12bpc
> modes. g4x is already caught, extend the check for vlv.
> 
> The other fixup is to restrict the lvds-specific dithering to early
> gen4 devices - g4x should use the pipeconf dither controls. Note that
> on gen2/3 the dither control is in the panel fitter even.
> 
> v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
> from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":
> 
> "Programming note: Dithering should only be enabled for 8 bpc or 6
> bpc."
> 
> v3: Actually drop the old ugly dither code.
> 
> v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
> pipes (Jesse).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
>  2 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8c36376..7f1ab8b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
>  	}
>  
> -	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (intel_crtc->config.has_dp_encoder) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -				    PIPECONF_DITHER_EN |
> +	/* only g4x and later have fancy bpc/dither controls */
> +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> +		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
                              ^^^^^^^^^^^^^^^^^

PIPECONF_DITHER_TYPE_MASK

The g4x docs are a bit confusing though. They seem to indicate the the
PIPECONF dither controls only affect DP.

> +
> +		/* Bspec claims that we can't use dithering for 30bpp pipes. */
> +		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
> +			pipeconf |= PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
> -		}
> -	}
>  
> -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
> -						      INTEL_OUTPUT_EDP)) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -					PIPECONF_ENABLE |
> -					I965_PIPECONF_ACTIVE;
> +		pipeconf &= ~PIPECONF_BPC_MASK;
> +		switch (intel_crtc->config.pipe_bpp) {
> +		case 18:
> +			pipeconf |= PIPECONF_6BPC;
> +			break;
> +		case 24:
> +			pipeconf |= PIPECONF_8BPC;
> +			break;
> +		case 30:
> +			pipeconf |= PIPECONF_10BPC;
> +			break;
> +		default:
> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
> +			BUG();
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 58a98ff..094f3c5 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
>  	/* Set the dithering flag on LVDS as needed, note that there is no
>  	 * special lvds dither control bit on pch-split platforms, dithering is
>  	 * only controlled through the PIPECONF reg. */
> -	if (INTEL_INFO(dev)->gen == 4) {
> +	if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) {
>  		if (intel_crtc->config.dither)
>  			temp |= LVDS_ENABLE_DITHER;
>  		else
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 01/15] drm/i915: fixup 12bpc hdmi dotclock handling

[Daniel Vetter]

On Tue, Apr 23, 2013 at 5:02 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Fri, Apr 19, 2013 at 11:24:33AM +0200, Daniel Vetter wrote:
>> We need to multiply the hdmi port dotclock by 1.5x since it's not
>> really a dotclock, but the 10/8 encoding bitclock divided by 10.
>>
>> Also add correct limit checks for the dotclock and reject modes which
>> don't fit. HDMI 1.4 would allow more, but our hw doesn't support that
>> unfortunately :(
>>
>> Somehow I suspect 12bpc hdmi output never really worked - we really
>> need an i-g-t testcase to check all the different pixel modes and
>> outputs.
>>
>> v2: Fixup the adjusted port clock handling - we need to make sure that
>> the fdi link code still gets the real pixelclock.
>>
>> v3: g4x/vlv don't support 12bpc hdmi output so drop the bogus comment.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Seems to match what the spec says. Although I'm getting confused about
> all the clocks we have. Somehow I'd prefer to store the pixel clock in
> the mode, and move the link clock out, but perhaps that makes things
> more messy elsewhere...

Yeah, I think we need to switch over to storing the adjusted pixel
clok in adjuste_mode->clock and keep the link clock in a separate
field in the pipe config. The current code is just a mess.

I haven't done this yet since I need some more clarity first, so that
we don't horribly break eDP ...

> Also I'm confused about the 225 MHz limit. That seems to be specific to
> PCH platforms. My g4x docs say that HDMI limit is 165 MHz. Also
> intel_limits_g4x_hdmi() has dot.max of 400 MHz which seems to contradict
> the docs (400 MHz is correct for the DAC apparently). But then again the
> docs also say that the p1_slow/p2_fast switchover point is 165 MHz, so
> if the max is 165 MHz too, there would never be any need to use p2_fast
> with HDMI. The more I read the docs, the more confused I get.

Yeah, it's a complete disaster. My thinking with the 225 MHz clock is that:
- 12 bpc is a hdmi-only feature, so we can forget about any fancy DVI cases
- hdmi spec up to 1.2 says 225MHz is the upper limit of the 8/10 symbol clock.

So I've figured by limiting things to the 1.2 hdmi spec I could avoid
trying to untangle this mess and trying to reconcile our code with
Bspec. I'm cheap, I know ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

On Tue, Apr 23, 2013 at 06:27:54PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 19, 2013 at 08:17:10PM +0200, Daniel Vetter wrote:
> > The current code is rather ... ugly. The only thing it managed to pull
> > off is getting 6bpc on DP working on g4x. Then someone added another
> > custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
> > implementing the PIPECONF-based dither/bpc controls on g4x/vlv.
> > 
> > Note that compared to pch based platforms g4x/vlv don't support 12bpc
> > modes. g4x is already caught, extend the check for vlv.
> > 
> > The other fixup is to restrict the lvds-specific dithering to early
> > gen4 devices - g4x should use the pipeconf dither controls. Note that
> > on gen2/3 the dither control is in the panel fitter even.
> > 
> > v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
> > from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":
> > 
> > "Programming note: Dithering should only be enabled for 8 bpc or 6
> > bpc."
> > 
> > v3: Actually drop the old ugly dither code.
> > 
> > v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
> > pipes (Jesse).
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++--------------
> >  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
> >  2 files changed, 22 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8c36376..7f1ab8b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
> >  			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> >  	}
> >  
> > -	/* default to 8bpc */
> > -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> > -	if (intel_crtc->config.has_dp_encoder) {
> > -		if (intel_crtc->config.dither) {
> > -			pipeconf |= PIPECONF_6BPC |
> > -				    PIPECONF_DITHER_EN |
> > +	/* only g4x and later have fancy bpc/dither controls */
> > +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> > +		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
>                               ^^^^^^^^^^^^^^^^^
> 
> PIPECONF_DITHER_TYPE_MASK

Will fix up, thansk for spotting this.

> The g4x docs are a bit confusing though. They seem to indicate the the
> PIPECONF dither controls only affect DP.

Hm, this could put a pending question from Jesse at ease whether we should
still enable dithering in the lvds port register on g4x. Can you point me
at the relevant bspec language? I didn't spot anything when hunting around
in the docs ...

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

On Tue, Apr 23, 2013 at 10:39:35PM +0200, Daniel Vetter wrote:
> On Tue, Apr 23, 2013 at 06:27:54PM +0300, Ville Syrjälä wrote:
> > The g4x docs are a bit confusing though. They seem to indicate the the
> > PIPECONF dither controls only affect DP.
> 
> Hm, this could put a pending question from Jesse at ease whether we should
> still enable dithering in the lvds port register on g4x. Can you point me
> at the relevant bspec language? I didn't spot anything when hunting around
> in the docs ...

Hm, I've done some testing and the pipeconf dithering seems to indeed have
no effect on the lvds panel. I've gotten away since this patch didn't
clear that bit correctly. I'll update the patch. If you can dig out the
Bspec reference, that'd still be great.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

The current code is rather ... ugly. The only thing it managed to pull
off is getting 6bpc on DP working on g4x. Then someone added another
custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
implementing the PIPECONF-based dither/bpc controls on g4x/vlv.

Note that compared to pch based platforms g4x/vlv don't support 12bpc
modes. g4x is already caught, extend the check for vlv.

The other fixup is to restrict the lvds-specific dithering to early
gen4 devices - g4x should use the pipeconf dither controls. Note that
on gen2/3 the dither control is in the panel fitter even.

v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":

"Programming note: Dithering should only be enabled for 8 bpc or 6
bpc."

v3: Actually drop the old ugly dither code.

v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
pipes (Jesse).

v5: Also clear the dither type correctly as spotted by Ville.

v6: As Ville pointed out we need to indeed set the dithering both in
the pipeconf register (for DP outputs) and in the LVDS port register
(for LVDS ouputs). Otherwise LVDS panel will not get properly
dithered. The old patch got away with this since it forgot to clear
the LVDS dither bit ...

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 505c2fc..3cdb921 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4591,22 +4591,30 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
 	}
 
-	/* default to 8bpc */
-	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
-	if (intel_crtc->config.has_dp_encoder) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-				    PIPECONF_DITHER_EN |
+	/* only g4x and later have fancy bpc/dither controls */
+	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
+		pipeconf &= ~(PIPECONF_BPC_MASK |
+			      PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
+
+		/* Bspec claims that we can't use dithering for 30bpp pipes. */
+		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
+			pipeconf |= PIPECONF_DITHER_EN |
 				    PIPECONF_DITHER_TYPE_SP;
-		}
-	}
 
-	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
-						      INTEL_OUTPUT_EDP)) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-					PIPECONF_ENABLE |
-					I965_PIPECONF_ACTIVE;
+		pipeconf &= ~PIPECONF_BPC_MASK;
+		switch (intel_crtc->config.pipe_bpp) {
+		case 18:
+			pipeconf |= PIPECONF_6BPC;
+			break;
+		case 24:
+			pipeconf |= PIPECONF_8BPC;
+			break;
+		case 30:
+			pipeconf |= PIPECONF_10BPC;
+			break;
+		default:
+			/* Case prevented by intel_choose_pipe_bpp_dither. */
+			BUG();
 		}
 	}
 
-- 
1.7.11.7

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

Re: [PATCH 02/15] drm/i915: Disable high-bpc on pre-1.4 EDID screens

[Daniel Vetter]

On Tue, Apr 23, 2013 at 06:07:16PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 19, 2013 at 11:24:34AM +0200, Daniel Vetter wrote:
> > Prevents black screens when using 30bpp framebuffers on my
> > HDMI screens here. The DP input on the same screen though reports a
> > 1.4 EDID with the correct 8bpc limit set.
> > 
> > v2: Actually check for the right thing!
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Ville Syrjälä]

On Wed, Apr 24, 2013 at 12:27:59AM +0200, Daniel Vetter wrote:
> On Tue, Apr 23, 2013 at 10:39:35PM +0200, Daniel Vetter wrote:
> > On Tue, Apr 23, 2013 at 06:27:54PM +0300, Ville Syrjälä wrote:
> > > The g4x docs are a bit confusing though. They seem to indicate the the
> > > PIPECONF dither controls only affect DP.
> > 
> > Hm, this could put a pending question from Jesse at ease whether we should
> > still enable dithering in the lvds port register on g4x. Can you point me
> > at the relevant bspec language? I didn't spot anything when hunting around
> > in the docs ...
> 
> Hm, I've done some testing and the pipeconf dithering seems to indeed have
> no effect on the lvds panel. I've gotten away since this patch didn't
> clear that bit correctly. I'll update the patch. If you can dig out the
> Bspec reference, that'd still be great.

It's mentioned in the PIPECONF description.

"4   | Dithering enable [DevCTG]: This bit enables dithering for DisplayPort 6bpc or 8bpc modes"
"3:2 | Dithering type [DevCTG]: This bit selects dithering type for DisplayPort 6bpc or 8bpc modes"

Also the description of "7:5 | Bits Per Color [DevCTG]" also talks a lot about
DP, which maybe implies that it too only affects DP. For SDVO/HDMI the
port register anyway only has 8bpc support so I guess that's covered, but
I'm not sure what the deal is with the CRT output.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Ville Syrjälä]

On Wed, Apr 24, 2013 at 12:30:14AM +0200, Daniel Vetter wrote:
> The current code is rather ... ugly. The only thing it managed to pull
> off is getting 6bpc on DP working on g4x. Then someone added another
> custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
> implementing the PIPECONF-based dither/bpc controls on g4x/vlv.
> 
> Note that compared to pch based platforms g4x/vlv don't support 12bpc
> modes. g4x is already caught, extend the check for vlv.
> 
> The other fixup is to restrict the lvds-specific dithering to early
> gen4 devices - g4x should use the pipeconf dither controls. Note that
> on gen2/3 the dither control is in the panel fitter even.
> 
> v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
> from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":
> 
> "Programming note: Dithering should only be enabled for 8 bpc or 6
> bpc."
> 
> v3: Actually drop the old ugly dither code.
> 
> v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
> pipes (Jesse).
> 
> v5: Also clear the dither type correctly as spotted by Ville.
> 
> v6: As Ville pointed out we need to indeed set the dithering both in
> the pipeconf register (for DP outputs) and in the LVDS port register
> (for LVDS ouputs). Otherwise LVDS panel will not get properly
> dithered. The old patch got away with this since it forgot to clear
> the LVDS dither bit ...
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 505c2fc..3cdb921 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4591,22 +4591,30 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
>  	}
>  
> -	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (intel_crtc->config.has_dp_encoder) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -				    PIPECONF_DITHER_EN |
> +	/* only g4x and later have fancy bpc/dither controls */
> +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> +		pipeconf &= ~(PIPECONF_BPC_MASK |
> +			      PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> +
> +		/* Bspec claims that we can't use dithering for 30bpp pipes. */
> +		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
> +			pipeconf |= PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
> -		}
> -	}
>  
> -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
> -						      INTEL_OUTPUT_EDP)) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -					PIPECONF_ENABLE |
> -					I965_PIPECONF_ACTIVE;
> +		pipeconf &= ~PIPECONF_BPC_MASK;

bpc bits were already cleared. This line can be dropped.

Otherwise:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		switch (intel_crtc->config.pipe_bpp) {
> +		case 18:
> +			pipeconf |= PIPECONF_6BPC;
> +			break;
> +		case 24:
> +			pipeconf |= PIPECONF_8BPC;
> +			break;
> +		case 30:
> +			pipeconf |= PIPECONF_10BPC;
> +			break;
> +		default:
> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
> +			BUG();
>  		}
>  	}
>  
> -- 
> 1.7.11.7

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 07/15] drm/i915: Fixup non-24bpp support for VGA screens on Haswell

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:39AM +0200, Daniel Vetter wrote:
> The LPT PCH only supports 8bpc, so we need to force the pipe bpp
> to the right value.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_crt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 58b4a53..1b9ebf4 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -207,6 +207,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
>  	if (HAS_PCH_SPLIT(dev))
>  		pipe_config->has_pch_encoder = true;
>  
> +	/* LPT FDI RX only supports 8bpc. */
> +	if (HAS_PCH_LPT(dev))
> +		pipe_config->pipe_bpp = 24;
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 09/15] drm/i915: hw state readout support for pipe_config->fdi_lanes

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:41AM +0200, Daniel Vetter wrote:
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7cb1abf..b7774c1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5766,9 +5766,13 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> -	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
> +	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>  		pipe_config->has_pch_encoder = true;
>  
> +		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
> +		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;

Should we add names for the shifts/mask values? Or maybe we have them
already...

And should we check the TX side too and make sure both sides agree on
the number of lanes?

> +	}
> +
>  	return true;
>  }
>  
> @@ -5905,9 +5909,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	 */
>  	tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
> -	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
> +	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE) {
>  		pipe_config->has_pch_encoder = true;
>  
> +		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
> +		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;
> +	}
> +
>  	return true;
>  }
>  
> @@ -7869,6 +7877,14 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
>  		return false;
>  	}
>  
> +	if (current_config->fdi_lanes != pipe_config->fdi_lanes) {
> +		DRM_ERROR("mismatch in fdi_lanes "
> +			  "(expected %i, found %i)\n",
> +			  current_config->fdi_lanes,
> +			  pipe_config->fdi_lanes);
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 10/15] drm/i915: split up fdi_set_m_n into computation and hw setup

[Ville Syrjälä]

On Fri, Apr 19, 2013 at 11:24:42AM +0200, Daniel Vetter wrote:
> And also move the computed m_n values into the pipe_config. This is a
> prep step to move the fdi state computation completely into the
> prepare phase of the modeset sequence. Which will allow us to handle
> fdi link bw constraints in a better way.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
>  2 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b7774c1..7abcd60 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5480,13 +5480,11 @@ void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>  	}
>  }
>  
> -static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
> +static void ironlake_fdi_compute_config(struct intel_crtc *intel_crtc)
>  {
> -	struct drm_device *dev = crtc->dev;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode =
>  		&intel_crtc->config.adjusted_mode;
> -	struct intel_link_m_n m_n = {0};
>  	int target_clock, lane, link_bw;
>  
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
> @@ -5511,9 +5509,7 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
>  	if (intel_crtc->config.pixel_multiplier > 1)
>  		link_bw *= intel_crtc->config.pixel_multiplier;
>  	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
> -			       link_bw, &m_n);
> -
> -	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
> +			       link_bw, &intel_crtc->config.fdi_m_n);
>  }
>  
>  static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
> @@ -5735,8 +5731,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
>  	 * ironlake_check_fdi_lanes. */
>  	intel_crtc->config.fdi_lanes = 0;
> -	if (intel_crtc->config.has_pch_encoder)
> -		ironlake_fdi_set_m_n(crtc);
> +	if (intel_crtc->config.has_pch_encoder) {
> +		ironlake_fdi_compute_config(intel_crtc);
> +
> +		intel_cpu_transcoder_set_m_n(intel_crtc,
> +					     &intel_crtc->config.fdi_m_n);
> +	}
>  
>  	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>  
> @@ -5866,8 +5866,12 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
> -	if (intel_crtc->config.has_pch_encoder)
> -		ironlake_fdi_set_m_n(crtc);
> +	if (intel_crtc->config.has_pch_encoder) {
> +		ironlake_fdi_compute_config(intel_crtc);
> +
> +		intel_cpu_transcoder_set_m_n(intel_crtc,
> +					     &intel_crtc->config.fdi_m_n);
> +	}
>  
>  	haswell_set_pipeconf(crtc);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 179b42b..f40b43f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -230,8 +230,9 @@ struct intel_crtc_config {
>  	/* Used by SDVO (and if we ever fix it, HDMI). */
>  	unsigned pixel_multiplier;
>  
> -	/* FDI lanes used, only valid if has_pch_encoder is set. */
> +	/* FDI configuration, only valid if has_pch_encoder is set. */
>  	int fdi_lanes;
> +	struct intel_link_m_n fdi_m_n;
>  };
>  
>  struct intel_crtc {
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 09/15] drm/i915: hw state readout support for pipe_config->fdi_lanes

[Daniel Vetter]

On Wed, Apr 24, 2013 at 02:23:20PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 19, 2013 at 11:24:41AM +0200, Daniel Vetter wrote:
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7cb1abf..b7774c1 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5766,9 +5766,13 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> >  	if (!(tmp & PIPECONF_ENABLE))
> >  		return false;
> >  
> > -	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
> > +	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> >  		pipe_config->has_pch_encoder = true;
> >  
> > +		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
> > +		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;
> 
> Should we add names for the shifts/mask values? Or maybe we have them
> already...

It's copy&pasta from where we store the lane counts, I can respin the
patch and de-magic both places.

> And should we check the TX side too and make sure both sides agree on
> the number of lanes?

Presumably when the RX is enabled, there's a TX for it. And TX state is a
bit more messy, especially on haswell. Hence just RX. We could add some
checks in the crtc disable code I guess to sanity check TX settings before
disabling things (so that we at least notice broken things), but I've
figured the black screen would be cause enough for a bug report. So imo ok
if we delay adding that until there's a real need for it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 07/15] drm/i915: Fixup non-24bpp support for VGA screens on Haswell

[Daniel Vetter]

On Wed, Apr 24, 2013 at 02:12:55PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 19, 2013 at 11:24:39AM +0200, Daniel Vetter wrote:
> > The LPT PCH only supports 8bpc, so we need to force the pipe bpp
> > to the right value.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Daniel Vetter]

The current code is rather ... ugly. The only thing it managed to pull
off is getting 6bpc on DP working on g4x. Then someone added another
custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
implementing the PIPECONF-based dither/bpc controls on g4x/vlv.

Note that compared to pch based platforms g4x/vlv don't support 12bpc
modes. g4x is already caught, extend the check for vlv.

The other fixup is to restrict the lvds-specific dithering to early
gen4 devices - g4x should use the pipeconf dither controls. Note that
on gen2/3 the dither control is in the panel fitter even.

v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":

"Programming note: Dithering should only be enabled for 8 bpc or 6
bpc."

v3: Actually drop the old ugly dither code.

v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
pipes (Jesse).

v5: Also clear the dither type correctly as spotted by Ville.

v6: As Ville pointed out we need to indeed set the dithering both in
the pipeconf register (for DP outputs) and in the LVDS port register
(for LVDS ouputs). Otherwise LVDS panel will not get properly
dithered. The old patch got away with this since it forgot to clear
the LVDS dither bit ...

v7: Remove redundant BPC_MASK clearing, spotted by Ville.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0fde86c..e10d9b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4601,22 +4601,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
 	}
 
-	/* default to 8bpc */
-	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
-	if (intel_crtc->config.has_dp_encoder) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-				    PIPECONF_DITHER_EN |
+	/* only g4x and later have fancy bpc/dither controls */
+	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
+		pipeconf &= ~(PIPECONF_BPC_MASK |
+			      PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
+
+		/* Bspec claims that we can't use dithering for 30bpp pipes. */
+		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
+			pipeconf |= PIPECONF_DITHER_EN |
 				    PIPECONF_DITHER_TYPE_SP;
-		}
-	}
 
-	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
-						      INTEL_OUTPUT_EDP)) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-					PIPECONF_ENABLE |
-					I965_PIPECONF_ACTIVE;
+		switch (intel_crtc->config.pipe_bpp) {
+		case 18:
+			pipeconf |= PIPECONF_6BPC;
+			break;
+		case 24:
+			pipeconf |= PIPECONF_8BPC;
+			break;
+		case 30:
+			pipeconf |= PIPECONF_10BPC;
+			break;
+		default:
+			/* Case prevented by intel_choose_pipe_bpp_dither. */
+			BUG();
 		}
 	}
 
-- 
1.7.11.7

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

Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

[Ville Syrjälä]

On Wed, Apr 24, 2013 at 02:57:17PM +0200, Daniel Vetter wrote:
> The current code is rather ... ugly. The only thing it managed to pull
> off is getting 6bpc on DP working on g4x. Then someone added another
> custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
> implementing the PIPECONF-based dither/bpc controls on g4x/vlv.
> 
> Note that compared to pch based platforms g4x/vlv don't support 12bpc
> modes. g4x is already caught, extend the check for vlv.
> 
> The other fixup is to restrict the lvds-specific dithering to early
> gen4 devices - g4x should use the pipeconf dither controls. Note that
> on gen2/3 the dither control is in the panel fitter even.
> 
> v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
> from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":
> 
> "Programming note: Dithering should only be enabled for 8 bpc or 6
> bpc."
> 
> v3: Actually drop the old ugly dither code.
> 
> v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
> pipes (Jesse).
> 
> v5: Also clear the dither type correctly as spotted by Ville.
> 
> v6: As Ville pointed out we need to indeed set the dithering both in
> the pipeconf register (for DP outputs) and in the LVDS port register
> (for LVDS ouputs). Otherwise LVDS panel will not get properly
> dithered. The old patch got away with this since it forgot to clear
> the LVDS dither bit ...
> 
> v7: Remove redundant BPC_MASK clearing, spotted by Ville.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0fde86c..e10d9b3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4601,22 +4601,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
>  	}
>  
> -	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (intel_crtc->config.has_dp_encoder) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -				    PIPECONF_DITHER_EN |
> +	/* only g4x and later have fancy bpc/dither controls */
> +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> +		pipeconf &= ~(PIPECONF_BPC_MASK |
> +			      PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> +
> +		/* Bspec claims that we can't use dithering for 30bpp pipes. */
> +		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
> +			pipeconf |= PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
> -		}
> -	}
>  
> -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
> -						      INTEL_OUTPUT_EDP)) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -					PIPECONF_ENABLE |
> -					I965_PIPECONF_ACTIVE;
> +		switch (intel_crtc->config.pipe_bpp) {
> +		case 18:
> +			pipeconf |= PIPECONF_6BPC;
> +			break;
> +		case 24:
> +			pipeconf |= PIPECONF_8BPC;
> +			break;
> +		case 30:
> +			pipeconf |= PIPECONF_10BPC;
> +			break;
> +		default:
> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
> +			BUG();
>  		}
>  	}
>  
> -- 
> 1.7.11.7

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: hw state readout support for pipe_config->fdi_lanes

[Daniel Vetter]

v2: Introduce some nice #defines for the FDI lane width fields and put
them to good use. Suggested by Ville.

Cc: Ville Syrjälä <ville.syrjala@linux.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     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++++++++----------
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 894d729..e96ee12 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4138,10 +4138,9 @@
 #define  FDI_LINK_TRAIN_600MV_3_5DB_SNB_B	(0x39<<22)
 #define  FDI_LINK_TRAIN_800MV_0DB_SNB_B		(0x38<<22)
 #define  FDI_LINK_TRAIN_VOL_EMP_MASK		(0x3f<<22)
-#define  FDI_DP_PORT_WIDTH_X1           (0<<19)
-#define  FDI_DP_PORT_WIDTH_X2           (1<<19)
-#define  FDI_DP_PORT_WIDTH_X3           (2<<19)
-#define  FDI_DP_PORT_WIDTH_X4           (3<<19)
+#define  FDI_DP_PORT_WIDTH_SHIFT		19
+#define  FDI_DP_PORT_WIDTH_MASK			(7 << 19)
+#define  FDI_DP_PORT_WIDTH(width)           (((width) - 1) << 19)
 #define  FDI_TX_ENHANCE_FRAME_ENABLE    (1<<18)
 /* Ironlake: hardwired to 1 */
 #define  FDI_TX_PLL_ENABLE              (1<<14)
@@ -4166,7 +4165,6 @@
 /* train, dp width same as FDI_TX */
 #define  FDI_FS_ERRC_ENABLE		(1<<27)
 #define  FDI_FE_ERRC_ENABLE		(1<<26)
-#define  FDI_DP_PORT_WIDTH_X8           (7<<19)
 #define  FDI_RX_POLARITY_REVERSED_LPT	(1<<16)
 #define  FDI_8BPC                       (0<<16)
 #define  FDI_10BPC                      (1<<16)
@@ -4188,9 +4186,6 @@
 #define  FDI_LINK_TRAIN_PATTERN_IDLE_CPT	(2<<8)
 #define  FDI_LINK_TRAIN_NORMAL_CPT		(3<<8)
 #define  FDI_LINK_TRAIN_PATTERN_MASK_CPT	(3<<8)
-/* LPT */
-#define  FDI_PORT_WIDTH_2X_LPT			(1<<19)
-#define  FDI_PORT_WIDTH_1X_LPT			(0<<19)
 
 #define _FDI_RXA_MISC			0xf0010
 #define _FDI_RXB_MISC			0xf1010
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1ecb0f3..96354a5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -182,7 +182,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	/* Enable the PCH Receiver FDI PLL */
 	rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
 		     FDI_RX_PLL_ENABLE |
-		     ((intel_crtc->config.fdi_lanes - 1) << 19);
+		     FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
 	POSTING_READ(_FDI_RXA_CTL);
 	udelay(220);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 652c6b8..85ccd2d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2419,8 +2419,8 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~(7 << 19);
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~FDI_DP_PORT_WIDTH_SHIFT;
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp &= ~FDI_LINK_TRAIN_NONE;
 	temp |= FDI_LINK_TRAIN_PATTERN_1;
 	I915_WRITE(reg, temp | FDI_TX_ENABLE);
@@ -2517,8 +2517,8 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~(7 << 19);
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~FDI_DP_PORT_WIDTH_MASK;
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp &= ~FDI_LINK_TRAIN_NONE;
 	temp |= FDI_LINK_TRAIN_PATTERN_1;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
@@ -2652,8 +2652,8 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~(7 << 19);
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~FDI_DP_PORT_WIDTH_MASK;
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp &= ~(FDI_LINK_TRAIN_AUTO | FDI_LINK_TRAIN_NONE_IVB);
 	temp |= FDI_LINK_TRAIN_PATTERN_1_IVB;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
@@ -2754,8 +2754,8 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
 	/* enable PCH FDI RX PLL, wait warmup plus DMI latency */
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~((0x7 << 19) | (0x7 << 16));
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~(FDI_DP_PORT_WIDTH_SHIFT | (0x7 << 16));
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
 	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
 
@@ -5763,9 +5763,14 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
-	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
+	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		pipe_config->has_pch_encoder = true;
 
+		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
+		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
+					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
+	}
+
 	return true;
 }
 
@@ -5902,9 +5907,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	 */
 	tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
-	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
+	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE) {
 		pipe_config->has_pch_encoder = true;
 
+		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
+		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
+					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
+	}
+
 	return true;
 }
 
@@ -7866,6 +7876,14 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 		return false;
 	}
 
+	if (current_config->fdi_lanes != pipe_config->fdi_lanes) {
+		DRM_ERROR("mismatch in fdi_lanes "
+			  "(expected %i, found %i)\n",
+			  current_config->fdi_lanes,
+			  pipe_config->fdi_lanes);
+		return false;
+	}
+
 	return true;
 }
 
-- 
1.7.11.7

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

Re: [PATCH 00/15] high-bpp fixes and fdi auto dithering

[Jani Nikula]

The version at Daniel's fdi-dither branch (which is without the hack in
"drm/i915: force bpp for eDP panels") is 

Tested-by: Jani Nikula <jani.nikula@intel.com>

on VLV.

On Fri, 19 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> This fixes all the bugs I've found in my various systems when using non-24bpp
> modes as the first part of the series.
>
> And with working non-standard bpp support I've figured we can go fancy and
> implemented auto-dithering if we hit an fdi bw limit. Which means that you can
> now use 3-pipe pch configurations on ivb on pretty much everywhere. The only
> restriction is that you need to fire up pipe C first, since without atomic
> modeset pipe B will otherwise too much bw.
>
> One big thing here is that this will break Paulo's hsw eDP machine, specifically
> the patch called "drm/i915: force bpp for eDP panels". But apparently without
> that my machine here is broken ... Ideas highly welcome about how we could quirk
> ourselves out of this mess.
>
> Cheers, Daniel
>
> Daniel Vetter (15):
>   drm/i915: fixup 12bpc hdmi dotclock handling
>   drm/i915: Disable high-bpc on pre-1.4 EDID screens
>   drm/i915: force bpp for eDP panels
>   drm/i915: drop adjusted_mode from *_set_pipeconf functions
>   drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv
>   drm/i915: allow high-bpc modes on DP
>   drm/i915: Fixup non-24bpp support for VGA screens on Haswell
>   drm/i915: move intel_crtc->fdi_lanes to pipe_config
>   drm/i915: hw state readout support for pipe_config->fdi_lanes
>   drm/i915: split up fdi_set_m_n into computation and hw setup
>   drm/i915: compute fdi lane config earlier
>   drm/i915: Split up ironlake_check_fdi_lanes
>   drm/i915: move fdi lane configuration checks ahead
>   drm/i915: don't count cpu ports for fdi B/C lane sharing
>   drm/i915: implement fdi auto-dithering
>
>  drivers/gpu/drm/i915/intel_crt.c     |   4 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   7 +-
>  drivers/gpu/drm/i915/intel_display.c | 349 ++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_dp.c      |  12 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  12 +-
>  drivers/gpu/drm/i915/intel_hdmi.c    |  31 +++-
>  drivers/gpu/drm/i915/intel_lvds.c    |   4 +-
>  7 files changed, 275 insertions(+), 144 deletions(-)
>
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/15] drm/i915: allow high-bpc modes on DP

[Imre Deak]

On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> Totally untested due to lack of screens supporting more than 8bpc. But
> now we should have closed all holes in our bpp handling, so this
> should be safe. The last missing piece was 10bpc support for g4x/vlv,
> since we directly use the pipe bpp to feed the display link (and
> anyway, only the cpt has any means to have a pipe bpp != the display
> link bpp).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1ac7235..50a9d9f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -744,7 +744,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
>  	 * bpc in between. */
> -	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
> +	bpp = pipe_config->pipe_bpp;
>  
>  	/* eDP panels are really fickle, try to enfore the bpp the firmware
>  	 * recomments. This means we'll up-dither 16bpp framebuffers on

Re: [PATCH 08/15] drm/i915: move intel_crtc->fdi_lanes to pipe_config

[Imre Deak]

On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> We need this for two reasons:
> - Correct handling of shared fdi lanes on ivb with fastboot.
> - Handling fdi link bw limits when we only have two fdi lanes by
>   dithering down a bit.
> 
> Just search&replace in this patch, no functional change at all.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |  7 ++++---
>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
>  3 files changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e14fe5f..2e12ee7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -181,7 +181,8 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  
>  	/* Enable the PCH Receiver FDI PLL */
>  	rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
> -		     FDI_RX_PLL_ENABLE | ((intel_crtc->fdi_lanes - 1) << 19);
> +		     FDI_RX_PLL_ENABLE |
> +		     ((intel_crtc->config.fdi_lanes - 1) << 19);
>  	I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
>  	POSTING_READ(_FDI_RXA_CTL);
>  	udelay(220);
> @@ -209,7 +210,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  		 * port reversal bit */
>  		I915_WRITE(DDI_BUF_CTL(PORT_E),
>  			   DDI_BUF_CTL_ENABLE |
> -			   ((intel_crtc->fdi_lanes - 1) << 1) |
> +			   ((intel_crtc->config.fdi_lanes - 1) << 1) |
>  			   hsw_ddi_buf_ctl_values[i / 2]);
>  		POSTING_READ(DDI_BUF_CTL(PORT_E));
>  
> @@ -1022,7 +1023,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
>  
>  	} else if (type == INTEL_OUTPUT_ANALOG) {
>  		temp |= TRANS_DDI_MODE_SELECT_FDI;
> -		temp |= (intel_crtc->fdi_lanes - 1) << 1;
> +		temp |= (intel_crtc->config.fdi_lanes - 1) << 1;
>  
>  	} else if (type == INTEL_OUTPUT_DISPLAYPORT ||
>  		   type == INTEL_OUTPUT_EDP) {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c2579c0..7cb1abf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2432,7 +2432,7 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
>  	reg = FDI_TX_CTL(pipe);
>  	temp = I915_READ(reg);
>  	temp &= ~(7 << 19);
> -	temp |= (intel_crtc->fdi_lanes - 1) << 19;
> +	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
>  	temp &= ~FDI_LINK_TRAIN_NONE;
>  	temp |= FDI_LINK_TRAIN_PATTERN_1;
>  	I915_WRITE(reg, temp | FDI_TX_ENABLE);
> @@ -2530,7 +2530,7 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
>  	reg = FDI_TX_CTL(pipe);
>  	temp = I915_READ(reg);
>  	temp &= ~(7 << 19);
> -	temp |= (intel_crtc->fdi_lanes - 1) << 19;
> +	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
>  	temp &= ~FDI_LINK_TRAIN_NONE;
>  	temp |= FDI_LINK_TRAIN_PATTERN_1;
>  	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
> @@ -2665,7 +2665,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
>  	reg = FDI_TX_CTL(pipe);
>  	temp = I915_READ(reg);
>  	temp &= ~(7 << 19);
> -	temp |= (intel_crtc->fdi_lanes - 1) << 19;
> +	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
>  	temp &= ~(FDI_LINK_TRAIN_AUTO | FDI_LINK_TRAIN_NONE_IVB);
>  	temp |= FDI_LINK_TRAIN_PATTERN_1_IVB;
>  	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
> @@ -2767,7 +2767,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>  	reg = FDI_RX_CTL(pipe);
>  	temp = I915_READ(reg);
>  	temp &= ~((0x7 << 19) | (0x7 << 16));
> -	temp |= (intel_crtc->fdi_lanes - 1) << 19;
> +	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
>  	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
>  
> @@ -5379,12 +5379,12 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
>  		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
>  
>  	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
> -		      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
> -	if (intel_crtc->fdi_lanes > 4) {
> +		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> +	if (intel_crtc->config.fdi_lanes > 4) {
>  		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
> -			      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
> +			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
>  		/* Clamp lanes to avoid programming the hw with bogus values. */
> -		intel_crtc->fdi_lanes = 4;
> +		intel_crtc->config.fdi_lanes = 4;
>  
>  		return false;
>  	}
> @@ -5397,28 +5397,28 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
>  		return true;
>  	case PIPE_B:
>  		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> -		    intel_crtc->fdi_lanes > 2) {
> +		    intel_crtc->config.fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> -				      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
> +				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
>  			/* Clamp lanes to avoid programming the hw with bogus values. */
> -			intel_crtc->fdi_lanes = 2;
> +			intel_crtc->config.fdi_lanes = 2;
>  
>  			return false;
>  		}
>  
> -		if (intel_crtc->fdi_lanes > 2)
> +		if (intel_crtc->config.fdi_lanes > 2)
>  			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
>  		else
>  			cpt_enable_fdi_bc_bifurcation(dev);
>  
>  		return true;
>  	case PIPE_C:
> -		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->fdi_lanes <= 2) {
> -			if (intel_crtc->fdi_lanes > 2) {
> +		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
> +			if (intel_crtc->config.fdi_lanes > 2) {
>  				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> -					      pipe_name(intel_crtc->pipe), intel_crtc->fdi_lanes);
> +					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
>  				/* Clamp lanes to avoid programming the hw with bogus values. */
> -				intel_crtc->fdi_lanes = 2;
> +				intel_crtc->config.fdi_lanes = 2;
>  
>  				return false;
>  			}
> @@ -5506,7 +5506,7 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
>  	lane = ironlake_get_lanes_required(target_clock, link_bw,
>  					   intel_crtc->config.pipe_bpp);
>  
> -	intel_crtc->fdi_lanes = lane;
> +	intel_crtc->config.fdi_lanes = lane;
>  
>  	if (intel_crtc->config.pixel_multiplier > 1)
>  		link_bw *= intel_crtc->config.pixel_multiplier;
> @@ -5734,7 +5734,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
>  	 * ironlake_check_fdi_lanes. */
> -	intel_crtc->fdi_lanes = 0;
> +	intel_crtc->config.fdi_lanes = 0;
>  	if (intel_crtc->config.has_pch_encoder)
>  		ironlake_fdi_set_m_n(crtc);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e3ca7e7..179b42b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -229,6 +229,9 @@ struct intel_crtc_config {
>  	int pixel_target_clock;
>  	/* Used by SDVO (and if we ever fix it, HDMI). */
>  	unsigned pixel_multiplier;
> +
> +	/* FDI lanes used, only valid if has_pch_encoder is set. */
> +	int fdi_lanes;
>  };
>  
>  struct intel_crtc {
> @@ -247,7 +250,6 @@ struct intel_crtc {
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
>  	struct intel_unpin_work *unpin_work;
> -	int fdi_lanes;
>  
>  	atomic_t unpin_work_count;
>  

Re: [PATCH] drm/i915: hw state readout support for pipe_config->fdi_lanes

[Imre Deak]

On Wed, 2013-04-24 at 15:30 +0200, Daniel Vetter wrote:
> v2: Introduce some nice #defines for the FDI lane width fields and put
> them to good use. Suggested by Ville.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.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     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++++++++----------
>  3 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 894d729..e96ee12 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4138,10 +4138,9 @@
>  #define  FDI_LINK_TRAIN_600MV_3_5DB_SNB_B	(0x39<<22)
>  #define  FDI_LINK_TRAIN_800MV_0DB_SNB_B		(0x38<<22)
>  #define  FDI_LINK_TRAIN_VOL_EMP_MASK		(0x3f<<22)
> -#define  FDI_DP_PORT_WIDTH_X1           (0<<19)
> -#define  FDI_DP_PORT_WIDTH_X2           (1<<19)
> -#define  FDI_DP_PORT_WIDTH_X3           (2<<19)
> -#define  FDI_DP_PORT_WIDTH_X4           (3<<19)
> +#define  FDI_DP_PORT_WIDTH_SHIFT		19
> +#define  FDI_DP_PORT_WIDTH_MASK			(7 << 19)
> +#define  FDI_DP_PORT_WIDTH(width)           (((width) - 1) << 19)

You could use FDI_DP_PORT_WIDTH_SHIFT in the above two macros.

>  #define  FDI_TX_ENHANCE_FRAME_ENABLE    (1<<18)
>  /* Ironlake: hardwired to 1 */
>  #define  FDI_TX_PLL_ENABLE              (1<<14)
> @@ -4166,7 +4165,6 @@
>  /* train, dp width same as FDI_TX */
>  #define  FDI_FS_ERRC_ENABLE		(1<<27)
>  #define  FDI_FE_ERRC_ENABLE		(1<<26)
> -#define  FDI_DP_PORT_WIDTH_X8           (7<<19)
>  #define  FDI_RX_POLARITY_REVERSED_LPT	(1<<16)
>  #define  FDI_8BPC                       (0<<16)
>  #define  FDI_10BPC                      (1<<16)
> @@ -4188,9 +4186,6 @@
>  #define  FDI_LINK_TRAIN_PATTERN_IDLE_CPT	(2<<8)
>  #define  FDI_LINK_TRAIN_NORMAL_CPT		(3<<8)
>  #define  FDI_LINK_TRAIN_PATTERN_MASK_CPT	(3<<8)
> -/* LPT */
> -#define  FDI_PORT_WIDTH_2X_LPT			(1<<19)
> -#define  FDI_PORT_WIDTH_1X_LPT			(0<<19)
>  
>  #define _FDI_RXA_MISC			0xf0010
>  #define _FDI_RXB_MISC			0xf1010
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 1ecb0f3..96354a5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -182,7 +182,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  	/* Enable the PCH Receiver FDI PLL */
>  	rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
>  		     FDI_RX_PLL_ENABLE |
> -		     ((intel_crtc->config.fdi_lanes - 1) << 19);
> +		     FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
>  	I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
>  	POSTING_READ(_FDI_RXA_CTL);
>  	udelay(220);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 652c6b8..85ccd2d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2419,8 +2419,8 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
>  	/* enable CPU FDI TX and PCH FDI RX */
>  	reg = FDI_TX_CTL(pipe);
>  	temp = I915_READ(reg);
> -	temp &= ~(7 << 19);
> -	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
> +	temp &= ~FDI_DP_PORT_WIDTH_SHIFT;

FDI_DP_PORT_WIDTH_MASK

> +	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
>  	temp &= ~FDI_LINK_TRAIN_NONE;
>  	temp |= FDI_LINK_TRAIN_PATTERN_1;
>  	I915_WRITE(reg, temp | FDI_TX_ENABLE);
> @@ -2517,8 +2517,8 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
>  	/* enable CPU FDI TX and PCH FDI RX */
>  	reg = FDI_TX_CTL(pipe);
>  	temp = I915_READ(reg);
> -	temp &= ~(7 << 19);
> -	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
> +	temp &= ~FDI_DP_PORT_WIDTH_MASK;
> +	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
>  	temp &= ~FDI_LINK_TRAIN_NONE;
>  	temp |= FDI_LINK_TRAIN_PATTERN_1;
>  	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
> @@ -2652,8 +2652,8 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
>  	/* enable CPU FDI TX and PCH FDI RX */
>  	reg = FDI_TX_CTL(pipe);
>  	temp = I915_READ(reg);
> -	temp &= ~(7 << 19);
> -	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
> +	temp &= ~FDI_DP_PORT_WIDTH_MASK;
> +	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
>  	temp &= ~(FDI_LINK_TRAIN_AUTO | FDI_LINK_TRAIN_NONE_IVB);
>  	temp |= FDI_LINK_TRAIN_PATTERN_1_IVB;
>  	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
> @@ -2754,8 +2754,8 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>  	/* enable PCH FDI RX PLL, wait warmup plus DMI latency */
>  	reg = FDI_RX_CTL(pipe);
>  	temp = I915_READ(reg);
> -	temp &= ~((0x7 << 19) | (0x7 << 16));
> -	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
> +	temp &= ~(FDI_DP_PORT_WIDTH_SHIFT | (0x7 << 16));

FDI_DP_PORT_WIDTH_MASK

Other than the above typos:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
>  	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
>  
> @@ -5763,9 +5763,14 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> -	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
> +	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>  		pipe_config->has_pch_encoder = true;
>  
> +		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
> +		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
> +					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
> +	}
> +
>  	return true;
>  }
>  
> @@ -5902,9 +5907,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	 */
>  	tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
> -	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
> +	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE) {
>  		pipe_config->has_pch_encoder = true;
>  
> +		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
> +		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
> +					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
> +	}
> +
>  	return true;
>  }
>  
> @@ -7866,6 +7876,14 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
>  		return false;
>  	}
>  
> +	if (current_config->fdi_lanes != pipe_config->fdi_lanes) {
> +		DRM_ERROR("mismatch in fdi_lanes "
> +			  "(expected %i, found %i)\n",
> +			  current_config->fdi_lanes,
> +			  pipe_config->fdi_lanes);
> +		return false;
> +	}
> +
>  	return true;
>  }
>  


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

Re: [PATCH 11/15] drm/i915: compute fdi lane config earlier

[Imre Deak]

On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> Now that it's split up, we can easily move it around and precompute
> the fdi lane configuration.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 71 +++++++++++++++++-------------------
>  1 file changed, 34 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7abcd60..8e10e1b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3955,6 +3955,37 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> +static void ironlake_fdi_compute_config(struct drm_device *dev,
> +					struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> +	int target_clock, lane, link_bw;
> +
> +	/* 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
> +	 * mode pixel clock is stored in units of 1KHz.
> +	 * Hence the bw of each lane in terms of the mode signal
> +	 * is:
> +	 */
> +	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
> +
> +	if (pipe_config->pixel_target_clock)
> +		target_clock = pipe_config->pixel_target_clock;
> +	else
> +		target_clock = adjusted_mode->clock;
> +
> +	lane = ironlake_get_lanes_required(target_clock, link_bw,
> +					   pipe_config->pipe_bpp);
> +
> +	pipe_config->fdi_lanes = lane;
> +
> +	if (pipe_config->pixel_multiplier > 1)
> +		link_bw *= pipe_config->pixel_multiplier;
> +	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
> +			       link_bw, &pipe_config->fdi_m_n);
> +}
> +
>  static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  				      struct intel_crtc_config *pipe_config)
>  {
> @@ -3989,6 +4020,9 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  		pipe_config->pipe_bpp = 8*3;
>  	}
>  
> +	if (pipe_config->has_pch_encoder)
> +		ironlake_fdi_compute_config(dev, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -5480,38 +5514,6 @@ void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>  	}
>  }
>  
> -static void ironlake_fdi_compute_config(struct intel_crtc *intel_crtc)
> -{
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_display_mode *adjusted_mode =
> -		&intel_crtc->config.adjusted_mode;
> -	int target_clock, lane, link_bw;
> -
> -	/* 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
> -	 * mode pixel clock is stored in units of 1KHz.
> -	 * Hence the bw of each lane in terms of the mode signal
> -	 * is:
> -	 */
> -	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
> -
> -	if (intel_crtc->config.pixel_target_clock)
> -		target_clock = intel_crtc->config.pixel_target_clock;
> -	else
> -		target_clock = adjusted_mode->clock;
> -
> -	lane = ironlake_get_lanes_required(target_clock, link_bw,
> -					   intel_crtc->config.pipe_bpp);
> -
> -	intel_crtc->config.fdi_lanes = lane;
> -
> -	if (intel_crtc->config.pixel_multiplier > 1)
> -		link_bw *= intel_crtc->config.pixel_multiplier;
> -	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
> -			       link_bw, &intel_crtc->config.fdi_m_n);
> -}
> -
>  static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
>  {
>  	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
> @@ -5730,10 +5732,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
>  	 * ironlake_check_fdi_lanes. */
> -	intel_crtc->config.fdi_lanes = 0;
>  	if (intel_crtc->config.has_pch_encoder) {
> -		ironlake_fdi_compute_config(intel_crtc);
> -
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
>  					     &intel_crtc->config.fdi_m_n);
>  	}
> @@ -5867,8 +5866,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
>  	if (intel_crtc->config.has_pch_encoder) {
> -		ironlake_fdi_compute_config(intel_crtc);
> -
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
>  					     &intel_crtc->config.fdi_m_n);
>  	}

Re: [PATCH 12/15] drm/i915: Split up ironlake_check_fdi_lanes

[Imre Deak]

On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> Again in preparation to move the configuration checks into the
> pipe_config computation stage of the modeset sequence.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8e10e1b..c9a2f0b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5440,11 +5440,6 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
>  			return false;
>  		}
>  
> -		if (intel_crtc->config.fdi_lanes > 2)
> -			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
> -		else
> -			cpt_enable_fdi_bc_bifurcation(dev);
> -
>  		return true;
>  	case PIPE_C:
>  		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
> @@ -5461,9 +5456,31 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
>  			return false;
>  		}
>  
> +		return true;
> +	default:
> +		BUG();
> +	}
> +}
> +
> +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_device *dev = intel_crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	switch (intel_crtc->pipe) {
> +	case PIPE_A:
> +		break;
> +	case PIPE_B:
> +		if (intel_crtc->config.fdi_lanes > 2)
> +			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
> +		else
> +			cpt_enable_fdi_bc_bifurcation(dev);
> +
> +		break;
> +	case PIPE_C:
>  		cpt_enable_fdi_bc_bifurcation(dev);
>  
> -		return true;
> +		break;
>  	default:
>  		BUG();
>  	}
> @@ -5738,6 +5755,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	}
>  
>  	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
> +	if (IS_IVYBRIDGE(dev))
> +		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
>  
>  	ironlake_set_pipeconf(crtc);
>  

Re: [PATCH] drm/i915: move fdi lane configuration checks ahead

[Imre Deak]

On Mon, 2013-04-22 at 17:13 +0200, Daniel Vetter wrote:
> This nicely allows us to drop some hacks which have only been used
> to work around modeset failures due to lack of fdi lanes.
> 
> v2: Implement proper checking for Haswell platforms - the fdi link to
> the LPT PCH has only 2 lanes. Note that we already filter out
> impossible modes in intel_crt_mode_valid. Unfortunately LPT does not
> support 6bpc on the fdi rx, so we can't pull clever tricks to squeeze
> in a few more modes.
> 
> v2: Rebased on top of Ben Widawsky's num_pipes reorg.
> 
> v3: Rebase on top of Ville's pipe debug output ocd rampage.
> 
> v4: Fixup rebase fail spotted by Ville.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 129 ++++++++++++++++++-----------------
>  1 file changed, 65 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9ff3cf6..53d6e2f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3943,9 +3943,68 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> -static void ironlake_fdi_compute_config(struct drm_device *dev,
> +static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> +				     struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *pipe_B_crtc =
> +		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> +
> +	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 false;
> +	}
> +
> +	if (IS_HASWELL(dev)) {
> +		if (pipe_config->fdi_lanes > 2) {
> +			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
> +				      pipe_config->fdi_lanes);
> +			return false;
> +		} else {
> +			return true;
> +		}
> +	}
> +
> +	if (INTEL_INFO(dev)->num_pipes)

Copy&paste error I assume, should be num_pipes == 2. Otherwise:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +		return true;
> +
> +	/* Ivybridge 3 pipe is really complicated */
> +	switch (pipe) {
> +	case PIPE_A:
> +		return true;
> +	case PIPE_B:
> +		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> +		    pipe_config->fdi_lanes > 2) {
> +			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> +				      pipe_name(pipe), pipe_config->fdi_lanes);
> +			return false;
> +		}
> +		return true;
> +	case PIPE_C:
> +		if (!pipe_B_crtc->base.enabled ||
> +		    pipe_B_crtc->config.fdi_lanes <= 2) {
> +			if (pipe_config->fdi_lanes > 2) {
> +				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> +					      pipe_name(pipe), pipe_config->fdi_lanes);
> +				return false;
> +			}
> +		} else {
> +			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> +			return false;
> +		}
> +		return true;
> +	default:
> +		BUG();
> +	}
> +}
> +
> +static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  					struct intel_crtc_config *pipe_config)
>  {
> +	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
>  	int target_clock, lane, link_bw;
>  
> @@ -3972,6 +4031,9 @@ static void ironlake_fdi_compute_config(struct drm_device *dev,
>  		link_bw *= pipe_config->pixel_multiplier;
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
>  			       link_bw, &pipe_config->fdi_m_n);
> +
> +	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
> +					intel_crtc->pipe, pipe_config);
>  }
>  
>  static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> @@ -4009,7 +4071,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  	}
>  
>  	if (pipe_config->has_pch_encoder)
> -		ironlake_fdi_compute_config(dev, pipe_config);
> +		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
>  
>  	return true;
>  }
> @@ -5393,63 +5455,6 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
>  	POSTING_READ(SOUTH_CHICKEN1);
>  }
>  
> -static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
> -{
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *pipe_B_crtc =
> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> -
> -	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
> -		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -	if (intel_crtc->config.fdi_lanes > 4) {
> -		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
> -			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -		/* Clamp lanes to avoid programming the hw with bogus values. */
> -		intel_crtc->config.fdi_lanes = 4;
> -
> -		return false;
> -	}
> -
> -	if (INTEL_INFO(dev)->num_pipes == 2)
> -		return true;
> -
> -	switch (intel_crtc->pipe) {
> -	case PIPE_A:
> -		return true;
> -	case PIPE_B:
> -		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> -		    intel_crtc->config.fdi_lanes > 2) {
> -			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> -				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -			/* Clamp lanes to avoid programming the hw with bogus values. */
> -			intel_crtc->config.fdi_lanes = 2;
> -
> -			return false;
> -		}
> -
> -		return true;
> -	case PIPE_C:
> -		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
> -			if (intel_crtc->config.fdi_lanes > 2) {
> -				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
> -					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
> -				/* Clamp lanes to avoid programming the hw with bogus values. */
> -				intel_crtc->config.fdi_lanes = 2;
> -
> -				return false;
> -			}
> -		} else {
> -			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> -			return false;
> -		}
> -
> -		return true;
> -	default:
> -		BUG();
> -	}
> -}
> -
>  static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
> @@ -5641,7 +5646,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	bool is_lvds = false;
>  	struct intel_encoder *encoder;
>  	int ret;
> -	bool fdi_config_ok;
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder) {
>  		switch (encoder->type) {
> @@ -5734,14 +5738,11 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
> -	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
> -	 * ironlake_check_fdi_lanes. */
>  	if (intel_crtc->config.has_pch_encoder) {
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
>  					     &intel_crtc->config.fdi_m_n);
>  	}
>  
> -	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>  	if (IS_IVYBRIDGE(dev))
>  		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
>  
> @@ -5757,7 +5758,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
>  
> -	return fdi_config_ok ? ret : -EINVAL;
> +	return ret;
>  }
>  
>  static bool ironlake_get_pipe_config(struct intel_crtc *crtc,


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

Re: [PATCH 14/15] drm/i915: don't count cpu ports for fdi B/C lane sharing

[Imre Deak]

On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> This allows us to use all 4 fdi lanes on fdi B when the cpu eDP is
> running on pipe C. Yay!
> 
> v2: Encapsulate test into a little helper function, as suggested by
> Chris Wilson.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6bcb196..c25dbdd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2381,6 +2381,11 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
>  			   FDI_FE_ERRC_ENABLE);
>  }
>  
> +static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
> +{
> +	return intel_crtc->base.enabled && intel_crtc->config.has_pch_encoder;
> +}
> +
>  static void ivb_modeset_global_resources(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2390,10 +2395,13 @@ static void ivb_modeset_global_resources(struct drm_device *dev)
>  		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
>  	uint32_t temp;
>  
> -	/* When everything is off disable fdi C so that we could enable fdi B
> -	 * with all lanes. XXX: This misses the case where a pipe is not using
> -	 * any pch resources and so doesn't need any fdi lanes. */
> -	if (!pipe_B_crtc->base.enabled && !pipe_C_crtc->base.enabled) {
> +	/*
> +	 * When everything is off disable fdi C so that we could enable fdi B
> +	 * with all lanes. Note that we don't care about enabled pipes without
> +	 * an enabled pch encoder.
> +	 */
> +	if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> +	    !pipe_has_enabled_pch(pipe_C_crtc)) {
>  		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
>  		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
>  
> @@ -3996,7 +4004,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  		}
>  		return true;
>  	case PIPE_C:
> -		if (!pipe_B_crtc->base.enabled ||
> +		if (!pipe_has_enabled_pch(pipe_B_crtc) ||
>  		    pipe_B_crtc->config.fdi_lanes <= 2) {
>  			if (pipe_config->fdi_lanes > 2) {
>  				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n",

Re: [PATCH 15/15] drm/i915: implement fdi auto-dithering

[Imre Deak]

On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> So on a bunch of setups we only have 2 fdi lanes available, e.g. hsw
> VGA or 3 pipes on ivb. And seemingly a lot of modes don't quite fit
> into this, among them the default 1080p mode.
> 
> The solution is to dither down the pipe a bit so that everything fits,
> which this patch implements.
> 
> But ports compute their state under the assumption that the bpp they
> pick will be the one selected, e.g. the display port bw computations
> won't work otherwise. Now we could adjust our code to again up-dither
> to the computed DP link parameters, but that's pointless.
> 
> So instead when the pipe needs to adjust parameters we need to retry
> the pipe_config computation at the encoder stage. Furthermore we need
> to inform encoders that they should not increase bandwidth
> requirements if possible. This is required for the hdmi code, which
> prefers the pipe to up-dither to either of the two possible hdmi bpc
> values.
> 
> LVDS has a similar requirement, although that's probably only
> theoretical in nature: It's unlikely that we'll ever see an 8bpc
> high-res lvds panel (which is required to hit the 2 fdi lane limit).
> 
> v2: Rebased on top of a bikeshed from Paulo.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 56 ++++++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 14 ++++++---
>  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
>  4 files changed, 62 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c25dbdd..6d35ccd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4021,13 +4021,16 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  	}
>  }
>  
> -static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> -					struct intel_crtc_config *pipe_config)
> +#define RETRY 1
> +static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> +				       struct intel_crtc_config *pipe_config)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
>  	int target_clock, lane, link_bw;
> +	bool setup_ok, needs_recompute = false;
>  
> +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
> @@ -4052,12 +4055,26 @@ static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
>  			       link_bw, &pipe_config->fdi_m_n);
>  
> -	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
> -					intel_crtc->pipe, pipe_config);
> +	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> +					    intel_crtc->pipe, pipe_config);
> +	if (!setup_ok && 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;
> +	}
> +
> +	if (needs_recompute)
> +		return RETRY;
> +
> +	return setup_ok ? 0 : -EINVAL;
>  }
>  
> -static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> -				      struct intel_crtc_config *pipe_config)
> +static int intel_crtc_compute_config(struct drm_crtc *crtc,
> +				     struct intel_crtc_config *pipe_config)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> @@ -4066,7 +4083,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  		/* FDI link clock is fixed at 2.7G */
>  		if (pipe_config->requested_mode.clock * 3
>  		    > IRONLAKE_FDI_FREQ * 4)
> -			return false;
> +			return -EINVAL;
>  	}
>  
>  	/* All interlaced capable intel hw wants timings in frames. Note though
> @@ -4080,7 +4097,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  	 */
>  	if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
>  		adjusted_mode->hsync_start == adjusted_mode->hdisplay)
> -		return false;
> +		return -EINVAL;
>  
>  	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) && pipe_config->pipe_bpp > 10*3) {
>  		pipe_config->pipe_bpp = 10*3; /* 12bpc is gen5+ */
> @@ -4093,7 +4110,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
>  	if (pipe_config->has_pch_encoder)
>  		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
>  
> -	return true;
> +	return 0;
>  }
>  
>  static int valleyview_get_display_clock_speed(struct drm_device *dev)
> @@ -7673,7 +7690,8 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  	struct drm_encoder_helper_funcs *encoder_funcs;
>  	struct intel_encoder *encoder;
>  	struct intel_crtc_config *pipe_config;
> -	int plane_bpp;
> +	int plane_bpp, ret = -EINVAL;
> +	bool retry = true;
>  
>  	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
>  	if (!pipe_config)
> @@ -7686,6 +7704,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  	if (plane_bpp < 0)
>  		goto fail;
>  
> +encoder_retry:
>  	/* Pass our mode to the connectors and the CRTC to give them a chance to
>  	 * adjust it according to limitations or connector properties, and also
>  	 * a chance to reject the mode entirely.
> @@ -7714,10 +7733,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  		}
>  	}
>  
> -	if (!(intel_crtc_compute_config(crtc, pipe_config))) {
> +	ret = intel_crtc_compute_config(crtc, pipe_config);
> +	if (ret < 0) {
>  		DRM_DEBUG_KMS("CRTC fixup failed\n");
>  		goto fail;
>  	}
> +
> +	if (ret == RETRY) {
> +		if (WARN(!retry, "loop in pipe configuration computation\n")) {
> +			ret = -EINVAL;
> +			goto fail;

Isn't it possible that intel_dp_compute_config increases pipe_bpp when
it forces pipe_bpp to what the firmware has set? In that case could hit
this WARN.

> +		}
> +
> +		DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
> +		retry = false;
> +		goto encoder_retry;
> +	}
> +
>  	DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
>  
>  	pipe_config->dither = pipe_config->pipe_bpp != plane_bpp;
> @@ -7727,7 +7759,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  	return pipe_config;
>  fail:
>  	kfree(pipe_config);
> -	return ERR_PTR(-EINVAL);
> +	return ERR_PTR(ret);
>  }
>  
>  /* Computes which crtcs are affected and sets the relevant bits in the mask. For
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f40b43f..c14afc6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -211,6 +211,13 @@ struct intel_crtc_config {
>  	/* Controls for the clock computation, to override various stages. */
>  	bool clock_set;
>  
> +	/*
> +	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
> +	 * required. This is set in the 2nd loop of calling encoder's
> +	 * ->compute_config if the first pick doesn't work out.
> +	 */
> +	bool bw_constrained;
> +
>  	/* Settings for the intel dpll used on pretty much everything but
>  	 * haswell. */
>  	struct dpll {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a8273c7..3942041 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -784,6 +784,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
>  	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
> +	int desired_bpp;
>  
>  	if (intel_hdmi->color_range_auto) {
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> @@ -808,16 +809,21 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	 */
>  	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
>  	    && HAS_PCH_SPLIT(dev)) {
> -		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
> -		pipe_config->pipe_bpp = 12*3;
> +		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
> +		desired_bpp = 12*3;
>  
>  		/* Need to adjust the port link by 1.5x for 12bpc. */
>  		adjusted_mode->clock = clock_12bpc;
>  		pipe_config->pixel_target_clock =
>  			pipe_config->requested_mode.clock;
>  	} else {
> -		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
> -		pipe_config->pipe_bpp = 8*3;
> +		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
> +		desired_bpp = 8*3;
> +	}
> +
> +	if (!pipe_config->bw_constrained) {
> +		DRM_DEBUG_KMS("forcing pipe bpc to %i for HDMI\n", desired_bpp);
> +		pipe_config->pipe_bpp = desired_bpp;
>  	}
>  
>  	if (adjusted_mode->clock > 225000) {
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 094f3c5..c426581 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -331,7 +331,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  	else
>  		lvds_bpp = 6*3;
>  
> -	if (lvds_bpp != pipe_config->pipe_bpp) {
> +	if (lvds_bpp != pipe_config->pipe_bpp && !pipe_config->bw_constrained) {
>  		DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
>  			      pipe_config->pipe_bpp, lvds_bpp);
>  		pipe_config->pipe_bpp = lvds_bpp;

Re: [PATCH 15/15] drm/i915: implement fdi auto-dithering

[Daniel Vetter]

On Mon, Apr 29, 2013 at 05:02:20PM +0300, Imre Deak wrote:
> On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> > So on a bunch of setups we only have 2 fdi lanes available, e.g. hsw
> > VGA or 3 pipes on ivb. And seemingly a lot of modes don't quite fit
> > into this, among them the default 1080p mode.
> > 
> > The solution is to dither down the pipe a bit so that everything fits,
> > which this patch implements.
> > 
> > But ports compute their state under the assumption that the bpp they
> > pick will be the one selected, e.g. the display port bw computations
> > won't work otherwise. Now we could adjust our code to again up-dither
> > to the computed DP link parameters, but that's pointless.
> > 
> > So instead when the pipe needs to adjust parameters we need to retry
> > the pipe_config computation at the encoder stage. Furthermore we need
> > to inform encoders that they should not increase bandwidth
> > requirements if possible. This is required for the hdmi code, which
> > prefers the pipe to up-dither to either of the two possible hdmi bpc
> > values.
> > 
> > LVDS has a similar requirement, although that's probably only
> > theoretical in nature: It's unlikely that we'll ever see an 8bpc
> > high-res lvds panel (which is required to hit the 2 fdi lane limit).
> > 
> > v2: Rebased on top of a bikeshed from Paulo.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 56 ++++++++++++++++++++++++++++--------
> >  drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
> >  drivers/gpu/drm/i915/intel_hdmi.c    | 14 ++++++---
> >  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
> >  4 files changed, 62 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c25dbdd..6d35ccd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4021,13 +4021,16 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> >  	}
> >  }
> >  
> > -static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > -					struct intel_crtc_config *pipe_config)
> > +#define RETRY 1
> > +static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > +				       struct intel_crtc_config *pipe_config)
> >  {
> >  	struct drm_device *dev = intel_crtc->base.dev;
> >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> >  	int target_clock, lane, link_bw;
> > +	bool setup_ok, needs_recompute = false;
> >  
> > +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
> > @@ -4052,12 +4055,26 @@ static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> >  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
> >  			       link_bw, &pipe_config->fdi_m_n);
> >  
> > -	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
> > -					intel_crtc->pipe, pipe_config);
> > +	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> > +					    intel_crtc->pipe, pipe_config);
> > +	if (!setup_ok && 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;
> > +	}
> > +
> > +	if (needs_recompute)
> > +		return RETRY;
> > +
> > +	return setup_ok ? 0 : -EINVAL;
> >  }
> >  
> > -static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> > -				      struct intel_crtc_config *pipe_config)
> > +static int intel_crtc_compute_config(struct drm_crtc *crtc,
> > +				     struct intel_crtc_config *pipe_config)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > @@ -4066,7 +4083,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> >  		/* FDI link clock is fixed at 2.7G */
> >  		if (pipe_config->requested_mode.clock * 3
> >  		    > IRONLAKE_FDI_FREQ * 4)
> > -			return false;
> > +			return -EINVAL;
> >  	}
> >  
> >  	/* All interlaced capable intel hw wants timings in frames. Note though
> > @@ -4080,7 +4097,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> >  	 */
> >  	if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
> >  		adjusted_mode->hsync_start == adjusted_mode->hdisplay)
> > -		return false;
> > +		return -EINVAL;
> >  
> >  	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) && pipe_config->pipe_bpp > 10*3) {
> >  		pipe_config->pipe_bpp = 10*3; /* 12bpc is gen5+ */
> > @@ -4093,7 +4110,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> >  	if (pipe_config->has_pch_encoder)
> >  		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
> >  
> > -	return true;
> > +	return 0;
> >  }
> >  
> >  static int valleyview_get_display_clock_speed(struct drm_device *dev)
> > @@ -7673,7 +7690,8 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  	struct drm_encoder_helper_funcs *encoder_funcs;
> >  	struct intel_encoder *encoder;
> >  	struct intel_crtc_config *pipe_config;
> > -	int plane_bpp;
> > +	int plane_bpp, ret = -EINVAL;
> > +	bool retry = true;
> >  
> >  	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> >  	if (!pipe_config)
> > @@ -7686,6 +7704,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  	if (plane_bpp < 0)
> >  		goto fail;
> >  
> > +encoder_retry:
> >  	/* Pass our mode to the connectors and the CRTC to give them a chance to
> >  	 * adjust it according to limitations or connector properties, and also
> >  	 * a chance to reject the mode entirely.
> > @@ -7714,10 +7733,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  		}
> >  	}
> >  
> > -	if (!(intel_crtc_compute_config(crtc, pipe_config))) {
> > +	ret = intel_crtc_compute_config(crtc, pipe_config);
> > +	if (ret < 0) {
> >  		DRM_DEBUG_KMS("CRTC fixup failed\n");
> >  		goto fail;
> >  	}
> > +
> > +	if (ret == RETRY) {
> > +		if (WARN(!retry, "loop in pipe configuration computation\n")) {
> > +			ret = -EINVAL;
> > +			goto fail;
> 
> Isn't it possible that intel_dp_compute_config increases pipe_bpp when
> it forces pipe_bpp to what the firmware has set? In that case could hit
> this WARN.

Yeah, but that would mean that the firmware asks us for a configuration
for which we simply do not have enough bandwidth. So I don't think that
we'll actually hit this in reality.

If we do I guess we need to add a check into intel_dp_compute_config to
see whether we're in the a) retry loop and b) try to increase bpp and then
just fail the compute_config stage. But as long as the entire eDP mess is
a bit unclear I'd like to avoid such complexity until we have a proven
need.

Want me to add something like the above to the commit message?

Cheers, Daniel

> 
> > +		}
> > +
> > +		DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
> > +		retry = false;
> > +		goto encoder_retry;
> > +	}
> > +
> >  	DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
> >  
> >  	pipe_config->dither = pipe_config->pipe_bpp != plane_bpp;
> > @@ -7727,7 +7759,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  	return pipe_config;
> >  fail:
> >  	kfree(pipe_config);
> > -	return ERR_PTR(-EINVAL);
> > +	return ERR_PTR(ret);
> >  }
> >  
> >  /* Computes which crtcs are affected and sets the relevant bits in the mask. For
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index f40b43f..c14afc6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -211,6 +211,13 @@ struct intel_crtc_config {
> >  	/* Controls for the clock computation, to override various stages. */
> >  	bool clock_set;
> >  
> > +	/*
> > +	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
> > +	 * required. This is set in the 2nd loop of calling encoder's
> > +	 * ->compute_config if the first pick doesn't work out.
> > +	 */
> > +	bool bw_constrained;
> > +
> >  	/* Settings for the intel dpll used on pretty much everything but
> >  	 * haswell. */
> >  	struct dpll {
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index a8273c7..3942041 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -784,6 +784,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >  	struct drm_device *dev = encoder->base.dev;
> >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> >  	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
> > +	int desired_bpp;
> >  
> >  	if (intel_hdmi->color_range_auto) {
> >  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> > @@ -808,16 +809,21 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >  	 */
> >  	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
> >  	    && HAS_PCH_SPLIT(dev)) {
> > -		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
> > -		pipe_config->pipe_bpp = 12*3;
> > +		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
> > +		desired_bpp = 12*3;
> >  
> >  		/* Need to adjust the port link by 1.5x for 12bpc. */
> >  		adjusted_mode->clock = clock_12bpc;
> >  		pipe_config->pixel_target_clock =
> >  			pipe_config->requested_mode.clock;
> >  	} else {
> > -		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
> > -		pipe_config->pipe_bpp = 8*3;
> > +		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
> > +		desired_bpp = 8*3;
> > +	}
> > +
> > +	if (!pipe_config->bw_constrained) {
> > +		DRM_DEBUG_KMS("forcing pipe bpc to %i for HDMI\n", desired_bpp);
> > +		pipe_config->pipe_bpp = desired_bpp;
> >  	}
> >  
> >  	if (adjusted_mode->clock > 225000) {
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 094f3c5..c426581 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -331,7 +331,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> >  	else
> >  		lvds_bpp = 6*3;
> >  
> > -	if (lvds_bpp != pipe_config->pipe_bpp) {
> > +	if (lvds_bpp != pipe_config->pipe_bpp && !pipe_config->bw_constrained) {
> >  		DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
> >  			      pipe_config->pipe_bpp, lvds_bpp);
> >  		pipe_config->pipe_bpp = lvds_bpp;
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 15/15] drm/i915: implement fdi auto-dithering

[Imre Deak]

On Mon, 2013-04-29 at 16:43 +0200, Daniel Vetter wrote:
> On Mon, Apr 29, 2013 at 05:02:20PM +0300, Imre Deak wrote:
> > On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:
> > > So on a bunch of setups we only have 2 fdi lanes available, e.g. hsw
> > > VGA or 3 pipes on ivb. And seemingly a lot of modes don't quite fit
> > > into this, among them the default 1080p mode.
> > > 
> > > The solution is to dither down the pipe a bit so that everything fits,
> > > which this patch implements.
> > > 
> > > But ports compute their state under the assumption that the bpp they
> > > pick will be the one selected, e.g. the display port bw computations
> > > won't work otherwise. Now we could adjust our code to again up-dither
> > > to the computed DP link parameters, but that's pointless.
> > > 
> > > So instead when the pipe needs to adjust parameters we need to retry
> > > the pipe_config computation at the encoder stage. Furthermore we need
> > > to inform encoders that they should not increase bandwidth
> > > requirements if possible. This is required for the hdmi code, which
> > > prefers the pipe to up-dither to either of the two possible hdmi bpc
> > > values.
> > > 
> > > LVDS has a similar requirement, although that's probably only
> > > theoretical in nature: It's unlikely that we'll ever see an 8bpc
> > > high-res lvds panel (which is required to hit the 2 fdi lane limit).
> > > 
> > > v2: Rebased on top of a bikeshed from Paulo.
> > > 
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 56 ++++++++++++++++++++++++++++--------
> > >  drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
> > >  drivers/gpu/drm/i915/intel_hdmi.c    | 14 ++++++---
> > >  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
> > >  4 files changed, 62 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index c25dbdd..6d35ccd 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4021,13 +4021,16 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> > >  	}
> > >  }
> > >  
> > > -static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > > -					struct intel_crtc_config *pipe_config)
> > > +#define RETRY 1
> > > +static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > > +				       struct intel_crtc_config *pipe_config)
> > >  {
> > >  	struct drm_device *dev = intel_crtc->base.dev;
> > >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > >  	int target_clock, lane, link_bw;
> > > +	bool setup_ok, needs_recompute = false;
> > >  
> > > +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
> > > @@ -4052,12 +4055,26 @@ static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > >  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
> > >  			       link_bw, &pipe_config->fdi_m_n);
> > >  
> > > -	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
> > > -					intel_crtc->pipe, pipe_config);
> > > +	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> > > +					    intel_crtc->pipe, pipe_config);
> > > +	if (!setup_ok && 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;
> > > +	}
> > > +
> > > +	if (needs_recompute)
> > > +		return RETRY;
> > > +
> > > +	return setup_ok ? 0 : -EINVAL;
> > >  }
> > >  
> > > -static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> > > -				      struct intel_crtc_config *pipe_config)
> > > +static int intel_crtc_compute_config(struct drm_crtc *crtc,
> > > +				     struct intel_crtc_config *pipe_config)
> > >  {
> > >  	struct drm_device *dev = crtc->dev;
> > >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > > @@ -4066,7 +4083,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> > >  		/* FDI link clock is fixed at 2.7G */
> > >  		if (pipe_config->requested_mode.clock * 3
> > >  		    > IRONLAKE_FDI_FREQ * 4)
> > > -			return false;
> > > +			return -EINVAL;
> > >  	}
> > >  
> > >  	/* All interlaced capable intel hw wants timings in frames. Note though
> > > @@ -4080,7 +4097,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> > >  	 */
> > >  	if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
> > >  		adjusted_mode->hsync_start == adjusted_mode->hdisplay)
> > > -		return false;
> > > +		return -EINVAL;
> > >  
> > >  	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) && pipe_config->pipe_bpp > 10*3) {
> > >  		pipe_config->pipe_bpp = 10*3; /* 12bpc is gen5+ */
> > > @@ -4093,7 +4110,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> > >  	if (pipe_config->has_pch_encoder)
> > >  		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
> > >  
> > > -	return true;
> > > +	return 0;
> > >  }
> > >  
> > >  static int valleyview_get_display_clock_speed(struct drm_device *dev)
> > > @@ -7673,7 +7690,8 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > >  	struct drm_encoder_helper_funcs *encoder_funcs;
> > >  	struct intel_encoder *encoder;
> > >  	struct intel_crtc_config *pipe_config;
> > > -	int plane_bpp;
> > > +	int plane_bpp, ret = -EINVAL;
> > > +	bool retry = true;
> > >  
> > >  	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> > >  	if (!pipe_config)
> > > @@ -7686,6 +7704,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > >  	if (plane_bpp < 0)
> > >  		goto fail;
> > >  
> > > +encoder_retry:
> > >  	/* Pass our mode to the connectors and the CRTC to give them a chance to
> > >  	 * adjust it according to limitations or connector properties, and also
> > >  	 * a chance to reject the mode entirely.
> > > @@ -7714,10 +7733,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > >  		}
> > >  	}
> > >  
> > > -	if (!(intel_crtc_compute_config(crtc, pipe_config))) {
> > > +	ret = intel_crtc_compute_config(crtc, pipe_config);
> > > +	if (ret < 0) {
> > >  		DRM_DEBUG_KMS("CRTC fixup failed\n");
> > >  		goto fail;
> > >  	}
> > > +
> > > +	if (ret == RETRY) {
> > > +		if (WARN(!retry, "loop in pipe configuration computation\n")) {
> > > +			ret = -EINVAL;
> > > +			goto fail;
> > 
> > Isn't it possible that intel_dp_compute_config increases pipe_bpp when
> > it forces pipe_bpp to what the firmware has set? In that case could hit
> > this WARN.
> 
> Yeah, but that would mean that the firmware asks us for a configuration
> for which we simply do not have enough bandwidth. So I don't think that
> we'll actually hit this in reality.
> 
> If we do I guess we need to add a check into intel_dp_compute_config to
> see whether we're in the a) retry loop and b) try to increase bpp and then
> just fail the compute_config stage. But as long as the entire eDP mess is
> a bit unclear I'd like to avoid such complexity until we have a proven
> need.
> 
> Want me to add something like the above to the commit message?

Ok, after the IRC discussion things are clearer now. Yea, would be good
to have have it in the commit message that eDP is a non-issue. With
that:

Reviewed-by: Imre Deak <imre.deak@intel.com>

--Imre

> 
> Cheers, Daniel
> 
> > 
> > > +		}
> > > +
> > > +		DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
> > > +		retry = false;
> > > +		goto encoder_retry;
> > > +	}
> > > +
> > >  	DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
> > >  
> > >  	pipe_config->dither = pipe_config->pipe_bpp != plane_bpp;
> > > @@ -7727,7 +7759,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > >  	return pipe_config;
> > >  fail:
> > >  	kfree(pipe_config);
> > > -	return ERR_PTR(-EINVAL);
> > > +	return ERR_PTR(ret);
> > >  }
> > >  
> > >  /* Computes which crtcs are affected and sets the relevant bits in the mask. For
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index f40b43f..c14afc6 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -211,6 +211,13 @@ struct intel_crtc_config {
> > >  	/* Controls for the clock computation, to override various stages. */
> > >  	bool clock_set;
> > >  
> > > +	/*
> > > +	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
> > > +	 * required. This is set in the 2nd loop of calling encoder's
> > > +	 * ->compute_config if the first pick doesn't work out.
> > > +	 */
> > > +	bool bw_constrained;
> > > +
> > >  	/* Settings for the intel dpll used on pretty much everything but
> > >  	 * haswell. */
> > >  	struct dpll {
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index a8273c7..3942041 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -784,6 +784,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> > >  	struct drm_device *dev = encoder->base.dev;
> > >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > >  	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
> > > +	int desired_bpp;
> > >  
> > >  	if (intel_hdmi->color_range_auto) {
> > >  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> > > @@ -808,16 +809,21 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> > >  	 */
> > >  	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
> > >  	    && HAS_PCH_SPLIT(dev)) {
> > > -		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
> > > -		pipe_config->pipe_bpp = 12*3;
> > > +		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
> > > +		desired_bpp = 12*3;
> > >  
> > >  		/* Need to adjust the port link by 1.5x for 12bpc. */
> > >  		adjusted_mode->clock = clock_12bpc;
> > >  		pipe_config->pixel_target_clock =
> > >  			pipe_config->requested_mode.clock;
> > >  	} else {
> > > -		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
> > > -		pipe_config->pipe_bpp = 8*3;
> > > +		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
> > > +		desired_bpp = 8*3;
> > > +	}
> > > +
> > > +	if (!pipe_config->bw_constrained) {
> > > +		DRM_DEBUG_KMS("forcing pipe bpc to %i for HDMI\n", desired_bpp);
> > > +		pipe_config->pipe_bpp = desired_bpp;
> > >  	}
> > >  
> > >  	if (adjusted_mode->clock > 225000) {
> > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > > index 094f3c5..c426581 100644
> > > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > > @@ -331,7 +331,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> > >  	else
> > >  		lvds_bpp = 6*3;
> > >  
> > > -	if (lvds_bpp != pipe_config->pipe_bpp) {
> > > +	if (lvds_bpp != pipe_config->pipe_bpp && !pipe_config->bw_constrained) {
> > >  		DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
> > >  			      pipe_config->pipe_bpp, lvds_bpp);
> > >  		pipe_config->pipe_bpp = lvds_bpp;
> > 
> > 
> 

[PATCH] drm/i915: put the right cpu_transcoder into pipe_config for hw state readout

[Daniel Vetter]

This hack is getting a bit messy, but this plugs the leak for now
until we have the cpu_transcoder properly pipe_config'ed.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 17fa13c..8b58930 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8096,6 +8096,7 @@ intel_modeset_check_state(struct drm_device *dev)
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
 		memset(&pipe_config, 0, sizeof(pipe_config));
+		pipe_config.cpu_transcoder = crtc->config.cpu_transcoder;
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
 		WARN(crtc->active != active,
-- 
1.7.11.7

[PATCH] drm/i915: hw state readout support for pipe_config->fdi_lanes

[Daniel Vetter]

v2: Introduce some nice #defines for the FDI lane width fields and put
them to good use. Suggested by Ville.

v3: Fixup the mask vs. shift copy&pasta fail Imre Deak spotted, and
use the shift #define also in the mask.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.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     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++++++++----------
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4d66898..cf35d37 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4142,10 +4142,9 @@
 #define  FDI_LINK_TRAIN_600MV_3_5DB_SNB_B	(0x39<<22)
 #define  FDI_LINK_TRAIN_800MV_0DB_SNB_B		(0x38<<22)
 #define  FDI_LINK_TRAIN_VOL_EMP_MASK		(0x3f<<22)
-#define  FDI_DP_PORT_WIDTH_X1           (0<<19)
-#define  FDI_DP_PORT_WIDTH_X2           (1<<19)
-#define  FDI_DP_PORT_WIDTH_X3           (2<<19)
-#define  FDI_DP_PORT_WIDTH_X4           (3<<19)
+#define  FDI_DP_PORT_WIDTH_SHIFT		19
+#define  FDI_DP_PORT_WIDTH_MASK			(7 << FDI_DP_PORT_WIDTH_SHIFT)
+#define  FDI_DP_PORT_WIDTH(width)           (((width) - 1) << FDI_DP_PORT_WIDTH_SHIFT)
 #define  FDI_TX_ENHANCE_FRAME_ENABLE    (1<<18)
 /* Ironlake: hardwired to 1 */
 #define  FDI_TX_PLL_ENABLE              (1<<14)
@@ -4170,7 +4169,6 @@
 /* train, dp width same as FDI_TX */
 #define  FDI_FS_ERRC_ENABLE		(1<<27)
 #define  FDI_FE_ERRC_ENABLE		(1<<26)
-#define  FDI_DP_PORT_WIDTH_X8           (7<<19)
 #define  FDI_RX_POLARITY_REVERSED_LPT	(1<<16)
 #define  FDI_8BPC                       (0<<16)
 #define  FDI_10BPC                      (1<<16)
@@ -4192,9 +4190,6 @@
 #define  FDI_LINK_TRAIN_PATTERN_IDLE_CPT	(2<<8)
 #define  FDI_LINK_TRAIN_NORMAL_CPT		(3<<8)
 #define  FDI_LINK_TRAIN_PATTERN_MASK_CPT	(3<<8)
-/* LPT */
-#define  FDI_PORT_WIDTH_2X_LPT			(1<<19)
-#define  FDI_PORT_WIDTH_1X_LPT			(0<<19)
 
 #define _FDI_RXA_MISC			0xf0010
 #define _FDI_RXB_MISC			0xf1010
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c9c4741..72941f9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -182,7 +182,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	/* Enable the PCH Receiver FDI PLL */
 	rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
 		     FDI_RX_PLL_ENABLE |
-		     ((intel_crtc->config.fdi_lanes - 1) << 19);
+		     FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
 	POSTING_READ(_FDI_RXA_CTL);
 	udelay(220);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e7c4d0b..ca29705 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2419,8 +2419,8 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~(7 << 19);
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~FDI_DP_PORT_WIDTH_MASK;
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp &= ~FDI_LINK_TRAIN_NONE;
 	temp |= FDI_LINK_TRAIN_PATTERN_1;
 	I915_WRITE(reg, temp | FDI_TX_ENABLE);
@@ -2517,8 +2517,8 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~(7 << 19);
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~FDI_DP_PORT_WIDTH_MASK;
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp &= ~FDI_LINK_TRAIN_NONE;
 	temp |= FDI_LINK_TRAIN_PATTERN_1;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
@@ -2652,8 +2652,8 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~(7 << 19);
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~FDI_DP_PORT_WIDTH_MASK;
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp &= ~(FDI_LINK_TRAIN_AUTO | FDI_LINK_TRAIN_NONE_IVB);
 	temp |= FDI_LINK_TRAIN_PATTERN_1_IVB;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
@@ -2754,8 +2754,8 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
 	/* enable PCH FDI RX PLL, wait warmup plus DMI latency */
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
-	temp &= ~((0x7 << 19) | (0x7 << 16));
-	temp |= (intel_crtc->config.fdi_lanes - 1) << 19;
+	temp &= ~(FDI_DP_PORT_WIDTH_MASK | (0x7 << 16));
+	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
 	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
 	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
 
@@ -5794,9 +5794,14 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
-	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
+	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		pipe_config->has_pch_encoder = true;
 
+		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
+		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
+					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
+	}
+
 	return true;
 }
 
@@ -5932,9 +5937,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	 */
 	tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
-	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
+	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE) {
 		pipe_config->has_pch_encoder = true;
 
+		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
+		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
+					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
+	}
+
 	return true;
 }
 
@@ -7896,6 +7906,14 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 		return false;
 	}
 
+	if (current_config->fdi_lanes != pipe_config->fdi_lanes) {
+		DRM_ERROR("mismatch in fdi_lanes "
+			  "(expected %i, found %i)\n",
+			  current_config->fdi_lanes,
+			  pipe_config->fdi_lanes);
+		return false;
+	}
+
 	return true;
 }
 
-- 
1.7.11.7

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

[PATCH] drm/i915: move fdi lane configuration checks ahead

[Daniel Vetter]

This nicely allows us to drop some hacks which have only been used
to work around modeset failures due to lack of fdi lanes.

v2: Implement proper checking for Haswell platforms - the fdi link to
the LPT PCH has only 2 lanes. Note that we already filter out
impossible modes in intel_crt_mode_valid. Unfortunately LPT does not
support 6bpc on the fdi rx, so we can't pull clever tricks to squeeze
in a few more modes.

v2: Rebased on top of Ben Widawsky's num_pipes reorg.

v3: Rebase on top of Ville's pipe debug output ocd rampage.

v4: Fixup rebase fail spotted by Ville.

v5: Fixup rebase fail spotted by Imre Deak. I suck.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 129 ++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a446a57..0950cdf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3974,9 +3974,68 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static void ironlake_fdi_compute_config(struct drm_device *dev,
+static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *pipe_B_crtc =
+		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
+
+	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 false;
+	}
+
+	if (IS_HASWELL(dev)) {
+		if (pipe_config->fdi_lanes > 2) {
+			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
+				      pipe_config->fdi_lanes);
+			return false;
+		} else {
+			return true;
+		}
+	}
+
+	if (INTEL_INFO(dev)->num_pipes == 2)
+		return true;
+
+	/* Ivybridge 3 pipe is really complicated */
+	switch (pipe) {
+	case PIPE_A:
+		return true;
+	case PIPE_B:
+		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
+		    pipe_config->fdi_lanes > 2) {
+			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
+				      pipe_name(pipe), pipe_config->fdi_lanes);
+			return false;
+		}
+		return true;
+	case PIPE_C:
+		if (!pipe_B_crtc->base.enabled ||
+		    pipe_B_crtc->config.fdi_lanes <= 2) {
+			if (pipe_config->fdi_lanes > 2) {
+				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
+					      pipe_name(pipe), pipe_config->fdi_lanes);
+				return false;
+			}
+		} else {
+			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
+			return false;
+		}
+		return true;
+	default:
+		BUG();
+	}
+}
+
+static bool ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 					struct intel_crtc_config *pipe_config)
 {
+	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	int target_clock, lane, link_bw;
 
@@ -4003,6 +4062,9 @@ static void ironlake_fdi_compute_config(struct drm_device *dev,
 		link_bw *= pipe_config->pixel_multiplier;
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
 			       link_bw, &pipe_config->fdi_m_n);
+
+	return ironlake_check_fdi_lanes(intel_crtc->base.dev,
+					intel_crtc->pipe, pipe_config);
 }
 
 static bool intel_crtc_compute_config(struct drm_crtc *crtc,
@@ -4040,7 +4102,7 @@ static bool intel_crtc_compute_config(struct drm_crtc *crtc,
 	}
 
 	if (pipe_config->has_pch_encoder)
-		ironlake_fdi_compute_config(dev, pipe_config);
+		return ironlake_fdi_compute_config(to_intel_crtc(crtc), pipe_config);
 
 	return true;
 }
@@ -5434,63 +5496,6 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
 	POSTING_READ(SOUTH_CHICKEN1);
 }
 
-static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *pipe_B_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
-
-	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
-		      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-	if (intel_crtc->config.fdi_lanes > 4) {
-		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
-			      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-		/* Clamp lanes to avoid programming the hw with bogus values. */
-		intel_crtc->config.fdi_lanes = 4;
-
-		return false;
-	}
-
-	if (INTEL_INFO(dev)->num_pipes == 2)
-		return true;
-
-	switch (intel_crtc->pipe) {
-	case PIPE_A:
-		return true;
-	case PIPE_B:
-		if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
-		    intel_crtc->config.fdi_lanes > 2) {
-			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-				      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-			/* Clamp lanes to avoid programming the hw with bogus values. */
-			intel_crtc->config.fdi_lanes = 2;
-
-			return false;
-		}
-
-		return true;
-	case PIPE_C:
-		if (!pipe_B_crtc->base.enabled || pipe_B_crtc->config.fdi_lanes <= 2) {
-			if (intel_crtc->config.fdi_lanes > 2) {
-				DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
-					      pipe_name(intel_crtc->pipe), intel_crtc->config.fdi_lanes);
-				/* Clamp lanes to avoid programming the hw with bogus values. */
-				intel_crtc->config.fdi_lanes = 2;
-
-				return false;
-			}
-		} else {
-			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return false;
-		}
-
-		return true;
-	default:
-		BUG();
-	}
-}
-
 static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -5682,7 +5687,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	bool is_lvds = false;
 	struct intel_encoder *encoder;
 	int ret;
-	bool fdi_config_ok;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		switch (encoder->type) {
@@ -5775,14 +5779,11 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
-	 * ironlake_check_fdi_lanes. */
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
 					     &intel_crtc->config.fdi_m_n);
 	}
 
-	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 	if (IS_IVYBRIDGE(dev))
 		ivybridge_update_fdi_bc_bifurcation(intel_crtc);
 
@@ -5798,7 +5799,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
 
-	return fdi_config_ok ? ret : -EINVAL;
+	return ret;
 }
 
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
-- 
1.7.11.7

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

Re: [PATCH 15/15] drm/i915: implement fdi auto-dithering

[Daniel Vetter]

On Mon, Apr 29, 2013 at 05:59:53PM +0300, Imre Deak wrote:
> On Mon, 2013-04-29 at 16:43 +0200, Daniel Vetter wrote:
> > On Mon, Apr 29, 2013 at 05:02:20PM +0300, Imre Deak wrote:
> > > On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:

[snip]

> > > > @@ -7714,10 +7733,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > > >  		}
> > > >  	}
> > > >  
> > > > -	if (!(intel_crtc_compute_config(crtc, pipe_config))) {
> > > > +	ret = intel_crtc_compute_config(crtc, pipe_config);
> > > > +	if (ret < 0) {
> > > >  		DRM_DEBUG_KMS("CRTC fixup failed\n");
> > > >  		goto fail;
> > > >  	}
> > > > +
> > > > +	if (ret == RETRY) {
> > > > +		if (WARN(!retry, "loop in pipe configuration computation\n")) {
> > > > +			ret = -EINVAL;
> > > > +			goto fail;
> > > 
> > > Isn't it possible that intel_dp_compute_config increases pipe_bpp when
> > > it forces pipe_bpp to what the firmware has set? In that case could hit
> > > this WARN.
> > 
> > Yeah, but that would mean that the firmware asks us for a configuration
> > for which we simply do not have enough bandwidth. So I don't think that
> > we'll actually hit this in reality.
> > 
> > If we do I guess we need to add a check into intel_dp_compute_config to
> > see whether we're in the a) retry loop and b) try to increase bpp and then
> > just fail the compute_config stage. But as long as the entire eDP mess is
> > a bit unclear I'd like to avoid such complexity until we have a proven
> > need.
> > 
> > Want me to add something like the above to the commit message?
> 
> Ok, after the IRC discussion things are clearer now. Yea, would be good
> to have have it in the commit message that eDP is a non-issue. With
> that:
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Ok, slurped in the entire series, with this patch's commit message amended
to reflect our discussion.

Thanks for the review, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/15] high-bpp fixes and fdi auto dithering

[Daniel Vetter]

On Thu, Apr 25, 2013 at 01:28:45PM +0300, Jani Nikula wrote:
> 
> The version at Daniel's fdi-dither branch (which is without the hack in
> "drm/i915: force bpp for eDP panels") is 
> 
> Tested-by: Jani Nikula <jani.nikula@intel.com>

Meh, I've accidentally merged the broken patch, but that's now fixed
again.
-Daniel

> 
> on VLV.
> 
> On Fri, 19 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Hi all,
> >
> > This fixes all the bugs I've found in my various systems when using non-24bpp
> > modes as the first part of the series.
> >
> > And with working non-standard bpp support I've figured we can go fancy and
> > implemented auto-dithering if we hit an fdi bw limit. Which means that you can
> > now use 3-pipe pch configurations on ivb on pretty much everywhere. The only
> > restriction is that you need to fire up pipe C first, since without atomic
> > modeset pipe B will otherwise too much bw.
> >
> > One big thing here is that this will break Paulo's hsw eDP machine, specifically
> > the patch called "drm/i915: force bpp for eDP panels". But apparently without
> > that my machine here is broken ... Ideas highly welcome about how we could quirk
> > ourselves out of this mess.
> >
> > Cheers, Daniel
> >
> > Daniel Vetter (15):
> >   drm/i915: fixup 12bpc hdmi dotclock handling
> >   drm/i915: Disable high-bpc on pre-1.4 EDID screens
> >   drm/i915: force bpp for eDP panels
> >   drm/i915: drop adjusted_mode from *_set_pipeconf functions
> >   drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv
> >   drm/i915: allow high-bpc modes on DP
> >   drm/i915: Fixup non-24bpp support for VGA screens on Haswell
> >   drm/i915: move intel_crtc->fdi_lanes to pipe_config
> >   drm/i915: hw state readout support for pipe_config->fdi_lanes
> >   drm/i915: split up fdi_set_m_n into computation and hw setup
> >   drm/i915: compute fdi lane config earlier
> >   drm/i915: Split up ironlake_check_fdi_lanes
> >   drm/i915: move fdi lane configuration checks ahead
> >   drm/i915: don't count cpu ports for fdi B/C lane sharing
> >   drm/i915: implement fdi auto-dithering
> >
> >  drivers/gpu/drm/i915/intel_crt.c     |   4 +
> >  drivers/gpu/drm/i915/intel_ddi.c     |   7 +-
> >  drivers/gpu/drm/i915/intel_display.c | 349 ++++++++++++++++++++++-------------
> >  drivers/gpu/drm/i915/intel_dp.c      |  12 +-
> >  drivers/gpu/drm/i915/intel_drv.h     |  12 +-
> >  drivers/gpu/drm/i915/intel_hdmi.c    |  31 +++-
> >  drivers/gpu/drm/i915/intel_lvds.c    |   4 +-
> >  7 files changed, 275 insertions(+), 144 deletions(-)
> >
> > -- 
> > 1.7.11.7
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch