intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 7/7] drm/i915: simplify config->pixel_multiplier handling
Date: Thu, 25 Apr 2013 15:08:32 +0300	[thread overview]
Message-ID: <20130425120832.GB4469@intel.com> (raw)
In-Reply-To: <1366362877-15446-8-git-send-email-daniel.vetter@ffwll.ch>

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

-- 
Ville Syrjälä
Intel OTC

  reply	other threads:[~2013-04-25 12:09 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-19  9:14 [PATCH 0/7] dp dpll pipe_config conversion + random stuff Daniel Vetter
2013-04-19  9:14 ` [PATCH 1/7] drm/i915: consolidate pch pll computations a bit Daniel Vetter
2013-04-25 10:58   ` Ville Syrjälä
2013-04-19  9:14 ` [PATCH 2/7] drm/i915: shovel compute clock into crtc->config.dpll on ilk Daniel Vetter
2013-04-19 10:14   ` Ville Syrjälä
2013-04-19 11:36     ` [RFC][PATCH] drm/i915: Make struct dpll == intel_clock_t ville.syrjala
2013-04-20 14:50       ` Daniel Vetter
2013-04-20 15:19     ` [PATCH] drm/i915: shovel compute clock into crtc->config.dpll on ilk Daniel Vetter
2013-04-22 11:13       ` Ville Syrjälä
2013-04-22 15:12         ` Daniel Vetter
2013-04-25 11:00       ` Ville Syrjälä
2013-04-19  9:14 ` [PATCH 3/7] drm/i915: move dp clock computations to encoder->compute_config Daniel Vetter
2013-04-25 11:34   ` Ville Syrjälä
2013-04-25 12:04     ` Daniel Vetter
2013-04-25 12:21       ` Ville Syrjälä
2013-04-19  9:14 ` [PATCH 4/7] drm/i915: use pipe_config for lvds dithering Daniel Vetter
2013-04-25 11:57   ` Ville Syrjälä
2013-04-25 12:24     ` Daniel Vetter
2013-04-25 12:42       ` Ville Syrjälä
2013-04-25 13:16         ` Daniel Vetter
2013-04-25 13:20         ` [PATCH] " Daniel Vetter
2013-04-25 15:08           ` Ville Syrjälä
2013-04-25 15:54             ` Daniel Vetter
2013-04-25 15:54             ` Daniel Vetter
2013-04-25 16:09               ` Ville Syrjälä
2013-04-19  9:14 ` [PATCH 5/7] drm/i915: don't force matching p1 for g4x/ilk+ reduced pll settings Daniel Vetter
2013-04-19 14:53   ` Sean Paul
2013-04-19  9:14 ` [PATCH 6/7] drm/i915: remove redundant has_pch_encoder check Daniel Vetter
2013-04-25 11:59   ` Ville Syrjälä
2013-04-19  9:14 ` [PATCH 7/7] drm/i915: simplify config->pixel_multiplier handling Daniel Vetter
2013-04-25 12:08   ` Ville Syrjälä [this message]
2013-04-25 12:27     ` Daniel Vetter
2013-04-25 19:22     ` 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=20130425120832.GB4469@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).