Hi Ulrich, On Monday, 17 September 2018 13:53:58 EEST Ulrich Hecht wrote: > > On September 14, 2018 at 11:10 AM Laurent Pinchart wrote: > > > > The LVDS encoders in the D3 and E3 SoCs differ significantly from those > > in the other R-Car Gen3 family members: > > > > - The LVDS PLL architecture is more complex and requires computing PLL > > parameters manually. > > > > - The PLL uses external clocks as inputs, which need to be retrieved > > from DT. > > > > - In addition to the different PLL setup, the startup sequence has > > changed *again* (seems someone had trouble making his/her mind). > > > > Supporting all this requires DT bindings extensions for external clocks, > > brand new PLL setup code, and a few quirks to handle the differences in > > the startup sequence. > > > > The implementation doesn't support all hardware features yet, namely > > > > - Using the LV[01] clocks generated by the CPG as PLL input. > > - Providing the LVDS PLL clock to the DU for use with the RGB output. > > > > Those features can be added later when the need will arise. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > Tested-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> > > --- > > Changes since v1: > > > > - Don't compile-out debug code based on DEBUG and CONFIG_DYNAMIC_DEBUG > > - Test all three input clocks (DOTCLKIN[01] and EXTAL) and pick the best > > one > > > > --- > > > > drivers/gpu/drm/rcar-du/rcar_lvds.c | 355 ++++++++++++++++++++++---- > > drivers/gpu/drm/rcar-du/rcar_lvds_regs.h | 43 +++- > > 2 files changed, 351 insertions(+), 47 deletions(-) > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > b/drivers/gpu/drm/rcar-du/rcar_lvds.c index ce0eb68c3416..23e7743144c8 > > 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c [snip] > > +static void rcar_lvds_d3_e3_pll_calc(struct rcar_lvds *lvds, struct clk > > *clk, > > + unsigned long target, struct pll_info *pll, > > + u32 clksel) > > +{ > > + unsigned long output; > > + unsigned long fin; > > + unsigned int m_min; > > + unsigned int m_max; > > + unsigned int m; > > + int error; > > + > > + if (!clk) > > + return; > > + > > + /* > > + * The LVDS PLL is made of a pre-divider and a multiplier (strangerly > > strangely Will be fixed in v2. > > + * enough called M and N respectively), followed by a post-divider E. > > + * > > + * ,-----. ,-----. ,-----. ,-----. > > + * Fin --> | 1/M | -Fpdf-> | PFD | --> | VCO | -Fvco-> | 1/E | --> Fout > > + * `-----' ,-> | | `-----' | `-----' > > + * | `-----' | > > + * | ,-----. | > > + * `-------- | 1/N | <-------' > > + * `-----' > > + * > > + * The clock output by the PLL is then further divided by a > > programmable > > + * divider DIV to achieve the desired target frequency. Finally, an > > + * optional fixed /7 divider is used to convert the bit clock to a > > pixel > > + * clock (as LVDS transmits 7 bits per lane per clock sample). > > + * > > + * ,-------. ,-----. |\ > > + * Fout --> | 1/DIV | --> | 1/7 | --> | | > > + * `-------' | `-----' | | --> dot clock > > + * `------------> | | > > + * |/ > > + * > > + * The /7 divider is optional when the LVDS PLL is used to generate a > > + * dot clock for the DU RGB output, without using the LVDS encoder. We > > + * don't support this configuration yet. > > + * > > + * The PLL allowed input frequency range is 12 MHz to 192 MHz. > > + */ > > + > > + fin = clk_get_rate(clk); > > + if (fin < 12000000 || fin > 192000000) > > + return; > > + > > + /* > > + * The comparison frequency range is 12 MHz to 24 MHz, which limits the > > + * allowed values for the pre-divider M (normal range 1-8). > > + * > > + * Fpfd = Fin / M > > + */ > > + m_min = max_t(unsigned int, 1, DIV_ROUND_UP(fin, 24000000)); > > + m_max = min_t(unsigned int, 8, fin / 12000000); > > + > > + for (m = m_min; m <= m_max; ++m) { > > + unsigned long fpfd; > > + unsigned int n_min; > > + unsigned int n_max; > > + unsigned int n; > > + > > + /* > > + * The VCO operating range is 900 Mhz to 1800 MHz, which limits > > + * the allowed values for the multiplier N (normal range > > + * 60-120). > > + * > > + * Fvco = Fin * N / M > > + */ > > + fpfd = fin / m; > > + n_min = max_t(unsigned int, 60, DIV_ROUND_UP(900000000, fpfd)); > > + n_max = min_t(unsigned int, 120, 1800000000 / fpfd); > > + > > + for (n = n_min; n < n_max; ++n) { > > + unsigned long fvco; > > + unsigned int e_min; > > + unsigned int e; > > + > > + /* > > + * The output frequency is limited to 1039.5 MHz, > > + * limiting again the allowed values for the > > + * post-divider E (normal value 1, 2 or 4). > > + * > > + * Fout = Fvco / E > > + */ > > + fvco = fpfd * n; > > + e_min = fvco > 1039500000 ? 1 : 0; > > + > > + for (e = e_min; e < 3; ++e) { > > + unsigned long fout; > > + unsigned long diff; > > + unsigned int div; > > + > > + /* > > + * Finally we have a programable divider after > > + * the PLL, followed by a an optional fixed /7 > > + * divider. > > + */ > > + fout = fvco / (1 << e) / 7; > > + div = DIV_ROUND_CLOSEST(fout, target); > > + diff = abs(fout / div - target); > > + > > + if (diff < pll->diff) { > > + pll->diff = diff; > > + pll->pll_m = m; > > + pll->pll_n = n; > > + pll->pll_e = e; > > + pll->div = div; > > + pll->clksel = clksel; > > + > > + if (diff == 0) > > + goto done; > > + } > > + } > > + } > > + } > > + > > +done: > > + output = fin * pll->pll_n / pll->pll_m / (1 << pll->pll_e) > > + / 7 / pll->div; > > + error = (long)(output - target) * 10000 / (long)target; > > + > > + dev_dbg(lvds->dev, > > + "%pC %lu Hz -> Fout %lu Hz (target %lu Hz, error %d.%02u%%), PLL > > M/N/E/DIV %u/%u/%u/%u\n", + clk, fin, output, target, error / 100, > > + error < 0 ? -error % 100 : error % 100, > > + pll->pll_m, pll->pll_n, pll->pll_e, pll->div); > > +} > > + > > +static void rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds, unsigned > > int freq) > > +{ > > + struct pll_info pll = { .diff = (unsigned long)-1 }; > > + u32 lvdpllcr; > > + > > + rcar_lvds_d3_e3_pll_calc(lvds, lvds->clocks.dotclkin[0], freq, &pll, > > + LVDPLLCR_CKSEL_DU_DOTCLKIN(0)); > > + rcar_lvds_d3_e3_pll_calc(lvds, lvds->clocks.dotclkin[1], freq, &pll, > > + LVDPLLCR_CKSEL_DU_DOTCLKIN(1)); > > + rcar_lvds_d3_e3_pll_calc(lvds, lvds->clocks.extal, freq, &pll, > > + LVDPLLCR_CKSEL_EXTAL); > > + > > + lvdpllcr = LVDPLLCR_PLLON | pll.clksel | LVDPLLCR_CLKOUT > > + | LVDPLLCR_PLLN(pll.pll_n - 1) | LVDPLLCR_PLLM(pll.pll_m - 1); > > + > > + if (pll.pll_e > 0) > > + lvdpllcr |= LVDPLLCR_STP_CLKOUTE | LVDPLLCR_OUTCLKSEL > > + | LVDPLLCR_PLLE(pll.pll_e - 1); > > + > > + rcar_lvds_write(lvds, LVDPLLCR, lvdpllcr); > > + > > + if (pll.div > 1) > > + rcar_lvds_write(lvds, LVDDIV, LVDDIV_DIVSEL | > > + LVDDIV_DIVRESET | LVDDIV_DIV(pll.div - 1)); > > + else > > + rcar_lvds_write(lvds, LVDDIV, 0); > > The datasheet says DIVRESET == 0 is "reset" and DIVRESET == 1 is "clear". I > haven't found anything clarifying what that exactly means, but my best > guess would have been that this should be the other way round: assert > LVDDIV_DIVRESET ("clear" the divider) if the divider is 1, and deassert it > ("reset" the divider) otherwise. My interpretation is opposite, 0 meaning asserting reset and 1 meaning clearing reset (and thus deasserting it). > The BSP driver, however, does it the same way as above, and the PLL setting > examples (table 37.5) in the datasheet also have DIVRESET asserted when the > divider is on, so the above code is likely to be correct. Maybe it's the > LVDDIV register description that is wrong? > > At any rate, I think this is confusing enough to deserve a comment. I agree, I'll add that. > > +} [snip] -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel