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
