| Message ID | 1470122579-32083-1-git-send-email-zhengxing@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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. :-)
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 --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 = {
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(-)