Re: [PATCH 2/3] drm/i915: Initialize CHV digital lock detect threshold

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Vijay Purushothaman <vijay.a.purushothaman@linux.intel.com>
Cc: Intel Graphics <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm/i915: Initialize CHV digital lock detect threshold
Date: Tue, 3 Mar 2015 17:38:29 +0200	[thread overview]
Message-ID: <20150303153829.GF11371@intel.com> (raw)
In-Reply-To: <1425395592-18432-1-git-send-email-vijay.a.purushothaman@linux.intel.com>

On Tue, Mar 03, 2015 at 08:43:12PM +0530, Vijay Purushothaman wrote:
> Initialize lock detect threshold and select coarse threshold for the
> case where M2 fraction division is disabled.
> 
> v2: Split the changes into multiple smaller patches based on review by
> Ville
> 
> v3: Addressed rest of the review comments. Clear out the old bits before
> we modify those bits as part of RMW
> 
> Signed-off-by: Vijay Purushothaman <vijay.a.purushothaman@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8200e98..1a0f94e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1046,6 +1046,7 @@ enum skl_disp_power_wells {
>  #define _CHV_PLL_DW9_CH0		0x8024
>  #define _CHV_PLL_DW9_CH1		0x81A4
>  #define  DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT		1 /* 3 bits */
> +#define  DPIO_CHV_INT_LOCK_THRESHOLD_MASK		(7 << 1)
>  #define  DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE	1 /* 1: coarse & 0 : fine  */
>  #define CHV_PLL_DW9(ch) _PIPE(ch, _CHV_PLL_DW9_CH0, _CHV_PLL_DW9_CH1)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 15904a8..a6b5786 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6176,11 +6176,24 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>  		dpio_val |= (2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT);
>  		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
>  
> +		/* Program digital lock detect threshold */
> +		dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW9(port));
> +		dpio_val &= ~DPIO_CHV_INT_LOCK_THRESHOLD_MASK;
> +		dpio_val |= (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT);

Missing
 dpio_val &= ~DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
here.

So I think to avoid such mistakes it's best to rewrite this too as:

val = read(PLL_DW9)
val &= ~(DPIO_CHV_INT_LOCK_THRESHOLD_MASK | DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE);
val |= 0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT)
if (!bestm2_frac)
	val |= DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
write(PLL_DW9, val);

> +		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port), dpio_val);
> +
>  	} else {
>  		/* M2 fraction division disable */
>  		dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
>  		dpio_val &= ~DPIO_CHV_FRAC_DIV_EN;
>  		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
> +
> +		/* Program digital lock detect threshold */
> +		dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW9(port));
> +		dpio_val &= ~DPIO_CHV_INT_LOCK_THRESHOLD_MASK;
> +		dpio_val |= (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT);
> +		dpio_val |= DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
> +		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port), dpio_val);
>  	}
>  
>  	/* Loop filter */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx