| Message ID | 1366362877-15446-8-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 19, 2013 at 11:14:37AM +0200, Daniel Vetter wrote: > We only ever check whether it's strictly bigger than one, so all the > is_sdvo/is_hdmi checks are redundant. Flatten the code a bit. > > Also, s/temp/dpll_md/ > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Looks OK. Note that we actually never set pixel_multipler on VLV since it doesn't support SDVO, but supposedly it could be used for HDMI too. So I guess it doesn't hurt to keep the code for it. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index eef5abd..6e265b0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4249,7 +4249,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) > u32 dpll, mdiv; > u32 bestn, bestm1, bestm2, bestp1, bestp2; > bool is_hdmi; > - u32 coreclk, reg_val, temp; > + u32 coreclk, reg_val, dpll_md; > > mutex_lock(&dev_priv->dpio_lock); > > @@ -4347,16 +4347,13 @@ static void vlv_update_pll(struct intel_crtc *crtc) > if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) > DRM_ERROR("DPLL %d failed to lock\n", pipe); > > - if (is_hdmi) { > - temp = 0; > - if (crtc->config.pixel_multiplier > 1) { > - temp = (crtc->config.pixel_multiplier - 1) > - << DPLL_MD_UDI_MULTIPLIER_SHIFT; > - } > - > - I915_WRITE(DPLL_MD(pipe), temp); > - POSTING_READ(DPLL_MD(pipe)); > + dpll_md = 0; > + if (crtc->config.pixel_multiplier > 1) { > + dpll_md = (crtc->config.pixel_multiplier - 1) > + << DPLL_MD_UDI_MULTIPLIER_SHIFT; > } > + I915_WRITE(DPLL_MD(pipe), dpll_md); > + POSTING_READ(DPLL_MD(pipe)); > > if (crtc->config.has_dp_encoder) > intel_dp_set_m_n(crtc); > @@ -4388,14 +4385,15 @@ static void i9xx_update_pll(struct intel_crtc *crtc, > else > dpll |= DPLLB_MODE_DAC_SERIAL; > > - if (is_sdvo) { > - if ((crtc->config.pixel_multiplier > 1) && > - (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) { > - dpll |= (crtc->config.pixel_multiplier - 1) > - << SDVO_MULTIPLIER_SHIFT_HIRES; > - } > - dpll |= DPLL_DVO_HIGH_SPEED; > + if ((crtc->config.pixel_multiplier > 1) && > + (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) { > + dpll |= (crtc->config.pixel_multiplier - 1) > + << SDVO_MULTIPLIER_SHIFT_HIRES; > } > + > + if (is_sdvo) > + dpll |= DPLL_DVO_HIGH_SPEED; > + > if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) > dpll |= DPLL_DVO_HIGH_SPEED; > > @@ -4455,15 +4453,12 @@ static void i9xx_update_pll(struct intel_crtc *crtc, > udelay(150); > > if (INTEL_INFO(dev)->gen >= 4) { > - u32 temp = 0; > - if (is_sdvo) { > - temp = 0; > - if (crtc->config.pixel_multiplier > 1) { > - temp = (crtc->config.pixel_multiplier - 1) > - << DPLL_MD_UDI_MULTIPLIER_SHIFT; > - } > + u32 dpll_md = 0; > + if (crtc->config.pixel_multiplier > 1) { > + dpll_md = (crtc->config.pixel_multiplier - 1) > + << DPLL_MD_UDI_MULTIPLIER_SHIFT; > } > - I915_WRITE(DPLL_MD(pipe), temp); > + I915_WRITE(DPLL_MD(pipe), dpll_md); > } else { > /* The pixel multiplier can only be updated once the > * DPLL is enabled and the clocks are stable. > @@ -5576,13 +5571,14 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > dpll |= DPLLB_MODE_LVDS; > else > dpll |= DPLLB_MODE_DAC_SERIAL; > - if (is_sdvo) { > - 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; > + > + if (intel_crtc->config.pixel_multiplier > 1) { > + dpll |= (intel_crtc->config.pixel_multiplier - 1) > + << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT; > } > + > + if (is_sdvo) > + dpll |= DPLL_DVO_HIGH_SPEED; > if (intel_crtc->config.has_dp_encoder) > dpll |= DPLL_DVO_HIGH_SPEED; > > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Apr 25, 2013 at 03:08:32PM +0300, Ville Syrjälä wrote: > On Fri, Apr 19, 2013 at 11:14:37AM +0200, Daniel Vetter wrote: > > We only ever check whether it's strictly bigger than one, so all the > > is_sdvo/is_hdmi checks are redundant. Flatten the code a bit. > > > > Also, s/temp/dpll_md/ > > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Looks OK. Note that we actually never set pixel_multipler on VLV since > it doesn't support SDVO, but supposedly it could be used for HDMI too. > So I guess it doesn't hurt to keep the code for it. Yeah, finally enabling correct pixel multiplier support for hdmi is on our eternal todo list. Last time I've tried it something blew up and it didn't really work out too well. We need to look into this again. -Daniel
On Thu, Apr 25, 2013 at 03:08:32PM +0300, Ville Syrjälä wrote: > On Fri, Apr 19, 2013 at 11:14:37AM +0200, Daniel Vetter wrote: > > We only ever check whether it's strictly bigger than one, so all the > > is_sdvo/is_hdmi checks are redundant. Flatten the code a bit. > > > > Also, s/temp/dpll_md/ > > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Looks OK. Note that we actually never set pixel_multipler on VLV since > it doesn't support SDVO, but supposedly it could be used for HDMI too. > So I guess it doesn't hurt to keep the code for it. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Merged the entire series, with the commit message for patch 3 massively pimped with my follow up. Thanks a lot for the review. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index eef5abd..6e265b0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4249,7 +4249,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) u32 dpll, mdiv; u32 bestn, bestm1, bestm2, bestp1, bestp2; bool is_hdmi; - u32 coreclk, reg_val, temp; + u32 coreclk, reg_val, dpll_md; mutex_lock(&dev_priv->dpio_lock); @@ -4347,16 +4347,13 @@ static void vlv_update_pll(struct intel_crtc *crtc) if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) DRM_ERROR("DPLL %d failed to lock\n", pipe); - if (is_hdmi) { - temp = 0; - if (crtc->config.pixel_multiplier > 1) { - temp = (crtc->config.pixel_multiplier - 1) - << DPLL_MD_UDI_MULTIPLIER_SHIFT; - } - - I915_WRITE(DPLL_MD(pipe), temp); - POSTING_READ(DPLL_MD(pipe)); + dpll_md = 0; + if (crtc->config.pixel_multiplier > 1) { + dpll_md = (crtc->config.pixel_multiplier - 1) + << DPLL_MD_UDI_MULTIPLIER_SHIFT; } + I915_WRITE(DPLL_MD(pipe), dpll_md); + POSTING_READ(DPLL_MD(pipe)); if (crtc->config.has_dp_encoder) intel_dp_set_m_n(crtc); @@ -4388,14 +4385,15 @@ static void i9xx_update_pll(struct intel_crtc *crtc, else dpll |= DPLLB_MODE_DAC_SERIAL; - if (is_sdvo) { - if ((crtc->config.pixel_multiplier > 1) && - (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) { - dpll |= (crtc->config.pixel_multiplier - 1) - << SDVO_MULTIPLIER_SHIFT_HIRES; - } - dpll |= DPLL_DVO_HIGH_SPEED; + if ((crtc->config.pixel_multiplier > 1) && + (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) { + dpll |= (crtc->config.pixel_multiplier - 1) + << SDVO_MULTIPLIER_SHIFT_HIRES; } + + if (is_sdvo) + dpll |= DPLL_DVO_HIGH_SPEED; + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) dpll |= DPLL_DVO_HIGH_SPEED; @@ -4455,15 +4453,12 @@ static void i9xx_update_pll(struct intel_crtc *crtc, udelay(150); if (INTEL_INFO(dev)->gen >= 4) { - u32 temp = 0; - if (is_sdvo) { - temp = 0; - if (crtc->config.pixel_multiplier > 1) { - temp = (crtc->config.pixel_multiplier - 1) - << DPLL_MD_UDI_MULTIPLIER_SHIFT; - } + u32 dpll_md = 0; + if (crtc->config.pixel_multiplier > 1) { + dpll_md = (crtc->config.pixel_multiplier - 1) + << DPLL_MD_UDI_MULTIPLIER_SHIFT; } - I915_WRITE(DPLL_MD(pipe), temp); + I915_WRITE(DPLL_MD(pipe), dpll_md); } else { /* The pixel multiplier can only be updated once the * DPLL is enabled and the clocks are stable. @@ -5576,13 +5571,14 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, dpll |= DPLLB_MODE_LVDS; else dpll |= DPLLB_MODE_DAC_SERIAL; - if (is_sdvo) { - 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; + + if (intel_crtc->config.pixel_multiplier > 1) { + dpll |= (intel_crtc->config.pixel_multiplier - 1) + << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT; } + + if (is_sdvo) + dpll |= DPLL_DVO_HIGH_SPEED; if (intel_crtc->config.has_dp_encoder) dpll |= DPLL_DVO_HIGH_SPEED;
We only ever check whether it's strictly bigger than one, so all the is_sdvo/is_hdmi checks are redundant. Flatten the code a bit. Also, s/temp/dpll_md/ Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++------------------- 1 file changed, 27 insertions(+), 31 deletions(-)