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

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

[Kuninori Morimoto]

Hi Laurent, David

These are v3 of DPLLCR patch for rcar-du.
[1/2] is added

Kuninori Morimoto (2):
  drm: rcar-du: use 1000 to avoid misunderstanding in rcar_du_dpll_divider()
  drm: rcar-du: calculate DPLLCR to be more small jitter

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 60 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[Kuninori Morimoto]

Hi Laurent, David

These are v3 of DPLLCR patch for rcar-du.
[1/2] is added

Kuninori Morimoto (2):
  drm: rcar-du: use 1000 to avoid misunderstanding in rcar_du_dpll_divider()
  drm: rcar-du: calculate DPLLCR to be more small jitter

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 60 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH v3 1/2] drm: rcar-du: use 1000 to avoid misunderstanding in rcar_du_dpll_divider()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

It is difficult to understand its scale if number has many 0s.
This patch uses "* 1000" to avoid it in rcar_du_dpll_divider().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3

 - new patch

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 5685d5a..6820461f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -132,7 +132,7 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc,
 
 				output = input * (n + 1) / (m + 1)
 				       / (fdpll + 1);
-				if (output >= 400000000)
+				if (output >= 400 * 1000 * 1000)
 					continue;
 
 				diff = abs((long)output - (long)target);
-- 
1.9.1

[PATCH v3 1/2] drm: rcar-du: use 1000 to avoid misunderstanding in rcar_du_dpll_divider()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

It is difficult to understand its scale if number has many 0s.
This patch uses "* 1000" to avoid it in rcar_du_dpll_divider().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3

 - new patch

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 5685d5a..6820461f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -132,7 +132,7 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc,
 
 				output = input * (n + 1) / (m + 1)
 				       / (fdpll + 1);
-				if (output >= 400000000)
+				if (output >= 400 * 1000 * 1000)
 					continue;
 
 				diff = abs((long)output - (long)target);
-- 
1.9.1

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

[Kuninori Morimoto]

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(-)

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;
 
-- 
1.9.1

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

[Kuninori Morimoto]

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(-)

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;
 
-- 
1.9.1

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

[Geert Uytterhoeven]

Hi Morimoto-san,

On Fri, Dec 15, 2017 at 2:24 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).

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

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

[Geert Uytterhoeven]

Hi Morimoto-san,

On Fri, Dec 15, 2017 at 2:24 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).

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Kuninori Morimoto]

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

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

[Geert Uytterhoeven]

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

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

[Kuninori Morimoto]

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