[PATCH 0/8] dp/fdi m/n rework + basic pipe_config readout

[PATCH 0/8] dp/fdi m/n rework + basic pipe_config readout

[Daniel Vetter]

Hi all,

So now that the basic infrastructure is in I'll (re)dump the next series. It
fixes up the mess around m/n dp/fdi handling, which has been nicely splattered
all over the code.

Missing bit here is that vlv doesn't have a set of specially selected dp pll
values, so will keep on using the generic clock computation code.

Two things in this patch series for the long-term goals:
- Start of dpll values precomputation, needed to proper handle clock sharing
  before touching hw state.
- Start of the pipe-config hw state readout support.

Especially for the 2nd item my plan is to start a nice diversion from my
outstanding pipe-config patches here and port the fastboot state readout from
Chris/Jesse over.

Flames, comments and review highly welcome!

Cheers, Daniel

Daniel Vetter (8):
  drm/i915: clear up the fdi/dp set_m_n confusion
  drm/i915: move dp_m_n computation to dp_encoder->compute_config
  drm/i915: track dp target_clock in pipe_config
  drm/i915: rip out superflous is_dp&is_cpu_edp tracking
  drm/i915: add hw state readout/checking for pipe_config
  drm/i915: hw readout support for ->has_pch_encoders
  drm/i915: create pipe_config->dpll for clock state
  drm/i915: move dp clock computations to encoder->compute_config

 drivers/gpu/drm/i915/i915_drv.h      |   5 +
 drivers/gpu/drm/i915/intel_display.c | 527 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c      | 125 ++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  30 +-
 4 files changed, 335 insertions(+), 352 deletions(-)

-- 
1.7.11.7

[PATCH 1/8] drm/i915: clear up the fdi/dp set_m_n confusion

[Daniel Vetter]

There's a rather decent confusion going on around transcoder m_n
values. So let's clarify:
- All dp encoders need this, either on the pch transcoder if it's a
  pch port, or on the cpu transcoder/pipe if it's a cpu port.
- fdi links need to have the right m_n values for the fdi link set in
  the cpu transcoder.

To handle the pch vs transcoder stuff a bit better, extract transcoder
set_m_n helpers. To make them simpler, set intel_crtc->cpu_transcoder
als in ironlake_crtc_mode_set, so that gen5+ (where the cpu m_n
registers are all at the same offset) can use it.

Haswell modeset is decently confused about dp vs. edp vs. fdi. dp vs.
edp works exactly the same as dp (since there's no pch dp any more),
so use that as a check. And only set up the fdi m_n values if we
really have a pch encoder present (which means we have a VGA encoder).

On ilk+ we've called ironlake_set_m_n both for cpu_edp and for pch
encoders. Now that dp_set_m_n handles all dp links (thanks to the
pch encoder check), we can ditch the cpu_edp stuff from the
fdi_set_m_n function.

Since the dp_m_n values are not readily available, we need to
carefully coax the edp values out of the encoder. Hence we can't (yet)
kill this superflous complexity.

v2: Rebase on top of the ivb fdi B/C check patch - we need to properly
clear intel_crtc->fdi_lane, otherwise those checks will misfire.

v3: Rebased on top of a s/IS_HASWELL/HAS_DDI/ patch from Paulo Zanoni.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 86 +++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      | 30 ++-----------
 drivers/gpu/drm/i915/intel_drv.h     |  8 ++++
 3 files changed, 67 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c71240..a53a02c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5314,15 +5314,47 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
 	return bps / (link_bw * 8) + 1;
 }
 
-static void ironlake_set_m_n(struct drm_crtc *crtc)
+void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+
+	I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+	I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
+	I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
+	I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
+}
+
+void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+	enum transcoder transcoder = crtc->cpu_transcoder;
+
+	if (INTEL_INFO(dev)->gen >= 5) {
+		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
+		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
+		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+	} else {
+		I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
+		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
+		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
+	}
+}
+
+static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
 	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
@@ -5342,22 +5374,14 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 		}
 	}
 
-	/* FDI link */
-	lane = 0;
-	/* CPU eDP doesn't require FDI link, so just set DP M/N
-	   according to current link config */
-	if (is_cpu_edp) {
-		intel_edp_link_config(edp_encoder, &lane, &link_bw);
-	} else {
-		/* 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;
-	}
+	/* 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;
 
 	/* [e]DP over FDI requires target mode clock instead of link clock. */
 	if (edp_encoder)
@@ -5367,9 +5391,8 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	else
 		target_clock = adjusted_mode->clock;
 
-	if (!lane)
-		lane = ironlake_get_lanes_required(target_clock, link_bw,
-						   intel_crtc->config.pipe_bpp);
+	lane = ironlake_get_lanes_required(target_clock, link_bw,
+					   intel_crtc->config.pipe_bpp);
 
 	intel_crtc->fdi_lanes = lane;
 
@@ -5378,10 +5401,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
 			       link_bw, &m_n);
 
-	I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
-	I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-	I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-	I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
+	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
@@ -5528,6 +5548,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
 	     "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
 
+	intel_crtc->cpu_transcoder = pipe;
+
 	ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
 				     &has_reduced_clock, &reduced_clock);
 	if (!ok) {
@@ -5566,7 +5588,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -5602,7 +5624,9 @@ 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. */
-	ironlake_set_m_n(crtc);
+	intel_crtc->fdi_lanes = 0;
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
@@ -5714,15 +5738,15 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	intel_crtc->lowfreq_avail = false;
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	if (!is_dp || is_cpu_edp)
-		ironlake_set_m_n(crtc);
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	haswell_set_pipeconf(crtc, adjusted_mode, dither);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 251aa6b..6b8a279 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -766,12 +766,9 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *intel_encoder;
 	struct intel_dp *intel_dp;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int lane_count = 4;
 	struct intel_link_m_n m_n;
-	int pipe = intel_crtc->pipe;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	int target_clock;
 
 	/*
@@ -805,29 +802,10 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
 			       target_clock, adjusted_mode->clock, &m_n);
 
-	if (HAS_DDI(dev)) {
-		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
-	} else if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(TRANSDATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(TRANSDPLINK_M1(pipe), m_n.link_m);
-		I915_WRITE(TRANSDPLINK_N1(pipe), m_n.link_n);
-	} else if (IS_VALLEYVIEW(dev)) {
-		I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
-	} else {
-		I915_WRITE(PIPE_GMCH_DATA_M(pipe),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n.link_m);
-		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n.link_n);
-	}
+	if (intel_crtc->config.has_pch_encoder)
+		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
+	else
+		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 void intel_dp_init_link_config(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5c7b04b..3a9b7be 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -192,8 +192,12 @@ struct intel_crtc_config {
 	 */
 	bool limited_color_range;
 
+	/* DP has a bunch of special case unfortunately, so mark the pipe
+	 * accordingly. */
+	bool has_dp_encoder;
 	bool dither;
 	int pipe_bpp;
+	struct intel_link_m_n dp_m_n;
 
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
@@ -641,6 +645,10 @@ extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
 			    struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
+extern void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
+extern void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
1.7.11.7

Re: [PATCH 1/8] drm/i915: clear up the fdi/dp set_m_n confusion

[Jesse Barnes]

On Thu, 28 Mar 2013 10:41:56 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

>  
> +	/* DP has a bunch of special case unfortunately, so mark the pipe
> +	 * accordingly. */
> +	bool has_dp_encoder;

Looks pretty good, but I don't think this field is used anywhere?
Maybe it belongs in a later patch?

Also, this makes me wonder if we should be clearing the m_n regs
somewhere and asserting for them in a few places.

Definitely looks better than the current code though; CPU vs PCH writes
sprinkled all about, and FDI thrown into the mix.

It's probably a bit unfair to say the HSW mode set is confused though;
it's just using existing code as best it can.  These new bits are
definitely clearer though.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 1/8] drm/i915: clear up the fdi/dp set_m_n confusion

[Jesse Barnes]

On Tue, 2 Apr 2013 13:47:52 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Thu, 28 Mar 2013 10:41:56 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> >  
> > +	/* DP has a bunch of special case unfortunately, so mark the pipe
> > +	 * accordingly. */
> > +	bool has_dp_encoder;
> 
> Looks pretty good, but I don't think this field is used anywhere?
> Maybe it belongs in a later patch?
> 
> Also, this makes me wonder if we should be clearing the m_n regs
> somewhere and asserting for them in a few places.
> 
> Definitely looks better than the current code though; CPU vs PCH writes
> sprinkled all about, and FDI thrown into the mix.
> 
> It's probably a bit unfair to say the HSW mode set is confused though;
> it's just using existing code as best it can.  These new bits are
> definitely clearer though.
> 

Oh and with the new field moved to another patch:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] drm/i915: clear up the fdi/dp set_m_n confusion

[Daniel Vetter]

There's a rather decent confusion going on around transcoder m_n
values. So let's clarify:
- All dp encoders need this, either on the pch transcoder if it's a
  pch port, or on the cpu transcoder/pipe if it's a cpu port.
- fdi links need to have the right m_n values for the fdi link set in
  the cpu transcoder.

To handle the pch vs transcoder stuff a bit better, extract transcoder
set_m_n helpers. To make them simpler, set intel_crtc->cpu_transcoder
als in ironlake_crtc_mode_set, so that gen5+ (where the cpu m_n
registers are all at the same offset) can use it.

Haswell modeset is decently confused about dp vs. edp vs. fdi. dp vs.
edp works exactly the same as dp (since there's no pch dp any more),
so use that as a check. And only set up the fdi m_n values if we
really have a pch encoder present (which means we have a VGA encoder).

On ilk+ we've called ironlake_set_m_n both for cpu_edp and for pch
encoders. Now that dp_set_m_n handles all dp links (thanks to the
pch encoder check), we can ditch the cpu_edp stuff from the
fdi_set_m_n function.

Since the dp_m_n values are not readily available, we need to
carefully coax the edp values out of the encoder. Hence we can't (yet)
kill this superflous complexity.

v2: Rebase on top of the ivb fdi B/C check patch - we need to properly
clear intel_crtc->fdi_lane, otherwise those checks will misfire.

v3: Rebased on top of a s/IS_HASWELL/HAS_DDI/ patch from Paulo Zanoni.

v4: Drop the addition of has_dp_encoder, it's in the wrong patch (Jesse).

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 86 +++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      | 30 ++-----------
 drivers/gpu/drm/i915/intel_drv.h     |  5 +++
 3 files changed, 64 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c71240..a53a02c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5314,15 +5314,47 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
 	return bps / (link_bw * 8) + 1;
 }
 
-static void ironlake_set_m_n(struct drm_crtc *crtc)
+void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+
+	I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+	I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
+	I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
+	I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
+}
+
+void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+	enum transcoder transcoder = crtc->cpu_transcoder;
+
+	if (INTEL_INFO(dev)->gen >= 5) {
+		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
+		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
+		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+	} else {
+		I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
+		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
+		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
+	}
+}
+
+static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
 	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
@@ -5342,22 +5374,14 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 		}
 	}
 
-	/* FDI link */
-	lane = 0;
-	/* CPU eDP doesn't require FDI link, so just set DP M/N
-	   according to current link config */
-	if (is_cpu_edp) {
-		intel_edp_link_config(edp_encoder, &lane, &link_bw);
-	} else {
-		/* 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;
-	}
+	/* 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;
 
 	/* [e]DP over FDI requires target mode clock instead of link clock. */
 	if (edp_encoder)
@@ -5367,9 +5391,8 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	else
 		target_clock = adjusted_mode->clock;
 
-	if (!lane)
-		lane = ironlake_get_lanes_required(target_clock, link_bw,
-						   intel_crtc->config.pipe_bpp);
+	lane = ironlake_get_lanes_required(target_clock, link_bw,
+					   intel_crtc->config.pipe_bpp);
 
 	intel_crtc->fdi_lanes = lane;
 
@@ -5378,10 +5401,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
 			       link_bw, &m_n);
 
-	I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
-	I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-	I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-	I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
+	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
@@ -5528,6 +5548,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
 	     "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
 
+	intel_crtc->cpu_transcoder = pipe;
+
 	ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
 				     &has_reduced_clock, &reduced_clock);
 	if (!ok) {
@@ -5566,7 +5588,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -5602,7 +5624,9 @@ 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. */
-	ironlake_set_m_n(crtc);
+	intel_crtc->fdi_lanes = 0;
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
@@ -5714,15 +5738,15 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	intel_crtc->lowfreq_avail = false;
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	if (!is_dp || is_cpu_edp)
-		ironlake_set_m_n(crtc);
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	haswell_set_pipeconf(crtc, adjusted_mode, dither);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 251aa6b..6b8a279 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -766,12 +766,9 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *intel_encoder;
 	struct intel_dp *intel_dp;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int lane_count = 4;
 	struct intel_link_m_n m_n;
-	int pipe = intel_crtc->pipe;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	int target_clock;
 
 	/*
@@ -805,29 +802,10 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
 			       target_clock, adjusted_mode->clock, &m_n);
 
-	if (HAS_DDI(dev)) {
-		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
-	} else if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(TRANSDATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(TRANSDPLINK_M1(pipe), m_n.link_m);
-		I915_WRITE(TRANSDPLINK_N1(pipe), m_n.link_n);
-	} else if (IS_VALLEYVIEW(dev)) {
-		I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
-	} else {
-		I915_WRITE(PIPE_GMCH_DATA_M(pipe),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n.link_m);
-		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n.link_n);
-	}
+	if (intel_crtc->config.has_pch_encoder)
+		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
+	else
+		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 void intel_dp_init_link_config(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 18bba6e..3df7380 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -192,6 +192,7 @@ struct intel_crtc_config {
 
 	bool dither;
 	int pipe_bpp;
+	struct intel_link_m_n dp_m_n;
 
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
@@ -639,6 +640,10 @@ extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
 			    struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
+extern void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
+extern void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
1.7.11.7

[PATCH 2/8] drm/i915: move dp_m_n computation to dp_encoder->compute_config

[Daniel Vetter]

We need a flag to designate dp encoders and the dp link m_n parameters
in the pipe config for that. And now that the pipe bpp computations
have been moved up and stored in the pipe config, too, we can do this
without losing our sanity.

v2: Rebased on top of Takashi Iwai's fix to (again) fix the target
clock handling for eDP. Luckily the new code is sane enough and just
does the right thing!

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 30 +++++++++---------
 drivers/gpu/drm/i915/intel_dp.c      | 59 ++++++------------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 --
 3 files changed, 25 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a53a02c..dfa8919 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4182,6 +4182,14 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
 	}
 }
 
+static void intel_dp_set_m_n(struct intel_crtc *crtc)
+{
+	if (crtc->config.has_pch_encoder)
+		intel_pch_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
+	else
+		intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
+}
+
 static void vlv_update_pll(struct drm_crtc *crtc,
 			   intel_clock_t *clock, intel_clock_t *reduced_clock,
 			   int num_connectors)
@@ -4189,9 +4197,6 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	u32 dpll, mdiv, pdiv;
 	u32 bestn, bestm1, bestm2, bestp1, bestp2;
@@ -4247,8 +4252,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 
 	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	I915_WRITE(DPLL(pipe), dpll);
 
@@ -4294,9 +4299,6 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
 	u32 dpll;
@@ -4371,8 +4373,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	I915_WRITE(DPLL(pipe), dpll);
 
@@ -5588,8 +5590,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp)
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_pll_enable)
@@ -5738,8 +5740,8 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp)
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	intel_crtc->lowfreq_avail = false;
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6b8a279..ef680d5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -193,6 +193,8 @@ intel_dp_mode_valid(struct drm_connector *connector,
 
 		if (mode->vdisplay > fixed_mode->vdisplay)
 			return MODE_PANEL;
+
+		target_clock = fixed_mode->clock;
 	}
 
 	max_link_clock = drm_dp_bw_code_to_link_rate(intel_dp_max_link_bw(intel_dp));
@@ -688,6 +690,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && !is_cpu_edp(intel_dp))
 		pipe_config->has_pch_encoder = true;
 
+	pipe_config->has_dp_encoder = true;
+
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
 				       adjusted_mode);
@@ -707,7 +711,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 = 8*3;
+	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
 	if (is_edp(intel_dp) && dev_priv->edp.bpp)
 		bpp = min_t(int, bpp, dev_priv->edp.bpp);
 
@@ -756,56 +760,11 @@ found:
 	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
 		      mode_rate, link_avail);
 
-	return true;
-}
-
-void
-intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
-		 struct drm_display_mode *adjusted_mode)
-{
-	struct drm_device *dev = crtc->dev;
-	struct intel_encoder *intel_encoder;
-	struct intel_dp *intel_dp;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int lane_count = 4;
-	struct intel_link_m_n m_n;
-	int target_clock;
-
-	/*
-	 * Find the lane count in the intel_encoder private
-	 */
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
-		intel_dp = enc_to_intel_dp(&intel_encoder->base);
-
-		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
-		    intel_encoder->type == INTEL_OUTPUT_EDP)
-		{
-			lane_count = intel_dp->lane_count;
-			break;
-		}
-	}
-
-	target_clock = mode->clock;
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
-		if (intel_encoder->type == INTEL_OUTPUT_EDP) {
-			target_clock = intel_edp_target_clock(intel_encoder,
-							      mode);
-			break;
-		}
-	}
-
-	/*
-	 * Compute the GMCH and Link ratios. The '3' here is
-	 * the number of bytes_per_pixel post-LUT, which we always
-	 * set up for 8-bits of R/G/B, or 3 bytes total.
-	 */
-	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
-			       target_clock, adjusted_mode->clock, &m_n);
+	intel_link_compute_m_n(bpp, lane_count,
+			       target_clock, adjusted_mode->clock,
+			       &pipe_config->dp_m_n);
 
-	if (intel_crtc->config.has_pch_encoder)
-		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
-	else
-		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
+	return true;
 }
 
 void intel_dp_init_link_config(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3a9b7be..41fabcb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -473,9 +473,6 @@ extern void intel_dp_init(struct drm_device *dev, int output_reg,
 			  enum port port);
 extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 				    struct intel_connector *intel_connector);
-void
-intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
-		 struct drm_display_mode *adjusted_mode);
 extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
 extern void intel_dp_start_link_train(struct intel_dp *intel_dp);
 extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);
-- 
1.7.11.7

Re: [PATCH 2/8] drm/i915: move dp_m_n computation to dp_encoder->compute_config

[Jesse Barnes]

On Thu, 28 Mar 2013 10:41:57 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> We need a flag to designate dp encoders and the dp link m_n parameters
> in the pipe config for that. And now that the pipe bpp computations
> have been moved up and stored in the pipe config, too, we can do this
> without losing our sanity.
> 
> v2: Rebased on top of Takashi Iwai's fix to (again) fix the target
> clock handling for eDP. Luckily the new code is sane enough and just
> does the right thing!
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 30 +++++++++---------
>  drivers/gpu/drm/i915/intel_dp.c      | 59 ++++++------------------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 --
>  3 files changed, 25 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a53a02c..dfa8919 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4182,6 +4182,14 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>  	}
>  }
>  
> +static void intel_dp_set_m_n(struct intel_crtc *crtc)
> +{
> +	if (crtc->config.has_pch_encoder)
> +		intel_pch_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
> +	else
> +		intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
> +}
> +
>  static void vlv_update_pll(struct drm_crtc *crtc,
>  			   intel_clock_t *clock, intel_clock_t *reduced_clock,
>  			   int num_connectors)
> @@ -4189,9 +4197,6 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_display_mode *adjusted_mode =
> -		&intel_crtc->config.adjusted_mode;
> -	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
>  	int pipe = intel_crtc->pipe;
>  	u32 dpll, mdiv, pdiv;
>  	u32 bestn, bestm1, bestm2, bestp1, bestp2;
> @@ -4247,8 +4252,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>  	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> -		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> +	if (intel_crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(intel_crtc);
>  
>  	I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4294,9 +4299,6 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_display_mode *adjusted_mode =
> -		&intel_crtc->config.adjusted_mode;
> -	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	u32 dpll;
> @@ -4371,8 +4373,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  		if (encoder->pre_pll_enable)
>  			encoder->pre_pll_enable(encoder);
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> -		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> +	if (intel_crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(intel_crtc);
>  
>  	I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -5588,8 +5590,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	} else
>  		intel_put_pch_pll(intel_crtc);
>  
> -	if (is_dp)
> -		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> +	if (intel_crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(intel_crtc);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		if (encoder->pre_pll_enable)
> @@ -5738,8 +5740,8 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
>  	drm_mode_debug_printmodeline(mode);
>  
> -	if (is_dp)
> -		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> +	if (intel_crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(intel_crtc);
>  
>  	intel_crtc->lowfreq_avail = false;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6b8a279..ef680d5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -193,6 +193,8 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  
>  		if (mode->vdisplay > fixed_mode->vdisplay)
>  			return MODE_PANEL;
> +
> +		target_clock = fixed_mode->clock;
>  	}
>  
>  	max_link_clock = drm_dp_bw_code_to_link_rate(intel_dp_max_link_bw(intel_dp));
> @@ -688,6 +690,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && !is_cpu_edp(intel_dp))
>  		pipe_config->has_pch_encoder = true;
>  
> +	pipe_config->has_dp_encoder = true;
> +
>  	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>  				       adjusted_mode);
> @@ -707,7 +711,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 = 8*3;
> +	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
>  	if (is_edp(intel_dp) && dev_priv->edp.bpp)
>  		bpp = min_t(int, bpp, dev_priv->edp.bpp);
>  
> @@ -756,56 +760,11 @@ found:
>  	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
>  		      mode_rate, link_avail);
>  
> -	return true;
> -}
> -
> -void
> -intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> -		 struct drm_display_mode *adjusted_mode)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct intel_encoder *intel_encoder;
> -	struct intel_dp *intel_dp;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int lane_count = 4;
> -	struct intel_link_m_n m_n;
> -	int target_clock;
> -
> -	/*
> -	 * Find the lane count in the intel_encoder private
> -	 */
> -	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> -		intel_dp = enc_to_intel_dp(&intel_encoder->base);
> -
> -		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> -		    intel_encoder->type == INTEL_OUTPUT_EDP)
> -		{
> -			lane_count = intel_dp->lane_count;
> -			break;
> -		}
> -	}
> -
> -	target_clock = mode->clock;
> -	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> -		if (intel_encoder->type == INTEL_OUTPUT_EDP) {
> -			target_clock = intel_edp_target_clock(intel_encoder,
> -							      mode);
> -			break;
> -		}
> -	}
> -
> -	/*
> -	 * Compute the GMCH and Link ratios. The '3' here is
> -	 * the number of bytes_per_pixel post-LUT, which we always
> -	 * set up for 8-bits of R/G/B, or 3 bytes total.
> -	 */
> -	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
> -			       target_clock, adjusted_mode->clock, &m_n);
> +	intel_link_compute_m_n(bpp, lane_count,
> +			       target_clock, adjusted_mode->clock,
> +			       &pipe_config->dp_m_n);
>  
> -	if (intel_crtc->config.has_pch_encoder)
> -		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
> -	else
> -		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
> +	return true;
>  }
>  
>  void intel_dp_init_link_config(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3a9b7be..41fabcb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -473,9 +473,6 @@ extern void intel_dp_init(struct drm_device *dev, int output_reg,
>  			  enum port port);
>  extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  				    struct intel_connector *intel_connector);
> -void
> -intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> -		 struct drm_display_mode *adjusted_mode);
>  extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
>  extern void intel_dp_start_link_train(struct intel_dp *intel_dp);
>  extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);

Looks good.

with the field from 1/8 moved to this patch:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] drm/i915: move dp_m_n computation to dp_encoder->compute_config

[Daniel Vetter]

We need a flag to designate dp encoders and the dp link m_n parameters
in the pipe config for that. And now that the pipe bpp computations
have been moved up and stored in the pipe config, too, we can do this
without losing our sanity.

v2: Rebased on top of Takashi Iwai's fix to (again) fix the target
clock handling for eDP. Luckily the new code is sane enough and just
does the right thing!

v3: Move ->has_dp_encoder to this patch (Jesse).

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 30 +++++++++---------
 drivers/gpu/drm/i915/intel_dp.c      | 59 ++++++------------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 ++--
 3 files changed, 28 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a53a02c..dfa8919 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4182,6 +4182,14 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
 	}
 }
 
+static void intel_dp_set_m_n(struct intel_crtc *crtc)
+{
+	if (crtc->config.has_pch_encoder)
+		intel_pch_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
+	else
+		intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
+}
+
 static void vlv_update_pll(struct drm_crtc *crtc,
 			   intel_clock_t *clock, intel_clock_t *reduced_clock,
 			   int num_connectors)
@@ -4189,9 +4197,6 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	u32 dpll, mdiv, pdiv;
 	u32 bestn, bestm1, bestm2, bestp1, bestp2;
@@ -4247,8 +4252,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 
 	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	I915_WRITE(DPLL(pipe), dpll);
 
@@ -4294,9 +4299,6 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
 	u32 dpll;
@@ -4371,8 +4373,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	I915_WRITE(DPLL(pipe), dpll);
 
@@ -5588,8 +5590,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp)
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_pll_enable)
@@ -5738,8 +5740,8 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp)
-		intel_dp_set_m_n(crtc, mode, adjusted_mode);
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
 
 	intel_crtc->lowfreq_avail = false;
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6b8a279..ef680d5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -193,6 +193,8 @@ intel_dp_mode_valid(struct drm_connector *connector,
 
 		if (mode->vdisplay > fixed_mode->vdisplay)
 			return MODE_PANEL;
+
+		target_clock = fixed_mode->clock;
 	}
 
 	max_link_clock = drm_dp_bw_code_to_link_rate(intel_dp_max_link_bw(intel_dp));
@@ -688,6 +690,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && !is_cpu_edp(intel_dp))
 		pipe_config->has_pch_encoder = true;
 
+	pipe_config->has_dp_encoder = true;
+
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
 				       adjusted_mode);
@@ -707,7 +711,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 = 8*3;
+	bpp = min_t(int, 8*3, pipe_config->pipe_bpp);
 	if (is_edp(intel_dp) && dev_priv->edp.bpp)
 		bpp = min_t(int, bpp, dev_priv->edp.bpp);
 
@@ -756,56 +760,11 @@ found:
 	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
 		      mode_rate, link_avail);
 
-	return true;
-}
-
-void
-intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
-		 struct drm_display_mode *adjusted_mode)
-{
-	struct drm_device *dev = crtc->dev;
-	struct intel_encoder *intel_encoder;
-	struct intel_dp *intel_dp;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int lane_count = 4;
-	struct intel_link_m_n m_n;
-	int target_clock;
-
-	/*
-	 * Find the lane count in the intel_encoder private
-	 */
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
-		intel_dp = enc_to_intel_dp(&intel_encoder->base);
-
-		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
-		    intel_encoder->type == INTEL_OUTPUT_EDP)
-		{
-			lane_count = intel_dp->lane_count;
-			break;
-		}
-	}
-
-	target_clock = mode->clock;
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
-		if (intel_encoder->type == INTEL_OUTPUT_EDP) {
-			target_clock = intel_edp_target_clock(intel_encoder,
-							      mode);
-			break;
-		}
-	}
-
-	/*
-	 * Compute the GMCH and Link ratios. The '3' here is
-	 * the number of bytes_per_pixel post-LUT, which we always
-	 * set up for 8-bits of R/G/B, or 3 bytes total.
-	 */
-	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
-			       target_clock, adjusted_mode->clock, &m_n);
+	intel_link_compute_m_n(bpp, lane_count,
+			       target_clock, adjusted_mode->clock,
+			       &pipe_config->dp_m_n);
 
-	if (intel_crtc->config.has_pch_encoder)
-		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
-	else
-		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
+	return true;
 }
 
 void intel_dp_init_link_config(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3df7380..c2ba117 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -190,6 +190,9 @@ struct intel_crtc_config {
 	 */
 	bool limited_color_range;
 
+	/* DP has a bunch of special case unfortunately, so mark the pipe
+	 * accordingly. */
+	bool has_dp_encoder;
 	bool dither;
 	int pipe_bpp;
 	struct intel_link_m_n dp_m_n;
@@ -468,9 +471,6 @@ extern void intel_dp_init(struct drm_device *dev, int output_reg,
 			  enum port port);
 extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 				    struct intel_connector *intel_connector);
-void
-intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
-		 struct drm_display_mode *adjusted_mode);
 extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
 extern void intel_dp_start_link_train(struct intel_dp *intel_dp);
 extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);
-- 
1.7.11.7

[PATCH 3/8] drm/i915: track dp target_clock in pipe_config

[Daniel Vetter]

We need it in the fdi m_n computation, which nicely kills almost
all ugly special cases in there.

It looks like we also need this to handle 12bpc hdmi correctly.

Eventually it might be better to switch things around and put the
target clock into adjusted_mode->clock and create a new pipe_config
parameter for the port link clock.

v2: Add a massive comment in the code to explain this mess.

v3: s/dp_target_clock/pixel_target_clock in anticipation of the hdmi
use-case.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dfa8919..464eb90 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5356,25 +5356,9 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
-	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
 	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
-	bool is_dp = false, is_cpu_edp = false;
-
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
-		switch (intel_encoder->type) {
-		case INTEL_OUTPUT_DISPLAYPORT:
-			is_dp = true;
-			break;
-		case INTEL_OUTPUT_EDP:
-			is_dp = true;
-			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
-				is_cpu_edp = true;
-			edp_encoder = intel_encoder;
-			break;
-		}
-	}
+	uint32_t bps;
 
 	/* FDI is a binary signal running at ~2.7GHz, encoding
 	 * each output octet as 10 bits. The actual frequency
@@ -5385,11 +5369,8 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
 	 */
 	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
 
-	/* [e]DP over FDI requires target mode clock instead of link clock. */
-	if (edp_encoder)
-		target_clock = intel_edp_target_clock(edp_encoder, mode);
-	else if (is_dp)
-		target_clock = mode->clock;
+	if (intel_crtc->config.pixel_target_clock)
+		target_clock = intel_crtc->config.pixel_target_clock;
 	else
 		target_clock = adjusted_mode->clock;
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ef680d5..b1bf00b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -753,6 +753,7 @@ found:
 	intel_dp->lane_count = lane_count;
 	adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
 	pipe_config->pipe_bpp = bpp;
+	pipe_config->pixel_target_clock = target_clock;
 
 	DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n",
 		      intel_dp->link_bw, intel_dp->lane_count,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 41fabcb..2a86a12 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -198,7 +198,12 @@ struct intel_crtc_config {
 	bool dither;
 	int pipe_bpp;
 	struct intel_link_m_n dp_m_n;
-
+	/**
+	 * This is currently used by DP and HDMI encoders since those can have a
+	 * target pixel clock != the port link clock (which is currently stored
+	 * in adjusted_mode->clock).
+	 */
+	int pixel_target_clock;
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
 };
-- 
1.7.11.7

Re: [PATCH 3/8] drm/i915: track dp target_clock in pipe_config

[Jesse Barnes]

On Thu, 28 Mar 2013 10:41:58 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> We need it in the fdi m_n computation, which nicely kills almost
> all ugly special cases in there.
> 
> It looks like we also need this to handle 12bpc hdmi correctly.
> 
> Eventually it might be better to switch things around and put the
> target clock into adjusted_mode->clock and create a new pipe_config
> parameter for the port link clock.
> 
> v2: Add a massive comment in the code to explain this mess.
> 
> v3: s/dp_target_clock/pixel_target_clock in anticipation of the hdmi
> use-case.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 25 +++----------------------
>  drivers/gpu/drm/i915/intel_dp.c      |  1 +
>  drivers/gpu/drm/i915/intel_drv.h     |  7 ++++++-
>  3 files changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dfa8919..464eb90 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5356,25 +5356,9 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_display_mode *adjusted_mode =
>  		&intel_crtc->config.adjusted_mode;
> -	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> -	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
>  	struct intel_link_m_n m_n = {0};
>  	int target_clock, lane, link_bw;
> -	bool is_dp = false, is_cpu_edp = false;
> -
> -	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> -		switch (intel_encoder->type) {
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
> -		case INTEL_OUTPUT_EDP:
> -			is_dp = true;
> -			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
> -				is_cpu_edp = true;
> -			edp_encoder = intel_encoder;
> -			break;
> -		}
> -	}
> +	uint32_t bps;
>  
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
>  	 * each output octet as 10 bits. The actual frequency
> @@ -5385,11 +5369,8 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
>  	 */
>  	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
>  
> -	/* [e]DP over FDI requires target mode clock instead of link clock. */
> -	if (edp_encoder)
> -		target_clock = intel_edp_target_clock(edp_encoder, mode);
> -	else if (is_dp)
> -		target_clock = mode->clock;
> +	if (intel_crtc->config.pixel_target_clock)
> +		target_clock = intel_crtc->config.pixel_target_clock;
>  	else
>  		target_clock = adjusted_mode->clock;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ef680d5..b1bf00b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -753,6 +753,7 @@ found:
>  	intel_dp->lane_count = lane_count;
>  	adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
>  	pipe_config->pipe_bpp = bpp;
> +	pipe_config->pixel_target_clock = target_clock;
>  
>  	DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n",
>  		      intel_dp->link_bw, intel_dp->lane_count,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 41fabcb..2a86a12 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -198,7 +198,12 @@ struct intel_crtc_config {
>  	bool dither;
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;
> -
> +	/**
> +	 * This is currently used by DP and HDMI encoders since those can have a
> +	 * target pixel clock != the port link clock (which is currently stored
> +	 * in adjusted_mode->clock).
> +	 */
> +	int pixel_target_clock;
>  	/* Used by SDVO (and if we ever fix it, HDMI). */
>  	unsigned pixel_multiplier;
>  };

Since you already explained there are other callers of
edp_target_clock():
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] drm/i915: remove leaky eDP functions

[Daniel Vetter]

Jesse Barnes noticed in his review of my DP cleanup series that
intel_edp_target_clock is now unused. Checking related code I've
noticed that also intel_edp_link_config is long unused.

Kill them both.

Wrt leaky eDP functions used in the common crtc code, the only thing
still left is intel_encoder_is_pch_edp. That one is just due to the
massive confusion between eDP vs. DP and port A vs. port D. Crtc code
should at most concern itself with the later, never with the former.

But that's material for another patch series.

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c  | 23 -----------------------
 drivers/gpu/drm/i915/intel_drv.h |  3 ---
 2 files changed, 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6bf144c..f90ce47 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -109,29 +109,6 @@ bool intel_encoder_is_pch_edp(struct drm_encoder *encoder)
 
 static void intel_dp_link_down(struct intel_dp *intel_dp);
 
-void
-intel_edp_link_config(struct intel_encoder *intel_encoder,
-		       int *lane_num, int *link_bw)
-{
-	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
-
-	*lane_num = intel_dp->lane_count;
-	*link_bw = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
-}
-
-int
-intel_edp_target_clock(struct intel_encoder *intel_encoder,
-		       struct drm_display_mode *mode)
-{
-	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
-	struct intel_connector *intel_connector = intel_dp->attached_connector;
-
-	if (intel_connector->panel.fixed_mode)
-		return intel_connector->panel.fixed_mode->clock;
-	else
-		return mode->clock;
-}
-
 static int
 intel_dp_max_link_bw(struct intel_dp *intel_dp)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 094c940..2b31d7e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -520,9 +520,6 @@ extern void ironlake_edp_panel_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_off(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
-extern void intel_edp_link_config(struct intel_encoder *, int *, int *);
-extern int intel_edp_target_clock(struct intel_encoder *,
-				  struct drm_display_mode *mode);
 extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
 extern int intel_plane_init(struct drm_device *dev, enum pipe pipe);
 extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
-- 
1.7.11.7

Re: [PATCH] drm/i915: remove leaky eDP functions

[Jesse Barnes]

On Tue,  2 Apr 2013 23:27:41 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Jesse Barnes noticed in his review of my DP cleanup series that
> intel_edp_target_clock is now unused. Checking related code I've
> noticed that also intel_edp_link_config is long unused.
> 
> Kill them both.
> 
> Wrt leaky eDP functions used in the common crtc code, the only thing
> still left is intel_encoder_is_pch_edp. That one is just due to the
> massive confusion between eDP vs. DP and port A vs. port D. Crtc code
> should at most concern itself with the later, never with the former.
> 
> But that's material for another patch series.
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 23 -----------------------
>  drivers/gpu/drm/i915/intel_drv.h |  3 ---
>  2 files changed, 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6bf144c..f90ce47 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -109,29 +109,6 @@ bool intel_encoder_is_pch_edp(struct drm_encoder *encoder)
>  
>  static void intel_dp_link_down(struct intel_dp *intel_dp);
>  
> -void
> -intel_edp_link_config(struct intel_encoder *intel_encoder,
> -		       int *lane_num, int *link_bw)
> -{
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> -
> -	*lane_num = intel_dp->lane_count;
> -	*link_bw = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
> -}
> -
> -int
> -intel_edp_target_clock(struct intel_encoder *intel_encoder,
> -		       struct drm_display_mode *mode)
> -{
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> -	struct intel_connector *intel_connector = intel_dp->attached_connector;
> -
> -	if (intel_connector->panel.fixed_mode)
> -		return intel_connector->panel.fixed_mode->clock;
> -	else
> -		return mode->clock;
> -}
> -
>  static int
>  intel_dp_max_link_bw(struct intel_dp *intel_dp)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 094c940..2b31d7e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -520,9 +520,6 @@ extern void ironlake_edp_panel_on(struct intel_dp *intel_dp);
>  extern void ironlake_edp_panel_off(struct intel_dp *intel_dp);
>  extern void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp);
>  extern void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
> -extern void intel_edp_link_config(struct intel_encoder *, int *, int *);
> -extern int intel_edp_target_clock(struct intel_encoder *,
> -				  struct drm_display_mode *mode);
>  extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
>  extern int intel_plane_init(struct drm_device *dev, enum pipe pipe);
>  extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 4/8] drm/i915: rip out superflous is_dp&is_cpu_edp tracking

[Daniel Vetter]

The only exception left is is_cpu_edp in the haswell modeset code.
We need that to assign the cpu transcoder, but we might want to
move that eventually into the encoder, too.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 464eb90..e925efe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4532,7 +4532,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	intel_clock_t clock, reduced_clock;
 	u32 dspcntr, pipeconf;
 	bool ok, has_reduced_clock = false, is_sdvo = false;
-	bool is_lvds = false, is_tv = false, is_dp = false;
+	bool is_lvds = false, is_tv = false;
 	struct intel_encoder *encoder;
 	const intel_limit_t *limit;
 	int ret;
@@ -4551,9 +4551,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		case INTEL_OUTPUT_TVOUT:
 			is_tv = true;
 			break;
-		case INTEL_OUTPUT_DISPLAYPORT:
-			is_dp = true;
-			break;
 		}
 
 		num_connectors++;
@@ -4636,7 +4633,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 
 	/* default to 8bpc */
 	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
-	if (is_dp) {
+	if (intel_crtc->config.has_dp_encoder) {
 		if (intel_crtc->config.dither) {
 			pipeconf |= PIPECONF_6BPC |
 				    PIPECONF_DITHER_EN |
@@ -5397,7 +5394,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	uint32_t dpll;
 	int factor, num_connectors = 0;
 	bool is_lvds = false, is_sdvo = false, is_tv = false;
-	bool is_dp = false, is_cpu_edp = false;
 
 	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
 		switch (intel_encoder->type) {
@@ -5413,14 +5409,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		case INTEL_OUTPUT_TVOUT:
 			is_tv = true;
 			break;
-		case INTEL_OUTPUT_DISPLAYPORT:
-			is_dp = true;
-			break;
-		case INTEL_OUTPUT_EDP:
-			is_dp = true;
-			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
-				is_cpu_edp = true;
-			break;
 		}
 
 		num_connectors++;
@@ -5452,7 +5440,8 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		}
 		dpll |= DPLL_DVO_HIGH_SPEED;
 	}
-	if (is_dp && !is_cpu_edp)
+	if (intel_crtc->config.has_dp_encoder &&
+	    intel_crtc->config.has_pch_encoder)
 		dpll |= DPLL_DVO_HIGH_SPEED;
 
 	/* compute bitmask from p1 value */
@@ -5505,7 +5494,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	intel_clock_t clock, reduced_clock;
 	u32 dpll, fp = 0, fp2 = 0;
 	bool ok, has_reduced_clock = false;
-	bool is_lvds = false, is_dp = false, is_cpu_edp = false;
+	bool is_lvds = false;
 	struct intel_encoder *encoder;
 	int ret;
 	bool dither, fdi_config_ok;
@@ -5515,14 +5504,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
 			break;
-		case INTEL_OUTPUT_DISPLAYPORT:
-			is_dp = true;
-			break;
-		case INTEL_OUTPUT_EDP:
-			is_dp = true;
-			if (!intel_encoder_is_pch_edp(&encoder->base))
-				is_cpu_edp = true;
-			break;
 		}
 
 		num_connectors++;
@@ -5559,7 +5540,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	drm_mode_debug_printmodeline(mode);
 
 	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
-	if (!is_cpu_edp) {
+	if (intel_crtc->config.has_pch_encoder) {
 		struct intel_pch_pll *pll;
 
 		pll = intel_get_pch_pll(intel_crtc, dpll, fp);
@@ -5672,18 +5653,14 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 	int num_connectors = 0;
-	bool is_dp = false, is_cpu_edp = false;
+	bool is_cpu_edp = false;
 	struct intel_encoder *encoder;
 	int ret;
 	bool dither;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		switch (encoder->type) {
-		case INTEL_OUTPUT_DISPLAYPORT:
-			is_dp = true;
-			break;
 		case INTEL_OUTPUT_EDP:
-			is_dp = true;
 			if (!intel_encoder_is_pch_edp(&encoder->base))
 				is_cpu_edp = true;
 			break;
-- 
1.7.11.7

Re: [PATCH 4/8] drm/i915: rip out superflous is_dp&is_cpu_edp tracking

[Jesse Barnes]

On Thu, 28 Mar 2013 10:41:59 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> The only exception left is is_cpu_edp in the haswell modeset code.
> We need that to assign the cpu transcoder, but we might want to
> move that eventually into the encoder, too.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 37 +++++++-----------------------------
>  1 file changed, 7 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 464eb90..e925efe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4532,7 +4532,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	intel_clock_t clock, reduced_clock;
>  	u32 dspcntr, pipeconf;
>  	bool ok, has_reduced_clock = false, is_sdvo = false;
> -	bool is_lvds = false, is_tv = false, is_dp = false;
> +	bool is_lvds = false, is_tv = false;
>  	struct intel_encoder *encoder;
>  	const intel_limit_t *limit;
>  	int ret;
> @@ -4551,9 +4551,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		case INTEL_OUTPUT_TVOUT:
>  			is_tv = true;
>  			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
>  		}
>  
>  		num_connectors++;
> @@ -4636,7 +4633,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	/* default to 8bpc */
>  	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (is_dp) {
> +	if (intel_crtc->config.has_dp_encoder) {
>  		if (intel_crtc->config.dither) {
>  			pipeconf |= PIPECONF_6BPC |
>  				    PIPECONF_DITHER_EN |
> @@ -5397,7 +5394,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	uint32_t dpll;
>  	int factor, num_connectors = 0;
>  	bool is_lvds = false, is_sdvo = false, is_tv = false;
> -	bool is_dp = false, is_cpu_edp = false;
>  
>  	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>  		switch (intel_encoder->type) {
> @@ -5413,14 +5409,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		case INTEL_OUTPUT_TVOUT:
>  			is_tv = true;
>  			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
> -		case INTEL_OUTPUT_EDP:
> -			is_dp = true;
> -			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
> -				is_cpu_edp = true;
> -			break;
>  		}
>  
>  		num_connectors++;
> @@ -5452,7 +5440,8 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		}
>  		dpll |= DPLL_DVO_HIGH_SPEED;
>  	}
> -	if (is_dp && !is_cpu_edp)
> +	if (intel_crtc->config.has_dp_encoder &&
> +	    intel_crtc->config.has_pch_encoder)
>  		dpll |= DPLL_DVO_HIGH_SPEED;
>  
>  	/* compute bitmask from p1 value */
> @@ -5505,7 +5494,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	intel_clock_t clock, reduced_clock;
>  	u32 dpll, fp = 0, fp2 = 0;
>  	bool ok, has_reduced_clock = false;
> -	bool is_lvds = false, is_dp = false, is_cpu_edp = false;
> +	bool is_lvds = false;
>  	struct intel_encoder *encoder;
>  	int ret;
>  	bool dither, fdi_config_ok;
> @@ -5515,14 +5504,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
>  			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
> -		case INTEL_OUTPUT_EDP:
> -			is_dp = true;
> -			if (!intel_encoder_is_pch_edp(&encoder->base))
> -				is_cpu_edp = true;
> -			break;
>  		}
>  
>  		num_connectors++;
> @@ -5559,7 +5540,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	drm_mode_debug_printmodeline(mode);
>  
>  	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
> -	if (!is_cpu_edp) {
> +	if (intel_crtc->config.has_pch_encoder) {
>  		struct intel_pch_pll *pll;
>  
>  		pll = intel_get_pch_pll(intel_crtc, dpll, fp);
> @@ -5672,18 +5653,14 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  	int num_connectors = 0;
> -	bool is_dp = false, is_cpu_edp = false;
> +	bool is_cpu_edp = false;
>  	struct intel_encoder *encoder;
>  	int ret;
>  	bool dither;
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder) {
>  		switch (encoder->type) {
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
>  		case INTEL_OUTPUT_EDP:
> -			is_dp = true;
>  			if (!intel_encoder_is_pch_edp(&encoder->base))
>  				is_cpu_edp = true;
>  			break;

\o/-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Untangling that stuff for ILK-IVB was a pain, glad to see it go.

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 5/8] drm/i915: add hw state readout/checking for pipe_config

[Daniel Vetter]

We need to be able to read out the hw state code for a bunch
of reasons:
- Correctly disabling boot-up/resume state.
- Pure paranoia.

Since not all of the pipe configuration is e.g. relevant for
fastboot (or at least we can allow some wiggle room in some
parameters, like the clocks), we need to add a strict_checking
parameter to intel_pipe_config_compare for fastboot.

For now intel_pipe_config_compare should be fully paranoid and
check everything that the hw state readout code supports. Which
for this infrastructure code is nothing.

I've gone a bit overboard with adding 3 get_pipe_config functions:
The ilk version will differ with the next patch, so it's not too
onerous.

v2: Don't check the hw config if the pipe is off, since an enabled,
but dpms off crtc will obviously have tons of difference with the hw
state.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h      |  5 +++
 drivers/gpu/drm/i915/intel_display.c | 77 +++++++++++++++++++++++++++++++-----
 2 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 86777c8..99d7f81 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -285,6 +285,7 @@ struct drm_i915_error_state {
 };
 
 struct intel_crtc_config;
+struct intel_crtc;
 
 struct drm_i915_display_funcs {
 	bool (*fbc_enabled)(struct drm_device *dev);
@@ -298,6 +299,10 @@ struct drm_i915_display_funcs {
 	void (*update_linetime_wm)(struct drm_device *dev, int pipe,
 				 struct drm_display_mode *mode);
 	void (*modeset_global_resources)(struct drm_device *dev);
+	/* Returns the active state of the crtc, and if the crtc is active,
+	 * fills out the pipe-config with the hw state. */
+	bool (*get_pipe_config)(struct intel_crtc *,
+				struct intel_crtc_config *);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e925efe..a5adaa0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4695,6 +4695,20 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	return ret;
 }
 
+static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t tmp;
+
+	tmp = I915_READ(PIPECONF(crtc->pipe));
+	if (!(tmp & PIPECONF_ENABLE))
+		return false;
+
+	return true;
+}
+
 static void ironlake_init_pch_refclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5355,7 +5369,6 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
 		&intel_crtc->config.adjusted_mode;
 	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
-	uint32_t bps;
 
 	/* FDI is a binary signal running at ~2.7GHz, encoding
 	 * each output octet as 10 bits. The actual frequency
@@ -5611,6 +5624,20 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	return fdi_config_ok ? ret : -EINVAL;
 }
 
+static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t tmp;
+
+	tmp = I915_READ(PIPECONF(crtc->pipe));
+	if (!(tmp & PIPECONF_ENABLE))
+		return false;
+
+	return true;
+}
+
 static void haswell_modeset_global_resources(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5725,6 +5752,20 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	return ret;
 }
 
+static bool haswell_get_pipe_config(struct intel_crtc *crtc,
+				    struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t tmp;
+
+	tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
+	if (!(tmp & PIPECONF_ENABLE))
+		return false;
+
+	return true;
+}
+
 static int intel_crtc_mode_set(struct drm_crtc *crtc,
 			       int x, int y,
 			       struct drm_framebuffer *fb)
@@ -7647,12 +7688,21 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 			    base.head) \
 		if (mask & (1 <<(intel_crtc)->pipe)) \
 
+static bool
+intel_pipe_config_compare(struct intel_crtc_config *current_config,
+			  struct intel_crtc_config *pipe_config)
+{
+	return true;
+}
+
 void
 intel_modeset_check_state(struct drm_device *dev)
 {
+	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_crtc *crtc;
 	struct intel_encoder *encoder;
 	struct intel_connector *connector;
+	struct intel_crtc_config pipe_config;
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list,
 			    base.head) {
@@ -7741,7 +7791,15 @@ intel_modeset_check_state(struct drm_device *dev)
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
-		assert_pipe(dev->dev_private, crtc->pipe, crtc->active);
+		active = dev_priv->display.get_pipe_config(crtc,
+							   &pipe_config);
+		WARN(crtc->active != active,
+		     "crtc active state doesn't match with hw state "
+		     "(expected %i, found %i)\n", crtc->active, active);
+
+		WARN(active &&
+		     !intel_pipe_config_compare(&crtc->config, &pipe_config),
+		     "pipe state doesn't match!\n");
 	}
 }
 
@@ -8549,18 +8607,21 @@ static void intel_init_display(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (HAS_DDI(dev)) {
+		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
 		dev_priv->display.off = haswell_crtc_off;
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
+		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
 		dev_priv->display.off = ironlake_crtc_off;
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else {
+		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -9092,14 +9153,10 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	}
 
 setup_pipes:
-	for_each_pipe(pipe) {
-		crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
-
-		tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
-		if (tmp & PIPECONF_ENABLE)
-			crtc->active = true;
-		else
-			crtc->active = false;
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		crtc->active = dev_priv->display.get_pipe_config(crtc,
+								 &crtc->config);
 
 		crtc->base.enabled = crtc->active;
 
-- 
1.7.11.7

Re: [PATCH 5/8] drm/i915: add hw state readout/checking for pipe_config

[Jesse Barnes]

On Thu, 28 Mar 2013 10:42:00 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> We need to be able to read out the hw state code for a bunch
> of reasons:
> - Correctly disabling boot-up/resume state.
> - Pure paranoia.
> 
> Since not all of the pipe configuration is e.g. relevant for
> fastboot (or at least we can allow some wiggle room in some
> parameters, like the clocks), we need to add a strict_checking
> parameter to intel_pipe_config_compare for fastboot.
> 
> For now intel_pipe_config_compare should be fully paranoid and
> check everything that the hw state readout code supports. Which
> for this infrastructure code is nothing.
> 
> I've gone a bit overboard with adding 3 get_pipe_config functions:
> The ilk version will differ with the next patch, so it's not too
> onerous.
> 
> v2: Don't check the hw config if the pipe is off, since an enabled,
> but dpms off crtc will obviously have tons of difference with the hw
> state.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  5 +++
>  drivers/gpu/drm/i915/intel_display.c | 77 +++++++++++++++++++++++++++++++-----
>  2 files changed, 72 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 86777c8..99d7f81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -285,6 +285,7 @@ struct drm_i915_error_state {
>  };
>  
>  struct intel_crtc_config;
> +struct intel_crtc;
>  
>  struct drm_i915_display_funcs {
>  	bool (*fbc_enabled)(struct drm_device *dev);
> @@ -298,6 +299,10 @@ struct drm_i915_display_funcs {
>  	void (*update_linetime_wm)(struct drm_device *dev, int pipe,
>  				 struct drm_display_mode *mode);
>  	void (*modeset_global_resources)(struct drm_device *dev);
> +	/* Returns the active state of the crtc, and if the crtc is active,
> +	 * fills out the pipe-config with the hw state. */
> +	bool (*get_pipe_config)(struct intel_crtc *,
> +				struct intel_crtc_config *);
>  	int (*crtc_mode_set)(struct drm_crtc *crtc,
>  			     int x, int y,
>  			     struct drm_framebuffer *old_fb);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e925efe..a5adaa0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4695,6 +4695,20 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	return ret;
>  }
>  
> +static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> +				 struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(PIPECONF(crtc->pipe));
> +	if (!(tmp & PIPECONF_ENABLE))
> +		return false;
> +
> +	return true;
> +}
> +
>  static void ironlake_init_pch_refclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5355,7 +5369,6 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
>  		&intel_crtc->config.adjusted_mode;
>  	struct intel_link_m_n m_n = {0};
>  	int target_clock, lane, link_bw;
> -	uint32_t bps;
>  
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
>  	 * each output octet as 10 bits. The actual frequency
> @@ -5611,6 +5624,20 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	return fdi_config_ok ? ret : -EINVAL;
>  }
>  
> +static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> +				     struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(PIPECONF(crtc->pipe));
> +	if (!(tmp & PIPECONF_ENABLE))
> +		return false;
> +
> +	return true;
> +}
> +
>  static void haswell_modeset_global_resources(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5725,6 +5752,20 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	return ret;
>  }
>  
> +static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> +				    struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
> +	if (!(tmp & PIPECONF_ENABLE))
> +		return false;
> +
> +	return true;
> +}
> +
>  static int intel_crtc_mode_set(struct drm_crtc *crtc,
>  			       int x, int y,
>  			       struct drm_framebuffer *fb)
> @@ -7647,12 +7688,21 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
>  			    base.head) \
>  		if (mask & (1 <<(intel_crtc)->pipe)) \
>  
> +static bool
> +intel_pipe_config_compare(struct intel_crtc_config *current_config,
> +			  struct intel_crtc_config *pipe_config)
> +{
> +	return true;
> +}
> +
>  void
>  intel_modeset_check_state(struct drm_device *dev)
>  {
> +	drm_i915_private_t *dev_priv = dev->dev_private;
>  	struct intel_crtc *crtc;
>  	struct intel_encoder *encoder;
>  	struct intel_connector *connector;
> +	struct intel_crtc_config pipe_config;
>  
>  	list_for_each_entry(connector, &dev->mode_config.connector_list,
>  			    base.head) {
> @@ -7741,7 +7791,15 @@ intel_modeset_check_state(struct drm_device *dev)
>  		     "crtc's computed enabled state doesn't match tracked enabled state "
>  		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
>  
> -		assert_pipe(dev->dev_private, crtc->pipe, crtc->active);
> +		active = dev_priv->display.get_pipe_config(crtc,
> +							   &pipe_config);
> +		WARN(crtc->active != active,
> +		     "crtc active state doesn't match with hw state "
> +		     "(expected %i, found %i)\n", crtc->active, active);
> +
> +		WARN(active &&
> +		     !intel_pipe_config_compare(&crtc->config, &pipe_config),
> +		     "pipe state doesn't match!\n");
>  	}
>  }
>  
> @@ -8549,18 +8607,21 @@ static void intel_init_display(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	if (HAS_DDI(dev)) {
> +		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
>  		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
>  		dev_priv->display.crtc_enable = haswell_crtc_enable;
>  		dev_priv->display.crtc_disable = haswell_crtc_disable;
>  		dev_priv->display.off = haswell_crtc_off;
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (HAS_PCH_SPLIT(dev)) {
> +		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
>  		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
>  		dev_priv->display.crtc_enable = ironlake_crtc_enable;
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
>  		dev_priv->display.off = ironlake_crtc_off;
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else {
> +		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9092,14 +9153,10 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  	}
>  
>  setup_pipes:
> -	for_each_pipe(pipe) {
> -		crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -
> -		tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
> -		if (tmp & PIPECONF_ENABLE)
> -			crtc->active = true;
> -		else
> -			crtc->active = false;
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> +			    base.head) {
> +		crtc->active = dev_priv->display.get_pipe_config(crtc,
> +								 &crtc->config);
>  
>  		crtc->base.enabled = crtc->active;
>  

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 6/8] drm/i915: hw readout support for ->has_pch_encoders

[Daniel Vetter]

Now we can ditch the checks in the Haswell disable code.

v2: add support for Haswell

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a5adaa0..c9e873e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2975,11 +2975,6 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 	mutex_unlock(&dev->struct_mutex);
 }
 
-static bool haswell_crtc_driving_pch(struct drm_crtc *crtc)
-{
-	return intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG);
-}
-
 /* Program iCLKIP clock to the desired frequency */
 static void lpt_program_iclkip(struct drm_crtc *crtc)
 {
@@ -3562,13 +3557,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
-	bool is_pch_port;
 
 	if (!intel_crtc->active)
 		return;
 
-	is_pch_port = haswell_crtc_driving_pch(crtc);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->disable(encoder);
 
@@ -3595,7 +3587,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	if (is_pch_port) {
+	if (intel_crtc->config.has_pch_encoder) {
 		lpt_disable_pch_transcoder(dev_priv);
 		intel_ddi_fdi_disable(crtc);
 	}
@@ -5635,6 +5627,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
+	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
+		pipe_config->has_pch_encoder = true;
+
 	return true;
 }
 
@@ -5763,6 +5758,17 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
+	/*
+	 * aswell has only FDI/PCH transcoder A. It is which is connected to
+	 * DDI E. So just check whether this pipe is wired to DDI E and whether
+	 * the PCH transcoder is on.
+	 */
+	tmp = I915_READ(TRANS_DDI_FUNC_CTL(crtc->pipe));
+	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
+	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
+		pipe_config->has_pch_encoder = true;
+
+
 	return true;
 }
 
@@ -7692,6 +7698,14 @@ static bool
 intel_pipe_config_compare(struct intel_crtc_config *current_config,
 			  struct intel_crtc_config *pipe_config)
 {
+	if (current_config->has_pch_encoder != pipe_config->has_pch_encoder) {
+		DRM_ERROR("mismatch in has_pch_encoder "
+			  "(expected %i, found %i)\n",
+			  current_config->has_pch_encoder,
+			  pipe_config->has_pch_encoder);
+		return false;
+	}
+
 	return true;
 }
 
@@ -7791,6 +7805,7 @@ intel_modeset_check_state(struct drm_device *dev)
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
+		memset(&pipe_config, 0, sizeof(pipe_config));
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
 		WARN(crtc->active != active,
@@ -9155,6 +9170,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 setup_pipes:
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
 			    base.head) {
+		memset(&crtc->config, 0, sizeof(crtc->config));
 		crtc->active = dev_priv->display.get_pipe_config(crtc,
 								 &crtc->config);
 
-- 
1.7.11.7

Re: [PATCH 6/8] drm/i915: hw readout support for ->has_pch_encoders

[Jesse Barnes]

On Thu, 28 Mar 2013 10:42:01 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Now we can ditch the checks in the Haswell disable code.
> 
> v2: add support for Haswell
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a5adaa0..c9e873e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2975,11 +2975,6 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> -static bool haswell_crtc_driving_pch(struct drm_crtc *crtc)
> -{
> -	return intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG);
> -}
> -
>  /* Program iCLKIP clock to the desired frequency */
>  static void lpt_program_iclkip(struct drm_crtc *crtc)
>  {
> @@ -3562,13 +3557,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> -	bool is_pch_port;
>  
>  	if (!intel_crtc->active)
>  		return;
>  
> -	is_pch_port = haswell_crtc_driving_pch(crtc);
> -
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		encoder->disable(encoder);
>  
> @@ -3595,7 +3587,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  		if (encoder->post_disable)
>  			encoder->post_disable(encoder);
>  
> -	if (is_pch_port) {
> +	if (intel_crtc->config.has_pch_encoder) {
>  		lpt_disable_pch_transcoder(dev_priv);
>  		intel_ddi_fdi_disable(crtc);
>  	}
> @@ -5635,6 +5627,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> +	if (I915_READ(TRANSCONF(crtc->pipe)) & TRANS_ENABLE)
> +		pipe_config->has_pch_encoder = true;
> +
>  	return true;
>  }
>  
> @@ -5763,6 +5758,17 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> +	/*
> +	 * aswell has only FDI/PCH transcoder A. It is which is connected to
> +	 * DDI E. So just check whether this pipe is wired to DDI E and whether
> +	 * the PCH transcoder is on.
> +	 */
> +	tmp = I915_READ(TRANS_DDI_FUNC_CTL(crtc->pipe));
> +	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
> +	    I915_READ(TRANSCONF(PIPE_A)) & TRANS_ENABLE)
> +		pipe_config->has_pch_encoder = true;
> +
> +
>  	return true;
>  }
>  
> @@ -7692,6 +7698,14 @@ static bool
>  intel_pipe_config_compare(struct intel_crtc_config *current_config,
>  			  struct intel_crtc_config *pipe_config)
>  {
> +	if (current_config->has_pch_encoder != pipe_config->has_pch_encoder) {
> +		DRM_ERROR("mismatch in has_pch_encoder "
> +			  "(expected %i, found %i)\n",
> +			  current_config->has_pch_encoder,
> +			  pipe_config->has_pch_encoder);
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> @@ -7791,6 +7805,7 @@ intel_modeset_check_state(struct drm_device *dev)
>  		     "crtc's computed enabled state doesn't match tracked enabled state "
>  		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
>  
> +		memset(&pipe_config, 0, sizeof(pipe_config));
>  		active = dev_priv->display.get_pipe_config(crtc,
>  							   &pipe_config);
>  		WARN(crtc->active != active,
> @@ -9155,6 +9170,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  setup_pipes:
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
>  			    base.head) {
> +		memset(&crtc->config, 0, sizeof(crtc->config));
>  		crtc->active = dev_priv->display.get_pipe_config(crtc,
>  								 &crtc->config);
>  

I think this is correct; I was worried about mode_set, crtc_disable,
crtc_enable.  But in that case, the pipe_config shouldn't be overridden
(that won't happen until the next mode_set).  So I think this is safe.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 7/8] drm/i915: create pipe_config->dpll for clock state

[Daniel Vetter]

Clock computations and handling are highly encoder specific, both in
the optimal clock selection and also in which clocks to use and when
sharing of clocks is possible.

So the best place to do this is somewhere in the encoders, with a
generic fallback for those encoders without special needs. To facility
this, add a pipe_config->clocks_set boolean.

This patch here is only prep work, it simply sets the computed clock
values in pipe_config->dpll, and uses that data in the hw clock
setting functions.

Haswell code isn't touched, simply because Haswell clocks work much
different and need their own infrastructure (with probably a
Haswell-specific config->ddi_clock substruct).

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c9e873e..5319133 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4118,37 +4118,38 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
 	return refclk;
 }
 
-static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
-				      intel_clock_t *clock)
+static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
 {
+	unsigned dotclock = crtc->config.adjusted_mode.clock;
+	struct dpll *clock = &crtc->config.dpll;
+
 	/* SDVO TV has fixed PLL values depend on its clock range,
 	   this mirrors vbios setting. */
-	if (adjusted_mode->clock >= 100000
-	    && adjusted_mode->clock < 140500) {
+	if (dotclock >= 100000 && dotclock < 140500) {
 		clock->p1 = 2;
 		clock->p2 = 10;
 		clock->n = 3;
 		clock->m1 = 16;
 		clock->m2 = 8;
-	} else if (adjusted_mode->clock >= 140500
-		   && adjusted_mode->clock <= 200000) {
+	} else if (dotclock >= 140500 && dotclock <= 200000) {
 		clock->p1 = 1;
 		clock->p2 = 10;
 		clock->n = 6;
 		clock->m1 = 12;
 		clock->m2 = 8;
 	}
+
+	crtc->config.clock_set = true;
 }
 
-static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
-				     intel_clock_t *clock,
+static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
 				     intel_clock_t *reduced_clock)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	int pipe = crtc->pipe;
 	u32 fp, fp2 = 0;
+	struct dpll *clock = &crtc->config.dpll;
 
 	if (IS_PINEVIEW(dev)) {
 		fp = (1 << clock->n) << 16 | clock->m1 << 8 | clock->m2;
@@ -4164,11 +4165,11 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
 
 	I915_WRITE(FP0(pipe), fp);
 
-	intel_crtc->lowfreq_avail = false;
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
+	crtc->lowfreq_avail = false;
+	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 	    reduced_clock && i915_powersave) {
 		I915_WRITE(FP1(pipe), fp2);
-		intel_crtc->lowfreq_avail = true;
+		crtc->lowfreq_avail = true;
 	} else {
 		I915_WRITE(FP1(pipe), fp);
 	}
@@ -4182,14 +4183,11 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
 		intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
 }
 
-static void vlv_update_pll(struct drm_crtc *crtc,
-			   intel_clock_t *clock, intel_clock_t *reduced_clock,
-			   int num_connectors)
+static void vlv_update_pll(struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	int pipe = crtc->pipe;
 	u32 dpll, mdiv, pdiv;
 	u32 bestn, bestm1, bestm2, bestp1, bestp2;
 	bool is_sdvo;
@@ -4197,8 +4195,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 
 	mutex_lock(&dev_priv->dpio_lock);
 
-	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
-		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
+	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
+		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
 
 	dpll = DPLL_VGA_MODE_DIS;
 	dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
@@ -4208,11 +4206,11 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 	I915_WRITE(DPLL(pipe), dpll);
 	POSTING_READ(DPLL(pipe));
 
-	bestn = clock->n;
-	bestm1 = clock->m1;
-	bestm2 = clock->m2;
-	bestp1 = clock->p1;
-	bestp2 = clock->p2;
+	bestn = crtc->config.dpll.n;
+	bestm1 = crtc->config.dpll.m1;
+	bestm2 = crtc->config.dpll.m2;
+	bestp1 = crtc->config.dpll.p1;
+	bestp2 = crtc->config.dpll.p2;
 
 	/*
 	 * In Valleyview PLL and program lane counter registers are exposed
@@ -4244,8 +4242,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 
 	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
 
-	if (intel_crtc->config.has_dp_encoder)
-		intel_dp_set_m_n(intel_crtc);
+	if (crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(crtc);
 
 	I915_WRITE(DPLL(pipe), dpll);
 
@@ -4256,8 +4254,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 	temp = 0;
 	if (is_sdvo) {
 		temp = 0;
-		if (intel_crtc->config.pixel_multiplier > 1) {
-			temp = (intel_crtc->config.pixel_multiplier - 1)
+		if (crtc->config.pixel_multiplier > 1) {
+			temp = (crtc->config.pixel_multiplier - 1)
 				<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
 		}
 	}
@@ -4265,16 +4263,15 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 	POSTING_READ(DPLL_MD(pipe));
 
 	/* Now program lane control registers */
-	if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
-			|| intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
-	{
+	if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)
+	   || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
 		temp = 0x1000C4;
 		if(pipe == 1)
 			temp |= (1 << 21);
 		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
 	}
-	if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
-	{
+
+	if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
 		temp = 0x1000C4;
 		if(pipe == 1)
 			temp |= (1 << 21);
@@ -4284,39 +4281,39 @@ static void vlv_update_pll(struct drm_crtc *crtc,
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
-static void i9xx_update_pll(struct drm_crtc *crtc,
-			    intel_clock_t *clock, intel_clock_t *reduced_clock,
+static void i9xx_update_pll(struct intel_crtc *crtc,
+			    intel_clock_t *reduced_clock,
 			    int num_connectors)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
-	int pipe = intel_crtc->pipe;
+	int pipe = crtc->pipe;
 	u32 dpll;
 	bool is_sdvo;
+	struct dpll *clock = &crtc->config.dpll;
 
-	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
+	i9xx_update_pll_dividers(crtc, reduced_clock);
 
-	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
-		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
+	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
+		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
+	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
 
 	if (is_sdvo) {
-		if ((intel_crtc->config.pixel_multiplier > 1) &&
+		if ((crtc->config.pixel_multiplier > 1) &&
 		    (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
-			dpll |= (intel_crtc->config.pixel_multiplier - 1)
+			dpll |= (crtc->config.pixel_multiplier - 1)
 				<< SDVO_MULTIPLIER_SHIFT_HIRES;
 		}
 		dpll |= DPLL_DVO_HIGH_SPEED;
 	}
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
 		dpll |= DPLL_DVO_HIGH_SPEED;
 
 	/* compute bitmask from p1 value */
@@ -4344,13 +4341,13 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 	if (INTEL_INFO(dev)->gen >= 4)
 		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
 
-	if (is_sdvo && intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
+	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
+	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
 		/* XXX: just matching BIOS for now */
 		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
 		dpll |= 3;
-	else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
+	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -4361,12 +4358,12 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 	POSTING_READ(DPLL(pipe));
 	udelay(150);
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	if (intel_crtc->config.has_dp_encoder)
-		intel_dp_set_m_n(intel_crtc);
+	if (crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(crtc);
 
 	I915_WRITE(DPLL(pipe), dpll);
 
@@ -4378,8 +4375,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 		u32 temp = 0;
 		if (is_sdvo) {
 			temp = 0;
-			if (intel_crtc->config.pixel_multiplier > 1) {
-				temp = (intel_crtc->config.pixel_multiplier - 1)
+			if (crtc->config.pixel_multiplier > 1) {
+				temp = (crtc->config.pixel_multiplier - 1)
 					<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
 			}
 		}
@@ -4394,23 +4391,23 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
 	}
 }
 
-static void i8xx_update_pll(struct drm_crtc *crtc,
+static void i8xx_update_pll(struct intel_crtc *crtc,
 			    struct drm_display_mode *adjusted_mode,
-			    intel_clock_t *clock, intel_clock_t *reduced_clock,
+			    intel_clock_t *reduced_clock,
 			    int num_connectors)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
-	int pipe = intel_crtc->pipe;
+	int pipe = crtc->pipe;
 	u32 dpll;
+	struct dpll *clock = &crtc->config.dpll;
 
-	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
+	i9xx_update_pll_dividers(crtc, reduced_clock);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
 		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
 	} else {
 		if (clock->p1 == 2)
@@ -4421,7 +4418,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -4432,7 +4429,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
 	POSTING_READ(DPLL(pipe));
 	udelay(150);
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
@@ -4579,20 +4576,26 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 						    &clock,
 						    &reduced_clock);
 	}
+	/* Compat-code for transition, will disappear. */
+	if (!intel_crtc->config.clock_set) {
+		intel_crtc->config.dpll.n = clock.n;
+		intel_crtc->config.dpll.m1 = clock.m1;
+		intel_crtc->config.dpll.m2 = clock.m2;
+		intel_crtc->config.dpll.p1 = clock.p1;
+		intel_crtc->config.dpll.p2 = clock.p2;
+	}
 
 	if (is_sdvo && is_tv)
-		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
+		i9xx_adjust_sdvo_tv_clock(intel_crtc);
 
 	if (IS_GEN2(dev))
-		i8xx_update_pll(crtc, adjusted_mode, &clock,
+		i8xx_update_pll(intel_crtc, adjusted_mode,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 	else if (IS_VALLEYVIEW(dev))
-		vlv_update_pll(crtc, &clock,
-				has_reduced_clock ? &reduced_clock : NULL,
-				num_connectors);
+		vlv_update_pll(intel_crtc);
 	else
-		i9xx_update_pll(crtc, &clock,
+		i9xx_update_pll(intel_crtc,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 
@@ -5221,7 +5224,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	}
 
 	if (is_sdvo && is_tv)
-		i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
+		i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
 
 	return true;
 }
@@ -5525,6 +5528,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
 		return -EINVAL;
 	}
+	/* Compat-code for transition, will disappear. */
+	if (!intel_crtc->config.clock_set) {
+		intel_crtc->config.dpll.n = clock.n;
+		intel_crtc->config.dpll.m1 = clock.m1;
+		intel_crtc->config.dpll.m2 = clock.m2;
+		intel_crtc->config.dpll.p1 = clock.p1;
+		intel_crtc->config.dpll.p2 = clock.p2;
+	}
 
 	/* Ensure that the cursor is valid for the new mode before changing... */
 	intel_crtc_update_cursor(crtc, true);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2a86a12..d27885a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -196,6 +196,18 @@ struct intel_crtc_config {
 	 * accordingly. */
 	bool has_dp_encoder;
 	bool dither;
+
+	/* Controls for the clock computation, to override various stages. */
+	bool clock_set;
+
+	/* Settings for the intel dpll used on pretty much everything but
+	 * haswell. */
+	struct dpll {
+		unsigned n;
+		unsigned m1, m2;
+		unsigned p1, p2;
+	} dpll;
+
 	int pipe_bpp;
 	struct intel_link_m_n dp_m_n;
 	/**
-- 
1.7.11.7

Re: [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state

[Jesse Barnes]

On Thu, 28 Mar 2013 10:42:02 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Clock computations and handling are highly encoder specific, both in
> the optimal clock selection and also in which clocks to use and when
> sharing of clocks is possible.
> 
> So the best place to do this is somewhere in the encoders, with a
> generic fallback for those encoders without special needs. To facility
> this, add a pipe_config->clocks_set boolean.
> 
> This patch here is only prep work, it simply sets the computed clock
> values in pipe_config->dpll, and uses that data in the hw clock
> setting functions.
> 
> Haswell code isn't touched, simply because Haswell clocks work much
> different and need their own infrastructure (with probably a
> Haswell-specific config->ddi_clock substruct).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 155 +++++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  12 +++
>  2 files changed, 95 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c9e873e..5319133 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4118,37 +4118,38 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
>  	return refclk;
>  }
>  
> -static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
> -				      intel_clock_t *clock)
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
>  {
> +	unsigned dotclock = crtc->config.adjusted_mode.clock;
> +	struct dpll *clock = &crtc->config.dpll;
> +
>  	/* SDVO TV has fixed PLL values depend on its clock range,
>  	   this mirrors vbios setting. */
> -	if (adjusted_mode->clock >= 100000
> -	    && adjusted_mode->clock < 140500) {
> +	if (dotclock >= 100000 && dotclock < 140500) {
>  		clock->p1 = 2;
>  		clock->p2 = 10;
>  		clock->n = 3;
>  		clock->m1 = 16;
>  		clock->m2 = 8;
> -	} else if (adjusted_mode->clock >= 140500
> -		   && adjusted_mode->clock <= 200000) {
> +	} else if (dotclock >= 140500 && dotclock <= 200000) {
>  		clock->p1 = 1;
>  		clock->p2 = 10;
>  		clock->n = 6;
>  		clock->m1 = 12;
>  		clock->m2 = 8;
>  	}
> +
> +	crtc->config.clock_set = true;
>  }
>  
> -static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
> -				     intel_clock_t *clock,
> +static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>  				     intel_clock_t *reduced_clock)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 fp, fp2 = 0;
> +	struct dpll *clock = &crtc->config.dpll;
>  
>  	if (IS_PINEVIEW(dev)) {
>  		fp = (1 << clock->n) << 16 | clock->m1 << 8 | clock->m2;
> @@ -4164,11 +4165,11 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>  
>  	I915_WRITE(FP0(pipe), fp);
>  
> -	intel_crtc->lowfreq_avail = false;
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	crtc->lowfreq_avail = false;
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  	    reduced_clock && i915_powersave) {
>  		I915_WRITE(FP1(pipe), fp2);
> -		intel_crtc->lowfreq_avail = true;
> +		crtc->lowfreq_avail = true;
>  	} else {
>  		I915_WRITE(FP1(pipe), fp);
>  	}
> @@ -4182,14 +4183,11 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
>  		intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
>  }
>  
> -static void vlv_update_pll(struct drm_crtc *crtc,
> -			   intel_clock_t *clock, intel_clock_t *reduced_clock,
> -			   int num_connectors)
> +static void vlv_update_pll(struct intel_crtc *crtc)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 dpll, mdiv, pdiv;
>  	u32 bestn, bestm1, bestm2, bestp1, bestp2;
>  	bool is_sdvo;
> @@ -4197,8 +4195,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>  	mutex_lock(&dev_priv->dpio_lock);
>  
> -	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> -		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> +	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> +		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  	dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> @@ -4208,11 +4206,11 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	I915_WRITE(DPLL(pipe), dpll);
>  	POSTING_READ(DPLL(pipe));
>  
> -	bestn = clock->n;
> -	bestm1 = clock->m1;
> -	bestm2 = clock->m2;
> -	bestp1 = clock->p1;
> -	bestp2 = clock->p2;
> +	bestn = crtc->config.dpll.n;
> +	bestm1 = crtc->config.dpll.m1;
> +	bestm2 = crtc->config.dpll.m2;
> +	bestp1 = crtc->config.dpll.p1;
> +	bestp2 = crtc->config.dpll.p2;
>  
>  	/*
>  	 * In Valleyview PLL and program lane counter registers are exposed
> @@ -4244,8 +4242,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>  	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
>  
> -	if (intel_crtc->config.has_dp_encoder)
> -		intel_dp_set_m_n(intel_crtc);
> +	if (crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(crtc);
>  
>  	I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4256,8 +4254,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	temp = 0;
>  	if (is_sdvo) {
>  		temp = 0;
> -		if (intel_crtc->config.pixel_multiplier > 1) {
> -			temp = (intel_crtc->config.pixel_multiplier - 1)
> +		if (crtc->config.pixel_multiplier > 1) {
> +			temp = (crtc->config.pixel_multiplier - 1)
>  				<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  		}
>  	}
> @@ -4265,16 +4263,15 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	POSTING_READ(DPLL_MD(pipe));
>  
>  	/* Now program lane control registers */
> -	if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
> -			|| intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> -	{
> +	if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)
> +	   || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
>  		temp = 0x1000C4;
>  		if(pipe == 1)
>  			temp |= (1 << 21);
>  		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
>  	}
> -	if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
> -	{
> +
> +	if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
>  		temp = 0x1000C4;
>  		if(pipe == 1)
>  			temp |= (1 << 21);
> @@ -4284,39 +4281,39 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> -static void i9xx_update_pll(struct drm_crtc *crtc,
> -			    intel_clock_t *clock, intel_clock_t *reduced_clock,
> +static void i9xx_update_pll(struct intel_crtc *crtc,
> +			    intel_clock_t *reduced_clock,
>  			    int num_connectors)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 dpll;
>  	bool is_sdvo;
> +	struct dpll *clock = &crtc->config.dpll;
>  
> -	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> +	i9xx_update_pll_dividers(crtc, reduced_clock);
>  
> -	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> -		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> +	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> +		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
>  		dpll |= DPLLB_MODE_LVDS;
>  	else
>  		dpll |= DPLLB_MODE_DAC_SERIAL;
>  
>  	if (is_sdvo) {
> -		if ((intel_crtc->config.pixel_multiplier > 1) &&
> +		if ((crtc->config.pixel_multiplier > 1) &&
>  		    (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
> -			dpll |= (intel_crtc->config.pixel_multiplier - 1)
> +			dpll |= (crtc->config.pixel_multiplier - 1)
>  				<< SDVO_MULTIPLIER_SHIFT_HIRES;
>  		}
>  		dpll |= DPLL_DVO_HIGH_SPEED;
>  	}
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
>  		dpll |= DPLL_DVO_HIGH_SPEED;
>  
>  	/* compute bitmask from p1 value */
> @@ -4344,13 +4341,13 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	if (INTEL_INFO(dev)->gen >= 4)
>  		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>  
> -	if (is_sdvo && intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> +	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
> -	else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> +	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>  		/* XXX: just matching BIOS for now */
>  		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
>  		dpll |= 3;
> -	else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -4361,12 +4358,12 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	POSTING_READ(DPLL(pipe));
>  	udelay(150);
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>  		if (encoder->pre_pll_enable)
>  			encoder->pre_pll_enable(encoder);
>  
> -	if (intel_crtc->config.has_dp_encoder)
> -		intel_dp_set_m_n(intel_crtc);
> +	if (crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(crtc);
>  
>  	I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4378,8 +4375,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  		u32 temp = 0;
>  		if (is_sdvo) {
>  			temp = 0;
> -			if (intel_crtc->config.pixel_multiplier > 1) {
> -				temp = (intel_crtc->config.pixel_multiplier - 1)
> +			if (crtc->config.pixel_multiplier > 1) {
> +				temp = (crtc->config.pixel_multiplier - 1)
>  					<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  			}
>  		}
> @@ -4394,23 +4391,23 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	}
>  }
>  
> -static void i8xx_update_pll(struct drm_crtc *crtc,
> +static void i8xx_update_pll(struct intel_crtc *crtc,
>  			    struct drm_display_mode *adjusted_mode,
> -			    intel_clock_t *clock, intel_clock_t *reduced_clock,
> +			    intel_clock_t *reduced_clock,
>  			    int num_connectors)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 dpll;
> +	struct dpll *clock = &crtc->config.dpll;
>  
> -	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> +	i9xx_update_pll_dividers(crtc, reduced_clock);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
>  		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
>  	} else {
>  		if (clock->p1 == 2)
> @@ -4421,7 +4418,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>  			dpll |= PLL_P2_DIVIDE_BY_4;
>  	}
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -4432,7 +4429,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>  	POSTING_READ(DPLL(pipe));
>  	udelay(150);
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>  		if (encoder->pre_pll_enable)
>  			encoder->pre_pll_enable(encoder);
>  
> @@ -4579,20 +4576,26 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  						    &clock,
>  						    &reduced_clock);
>  	}
> +	/* Compat-code for transition, will disappear. */
> +	if (!intel_crtc->config.clock_set) {
> +		intel_crtc->config.dpll.n = clock.n;
> +		intel_crtc->config.dpll.m1 = clock.m1;
> +		intel_crtc->config.dpll.m2 = clock.m2;
> +		intel_crtc->config.dpll.p1 = clock.p1;
> +		intel_crtc->config.dpll.p2 = clock.p2;
> +	}
>  
>  	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> +		i9xx_adjust_sdvo_tv_clock(intel_crtc);
>  
>  	if (IS_GEN2(dev))
> -		i8xx_update_pll(crtc, adjusted_mode, &clock,
> +		i8xx_update_pll(intel_crtc, adjusted_mode,
>  				has_reduced_clock ? &reduced_clock : NULL,
>  				num_connectors);
>  	else if (IS_VALLEYVIEW(dev))
> -		vlv_update_pll(crtc, &clock,
> -				has_reduced_clock ? &reduced_clock : NULL,
> -				num_connectors);
> +		vlv_update_pll(intel_crtc);
>  	else
> -		i9xx_update_pll(crtc, &clock,
> +		i9xx_update_pll(intel_crtc,
>  				has_reduced_clock ? &reduced_clock : NULL,
>  				num_connectors);
>  
> @@ -5221,7 +5224,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  	}
>  
>  	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> +		i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
>  
>  	return true;
>  }
> @@ -5525,6 +5528,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  		DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  		return -EINVAL;
>  	}
> +	/* Compat-code for transition, will disappear. */
> +	if (!intel_crtc->config.clock_set) {
> +		intel_crtc->config.dpll.n = clock.n;
> +		intel_crtc->config.dpll.m1 = clock.m1;
> +		intel_crtc->config.dpll.m2 = clock.m2;
> +		intel_crtc->config.dpll.p1 = clock.p1;
> +		intel_crtc->config.dpll.p2 = clock.p2;
> +	}
>  
>  	/* Ensure that the cursor is valid for the new mode before changing... */
>  	intel_crtc_update_cursor(crtc, true);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2a86a12..d27885a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -196,6 +196,18 @@ struct intel_crtc_config {
>  	 * accordingly. */
>  	bool has_dp_encoder;
>  	bool dither;
> +
> +	/* Controls for the clock computation, to override various stages. */
> +	bool clock_set;
> +
> +	/* Settings for the intel dpll used on pretty much everything but
> +	 * haswell. */
> +	struct dpll {
> +		unsigned n;
> +		unsigned m1, m2;
> +		unsigned p1, p2;
> +	} dpll;
> +
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;
>  	/**

This one's hard to review since you mixed in a drm_crtc->intel_crtc
function arg change.

I'd rather have that split out, but it looks ok.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state

[Daniel Vetter]

On Tue, Apr 02, 2013 at 02:14:23PM -0700, Jesse Barnes wrote:
> This one's hard to review since you mixed in a drm_crtc->intel_crtc
> function arg change.
> 
> I'd rather have that split out, but it looks ok.

Yeah, I've fumbled this one a bit, but decided to punt on the split-up.

Generally I'm always a bit unsure when exactly we should do rote
refactoring like this: We have a similar conversion going on from
drm_encoder->intel_encoder, also with the switch away from the drm crtc
helper vtables to our own.
 
Usually I don't switch code I don't yet touch (Paulo complained about
that, too) since such massive sed jobs simply make patch rebasing complete
hell. Both for me, but also for anyone else with an in-flight patch series
touching the same area. But once in a while I get fed up and convert a few
more things while touching them, leading the slightly ugly patches ...

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

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

[PATCH 8/8] drm/i915: move dp clock computations to encoder->compute_config

[Daniel Vetter]

With the exception of hsw, which has dedicated DP clocks which run at
the fixed frequency already, and vlv, which doesn't have optmized
pre-defined dp clock parameters (yet).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 97 +-----------------------------------
 drivers/gpu/drm/i915/intel_dp.c      | 45 +++++++++++++++++
 2 files changed, 46 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5319133..f1cabf3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -114,15 +114,6 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 			intel_clock_t *best_clock);
 
 static bool
-intel_find_pll_g4x_dp(const intel_limit_t *, struct drm_crtc *crtc,
-		      int target, int refclk, intel_clock_t *match_clock,
-		      intel_clock_t *best_clock);
-static bool
-intel_find_pll_ironlake_dp(const intel_limit_t *, struct drm_crtc *crtc,
-			   int target, int refclk, intel_clock_t *match_clock,
-			   intel_clock_t *best_clock);
-
-static bool
 intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
 			int target, int refclk, intel_clock_t *match_clock,
 			intel_clock_t *best_clock);
@@ -254,20 +245,6 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
 	.find_pll = intel_g4x_find_best_PLL,
 };
 
-static const intel_limit_t intel_limits_g4x_display_port = {
-	.dot = { .min = 161670, .max = 227000 },
-	.vco = { .min = 1750000, .max = 3500000},
-	.n = { .min = 1, .max = 2 },
-	.m = { .min = 97, .max = 108 },
-	.m1 = { .min = 0x10, .max = 0x12 },
-	.m2 = { .min = 0x05, .max = 0x06 },
-	.p = { .min = 10, .max = 20 },
-	.p1 = { .min = 1, .max = 2},
-	.p2 = { .dot_limit = 0,
-		.p2_slow = 10, .p2_fast = 10 },
-	.find_pll = intel_find_pll_g4x_dp,
-};
-
 static const intel_limit_t intel_limits_pineview_sdvo = {
 	.dot = { .min = 20000, .max = 400000},
 	.vco = { .min = 1700000, .max = 3500000 },
@@ -374,20 +351,6 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
 	.find_pll = intel_g4x_find_best_PLL,
 };
 
-static const intel_limit_t intel_limits_ironlake_display_port = {
-	.dot = { .min = 25000, .max = 350000 },
-	.vco = { .min = 1760000, .max = 3510000},
-	.n = { .min = 1, .max = 2 },
-	.m = { .min = 81, .max = 90 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
-	.p = { .min = 10, .max = 20 },
-	.p1 = { .min = 1, .max = 2},
-	.p2 = { .dot_limit = 0,
-		.p2_slow = 10, .p2_fast = 10 },
-	.find_pll = intel_find_pll_ironlake_dp,
-};
-
 static const intel_limit_t intel_limits_vlv_dac = {
 	.dot = { .min = 25000, .max = 270000 },
 	.vco = { .min = 4000000, .max = 6000000 },
@@ -497,10 +460,7 @@ static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
 			else
 				limit = &intel_limits_ironlake_single_lvds;
 		}
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
-		   intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
-		limit = &intel_limits_ironlake_display_port;
-	else
+	} else
 		limit = &intel_limits_ironlake_dac;
 
 	return limit;
@@ -521,8 +481,6 @@ static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
 		limit = &intel_limits_g4x_hdmi;
 	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO)) {
 		limit = &intel_limits_g4x_sdvo;
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
-		limit = &intel_limits_g4x_display_port;
 	} else /* The option is for other outputs */
 		limit = &intel_limits_i9xx_sdvo;
 
@@ -765,59 +723,6 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 }
 
 static bool
-intel_find_pll_ironlake_dp(const intel_limit_t *limit, struct drm_crtc *crtc,
-			   int target, int refclk, intel_clock_t *match_clock,
-			   intel_clock_t *best_clock)
-{
-	struct drm_device *dev = crtc->dev;
-	intel_clock_t clock;
-
-	if (target < 200000) {
-		clock.n = 1;
-		clock.p1 = 2;
-		clock.p2 = 10;
-		clock.m1 = 12;
-		clock.m2 = 9;
-	} else {
-		clock.n = 2;
-		clock.p1 = 1;
-		clock.p2 = 10;
-		clock.m1 = 14;
-		clock.m2 = 8;
-	}
-	intel_clock(dev, refclk, &clock);
-	memcpy(best_clock, &clock, sizeof(intel_clock_t));
-	return true;
-}
-
-/* DisplayPort has only two frequencies, 162MHz and 270MHz */
-static bool
-intel_find_pll_g4x_dp(const intel_limit_t *limit, struct drm_crtc *crtc,
-		      int target, int refclk, intel_clock_t *match_clock,
-		      intel_clock_t *best_clock)
-{
-	intel_clock_t clock;
-	if (target < 200000) {
-		clock.p1 = 2;
-		clock.p2 = 10;
-		clock.n = 2;
-		clock.m1 = 23;
-		clock.m2 = 8;
-	} else {
-		clock.p1 = 1;
-		clock.p2 = 10;
-		clock.n = 1;
-		clock.m1 = 14;
-		clock.m2 = 2;
-	}
-	clock.m = 5 * (clock.m1 + 2) + (clock.m2 + 2);
-	clock.p = (clock.p1 * clock.p2);
-	clock.dot = 96000 * clock.m / (clock.n + 2) / clock.p;
-	clock.vco = 0;
-	memcpy(best_clock, &clock, sizeof(intel_clock_t));
-	return true;
-}
-static bool
 intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
 			int target, int refclk, intel_clock_t *match_clock,
 			intel_clock_t *best_clock)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b1bf00b..78006a7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -670,6 +670,49 @@ intel_dp_i2c_init(struct intel_dp *intel_dp,
 	return ret;
 }
 
+static void
+intel_dp_set_clock(struct intel_encoder *encoder,
+		   struct intel_crtc_config *pipe_config, int link_bw)
+{
+	struct drm_device *dev = encoder->base.dev;
+
+	if (IS_G4X(dev)) {
+		if (link_bw == DP_LINK_BW_1_62) {
+			pipe_config->dpll.p1 = 2;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.n = 2;
+			pipe_config->dpll.m1 = 23;
+			pipe_config->dpll.m2 = 8;
+		} else {
+			pipe_config->dpll.p1 = 1;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.n = 1;
+			pipe_config->dpll.m1 = 14;
+			pipe_config->dpll.m2 = 2;
+		}
+		pipe_config->clock_set = true;
+	} else if (IS_HASWELL(dev)) {
+		/* Haswell has special-purpose DP DDI clocks. */
+	} else if (HAS_PCH_SPLIT(dev)) {
+		if (link_bw == DP_LINK_BW_1_62) {
+			pipe_config->dpll.n = 1;
+			pipe_config->dpll.p1 = 2;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.m1 = 12;
+			pipe_config->dpll.m2 = 9;
+		} else {
+			pipe_config->dpll.n = 2;
+			pipe_config->dpll.p1 = 1;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.m1 = 14;
+			pipe_config->dpll.m2 = 8;
+		}
+		pipe_config->clock_set = true;
+	} else if (IS_VALLEYVIEW(dev)) {
+		/* FIXME: Need to figure out optimized DP clocks for vlv. */
+	}
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_config *pipe_config)
@@ -765,6 +808,8 @@ found:
 			       target_clock, adjusted_mode->clock,
 			       &pipe_config->dp_m_n);
 
+	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
+
 	return true;
 }
 
-- 
1.7.11.7

Re: [PATCH 8/8] drm/i915: move dp clock computations to encoder->compute_config

[Jesse Barnes]

On Thu, 28 Mar 2013 10:42:03 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> With the exception of hsw, which has dedicated DP clocks which run at
> the fixed frequency already, and vlv, which doesn't have optmized
> pre-defined dp clock parameters (yet).
> 

Nice.  I think we should do this for common HDMI modes too.  We have
some extra clock manipulation regs we can use to tune things, so having
fixed dividers for 720p and 1080p along with the tuning params should
give us better behavior than what we have today.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 8/8] drm/i915: move dp clock computations to encoder->compute_config

[Daniel Vetter]

[-- Attachment #1.1: Type: text/plain, Size: 1082 bytes --]

On Tue, Apr 2, 2013 at 11:20 PM, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:

> On Thu, 28 Mar 2013 10:42:03 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > With the exception of hsw, which has dedicated DP clocks which run at
> > the fixed frequency already, and vlv, which doesn't have optmized
> > pre-defined dp clock parameters (yet).
> >
>
> Nice.  I think we should do this for common HDMI modes too.  We have
> some extra clock manipulation regs we can use to tune things, so having
> fixed dividers for 720p and 1080p along with the tuning params should
> give us better behavior than what we have today.


Imo we still have a few lower-hanging fruit before we need to start doing
clock fine-tuning in hdmi-land: Atm we don't really bother with supporting
the 1001/1000 modified clocks in the CEA spec at all ... Once we have the
support code for that, adding fine-tuned clocks for those modes starts to
make sense, so that we really hit them spot-on.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[-- Attachment #1.2: Type: text/html, Size: 1639 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 8/8] drm/i915: move dp clock computations to encoder->compute_config

[Daniel Vetter]

On Tue, Apr 02, 2013 at 11:26:06PM +0200, Daniel Vetter wrote:
> On Tue, Apr 2, 2013 at 11:20 PM, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:
> 
> > On Thu, 28 Mar 2013 10:42:03 +0100
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > > With the exception of hsw, which has dedicated DP clocks which run at
> > > the fixed frequency already, and vlv, which doesn't have optmized
> > > pre-defined dp clock parameters (yet).
> > >
> >
> > Nice.  I think we should do this for common HDMI modes too.  We have
> > some extra clock manipulation regs we can use to tune things, so having
> > fixed dividers for 720p and 1080p along with the tuning params should
> > give us better behavior than what we have today.
> 
> 
> Imo we still have a few lower-hanging fruit before we need to start doing
> clock fine-tuning in hdmi-land: Atm we don't really bother with supporting
> the 1001/1000 modified clocks in the CEA spec at all ... Once we have the
> support code for that, adding fine-tuned clocks for those modes starts to
> make sense, so that we really hit them spot-on.

I've dropped this patch here again for now, since the previous patch is
horribly broken: It completely missed to transform the ironlake dpll
functions over to use pipe_config->dpll instead of the intel_clock_t
computed. Thanks to Paulo for dissecting this embarassement.

/me hides in shame

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

[PATCH 00/10] bpc handling fixes

[Daniel Vetter]

Hi all,

To actually be able to implement fdi link bpc dithering we need to have working
plane/pipe bpp handling. The current code doesn't manage that - I've had way too
many black screens and other funny issues.

Note that I currently lack a high-color-depth screen, so I couldn't test
anything with a 10bpc dp screen or a 12bpc hdmi capable screen. Anyone who has
such a thing, testing highly welcome.

After these patches I can at least fire up X in any sane bitdepth and things
still work.

Cheers, Daniel

Daniel Vetter (10):
  drm/i915: use pipe_config for lvds dithering
  drm/i915: consolidate pch pll computations a bit
  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: extract i9xx_set_pipeconf
  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

 drivers/gpu/drm/i915/intel_crt.c     |   4 +
 drivers/gpu/drm/i915/intel_display.c | 179 ++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dp.c      |  13 ++-
 drivers/gpu/drm/i915/intel_drv.h     |   5 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  23 ++++-
 drivers/gpu/drm/i915/intel_lvds.c    |  14 +--
 6 files changed, 139 insertions(+), 99 deletions(-)

-- 
1.7.11.4

[PATCH 1/8] drm/i915: clear up the fdi/dp set_m_n confusion

[Daniel Vetter]

There's a rather decent confusion going on around transcoder m_n
values. So let's clarify:
- All dp encoders need this, either on the pch transcoder if it's a
  pch port, or on the cpu transcoder/pipe if it's a cpu port.
- fdi links need to have the right m_n values for the fdi link set in
  the cpu transcoder.

To handle the pch vs transcoder stuff a bit better, extract transcoder
set_m_n helpers. To make them simpler, set intel_crtc->cpu_transcoder
als in ironlake_crtc_mode_set, so that gen5+ (where the cpu m_n
registers are all at the same offset) can use it.

Haswell modeset is decently confused about dp vs. edp vs. fdi. dp vs.
edp works exactly the same as dp (since there's no pch dp any more),
so use that as a check. And only set up the fdi m_n values if we
really have a pch encoder present (which means we have a VGA encoder).

On ilk+ we've called ironlake_set_m_n both for cpu_edp and for pch
encoders. Now that dp_set_m_n handles all dp links (thanks to the
pch encoder check), we can ditch the cpu_edp stuff from the
fdi_set_m_n function.

Since the dp_m_n values are not readily available, we need to
carefully coax the edp values out of the encoder. Hence we can't (yet)
kill this superflous complexity.

v2: Rebase on top of the ivb fdi B/C check patch - we need to properly
clear intel_crtc->fdi_lane, otherwise those checks will misfire.

v3: Rebased on top of a s/IS_HASWELL/HAS_DDI/ patch from Paulo Zanoni.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 86 +++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      | 30 ++-----------
 drivers/gpu/drm/i915/intel_drv.h     |  8 ++++
 3 files changed, 67 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f1d47dd..8335fd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5255,15 +5255,47 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
 	return bps / (link_bw * 8) + 1;
 }
 
-static void ironlake_set_m_n(struct drm_crtc *crtc)
+void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+
+	I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+	I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
+	I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
+	I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
+}
+
+void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+	enum transcoder transcoder = crtc->cpu_transcoder;
+
+	if (INTEL_INFO(dev)->gen >= 5) {
+		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
+		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
+		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+	} else {
+		I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
+		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
+		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
+	}
+}
+
+static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
 	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
@@ -5283,22 +5315,14 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 		}
 	}
 
-	/* FDI link */
-	lane = 0;
-	/* CPU eDP doesn't require FDI link, so just set DP M/N
-	   according to current link config */
-	if (is_cpu_edp) {
-		intel_edp_link_config(edp_encoder, &lane, &link_bw);
-	} else {
-		/* 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;
-	}
+	/* 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;
 
 	/* [e]DP over FDI requires target mode clock instead of link clock. */
 	if (edp_encoder)
@@ -5308,9 +5332,8 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	else
 		target_clock = adjusted_mode->clock;
 
-	if (!lane)
-		lane = ironlake_get_lanes_required(target_clock, link_bw,
-						   intel_crtc->config.pipe_bpp);
+	lane = ironlake_get_lanes_required(target_clock, link_bw,
+					   intel_crtc->config.pipe_bpp);
 
 	intel_crtc->fdi_lanes = lane;
 
@@ -5319,10 +5342,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
 			       link_bw, &m_n);
 
-	I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
-	I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-	I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-	I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
+	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
@@ -5469,6 +5489,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
 	     "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
 
+	intel_crtc->cpu_transcoder = pipe;
+
 	ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
 				     &has_reduced_clock, &reduced_clock);
 	if (!ok) {
@@ -5507,7 +5529,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -5543,7 +5565,9 @@ 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. */
-	ironlake_set_m_n(crtc);
+	intel_crtc->fdi_lanes = 0;
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
@@ -5650,15 +5674,15 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	intel_crtc->lowfreq_avail = false;
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	if (!is_dp || is_cpu_edp)
-		ironlake_set_m_n(crtc);
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	haswell_set_pipeconf(crtc, adjusted_mode, dither);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a4c9858..a4caf1e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -766,12 +766,9 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *intel_encoder;
 	struct intel_dp *intel_dp;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int lane_count = 4;
 	struct intel_link_m_n m_n;
-	int pipe = intel_crtc->pipe;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 
 	/*
 	 * Find the lane count in the intel_encoder private
@@ -795,29 +792,10 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
 			       mode->clock, adjusted_mode->clock, &m_n);
 
-	if (HAS_DDI(dev)) {
-		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
-	} else if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(TRANSDATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(TRANSDPLINK_M1(pipe), m_n.link_m);
-		I915_WRITE(TRANSDPLINK_N1(pipe), m_n.link_n);
-	} else if (IS_VALLEYVIEW(dev)) {
-		I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
-	} else {
-		I915_WRITE(PIPE_GMCH_DATA_M(pipe),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n.link_m);
-		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n.link_n);
-	}
+	if (intel_crtc->config.has_pch_encoder)
+		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
+	else
+		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 void intel_dp_init_link_config(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e43155a..aa4dee4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -191,8 +191,12 @@ struct intel_crtc_config {
 	 */
 	bool limited_color_range;
 
+	/* DP has a bunch of special case unfortunately, so mark the pipe
+	 * accordingly. */
+	bool has_dp_encoder;
 	bool dither;
 	int pipe_bpp;
+	struct intel_link_m_n dp_m_n;
 
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
@@ -635,6 +639,10 @@ extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
 			    struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
+extern void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
+extern void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
1.7.11.4

[PATCH 0/8] fdi/dp m_n reorg and a few other clock changes

[Daniel Vetter]

Hi all,

This here is a bit a grab-bag of changes, all around the clock handling in our
code. Originally I've wanted to push this further so that we can compute the
desired dpll settings in the pipe_config computation stage. We need that to
properly figure out pll sharing for atomic modeset. But it's messy, so this only
moves us a bit further.

Also contained in here is the hw state readout support infrastructure for pipe
configurations. It helps in simplifying things a bit, but otherwise not used for
a lot of things yet.

Cheers, Daniel

Daniel Vetter (8):
  drm/i915: clear up the fdi/dp set_m_n confusion
  drm/i915: move dp_m_n computation to dp_encoder->compute_config
  drm/i915: track dp target_clock in pipe_config
  drm/i915: rip out superflous is_dp&is_cpu_edp tracking
  drm/i915: add hw state readout/checking for pipe_config
  drm/i915: hw readout support for ->has_pch_encoders
  drm/i915: create pipe_config->dpll for clock state
  drm/i915: move dp clock computations to encoder->compute_config

 drivers/gpu/drm/i915/i915_drv.h      |   5 +
 drivers/gpu/drm/i915/intel_display.c | 527 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c      | 115 ++++----
 drivers/gpu/drm/i915/intel_drv.h     |  30 +-
 4 files changed, 335 insertions(+), 342 deletions(-)

-- 
1.7.11.4

[PATCH 1/8] drm/i915: clear up the fdi/dp set_m_n confusion

[Daniel Vetter]

There's a rather decent confusion going on around transcoder m_n
values. So let's clarify:
- All dp encoders need this, either on the pch transcoder if it's a
  pch port, or on the cpu transcoder/pipe if it's a cpu port.
- fdi links need to have the right m_n values for the fdi link set in
  the cpu transcoder.

To handle the pch vs transcoder stuff a bit better, extract transcoder
set_m_n helpers. To make them simpler, set intel_crtc->cpu_transcoder
als in ironlake_crtc_mode_set, so that gen5+ (where the cpu m_n
registers are all at the same offset) can use it.

Haswell modeset is decently confused about dp vs. edp vs. fdi. dp vs.
edp works exactly the same as dp (since there's no pch dp any more),
so use that as a check. And only set up the fdi m_n values if we
really have a pch encoder present (which means we have a VGA encoder).

On ilk+ we've called ironlake_set_m_n both for cpu_edp and for pch
encoders. Now that dp_set_m_n handles all dp links (thanks to the
pch encoder check), we can ditch the cpu_edp stuff from the
fdi_set_m_n function.

Since the dp_m_n values are not readily available, we need to
carefully coax the edp values out of the encoder. Hence we can't (yet)
kill this superflous complexity.

v2: Rebase on top of the ivb fdi B/C check patch - we need to properly
clear intel_crtc->fdi_lane, otherwise those checks will misfire.

v3: Rebased on top of a s/IS_HASWELL/HAS_DDI/ patch from Paulo Zanoni.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 86 +++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      | 30 ++-----------
 drivers/gpu/drm/i915/intel_drv.h     |  8 ++++
 3 files changed, 67 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f1d47dd..8335fd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5255,15 +5255,47 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
 	return bps / (link_bw * 8) + 1;
 }
 
-static void ironlake_set_m_n(struct drm_crtc *crtc)
+void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+
+	I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+	I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
+	I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
+	I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
+}
+
+void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+				  struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = crtc->pipe;
+	enum transcoder transcoder = crtc->cpu_transcoder;
+
+	if (INTEL_INFO(dev)->gen >= 5) {
+		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
+		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
+		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+	} else {
+		I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
+		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
+		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
+	}
+}
+
+static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
 	struct intel_link_m_n m_n = {0};
 	int target_clock, lane, link_bw;
@@ -5283,22 +5315,14 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 		}
 	}
 
-	/* FDI link */
-	lane = 0;
-	/* CPU eDP doesn't require FDI link, so just set DP M/N
-	   according to current link config */
-	if (is_cpu_edp) {
-		intel_edp_link_config(edp_encoder, &lane, &link_bw);
-	} else {
-		/* 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;
-	}
+	/* 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;
 
 	/* [e]DP over FDI requires target mode clock instead of link clock. */
 	if (edp_encoder)
@@ -5308,9 +5332,8 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	else
 		target_clock = adjusted_mode->clock;
 
-	if (!lane)
-		lane = ironlake_get_lanes_required(target_clock, link_bw,
-						   intel_crtc->config.pipe_bpp);
+	lane = ironlake_get_lanes_required(target_clock, link_bw,
+					   intel_crtc->config.pipe_bpp);
 
 	intel_crtc->fdi_lanes = lane;
 
@@ -5319,10 +5342,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
 			       link_bw, &m_n);
 
-	I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
-	I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-	I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-	I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
+	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
@@ -5469,6 +5489,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
 	     "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
 
+	intel_crtc->cpu_transcoder = pipe;
+
 	ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
 				     &has_reduced_clock, &reduced_clock);
 	if (!ok) {
@@ -5507,7 +5529,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -5543,7 +5565,9 @@ 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. */
-	ironlake_set_m_n(crtc);
+	intel_crtc->fdi_lanes = 0;
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
@@ -5650,15 +5674,15 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp && !is_cpu_edp)
+	if (is_dp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 
 	intel_crtc->lowfreq_avail = false;
 
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
-	if (!is_dp || is_cpu_edp)
-		ironlake_set_m_n(crtc);
+	if (intel_crtc->config.has_pch_encoder)
+		ironlake_fdi_set_m_n(crtc);
 
 	haswell_set_pipeconf(crtc, adjusted_mode, dither);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a4c9858..a4caf1e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -766,12 +766,9 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *intel_encoder;
 	struct intel_dp *intel_dp;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int lane_count = 4;
 	struct intel_link_m_n m_n;
-	int pipe = intel_crtc->pipe;
-	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 
 	/*
 	 * Find the lane count in the intel_encoder private
@@ -795,29 +792,10 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
 			       mode->clock, adjusted_mode->clock, &m_n);
 
-	if (HAS_DDI(dev)) {
-		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
-	} else if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(TRANSDATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(TRANSDPLINK_M1(pipe), m_n.link_m);
-		I915_WRITE(TRANSDPLINK_N1(pipe), m_n.link_n);
-	} else if (IS_VALLEYVIEW(dev)) {
-		I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
-		I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
-	} else {
-		I915_WRITE(PIPE_GMCH_DATA_M(pipe),
-			   TU_SIZE(m_n.tu) | m_n.gmch_m);
-		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n.gmch_n);
-		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n.link_m);
-		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n.link_n);
-	}
+	if (intel_crtc->config.has_pch_encoder)
+		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
+	else
+		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
 }
 
 void intel_dp_init_link_config(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e43155a..aa4dee4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -191,8 +191,12 @@ struct intel_crtc_config {
 	 */
 	bool limited_color_range;
 
+	/* DP has a bunch of special case unfortunately, so mark the pipe
+	 * accordingly. */
+	bool has_dp_encoder;
 	bool dither;
 	int pipe_bpp;
+	struct intel_link_m_n dp_m_n;
 
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
@@ -635,6 +639,10 @@ extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
 			    struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
+extern void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
+extern void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n);
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
1.7.11.4