Re: [PATCH v2 4.1/5] drm/i915: calculate the port clock rate along with other PLL params

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 24, 2015 at 03:53:23PM +0300, Ville Syrjälä wrote:
> On Mon, Jun 22, 2015 at 11:35:51PM +0300, Imre Deak wrote:
> > Depending on the platform the port clock fed to the pipe can be the PLL's
> > post-divided fast clock rate or a /5 divided version of it. To make this
> > more obvious across the platforms calculate this port clock along with
> > the rest of the PLL parameters.
> > 
> > This is also needed by the next patch where we can reuse the CHV helper
> > for the BXT PLL HW readout code; so export the corresponding helper.
> > 
> > While at it also add a more descriptive name to the helpers and a
> > comment explaining what's being calculated.
> > 
> > No functional change.
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> I'm not sure I entirely like the new name, but the old name wasn't
> good either, and I can't think of anything better.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

All merged to dinq, thanks.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++---------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >  2 files changed, 38 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index fb7fd5f..a11ce7fa 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -553,15 +553,25 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
> >  	return limit;
> >  }
> >  
> > +/*
> > + * Platform specific helpers to calculate the port PLL loopback- (clock.m),
> > + * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast
> > + * (clock.dot) clock rates. This fast dot clock is fed to the port's IO logic.
> > + * The helpers' return value is the rate of the clock that is fed to the
> > + * display engine's pipe which can be the above fast dot clock rate or a
> > + * divided-down version of it.
> > + */
> >  /* m1 is reserved as 0 in Pineview, n is a ring counter */
> > -static void pineview_clock(int refclk, intel_clock_t *clock)
> > +static int pnv_calc_dpll_params(int refclk, intel_clock_t *clock)
> >  {
> >  	clock->m = clock->m2 + 2;
> >  	clock->p = clock->p1 * clock->p2;
> >  	if (WARN_ON(clock->n == 0 || clock->p == 0))
> > -		return;
> > +		return 0;
> >  	clock->vco = DIV_ROUND_CLOSEST(refclk * clock->m, clock->n);
> >  	clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> > +
> > +	return clock->dot;
> >  }
> >  
> >  static uint32_t i9xx_dpll_compute_m(struct dpll *dpll)
> > @@ -569,35 +579,41 @@ static uint32_t i9xx_dpll_compute_m(struct dpll *dpll)
> >  	return 5 * (dpll->m1 + 2) + (dpll->m2 + 2);
> >  }
> >  
> > -static void i9xx_clock(int refclk, intel_clock_t *clock)
> > +static int i9xx_calc_dpll_params(int refclk, intel_clock_t *clock)
> >  {
> >  	clock->m = i9xx_dpll_compute_m(clock);
> >  	clock->p = clock->p1 * clock->p2;
> >  	if (WARN_ON(clock->n + 2 == 0 || clock->p == 0))
> > -		return;
> > +		return 0;
> >  	clock->vco = DIV_ROUND_CLOSEST(refclk * clock->m, clock->n + 2);
> >  	clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> > +
> > +	return clock->dot;
> >  }
> >  
> > -static void vlv_clock(int refclk, intel_clock_t *clock)
> > +static int vlv_calc_dpll_params(int refclk, intel_clock_t *clock)
> >  {
> >  	clock->m = clock->m1 * clock->m2;
> >  	clock->p = clock->p1 * clock->p2;
> >  	if (WARN_ON(clock->n == 0 || clock->p == 0))
> > -		return;
> > +		return 0;
> >  	clock->vco = DIV_ROUND_CLOSEST(refclk * clock->m, clock->n);
> >  	clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> > +
> > +	return clock->dot / 5;
> >  }
> >  
> > -static void chv_clock(int refclk, intel_clock_t *clock)
> > +int chv_calc_dpll_params(int refclk, intel_clock_t *clock)
> >  {
> >  	clock->m = clock->m1 * clock->m2;
> >  	clock->p = clock->p1 * clock->p2;
> >  	if (WARN_ON(clock->n == 0 || clock->p == 0))
> > -		return;
> > +		return 0;
> >  	clock->vco = DIV_ROUND_CLOSEST_ULL((uint64_t)refclk * clock->m,
> >  			clock->n << 22);
> >  	clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> > +
> > +	return clock->dot / 5;
> >  }
> >  
> >  #define INTELPllInvalid(s)   do { /* DRM_DEBUG(s); */ return false; } while (0)
> > @@ -692,7 +708,7 @@ i9xx_find_best_dpll(const intel_limit_t *limit,
> >  					clock.p1 <= limit->p1.max; clock.p1++) {
> >  					int this_err;
> >  
> > -					i9xx_clock(refclk, &clock);
> > +					i9xx_calc_dpll_params(refclk, &clock);
> >  					if (!intel_PLL_is_valid(dev, limit,
> >  								&clock))
> >  						continue;
> > @@ -737,7 +753,7 @@ pnv_find_best_dpll(const intel_limit_t *limit,
> >  					clock.p1 <= limit->p1.max; clock.p1++) {
> >  					int this_err;
> >  
> > -					pineview_clock(refclk, &clock);
> > +					pnv_calc_dpll_params(refclk, &clock);
> >  					if (!intel_PLL_is_valid(dev, limit,
> >  								&clock))
> >  						continue;
> > @@ -787,7 +803,7 @@ g4x_find_best_dpll(const intel_limit_t *limit,
> >  				     clock.p1 >= limit->p1.min; clock.p1--) {
> >  					int this_err;
> >  
> > -					i9xx_clock(refclk, &clock);
> > +					i9xx_calc_dpll_params(refclk, &clock);
> >  					if (!intel_PLL_is_valid(dev, limit,
> >  								&clock))
> >  						continue;
> > @@ -877,7 +893,7 @@ vlv_find_best_dpll(const intel_limit_t *limit,
> >  					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
> >  								     refclk * clock.m1);
> >  
> > -					vlv_clock(refclk, &clock);
> > +					vlv_calc_dpll_params(refclk, &clock);
> >  
> >  					if (!intel_PLL_is_valid(dev, limit,
> >  								&clock))
> > @@ -940,7 +956,7 @@ chv_find_best_dpll(const intel_limit_t *limit,
> >  
> >  			clock.m2 = m2;
> >  
> > -			chv_clock(refclk, &clock);
> > +			chv_calc_dpll_params(refclk, &clock);
> >  
> >  			if (!intel_PLL_is_valid(dev, limit, &clock))
> >  				continue;
> > @@ -7926,10 +7942,7 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> >  	clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
> >  	clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
> >  
> > -	vlv_clock(refclk, &clock);
> > -
> > -	/* clock.dot is the fast clock */
> > -	pipe_config->port_clock = clock.dot / 5;
> > +	pipe_config->port_clock = vlv_calc_dpll_params(refclk, &clock);
> >  }
> >  
> >  static void
> > @@ -8025,10 +8038,7 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
> >  	clock.p1 = (cmn_dw13 >> DPIO_CHV_P1_DIV_SHIFT) & 0x7;
> >  	clock.p2 = (cmn_dw13 >> DPIO_CHV_P2_DIV_SHIFT) & 0x1f;
> >  
> > -	chv_clock(refclk, &clock);
> > -
> > -	/* clock.dot is the fast clock */
> > -	pipe_config->port_clock = clock.dot / 5;
> > +	pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock);
> >  }
> >  
> >  static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> > @@ -10481,6 +10491,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >  	u32 fp;
> >  	intel_clock_t clock;
> > +	int port_clock;
> >  	int refclk = i9xx_pll_refclk(dev, pipe_config);
> >  
> >  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> > @@ -10521,9 +10532,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  		}
> >  
> >  		if (IS_PINEVIEW(dev))
> > -			pineview_clock(refclk, &clock);
> > +			port_clock = pnv_calc_dpll_params(refclk, &clock);
> >  		else
> > -			i9xx_clock(refclk, &clock);
> > +			port_clock = i9xx_calc_dpll_params(refclk, &clock);
> >  	} else {
> >  		u32 lvds = IS_I830(dev) ? 0 : I915_READ(LVDS);
> >  		bool is_lvds = (pipe == 1) && (lvds & LVDS_PORT_EN);
> > @@ -10549,7 +10560,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  				clock.p2 = 2;
> >  		}
> >  
> > -		i9xx_clock(refclk, &clock);
> > +		port_clock = i9xx_calc_dpll_params(refclk, &clock);
> >  	}
> >  
> >  	/*
> > @@ -10557,7 +10568,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  	 * port_clock to compute adjusted_mode.crtc_clock in the
> >  	 * encoder's get_config() function.
> >  	 */
> > -	pipe_config->port_clock = clock.dot;
> > +	pipe_config->port_clock = port_clock;
> >  }
> >  
> >  int intel_dotclock_calculate(int link_freq,
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index e2174fd..7a5b3bc 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1138,6 +1138,8 @@ ironlake_check_encoder_dotclock(const struct intel_crtc_state *pipe_config,
> >  				int dotclock);
> >  bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
> >  			intel_clock_t *best_clock);
> > +int chv_calc_dpll_params(int refclk, intel_clock_t *pll_clock);
> > +
> >  bool intel_crtc_active(struct drm_crtc *crtc);
> >  void hsw_enable_ips(struct intel_crtc *crtc);
> >  void hsw_disable_ips(struct intel_crtc *crtc);
> > -- 
> > 2.1.4
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx