All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 13/13] drm/i915: clear up the fdi/dp set_m_n confusion
Date: Wed, 27 Mar 2013 00:45:02 +0100	[thread overview]
Message-ID: <1364341502-1184-14-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1364341502-1184-1-git-send-email-daniel.vetter@ffwll.ch>

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 8ab7520..b076665 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5297,15 +5297,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;
@@ -5325,22 +5357,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)
@@ -5350,9 +5374,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;
 
@@ -5361,10 +5384,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,
@@ -5511,6 +5531,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) {
@@ -5549,7 +5571,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)
@@ -5585,7 +5607,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);
 
@@ -5697,15 +5721,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

  parent reply	other threads:[~2013-03-26 23:45 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-26 23:44 [PATCH 00/13] pipe_config basic infrastructure Daniel Vetter
2013-03-26 23:44 ` [PATCH 01/13] drm/i915: introduce struct intel_crtc_config Daniel Vetter
2013-03-27 16:43   ` Jesse Barnes
2013-03-26 23:44 ` [PATCH 02/13] drm/i915: compute pipe_config earlier Daniel Vetter
2013-03-27 16:45   ` Jesse Barnes
2013-03-26 23:44 ` [PATCH 03/13] drm/i915: add pipe_config->timings_set Daniel Vetter
2013-03-27 16:49   ` Jesse Barnes
2013-03-27 16:59     ` Daniel Vetter
2013-03-27 17:00     ` Daniel Vetter
2013-03-27 16:59   ` Jesse Barnes
2013-03-27 17:06     ` Daniel Vetter
2013-03-27 17:15       ` Jesse Barnes
2013-03-26 23:44 ` [PATCH 04/13] drm/i915: add pipe_config->pixel_multiplier Daniel Vetter
2013-03-27 16:54   ` Jesse Barnes
2013-03-27 17:03     ` Daniel Vetter
2013-03-26 23:44 ` [PATCH 05/13] drm/i915: drop helper vtable for sdvo encoder Daniel Vetter
2013-03-27 16:55   ` Jesse Barnes
2013-03-26 23:44 ` [PATCH 06/13] drm/i915: add pipe_config->has_pch_encoder Daniel Vetter
2013-03-27 17:06   ` Jesse Barnes
2013-03-27 17:11     ` Daniel Vetter
2013-03-26 23:44 ` [PATCH 07/13] drm/i915: add pipe_config->limited_color_range Daniel Vetter
2013-03-27 17:09   ` Jesse Barnes
2013-03-26 23:44 ` [PATCH 08/13] drm/i915: introduce pipe_config->dither|pipe_bpp Daniel Vetter
2013-03-27 17:11   ` Jesse Barnes
2013-03-26 23:44 ` [PATCH 09/13] drm/i915: precompute pipe bpp before touching the hw Daniel Vetter
2013-03-27 17:24   ` Jesse Barnes
2013-03-27 18:58     ` Daniel Vetter
2013-03-26 23:44 ` [PATCH 10/13] drm/i915: convert DP autodither code to new infrastructure Daniel Vetter
2013-03-27 21:13   ` Jesse Barnes
2013-03-26 23:45 ` [PATCH 11/13] drm/i915: clean up plane bpp confusion Daniel Vetter
2013-03-27 21:15   ` Jesse Barnes
2013-03-28 11:26   ` Ville Syrjälä
2013-03-28 11:46     ` Daniel Vetter
2013-03-28 11:59       ` Ville Syrjälä
2013-03-28 12:49         ` [PATCH 1/2] drm/i915: check fb->pixel_format instead of bits_per_pixel Daniel Vetter
2013-03-28 12:49           ` [PATCH 2/2] drm/i915: fixup fb bpp computation in pipe_config_set_bpp Daniel Vetter
2013-03-28 15:13             ` Ville Syrjälä
2013-03-28 15:36               ` [PATCH] drm/i915: remove "inline" keyword from ironlake_disable_display_irq Daniel Vetter
2013-03-28 15:38               ` [PATCH] drm/i915: fixup fb bpp computation in pipe_config_set_bpp Daniel Vetter
2013-03-28 15:45                 ` Ville Syrjälä
2013-03-28 15:55                   ` Daniel Vetter
2013-03-28 14:42           ` [PATCH 1/2] drm/i915: check fb->pixel_format instead of bits_per_pixel Ville Syrjälä
2013-03-28 15:01             ` [PATCH] " Daniel Vetter
2013-03-28 15:16               ` Ville Syrjälä
2013-03-28 12:51         ` [PATCH 11/13] drm/i915: clean up plane bpp confusion Daniel Vetter
2013-03-26 23:45 ` [PATCH 12/13] drm/i915: clean up pipe " Daniel Vetter
2013-03-27 21:28   ` Jesse Barnes
2013-03-27 22:41     ` Daniel Vetter
2013-03-27 23:13       ` Jesse Barnes
2013-03-27 23:50         ` Daniel Vetter
2013-03-26 23:45 ` Daniel Vetter [this message]
2013-03-27  0:14   ` [PATCH 13/13] drm/i915: clear up the fdi/dp set_m_n confusion Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1364341502-1184-14-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.