Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Store the /5 target clock in struct dpll on vlv/chv

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 08 Mar 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Unify vlv/chv with earlier platforms so that the sturct dpll::dot
> represents the /5 clock frequency (ie. DP symbol rate or HDMI
> TMDS rate) rather than the *5 fast clock (/2 of the bitrate).
> Makes life a little less confusing to get the same number back
> in .dot which we fed into the DPLL algorithm.
>
> v2: Actually just include the 5x in the final P divider
>     Do the same change to the hand rolled gvt code
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

For some reason it was really hard for me to convince myself that I'd
checked every code path for the right amount of *5 and /5 in the right
places... I'll just say I didn't spot anything obviously wrong
here. Fingers crossed.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/display/intel_dpll.c | 19 ++++++++-----------
>  drivers/gpu/drm/i915/gvt/handlers.c       |  4 ++--
>  2 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
> index 0ae37fdbf2a5..b3fd94538c44 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -254,12 +254,12 @@ static const struct intel_limit ilk_limits_dual_lvds_100m = {
>  
>  static const struct intel_limit intel_limits_vlv = {
>  	 /*
> -	  * These are the data rate limits (measured in fast clocks)
> +	  * These are based on the data rate limits (measured in fast clocks)
>  	  * since those are the strictest limits we have. The fast
>  	  * clock and actual rate limits are more relaxed, so checking
>  	  * them would make no difference.
>  	  */
> -	.dot = { .min = 25000 * 5, .max = 270000 * 5 },
> +	.dot = { .min = 25000, .max = 270000 },
>  	.vco = { .min = 4000000, .max = 6000000 },
>  	.n = { .min = 1, .max = 7 },
>  	.m1 = { .min = 2, .max = 3 },
> @@ -270,12 +270,12 @@ static const struct intel_limit intel_limits_vlv = {
>  
>  static const struct intel_limit intel_limits_chv = {
>  	/*
> -	 * These are the data rate limits (measured in fast clocks)
> +	 * These are based on the data rate limits (measured in fast clocks)
>  	 * since those are the strictest limits we have.  The fast
>  	 * clock and actual rate limits are more relaxed, so checking
>  	 * them would make no difference.
>  	 */
> -	.dot = { .min = 25000 * 5, .max = 540000 * 5},
> +	.dot = { .min = 25000, .max = 540000 },
>  	.vco = { .min = 4800000, .max = 6480000 },
>  	.n = { .min = 1, .max = 1 },
>  	.m1 = { .min = 2, .max = 2 },
> @@ -337,26 +337,26 @@ int i9xx_calc_dpll_params(int refclk, struct dpll *clock)
>  int vlv_calc_dpll_params(int refclk, struct dpll *clock)
>  {
>  	clock->m = clock->m1 * clock->m2;
> -	clock->p = clock->p1 * clock->p2;
> +	clock->p = clock->p1 * clock->p2 * 5;
>  	if (WARN_ON(clock->n == 0 || clock->p == 0))
>  		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;
> +	return clock->dot;
>  }
>  
>  int chv_calc_dpll_params(int refclk, struct dpll *clock)
>  {
>  	clock->m = clock->m1 * clock->m2;
> -	clock->p = clock->p1 * clock->p2;
> +	clock->p = clock->p1 * clock->p2 * 5;
>  	if (WARN_ON(clock->n == 0 || clock->p == 0))
>  		return 0;
>  	clock->vco = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(refclk, clock->m),
>  					   clock->n << 22);
>  	clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
>  
> -	return clock->dot / 5;
> +	return clock->dot;
>  }
>  
>  /*
> @@ -659,8 +659,6 @@ vlv_find_best_dpll(const struct intel_limit *limit,
>  	int max_n = min(limit->n.max, refclk / 19200);
>  	bool found = false;
>  
> -	target *= 5; /* fast clock */
> -
>  	memset(best_clock, 0, sizeof(*best_clock));
>  
>  	/* based on hardware requirement, prefer smaller n to precision */
> @@ -729,7 +727,6 @@ chv_find_best_dpll(const struct intel_limit *limit,
>  	 */
>  	clock.n = 1;
>  	clock.m1 = 2;
> -	target *= 5;	/* fast clock */
>  
>  	for (clock.p1 = limit->p1.max; clock.p1 >= limit->p1.min; clock.p1--) {
>  		for (clock.p2 = limit->p2.p2_fast;
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index 520a7e1942f3..efdd2f3f9d73 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -583,7 +583,7 @@ static u32 bxt_vgpu_get_dp_bitrate(struct intel_vgpu *vgpu, enum port port)
>  	clock.p1 = (vgpu_vreg_t(vgpu, BXT_PORT_PLL_EBB_0(phy, ch)) & PORT_PLL_P1_MASK) >> PORT_PLL_P1_SHIFT;
>  	clock.p2 = (vgpu_vreg_t(vgpu, BXT_PORT_PLL_EBB_0(phy, ch)) & PORT_PLL_P2_MASK) >> PORT_PLL_P2_SHIFT;
>  	clock.m = clock.m1 * clock.m2;
> -	clock.p = clock.p1 * clock.p2;
> +	clock.p = clock.p1 * clock.p2 * 5;
>  
>  	if (clock.n == 0 || clock.p == 0) {
>  		gvt_dbg_dpy("vgpu-%d PORT_%c PLL has invalid divider\n", vgpu->id, port_name(port));
> @@ -593,7 +593,7 @@ static u32 bxt_vgpu_get_dp_bitrate(struct intel_vgpu *vgpu, enum port port)
>  	clock.vco = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(refclk, clock.m), clock.n << 22);
>  	clock.dot = DIV_ROUND_CLOSEST(clock.vco, clock.p);
>  
> -	dp_br = clock.dot / 5;
> +	dp_br = clock.dot;
>  
>  out:
>  	return dp_br;

-- 
Jani Nikula, Intel Open Source Graphics Center