diff mbox

[04/13] drm/i915: add pipe_config->pixel_multiplier

Message ID 1364341502-1184-5-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter March 26, 2013, 11:44 p.m. UTC 
Used by SDVO (and hopefully, eventually HDMI, if we ever get around
to fixing up the low dotclock CEA modes ...).

This required adding a new encoder->mode_set callback to be able to
pass around the intel_crtc_config.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 80 +++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h     | 19 ++-------
 drivers/gpu/drm/i915/intel_sdvo.c    | 39 +++++++++---------
 3 files changed, 66 insertions(+), 72 deletions(-)

Comments

Jesse Barnes March 27, 2013, 4:54 p.m. UTC | #1 
On Wed, 27 Mar 2013 00:44:53 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Used by SDVO (and hopefully, eventually HDMI, if we ever get around
> to fixing up the low dotclock CEA modes ...).
> 
> This required adding a new encoder->mode_set callback to be able to
> pass around the intel_crtc_config.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 80 +++++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_drv.h     | 19 ++-------
>  drivers/gpu/drm/i915/intel_sdvo.c    | 39 +++++++++---------
>  3 files changed, 66 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3e22305..3672b8d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4320,14 +4320,15 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>  }
>  
>  static void vlv_update_pll(struct drm_crtc *crtc,
> -			   struct drm_display_mode *mode,
> -			   struct drm_display_mode *adjusted_mode,
>  			   intel_clock_t *clock, intel_clock_t *reduced_clock,
>  			   int num_connectors)
>  {
>  	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;

These arg compaction changes could probably be squashed into the
initial crtc_config patch to make this one smaller.

> @@ -5907,8 +5909,12 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
>  			encoder->base.base.id,
>  			drm_get_encoder_name(&encoder->base),
>  			mode->base.id, mode->name);
> -		encoder_funcs = encoder->base.helper_private;
> -		encoder_funcs->mode_set(&encoder->base, mode, adjusted_mode);
> +		if (encoder->mode_set) {
> +			encoder->mode_set(encoder);
> +		} else {
> +			encoder_funcs = encoder->base.helper_private;
> +			encoder_funcs->mode_set(&encoder->base, mode, adjusted_mode);
> +		}
>  	}

This made me do a double take; maybe it's time to
s/encoder/intel_encoder in this function...

Looks good otherwise.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Daniel Vetter March 27, 2013, 5:03 p.m. UTC | #2 
On Wed, Mar 27, 2013 at 5:54 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3e22305..3672b8d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4320,14 +4320,15 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>>  }
>>
>>  static void vlv_update_pll(struct drm_crtc *crtc,
>> -                        struct drm_display_mode *mode,
>> -                        struct drm_display_mode *adjusted_mode,
>>                          intel_clock_t *clock, intel_clock_t *reduced_clock,
>>                          int num_connectors)
>>  {
>>       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;
>
> These arg compaction changes could probably be squashed into the
> initial crtc_config patch to make this one smaller.

See my little commit message update, but I've done things in such an
incremental way to avoid rebase hell and allow me to move things
around easier. What you see here is by far not my very first approach
;-)

To make everyone's OCD happy we can do a cleanup pass at the end, but
atm I'd like to avoid massive sed patches - too high churn.

>> @@ -5907,8 +5909,12 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
>>                       encoder->base.base.id,
>>                       drm_get_encoder_name(&encoder->base),
>>                       mode->base.id, mode->name);
>> -             encoder_funcs = encoder->base.helper_private;
>> -             encoder_funcs->mode_set(&encoder->base, mode, adjusted_mode);
>> +             if (encoder->mode_set) {
>> +                     encoder->mode_set(encoder);
>> +             } else {
>> +                     encoder_funcs = encoder->base.helper_private;
>> +                     encoder_funcs->mode_set(&encoder->base, mode, adjusted_mode);
>> +             }
>>       }
>
> This made me do a double take; maybe it's time to
> s/encoder/intel_encoder in this function...

Yeah, we're halfway between mostly using intel_encoder and not so much
drm_encoder. Again, I think we can do a sed jobs once things settle,
meanwhile it's gonna be a bit ugly. Personally I don't care ;-)
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e22305..3672b8d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4320,14 +4320,15 @@  static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
 }
 
 static void vlv_update_pll(struct drm_crtc *crtc,
-			   struct drm_display_mode *mode,
-			   struct drm_display_mode *adjusted_mode,
 			   intel_clock_t *clock, intel_clock_t *reduced_clock,
 			   int num_connectors)
 {
 	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;
@@ -4394,11 +4395,11 @@  static void vlv_update_pll(struct drm_crtc *crtc,
 
 	temp = 0;
 	if (is_sdvo) {
-		temp = intel_mode_get_pixel_multiplier(adjusted_mode);
-		if (temp > 1)
-			temp = (temp - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
-		else
-			temp = 0;
+		temp = 0;
+		if (intel_crtc->config.pixel_multiplier > 1) {
+			temp = (intel_crtc->config.pixel_multiplier - 1)
+				<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
+		}
 	}
 	I915_WRITE(DPLL_MD(pipe), temp);
 	POSTING_READ(DPLL_MD(pipe));
@@ -4424,14 +4425,15 @@  static void vlv_update_pll(struct drm_crtc *crtc,
 }
 
 static void i9xx_update_pll(struct drm_crtc *crtc,
-			    struct drm_display_mode *mode,
-			    struct drm_display_mode *adjusted_mode,
 			    intel_clock_t *clock, intel_clock_t *reduced_clock,
 			    int num_connectors)
 {
 	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;
@@ -4448,11 +4450,12 @@  static void i9xx_update_pll(struct drm_crtc *crtc,
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
+
 	if (is_sdvo) {
-		int pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
-		if (pixel_multiplier > 1) {
-			if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
-				dpll |= (pixel_multiplier - 1) << SDVO_MULTIPLIER_SHIFT_HIRES;
+		if ((intel_crtc->config.pixel_multiplier > 1) &&
+		    (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
+			dpll |= (intel_crtc->config.pixel_multiplier - 1)
+				<< SDVO_MULTIPLIER_SHIFT_HIRES;
 		}
 		dpll |= DPLL_DVO_HIGH_SPEED;
 	}
@@ -4517,11 +4520,11 @@  static void i9xx_update_pll(struct drm_crtc *crtc,
 	if (INTEL_INFO(dev)->gen >= 4) {
 		u32 temp = 0;
 		if (is_sdvo) {
-			temp = intel_mode_get_pixel_multiplier(adjusted_mode);
-			if (temp > 1)
-				temp = (temp - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
-			else
-				temp = 0;
+			temp = 0;
+			if (intel_crtc->config.pixel_multiplier > 1) {
+				temp = (intel_crtc->config.pixel_multiplier - 1)
+					<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
+			}
 		}
 		I915_WRITE(DPLL_MD(pipe), temp);
 	} else {
@@ -4731,11 +4734,11 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 	else if (IS_VALLEYVIEW(dev))
-		vlv_update_pll(crtc, mode, adjusted_mode, &clock,
+		vlv_update_pll(crtc, &clock,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 	else
-		i9xx_update_pll(crtc, mode, adjusted_mode, &clock,
+		i9xx_update_pll(crtc, &clock,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 
@@ -5449,17 +5452,18 @@  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,
-			     struct drm_display_mode *mode,
-			     struct drm_display_mode *adjusted_mode)
+static void ironlake_set_m_n(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;
 	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, pixel_multiplier, lane, link_bw;
+	int target_clock, lane, link_bw;
 	bool is_dp = false, is_cpu_edp = false;
 
 	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
@@ -5477,7 +5481,6 @@  static void ironlake_set_m_n(struct drm_crtc *crtc,
 	}
 
 	/* FDI link */
-	pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
 	lane = 0;
 	/* CPU eDP doesn't require FDI link, so just set DP M/N
 	   according to current link config */
@@ -5508,8 +5511,8 @@  static void ironlake_set_m_n(struct drm_crtc *crtc,
 
 	intel_crtc->fdi_lanes = lane;
 
-	if (pixel_multiplier > 1)
-		link_bw *= pixel_multiplier;
+	if (intel_crtc->config.pixel_multiplier > 1)
+		link_bw *= intel_crtc->config.pixel_multiplier;
 	intel_link_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw, &m_n);
 
 	I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
@@ -5519,7 +5522,6 @@  static void ironlake_set_m_n(struct drm_crtc *crtc,
 }
 
 static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
-				      struct drm_display_mode *adjusted_mode,
 				      intel_clock_t *clock, u32 fp)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
@@ -5527,7 +5529,7 @@  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *intel_encoder;
 	uint32_t dpll;
-	int factor, pixel_multiplier, num_connectors = 0;
+	int factor, num_connectors = 0;
 	bool is_lvds = false, is_sdvo = false, is_tv = false;
 	bool is_dp = false, is_cpu_edp = false;
 
@@ -5578,9 +5580,9 @@  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
 	if (is_sdvo) {
-		pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
-		if (pixel_multiplier > 1) {
-			dpll |= (pixel_multiplier - 1) << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT;
+		if (intel_crtc->config.pixel_multiplier > 1) {
+			dpll |= (intel_crtc->config.pixel_multiplier - 1)
+				<< PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT;
 		}
 		dpll |= DPLL_DVO_HIGH_SPEED;
 	}
@@ -5684,7 +5686,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 |
 			reduced_clock.m2;
 
-	dpll = ironlake_compute_dpll(intel_crtc, adjusted_mode, &clock, fp);
+	dpll = ironlake_compute_dpll(intel_crtc, &clock, fp);
 
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
@@ -5738,7 +5740,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
 	 * ironlake_check_fdi_lanes. */
-	ironlake_set_m_n(crtc, mode, adjusted_mode);
+	ironlake_set_m_n(crtc);
 
 	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
 
@@ -5859,7 +5861,7 @@  static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
 	if (!is_dp || is_cpu_edp)
-		ironlake_set_m_n(crtc, mode, adjusted_mode);
+		ironlake_set_m_n(crtc);
 
 	haswell_set_pipeconf(crtc, adjusted_mode, dither);
 
@@ -5907,8 +5909,12 @@  static int intel_crtc_mode_set(struct drm_crtc *crtc,
 			encoder->base.base.id,
 			drm_get_encoder_name(&encoder->base),
 			mode->base.id, mode->name);
-		encoder_funcs = encoder->base.helper_private;
-		encoder_funcs->mode_set(&encoder->base, mode, adjusted_mode);
+		if (encoder->mode_set) {
+			encoder->mode_set(encoder);
+		} else {
+			encoder_funcs = encoder->base.helper_private;
+			encoder_funcs->mode_set(&encoder->base, mode, adjusted_mode);
+		}
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 054032a..f0e5462 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -102,8 +102,6 @@ 
 #define INTEL_DVO_CHIP_TVOUT 4
 
 /* drm_display_mode->private_flags */
-#define INTEL_MODE_PIXEL_MULTIPLIER_SHIFT (0x0)
-#define INTEL_MODE_PIXEL_MULTIPLIER_MASK (0xf << INTEL_MODE_PIXEL_MULTIPLIER_SHIFT)
 #define INTEL_MODE_DP_FORCE_6BPC (0x10)
 /*
  * Set when limited 16-235 (as opposed to full 0-255) RGB color range is
@@ -111,20 +109,6 @@ 
  */
 #define INTEL_MODE_LIMITED_COLOR_RANGE (0x40)
 
-static inline void
-intel_mode_set_pixel_multiplier(struct drm_display_mode *mode,
-				int multiplier)
-{
-	mode->clock *= multiplier;
-	mode->private_flags |= multiplier;
-}
-
-static inline int
-intel_mode_get_pixel_multiplier(const struct drm_display_mode *mode)
-{
-	return (mode->private_flags & INTEL_MODE_PIXEL_MULTIPLIER_MASK) >> INTEL_MODE_PIXEL_MULTIPLIER_SHIFT;
-}
-
 struct intel_framebuffer {
 	struct drm_framebuffer base;
 	struct drm_i915_gem_object *obj;
@@ -159,6 +143,7 @@  struct intel_encoder {
 	void (*pre_pll_enable)(struct intel_encoder *);
 	void (*pre_enable)(struct intel_encoder *);
 	void (*enable)(struct intel_encoder *);
+	void (*mode_set)(struct intel_encoder *intel_encoder);
 	void (*disable)(struct intel_encoder *);
 	void (*post_disable)(struct intel_encoder *);
 	/* Read out the current hw state of this connector, returning true if
@@ -205,6 +190,8 @@  struct intel_crtc_config {
 	 * changes the crtc timings in the mode to prevent the crtc fixup from
 	 * overwriting them.  Currently only lvds needs that. */
 	bool timings_set;
+	/* Used by SDVO (and if we ever fix it, HDMI). */
+	unsigned pixel_multiplier;
 };
 
 struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 8fdd8f8..4d9fede 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -788,7 +788,6 @@  static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
 	v_sync_offset = mode->vsync_start - mode->vdisplay;
 
 	mode_clock = mode->clock;
-	mode_clock /= intel_mode_get_pixel_multiplier(mode) ?: 1;
 	mode_clock /= 10;
 	dtd->part1.clock = mode_clock;
 
@@ -1041,12 +1040,12 @@  intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
 	return true;
 }
 
-static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
-				  const struct drm_display_mode *mode,
-				  struct drm_display_mode *adjusted_mode)
+static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
+				      struct intel_crtc_config *pipe_config)
 {
-	struct intel_sdvo *intel_sdvo = to_intel_sdvo(encoder);
-	int multiplier;
+	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
+	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	struct drm_display_mode *mode = &pipe_config->requested_mode;
 
 	/* We need to construct preferred input timings based on our
 	 * output timings.  To do that, we have to set the output
@@ -1073,8 +1072,9 @@  static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 	/* Make the CRTC code factor in the SDVO pixel multiplier.  The
 	 * SDVO device will factor out the multiplier during mode_set.
 	 */
-	multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode);
-	intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
+	pipe_config->pixel_multiplier =
+		intel_sdvo_get_pixel_multiplier(adjusted_mode);
+	adjusted_mode->clock *= pipe_config->pixel_multiplier;
 
 	if (intel_sdvo->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -1093,19 +1093,19 @@  static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 	return true;
 }
 
-static void intel_sdvo_mode_set(struct drm_encoder *encoder,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode)
+static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder)
 {
-	struct drm_device *dev = encoder->dev;
+	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = encoder->crtc;
+	struct drm_crtc *crtc = intel_encoder->base.crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_sdvo *intel_sdvo = to_intel_sdvo(encoder);
+	struct drm_display_mode *adjusted_mode =
+		&intel_crtc->config.adjusted_mode;
+	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
+	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&intel_encoder->base);
 	u32 sdvox;
 	struct intel_sdvo_in_out_map in_out;
 	struct intel_sdvo_dtd input_dtd, output_dtd;
-	int pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
 	int rate;
 
 	if (!mode)
@@ -1165,7 +1165,7 @@  static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 		DRM_INFO("Setting input timings on %s failed\n",
 			 SDVO_NAME(intel_sdvo));
 
-	switch (pixel_multiplier) {
+	switch (intel_crtc->config.pixel_multiplier) {
 	default:
 	case 1: rate = SDVO_CLOCK_RATE_MULT_1X; break;
 	case 2: rate = SDVO_CLOCK_RATE_MULT_2X; break;
@@ -1209,7 +1209,8 @@  static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	} else if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) {
 		/* done in crtc_mode_set as it lives inside the dpll register */
 	} else {
-		sdvox |= (pixel_multiplier - 1) << SDVO_PORT_MULTIPLY_SHIFT;
+		sdvox |= (intel_crtc->config.pixel_multiplier - 1)
+			<< SDVO_PORT_MULTIPLY_SHIFT;
 	}
 
 	if (input_dtd.part2.sdvo_flags & SDVO_NEED_TO_STALL &&
@@ -2041,8 +2042,6 @@  done:
 }
 
 static const struct drm_encoder_helper_funcs intel_sdvo_helper_funcs = {
-	.mode_fixup = intel_sdvo_mode_fixup,
-	.mode_set = intel_sdvo_mode_set,
 };
 
 static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
@@ -2787,7 +2786,9 @@  bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 
 	drm_encoder_helper_add(&intel_encoder->base, &intel_sdvo_helper_funcs);
 
+	intel_encoder->compute_config = intel_sdvo_compute_config;
 	intel_encoder->disable = intel_disable_sdvo;
+	intel_encoder->mode_set = intel_sdvo_mode_set;
 	intel_encoder->enable = intel_enable_sdvo;
 	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;