diff mbox

[v3,7/7] clk: rockchip: rk3399: Add support frac mode frequencies

Message ID 1470122579-32083-1-git-send-email-zhengxing@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

zhengxing Aug. 2, 2016, 7:22 a.m. UTC
We need to support various display resolutions for external
display devices like HDMI/DP, the frac mode can help us to
acquire almost any frequencies, and need higher VCOs to reduce
clock jitters.

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

Changes in v3: None
Changes in v2: None

 drivers/clk/rockchip/clk-rk3399.c |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Heiko Stübner Aug. 4, 2016, 7:19 p.m. UTC | #1
Hi Xing,

Am Dienstag, 2. August 2016, 15:22:59 schrieb Xing Zheng:
> We need to support various display resolutions for external
> display devices like HDMI/DP, the frac mode can help us to
> acquire almost any frequencies, and need higher VCOs to reduce
> clock jitters.
> 
> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>

why does this need to be a separate rate array and cannot live in the general 
pll rate array?

The plls are general purpose, so we shouldn't limit them arbitarily.

I currently only see some frequencies (594MHz, 297MHz, 54MHz) that are present 
in both arrays but have different settings. As your patch description says 
that these settings reduce clock jitter, wouldn't the general frequencies also 
profit from merging these new values into the general rate array?


Heiko
zhengxing Aug. 5, 2016, 2:26 a.m. UTC | #2
Hi Heiko,

On 2016年08月05日 03:19, Heiko Stübner wrote:
> Hi Xing,
>
> Am Dienstag, 2. August 2016, 15:22:59 schrieb Xing Zheng:
>> We need to support various display resolutions for external
>> display devices like HDMI/DP, the frac mode can help us to
>> acquire almost any frequencies, and need higher VCOs to reduce
>> clock jitters.
>>
>> Signed-off-by: Xing Zheng<zhengxing@rock-chips.com>
> why does this need to be a separate rate array and cannot live in the general
> pll rate array?
>
> The plls are general purpose, so we shouldn't limit them arbitarily.
Yes, I understand your mean. :-)
>
> I currently only see some frequencies (594MHz, 297MHz, 54MHz) that are present
> in both arrays but have different settings. As your patch description says
> that these settings reduce clock jitter, wouldn't the general frequencies also
> profit from merging these new values into the general rate array?
>
>
and here are some of our ideas:

"WIth the frac mode and higher VCO to reduce clock jitters" that 
suggestion is from IC designer.
There are many and various kinds resolution and needed frequencies for 
external disaplay devices. For example, the DP needs:
3840x2160 533250KHz
3840x2160 297000KHz
3840x2160 296703KHz
2560x1440 241500KHz
1920x1080 148500KHz
1920x1080 148352KHz
1680x1050 146250KHz
1600x900 108000KHz
1280x1024 135000KHz
1280x1024 108000KHz
... and so on

There some frequencies must be allocated with frac mode. We separate 
these frequencies that are only used for display (VPLL) from the general 
rate table, and put them to be classified into a frac mode table, we can 
reduce the frequency of the query time, the two rate tables will not 
interfere with each other. Because other PLLs don't need to assgin these 
various frequencies with frac mode.

Thanks
Heiko Stübner Aug. 5, 2016, 8:48 a.m. UTC | #3
Hi Xing,

Am Freitag, 5. August 2016, 10:26:57 schrieb Xing Zheng:
> On 2016年08月05日 03:19, Heiko Stübner wrote:
> > Am Dienstag, 2. August 2016, 15:22:59 schrieb Xing Zheng:
> >> We need to support various display resolutions for external
> >> display devices like HDMI/DP, the frac mode can help us to
> >> acquire almost any frequencies, and need higher VCOs to reduce
> >> clock jitters.
> >> 
> >> Signed-off-by: Xing Zheng<zhengxing@rock-chips.com>
> > 
> > why does this need to be a separate rate array and cannot live in the
> > general pll rate array?
> > 
> > The plls are general purpose, so we shouldn't limit them arbitarily.
> 
> Yes, I understand your mean. :-)
> 
> > I currently only see some frequencies (594MHz, 297MHz, 54MHz) that are
> > present in both arrays but have different settings. As your patch
> > description says that these settings reduce clock jitter, wouldn't the
> > general frequencies also profit from merging these new values into the
> > general rate array?
> 
> and here are some of our ideas:
> 
> "WIth the frac mode and higher VCO to reduce clock jitters" that
> suggestion is from IC designer.
> There are many and various kinds resolution and needed frequencies for
> external disaplay devices. For example, the DP needs:
> 3840x2160 533250KHz
> 3840x2160 297000KHz
> 3840x2160 296703KHz
> 2560x1440 241500KHz
> 1920x1080 148500KHz
> 1920x1080 148352KHz
> 1680x1050 146250KHz
> 1600x900 108000KHz
> 1280x1024 135000KHz
> 1280x1024 108000KHz
> ... and so on
> 
> There some frequencies must be allocated with frac mode. We separate
> these frequencies that are only used for display (VPLL) from the general
> rate table, and put them to be classified into a frac mode table, we can
> reduce the frequency of the query time, the two rate tables will not
> interfere with each other. Because other PLLs don't need to assgin these
> various frequencies with frac mode.

Hmm, you're adding 14 frequencies to that new table (4 or so of them 
duplicating existing frequencies). So even if the effective number of new 
frequencies goes from now 10 to 20, I don't think walking that table will take 
an excessive time longer than now.

After the patch introducing the automatic rate calculation, the rate table we 
need to walk, will even get smaller.

Other components might also profit from the updated standard frequencies with 
less jitter you're introducing here.

And of course there is also the possibility somebody might want to build some 
rk3399 device without any graphics output at all [arm-server seem to be the 
new hype :-) ], so may want to use the vpll for something else completely.

So I still don't see an argument why it needs to be a separate table, as I 
currently don't see a case were it will really hurt the other PLLs.


Heiko
zhengxing Aug. 5, 2016, 1:23 p.m. UTC | #4
Hi Heiko,

On 2016年08月05日 16:48, Heiko Stübner wrote:
> Hi Xing,
>
> Am Freitag, 5. August 2016, 10:26:57 schrieb Xing Zheng:
>> On 2016年08月05日 03:19, Heiko Stübner wrote:
>>> Am Dienstag, 2. August 2016, 15:22:59 schrieb Xing Zheng:
>>>> We need to support various display resolutions for external
>>>> display devices like HDMI/DP, the frac mode can help us to
>>>> acquire almost any frequencies, and need higher VCOs to reduce
>>>> clock jitters.
>>>>
>>>> Signed-off-by: Xing Zheng<zhengxing@rock-chips.com>
>>> why does this need to be a separate rate array and cannot live in the
>>> general pll rate array?
>>>
>>> The plls are general purpose, so we shouldn't limit them arbitarily.
>> Yes, I understand your mean. :-)
>>
>>> I currently only see some frequencies (594MHz, 297MHz, 54MHz) that are
>>> present in both arrays but have different settings. As your patch
>>> description says that these settings reduce clock jitter, wouldn't the
>>> general frequencies also profit from merging these new values into the
>>> general rate array?
>> and here are some of our ideas:
>>
>> "WIth the frac mode and higher VCO to reduce clock jitters" that
>> suggestion is from IC designer.
>> There are many and various kinds resolution and needed frequencies for
>> external disaplay devices. For example, the DP needs:
>> 3840x2160 533250KHz
>> 3840x2160 297000KHz
>> 3840x2160 296703KHz
>> 2560x1440 241500KHz
>> 1920x1080 148500KHz
>> 1920x1080 148352KHz
>> 1680x1050 146250KHz
>> 1600x900 108000KHz
>> 1280x1024 135000KHz
>> 1280x1024 108000KHz
>> ... and so on
>>
>> There some frequencies must be allocated with frac mode. We separate
>> these frequencies that are only used for display (VPLL) from the general
>> rate table, and put them to be classified into a frac mode table, we can
>> reduce the frequency of the query time, the two rate tables will not
>> interfere with each other. Because other PLLs don't need to assgin these
>> various frequencies with frac mode.
> Hmm, you're adding 14 frequencies to that new table (4 or so of them
> duplicating existing frequencies). So even if the effective number of new
> frequencies goes from now 10 to 20, I don't think walking that table will take
> an excessive time longer than now.
>
> After the patch introducing the automatic rate calculation, the rate table we
> need to walk, will even get smaller.
>
> Other components might also profit from the updated standard frequencies with
> less jitter you're introducing here.
>
> And of course there is also the possibility somebody might want to build some
> rk3399 device without any graphics output at all [arm-server seem to be the
> new hype :-) ], so may want to use the vpll for something else completely.
>
> So I still don't see an argument why it needs to be a separate table, as I
> currently don't see a case were it will really hurt the other PLLs.
>
>
> Heiko
>
Yes, sorry to this idea is not comprehensive. I will try to find a 
better way.

Thanks for your comments. :-)
Heiko Stübner Aug. 5, 2016, 1:26 p.m. UTC | #5
Am Freitag, 5. August 2016, 21:23:14 schrieb Xing Zheng:
> Hi Heiko,
> 
> On 2016年08月05日 16:48, Heiko Stübner wrote:
> > Hi Xing,
> > 
> > Am Freitag, 5. August 2016, 10:26:57 schrieb Xing Zheng:
> >> On 2016年08月05日 03:19, Heiko Stübner wrote:
> >>> Am Dienstag, 2. August 2016, 15:22:59 schrieb Xing Zheng:
> >>>> We need to support various display resolutions for external
> >>>> display devices like HDMI/DP, the frac mode can help us to
> >>>> acquire almost any frequencies, and need higher VCOs to reduce
> >>>> clock jitters.
> >>>> 
> >>>> Signed-off-by: Xing Zheng<zhengxing@rock-chips.com>
> >>> 
> >>> why does this need to be a separate rate array and cannot live in the
> >>> general pll rate array?
> >>> 
> >>> The plls are general purpose, so we shouldn't limit them arbitarily.
> >> 
> >> Yes, I understand your mean. :-)
> >> 
> >>> I currently only see some frequencies (594MHz, 297MHz, 54MHz) that are
> >>> present in both arrays but have different settings. As your patch
> >>> description says that these settings reduce clock jitter, wouldn't the
> >>> general frequencies also profit from merging these new values into the
> >>> general rate array?
> >> 
> >> and here are some of our ideas:
> >> 
> >> "WIth the frac mode and higher VCO to reduce clock jitters" that
> >> suggestion is from IC designer.
> >> There are many and various kinds resolution and needed frequencies for
> >> external disaplay devices. For example, the DP needs:
> >> 3840x2160 533250KHz
> >> 3840x2160 297000KHz
> >> 3840x2160 296703KHz
> >> 2560x1440 241500KHz
> >> 1920x1080 148500KHz
> >> 1920x1080 148352KHz
> >> 1680x1050 146250KHz
> >> 1600x900 108000KHz
> >> 1280x1024 135000KHz
> >> 1280x1024 108000KHz
> >> ... and so on
> >> 
> >> There some frequencies must be allocated with frac mode. We separate
> >> these frequencies that are only used for display (VPLL) from the general
> >> rate table, and put them to be classified into a frac mode table, we can
> >> reduce the frequency of the query time, the two rate tables will not
> >> interfere with each other. Because other PLLs don't need to assgin these
> >> various frequencies with frac mode.
> > 
> > Hmm, you're adding 14 frequencies to that new table (4 or so of them
> > duplicating existing frequencies). So even if the effective number of new
> > frequencies goes from now 10 to 20, I don't think walking that table will
> > take an excessive time longer than now.
> > 
> > After the patch introducing the automatic rate calculation, the rate table
> > we need to walk, will even get smaller.
> > 
> > Other components might also profit from the updated standard frequencies
> > with less jitter you're introducing here.
> > 
> > And of course there is also the possibility somebody might want to build
> > some rk3399 device without any graphics output at all [arm-server seem to
> > be the new hype :-) ], so may want to use the vpll for something else
> > completely.
> > 
> > So I still don't see an argument why it needs to be a separate table, as I
> > currently don't see a case were it will really hurt the other PLLs.
> > 
> > 
> > Heiko
> 
> Yes, sorry to this idea is not comprehensive. I will try to find a
> better way.
> 
> Thanks for your comments. :-)

as I said, to me just merging the new clock rates into the existing pll rate 
array looks like it should work just fine :-)
diff mbox

Patch

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 2792404..ddd209b 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -109,6 +109,25 @@  static struct rockchip_pll_rate_table rk3399_pll_rates[] = {
 	{ /* sentinel */ },
 };
 
+static struct rockchip_pll_rate_table rk3399_pll_frates[] = {
+	/* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */
+	RK3036_PLL_RATE( 594000000, 1, 123, 5, 1, 0, 12582912),  /* vco = 2970000000 */
+	RK3036_PLL_RATE( 593406593, 1, 123, 5, 1, 0, 10508804),  /* vco = 2967032965 */
+	RK3036_PLL_RATE( 297000000, 1, 123, 5, 2, 0, 12582912),  /* vco = 2970000000 */
+	RK3036_PLL_RATE( 296703297, 1, 123, 5, 2, 0, 10508807),  /* vco = 2967032970 */
+	RK3036_PLL_RATE( 148500000, 1, 129, 7, 3, 0, 15728640),  /* vco = 3118500000 */
+	RK3036_PLL_RATE( 148351648, 1, 123, 5, 4, 0, 10508800),  /* vco = 2967032960 */
+	RK3036_PLL_RATE( 106500000, 1, 124, 7, 4, 0,  4194304),  /* vco = 2982000000 */
+	RK3036_PLL_RATE(  74250000, 1, 129, 7, 6, 0, 15728640),  /* vco = 3118500000 */
+	RK3036_PLL_RATE(  74175824, 1, 129, 7, 6, 0, 13550823),  /* vco = 3115384608 */
+	RK3036_PLL_RATE(  65000000, 1, 113, 7, 6, 0, 12582912),  /* vco = 2730000000 */
+	RK3036_PLL_RATE(  59340659, 1, 121, 7, 7, 0,  2581098),  /* vco = 2907692291 */
+	RK3036_PLL_RATE(  54000000, 1, 110, 7, 7, 0,  4194304),  /* vco = 2646000000 */
+	RK3036_PLL_RATE(  27000000, 1,  55, 7, 7, 0,  2097152),  /* vco = 1323000000 */
+	RK3036_PLL_RATE(  26973027, 1,  55, 7, 7, 0,  1173232),  /* vco = 1321678323 */
+	{ /* sentinel */ },
+};
+
 /* CRU parents */
 PNAME(mux_pll_p)				= { "xin24m", "xin32k" };
 
@@ -229,7 +248,7 @@  static struct rockchip_pll_clock rk3399_pll_clks[] __initdata = {
 	[npll] = PLL(pll_rk3399, PLL_NPLL, "npll",  mux_pll_p, 0, RK3399_PLL_CON(40),
 		     RK3399_PLL_CON(43), 8, 31, ROCKCHIP_PLL_SYNC_RATE, rk3399_pll_rates),
 	[vpll] = PLL(pll_rk3399, PLL_VPLL, "vpll",  mux_pll_p, 0, RK3399_PLL_CON(48),
-		     RK3399_PLL_CON(51), 8, 31, ROCKCHIP_PLL_SYNC_RATE, rk3399_pll_rates),
+		     RK3399_PLL_CON(51), 8, 31, ROCKCHIP_PLL_SYNC_RATE, rk3399_pll_frates),
 };
 
 static struct rockchip_pll_clock rk3399_pmu_pll_clks[] __initdata = {