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+rene...@ideasonboard.com>
> > Tested-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> > ---
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to