diff mbox

[v3,2/2] drm: rcar-du: calculate DPLLCR to be more small jitter

Message ID 871sjweqr4.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kuninori Morimoto Dec. 15, 2017, 1:24 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

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 2000 < 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@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3

 - uses "* 1000" for number
 - uses "xx000U" for number
 - uses finnm to avoid duplicate calculation

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 58 +++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 4 deletions(-)

Comments

Kuninori Morimoto Dec. 15, 2017, 8:12 a.m. UTC | #1 
Hi Geert

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 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.
> 
> Thanks for your patch!
> 
> > One note here is that it should be 2000 < fvco < 4096MHz
> 
> 2000 Hz? (else it could be misinterpreted that MHz applies to both values).

Laurent had asked same question ;)
But, yes, it is 2000 Hz

Best regards
---
Kuninori Morimoto
Geert Uytterhoeven Dec. 15, 2017, 8:36 a.m. UTC | #2 
Hi Morimoto-san,

On Fri, Dec 15, 2017 at 9:12 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>> > 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.
>>
>> Thanks for your patch!
>>
>> > One note here is that it should be 2000 < fvco < 4096MHz
>>
>> 2000 Hz? (else it could be misinterpreted that MHz applies to both values).
>
> Laurent had asked same question ;)
> But, yes, it is 2000 Hz

I've seen his question, that's why I think you should make it unambiguous.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Kuninori Morimoto Dec. 17, 2017, 11:46 p.m. UTC | #3 
Hi Geert

> >> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >> > 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.
> >>
> >> Thanks for your patch!
> >>
> >> > One note here is that it should be 2000 < fvco < 4096MHz
> >>
> >> 2000 Hz? (else it could be misinterpreted that MHz applies to both values).
> >
> > Laurent had asked same question ;)
> > But, yes, it is 2000 Hz
> 
> I've seen his question, that's why I think you should make it unambiguous.

Ahh... OK yes indeed
will fix in v4

Best regards
---
Kuninori Morimoto
diff mbox

Patch

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 6820461f..bb5ead6 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
+	 *	2000 < fvco < 4096Mhz
+	 *
+	 * To be small 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
+			 */
+
+			/*
+			 *	fvco	= fin * P *  N / M
+			 *	fclkout	= fin      * N / M / FDPLL
+			 *
+			 * To avoid duplicate calculation, let's use below
+			 *
+			 *	finnm	= fin * N / M
+			 *	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;
+
 			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;