Hello Morimoto-san, On Monday, 18 December 2017 02:35:56 EET Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> > > In general, PLL has VCO (= Voltage controlled oscillator), > one of the very important electronic feature called as "jitter" > is related to this VCO. > In academic generalism, VCO should be maximum to be more small jitter. > In high frequency clock, jitter will be large impact. > Thus, selecting Hi VCO is general theory. > > fin fvco fout fclkout > in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> out > +-> | | | > | | > +-----------------[1/N]<-------------+ > > fclkout = fvco / P / FDPLL -- (1) > > In PD, it will loop until fin/M = fvco/P/N > > fvco = fin * P * N / M -- (2) > > (1) + (2) indicates > > fclkout = fin * N / M / FDPLL > > In this device, N = (n + 1), M = (m + 1), P = 2, FDPLL = (fdpll + 1). > > fclkout = fin * (n + 1) / (m + 1) / (fdpll + 1) > > This is the datasheet formula. > One note here is that it should be 2kHz < fvco < 4096MHz > To be smaller jitter, fvco should be maximum, > in other words, N as large as possible, M as small as possible driver > should select. Here, basically M=1. > This patch do it. > > Reported-by: HIROSHI INOSE <hiroshi.inose.rb@xxxxxxxxxxx> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> > --- > v3 -> v4 > > - 2000 -> 2kHz > > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 58 ++++++++++++++++++++++++++++--- > 1 file changed, 54 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 6820461f..574854a 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -125,13 +125,63 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc > *rcrtc, unsigned int m; > unsigned int n; > > - for (n = 39; n < 120; n++) { > - for (m = 0; m < 4; m++) { > + /* > + * fin fvco fout fclkout > + * in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> out > + * +-> | | | > + * | | > + * +-----------------[1/N]<-------------+ > + * > + * fclkout = fvco / P / FDPLL -- (1) > + * > + * fin/M = fvco/P/N > + * > + * fvco = fin * P * N / M -- (2) > + * > + * (1) + (2) indicates > + * > + * fclkout = fin * N / M / FDPLL > + * > + * NOTES > + * N : (n + 1) > + * M : (m + 1) > + * FDPLL : (fdpll + 1) > + * P : 2 > + * 2kHz < fvco < 4096MHz > + * > + * To be small jitter, Nitpicking, I would write this "to minimize the jitter". > + * N : as large as possible > + * M : as small as possible > + */ > + for (m = 0; m < 4; m++) { > + for (n = 119; n > 38; n--) { > + /* > + * NOTE: > + * > + * This code is assuming "used" from 64bit CPU only, > + * not from 32bit CPU. But both can compile correctly Nitpicking again, I would write this "This code only runs on 64-bit architectures, the unsigned long type can thus be used for 64-bit computation. It will still compile without any warning on 32-bit architectures." > + */ > + > + /* > + * fvco = fin * P * N / M > + * fclkout = fin * N / M / FDPLL > + * > + * To avoid duplicate calculation, let's use below > + * > + * finnm = fin * N / M This is called fout in your diagram above, I would use the same name here. > + * fvco = finnm * P > + * fclkout = finnm / FDPLL > + */ > + unsigned long finnm = input * (n + 1) / (m + 1); > + unsigned long fvco = finnm * 2; > + > + if (fvco < 2000 || fvco > 4096 * 1000 * 1000U) > + continue; How about if (fvco < 1000 || fvco > 2048 * 1000 * 1000) to avoid computing the intermediate fvco variable ? If you agree with these small changes there's no need to resubmit the patch, I'll modify it when applying, and Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > for (fdpll = 1; fdpll < 32; fdpll++) { > unsigned long output; > > - output = input * (n + 1) / (m + 1) > - / (fdpll + 1); > + output = finnm / (fdpll + 1); > if (output >= 400 * 1000 * 1000) > continue; -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel