| Message ID | 20230804-tc358768-v1-3-1afd44b7826b@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: tc358768: Fixes and timings improvements | expand |
On 04/08/2023 13:44, Tomi Valkeinen wrote: > As is quite common, some of TC358768's PLL register fields are to be > programmed with (value - 1). Specifically, the FBD and PRD, multiplier > and divider, are such fields. > > However, what the driver currently does is that it considers that the > formula used for PLL rate calculation is: > > RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] > > where FBD and PRD are values directly from the registers, while a more > sensible way to look at it is: > > RefClk * FBD / PRD * (1 / (2^FRS)) > > and when the FBD and PRD values are written to the registers, they will > be subtracted by one. > > Change the driver accordingly, as it simplifies the PLL code. Reviewed-by: Peter Ujfalusi <peter.ujfalusi@gmail.com> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/gpu/drm/bridge/tc358768.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c > index b668f77673c3..d5831a1236e9 100644 > --- a/drivers/gpu/drm/bridge/tc358768.c > +++ b/drivers/gpu/drm/bridge/tc358768.c > @@ -316,7 +316,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, > > target_pll = tc358768_pclk_to_pll(priv, mode->clock * 1000); > > - /* pll_clk = RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] */ > + /* pll_clk = RefClk * FBD / PRD * (1 / (2^FRS)) */ > > for (i = 0; i < ARRAY_SIZE(frs_limits); i++) > if (target_pll >= frs_limits[i]) > @@ -336,19 +336,19 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, > best_prd = 0; > best_fbd = 0; > > - for (prd = 0; prd < 16; ++prd) { > - u32 divisor = (prd + 1) * (1 << frs); > + for (prd = 1; prd <= 16; ++prd) { > + u32 divisor = prd * (1 << frs); > u32 fbd; > > - for (fbd = 0; fbd < 512; ++fbd) { > + for (fbd = 1; fbd <= 512; ++fbd) { > u32 pll, diff, pll_in; > > - pll = (u32)div_u64((u64)refclk * (fbd + 1), divisor); > + pll = (u32)div_u64((u64)refclk * fbd, divisor); > > if (pll >= max_pll || pll < min_pll) > continue; > > - pll_in = (u32)div_u64((u64)refclk, prd + 1); > + pll_in = (u32)div_u64((u64)refclk, prd); > if (pll_in < 4000000) > continue; > > @@ -611,7 +611,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, > mode->clock * 1000); > > /* PRD[15:12] FBD[8:0] */ > - tc358768_write(priv, TC358768_PLLCTL0, (prd << 12) | fbd); > + tc358768_write(priv, TC358768_PLLCTL0, ((prd - 1) << 12) | (fbd - 1)); > > /* FRS[11:10] LBWS[9:8] CKEN[4] RESETB[1] EN[0] */ > tc358768_write(priv, TC358768_PLLCTL1, >
diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index b668f77673c3..d5831a1236e9 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -316,7 +316,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, target_pll = tc358768_pclk_to_pll(priv, mode->clock * 1000); - /* pll_clk = RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] */ + /* pll_clk = RefClk * FBD / PRD * (1 / (2^FRS)) */ for (i = 0; i < ARRAY_SIZE(frs_limits); i++) if (target_pll >= frs_limits[i]) @@ -336,19 +336,19 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, best_prd = 0; best_fbd = 0; - for (prd = 0; prd < 16; ++prd) { - u32 divisor = (prd + 1) * (1 << frs); + for (prd = 1; prd <= 16; ++prd) { + u32 divisor = prd * (1 << frs); u32 fbd; - for (fbd = 0; fbd < 512; ++fbd) { + for (fbd = 1; fbd <= 512; ++fbd) { u32 pll, diff, pll_in; - pll = (u32)div_u64((u64)refclk * (fbd + 1), divisor); + pll = (u32)div_u64((u64)refclk * fbd, divisor); if (pll >= max_pll || pll < min_pll) continue; - pll_in = (u32)div_u64((u64)refclk, prd + 1); + pll_in = (u32)div_u64((u64)refclk, prd); if (pll_in < 4000000) continue; @@ -611,7 +611,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, mode->clock * 1000); /* PRD[15:12] FBD[8:0] */ - tc358768_write(priv, TC358768_PLLCTL0, (prd << 12) | fbd); + tc358768_write(priv, TC358768_PLLCTL0, ((prd - 1) << 12) | (fbd - 1)); /* FRS[11:10] LBWS[9:8] CKEN[4] RESETB[1] EN[0] */ tc358768_write(priv, TC358768_PLLCTL1,
As is quite common, some of TC358768's PLL register fields are to be programmed with (value - 1). Specifically, the FBD and PRD, multiplier and divider, are such fields. However, what the driver currently does is that it considers that the formula used for PLL rate calculation is: RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] where FBD and PRD are values directly from the registers, while a more sensible way to look at it is: RefClk * FBD / PRD * (1 / (2^FRS)) and when the FBD and PRD values are written to the registers, they will be subtracted by one. Change the driver accordingly, as it simplifies the PLL code. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- drivers/gpu/drm/bridge/tc358768.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)