On Tue, 08 Mar 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> > --- > 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