On Wed, Mar 27, 2013 at 12:45:02AM +0100, Daniel Vetter wrote:
> 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 at ffwll.ch>
Oops, that patch is already part of the next pile of pipe_config stuff.
Please disregard (for now ...).
-Daniel
> ---
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
