From: Frank Oltmanns <hidden> Date: 2023-12-18 13:43:44
On some pinephones the video output sometimes freezes (flips between two
frames) [1]. It seems to be that the reason for this behaviour is that
PLL-MIPI and PLL-VIDEO0 are operating outside there specified limits.
The changes I propose in this patch series consists of two major parts:
1. sunxi-ng: Adhere to the following constraints given in the
Allwinner A64 Manual:
a. PLL-MIPI:
* M/N >= 3
* (PLL_VIDEO0)/M >= 24MHz
b. PLL-VIDEO0:
* 8 <= N/M <= 25
2. Choose a higher clock rate for the ST7703 based XDB599 panel, so
that the panel functions with the Allwinner A64 SOC. PLL-MIPI
must run between 500 MHz and 1.4 GHz. As PLL-MIPI runs at 6 times
the panel's clock rate, we need its clock to be at least 83.333
MHz.
So far, I've tested the patches only on my pinephone. Before the patches
it would freeze at least every other day. With the patches it has not
shown this behavior in over a week.
I very much appreciate your feedback!
[1] https://gitlab.com/postmarketOS/pmaports/-/issues/805
Signed-off-by: Frank Oltmanns <redacted>
---
Frank Oltmanns (5):
clk: sunxi-ng: nkm: Support constraints on m/n ratio and parent rate
clk: sunxi-ng: a64: Add constraints on PLL-MIPI's n/m ratio and parent rate
clk: sunxi-ng: nm: Support constraints on n/m ratio and parent rate
clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
drm/panel: st7703: Drive XBD599 panel at higher clock rate
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 10 ++++++--
drivers/clk/sunxi-ng/ccu_nkm.c | 23 ++++++++++++++++++
drivers/clk/sunxi-ng/ccu_nkm.h | 8 +++++++
drivers/clk/sunxi-ng/ccu_nm.c | 21 +++++++++++++++--
drivers/clk/sunxi-ng/ccu_nm.h | 34 +++++++++++++++++++++++++--
drivers/gpu/drm/panel/panel-sitronix-st7703.c | 14 +++++------
6 files changed, 97 insertions(+), 13 deletions(-)
---
base-commit: d0ac5722dae5f4302bb4ef6df10d0afa718df80b
change-id: 20231218-pinephone-pll-fixes-0ccdfde273e4
Best regards,
--
Frank Oltmanns [off-list ref]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Frank Oltmanns <hidden> Date: 2023-12-18 13:36:16
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Frank Oltmanns <hidden> Date: 2023-12-18 13:36:21
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/gpu/drm/panel/panel-sitronix-st7703.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
From: Frank Oltmanns <hidden> Date: 2023-12-18 13:41:49
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
The PLL-MIPI clock is implemented as ccu_nm. Therefore, add support for
this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nm.c | 21 +++++++++++++++++++--
drivers/clk/sunxi-ng/ccu_nm.h | 34 ++++++++++++++++++++++++++++++++--
2 files changed, 51 insertions(+), 4 deletions(-)
@@ -27,6 +27,19 @@ static unsigned long ccu_nm_calc_rate(unsigned long parent,returnrate;}+staticboolccu_nm_is_valid_rate(structccu_common*common,unsignedlongn,unsignedlongm)+{+structccu_nm*nm=container_of(common,structccu_nm,common);++if(nm->max_nm_ratio&&(n>nm->max_nm_ratio*m))+returnfalse;++if(nm->min_nm_ratio&&(n<nm->min_nm_ratio*m))+returnfalse;++returntrue;+}+staticunsignedlongccu_nm_find_best(structccu_common*common,unsignedlongparent,unsignedlongrate,struct_ccu_nm*nm){
@@ -36,8 +49,12 @@ static unsigned long ccu_nm_find_best(struct ccu_common *common, unsigned long pfor(_n=nm->min_n;_n<=nm->max_n;_n++){for(_m=nm->min_m;_m<=nm->max_m;_m++){-unsignedlongtmp_rate=ccu_nm_calc_rate(parent,-_n,_m);+unsignedlongtmp_rate;++if(!ccu_nm_is_valid_rate(common,_n,_m))+continue;++tmp_rate=ccu_nm_calc_rate(parent,_n,_m);if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)){best_rate=tmp_rate;
@@ -31,6 +31,8 @@ struct ccu_nm {unsignedintfixed_post_div;unsignedintmin_rate;unsignedintmax_rate;+unsignedlongmin_nm_ratio;/* minimum value for m/n */+unsignedlongmax_nm_ratio;/* maximum value for m/n */structccu_commoncommon;};
From: Frank Oltmanns <hidden> Date: 2023-12-18 13:42:54
The Allwinner A64 manual lists the following constraints for the
PLL-MIPI clock:
- M/N >= 3
- (PLL_VIDEO0)/M >= 24MHz
Use these constraints.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 2 ++
1 file changed, 2 insertions(+)
From: Frank Oltmanns <hidden> Date: 2023-12-18 13:42:54
The Allwinner A64 manual lists the following constraints for the
PLL-MIPI clock:
- M/N >= 3
- (PLL_VIDEO0)/M >= 24MHz
The PLL-MIPI clock is implemented as ccu_nkm. Therefore, add support for
these constraints.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nkm.c | 23 +++++++++++++++++++++++
drivers/clk/sunxi-ng/ccu_nkm.h | 8 ++++++++
2 files changed, 31 insertions(+)
@@ -32,6 +46,9 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(struct ccu_common *commontmp_parent=clk_hw_round_rate(parent_hw,rate*_m/(_n*_k));+if(!ccu_nkm_is_valid_rate(common,tmp_parent,_n,_m))+continue;+tmp_rate=tmp_parent*_n*_k/_m;if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)||
@@ -65,6 +82,12 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,for(_k=nkm->min_k;_k<=nkm->max_k;_k++){for(_n=nkm->min_n;_n<=nkm->max_n;_n++){for(_m=nkm->min_m;_m<=nkm->max_m;_m++){+if((common->reg==0x040)&&(_m>3*_n))+break;++if((common->reg==0x040)&&(parent<24000000*_m))+continue;+unsignedlongtmp_rate;tmp_rate=parent*_n*_k/_m;
From: Frank Oltmanns <hidden> Date: 2023-12-18 17:32:13
On 2023-12-18 at 14:35:19 +0100, Frank Oltmanns [off-list ref] wrote:
quoted hunk
The Allwinner A64 manual lists the following constraints for the
PLL-MIPI clock:
- M/N >= 3
- (PLL_VIDEO0)/M >= 24MHz
The PLL-MIPI clock is implemented as ccu_nkm. Therefore, add support for
these constraints.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nkm.c | 23 +++++++++++++++++++++++
drivers/clk/sunxi-ng/ccu_nkm.h | 8 ++++++++
2 files changed, 31 insertions(+)
@@ -32,6 +46,9 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(struct ccu_common *commontmp_parent=clk_hw_round_rate(parent_hw,rate*_m/(_n*_k));+if(!ccu_nkm_is_valid_rate(common,tmp_parent,_n,_m))+continue;+tmp_rate=tmp_parent*_n*_k/_m;if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)||
@@ -65,6 +82,12 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,for(_k=nkm->min_k;_k<=nkm->max_k;_k++){for(_n=nkm->min_n;_n<=nkm->max_n;_n++){for(_m=nkm->min_m;_m<=nkm->max_m;_m++){+if((common->reg==0x040)&&(_m>3*_n))+break;++if((common->reg==0x040)&&(parent<24000000*_m))+continue;+
This, of course, is rubbish and should be this instead:
+ if (!ccu_nkm_is_valid_rate(common, parent, _n, _m))
+ continue;
+
I'll submit a V2 after receiving some feedback.
Hi Frank!
Dne ponedeljek, 18. december 2023 ob 14:35:19 CET je Frank Oltmanns napisal(a):
The Allwinner A64 manual lists the following constraints for the
PLL-MIPI clock:
- M/N >= 3
This should be "<="
quoted hunk
- (PLL_VIDEO0)/M >= 24MHz
The PLL-MIPI clock is implemented as ccu_nkm. Therefore, add support for
these constraints.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nkm.c | 23 +++++++++++++++++++++++
drivers/clk/sunxi-ng/ccu_nkm.h | 8 ++++++++
2 files changed, 31 insertions(+)
@@ -32,6 +46,9 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(struct ccu_common *commontmp_parent=clk_hw_round_rate(parent_hw,rate*_m/(_n*_k));+if(!ccu_nkm_is_valid_rate(common,tmp_parent,_n,_m))+continue;+tmp_rate=tmp_parent*_n*_k/_m;if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)||
@@ -65,6 +82,12 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,for(_k=nkm->min_k;_k<=nkm->max_k;_k++){for(_n=nkm->min_n;_n<=nkm->max_n;_n++){for(_m=nkm->min_m;_m<=nkm->max_m;_m++){+if((common->reg==0x040)&&(_m>3*_n))+break;++if((common->reg==0x040)&&(parent<24000000*_m))+continue;+
Dne ponedeljek, 18. december 2023 ob 14:35:21 CET je Frank Oltmanns napisal(a):
quoted hunk
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
The PLL-MIPI clock is implemented as ccu_nm. Therefore, add support for
this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nm.c | 21 +++++++++++++++++++--
drivers/clk/sunxi-ng/ccu_nm.h | 34 ++++++++++++++++++++++++++++++++--
2 files changed, 51 insertions(+), 4 deletions(-)
@@ -27,6 +27,19 @@ static unsigned long ccu_nm_calc_rate(unsigned long parent,returnrate;}+staticboolccu_nm_is_valid_rate(structccu_common*common,unsignedlongn,unsignedlongm)+{+structccu_nm*nm=container_of(common,structccu_nm,common);++if(nm->max_nm_ratio&&(n>nm->max_nm_ratio*m))+returnfalse;++if(nm->min_nm_ratio&&(n<nm->min_nm_ratio*m))+returnfalse;++returntrue;+}+staticunsignedlongccu_nm_find_best(structccu_common*common,unsignedlongparent,unsignedlongrate,struct_ccu_nm*nm){
@@ -36,8 +49,12 @@ static unsigned long ccu_nm_find_best(struct ccu_common *common, unsigned long pfor(_n=nm->min_n;_n<=nm->max_n;_n++){for(_m=nm->min_m;_m<=nm->max_m;_m++){-unsignedlongtmp_rate=ccu_nm_calc_rate(parent,-_n,_m);+unsignedlongtmp_rate;++if(!ccu_nm_is_valid_rate(common,_n,_m))+continue;++tmp_rate=ccu_nm_calc_rate(parent,_n,_m);if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)){best_rate=tmp_rate;
@@ -31,6 +31,8 @@ struct ccu_nm {unsignedintfixed_post_div;unsignedintmin_rate;unsignedintmax_rate;+unsignedlongmin_nm_ratio;/* minimum value for m/n */+unsignedlongmax_nm_ratio;/* maximum value for m/n */
Comment is wrong, it should be "n/m". For consistency with nkm patch,
min_n_m_ratio and max_n_m_ratio.
Best regards,
Jernej
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a):
quoted hunk
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
Best regards,
Jernej
From: Frank Oltmanns <hidden> Date: 2023-12-20 07:08:41
Hi Jernej!
On 2023-12-19 at 17:46:08 +0100, Jernej Škrabec [off-list ref] wrote:
Hi Frank!
Dne ponedeljek, 18. december 2023 ob 14:35:19 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraints for the
PLL-MIPI clock:
- M/N >= 3
This should be "<="
Yes, good catch! I will fix it in V2.
quoted
- (PLL_VIDEO0)/M >= 24MHz
The PLL-MIPI clock is implemented as ccu_nkm. Therefore, add support for
these constraints.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nkm.c | 23 +++++++++++++++++++++++
drivers/clk/sunxi-ng/ccu_nkm.h | 8 ++++++++
2 files changed, 31 insertions(+)
@@ -32,6 +46,9 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(struct ccu_common *commontmp_parent=clk_hw_round_rate(parent_hw,rate*_m/(_n*_k));+if(!ccu_nkm_is_valid_rate(common,tmp_parent,_n,_m))+continue;+tmp_rate=tmp_parent*_n*_k/_m;if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)||
@@ -65,6 +82,12 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,for(_k=nkm->min_k;_k<=nkm->max_k;_k++){for(_n=nkm->min_n;_n<=nkm->max_n;_n++){for(_m=nkm->min_m;_m<=nkm->max_m;_m++){+if((common->reg==0x040)&&(_m>3*_n))+break;++if((common->reg==0x040)&&(parent<24000000*_m))+continue;+
What about max_m_n_ratio and max_parent_m_ratio, to be consistent? This
should also allow to simplify description.
Jernej, thank you so much! This is brilliant! I was racking my brain for
a good name but failed. Now, that I see your proposal, I don't know why
I hadn't come up with it. It's the obvious choice.
I'd say with the new names we should be able to get rid of the comments
describing the new struct members (also in ccu_nm.h). What are your
thoughts on that?
Best regards,
Frank
From: Frank Oltmanns <hidden> Date: 2023-12-20 07:14:07
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec [off-list ref] wrote:
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Above flags are unrelated change, put them in new patch if needed.
You might notice that I am using a new macro for initializing the
pll_video0_clk struct:
New: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO
Old: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST
Setting the two CCU_FEATURE flags is part of the old initialization
macro.
I'll add SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_NM_RATIO_CLOSEST which
hopefully resolves the confusion.
Thanks,
Frank
From: Frank Oltmanns <hidden> Date: 2023-12-20 08:08:32
On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec [off-list ref] wrote:
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
quoted
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
This one is really a tough nut. Unfortunately, the PLL_MIPI clock for
this panel has to run exactly at 6 * panel clock. Let me start by
showing the relevant part of the clock tree (this is on the pinephone
after applying the patches):
pll-video0 393600000
pll-mipi 500945454
tcon0 500945454
tcon-data-clock 125236363
To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit
rate [1]. It's a fixed divisor
The panel I'm proposing to change is defined as this:
static const struct st7703_panel_desc xbd599_desc = {
.mode = &xbd599_mode,
.lanes = 4,
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
.format = MIPI_DSI_FMT_RGB888,
.init_sequence = xbd599_init_sequence,
};
So, we have 24 bpp and 4 lanes. Therefore, the resulting requested
tcon-data-clock rate is
crtc_clock * 1000 * (24 / 4) / 4
tcon-data-clock therefore requests a parent rate of
4 * (crtc_clock * 1000 * (24 / 4) / 4)
The initial 4 is the fixed divisor between tcon0 and tcon-data-clock.
Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi.
Since PLL-MIPI has to run at at least at 500MHz this forces us to have a
crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of
83.502 MHz.
If we only changed the constraints on the PLL_MIPI without changing the
panel mode, we end up with a mismatch. This, in turn, would result in
dropped frames, right?
Best regards,
Frank
[1] Source:
https://elixir.bootlin.com/linux/v6.6.7/source/drivers/gpu/drm/sun4i/sun4i_tcon.c#L346
Dne sreda, 20. december 2023 ob 07:58:07 CET je Frank Oltmanns napisal(a):
Hi Jernej!
On 2023-12-19 at 17:46:08 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Hi Frank!
Dne ponedeljek, 18. december 2023 ob 14:35:19 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraints for the
PLL-MIPI clock:
- M/N >= 3
This should be "<="
Yes, good catch! I will fix it in V2.
quoted
quoted
- (PLL_VIDEO0)/M >= 24MHz
The PLL-MIPI clock is implemented as ccu_nkm. Therefore, add support for
these constraints.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu_nkm.c | 23 +++++++++++++++++++++++
drivers/clk/sunxi-ng/ccu_nkm.h | 8 ++++++++
2 files changed, 31 insertions(+)
@@ -32,6 +46,9 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(struct ccu_common *commontmp_parent=clk_hw_round_rate(parent_hw,rate*_m/(_n*_k));+if(!ccu_nkm_is_valid_rate(common,tmp_parent,_n,_m))+continue;+tmp_rate=tmp_parent*_n*_k/_m;if(ccu_is_better_rate(common,rate,tmp_rate,best_rate)||
@@ -65,6 +82,12 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,for(_k=nkm->min_k;_k<=nkm->max_k;_k++){for(_n=nkm->min_n;_n<=nkm->max_n;_n++){for(_m=nkm->min_m;_m<=nkm->max_m;_m++){+if((common->reg==0x040)&&(_m>3*_n))+break;++if((common->reg==0x040)&&(parent<24000000*_m))+continue;+
What about max_m_n_ratio and max_parent_m_ratio, to be consistent? This
should also allow to simplify description.
Jernej, thank you so much! This is brilliant! I was racking my brain for
a good name but failed. Now, that I see your proposal, I don't know why
I hadn't come up with it. It's the obvious choice.
I'd say with the new names we should be able to get rid of the comments
describing the new struct members (also in ccu_nm.h). What are your
thoughts on that?
Ah, I missed that only new ones are documented. Yeah, you can skip it.
Best regards,
Jernej
Dne sreda, 20. december 2023 ob 08:09:28 CET je Frank Oltmanns napisal(a):
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Above flags are unrelated change, put them in new patch if needed.
You might notice that I am using a new macro for initializing the
pll_video0_clk struct:
New: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO
Old: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST
Setting the two CCU_FEATURE flags is part of the old initialization
macro.
I'll add SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_NM_RATIO_CLOSEST which
hopefully resolves the confusion.
I'm in doubt if we need so many macros. How many users of these macro we'll have?
I see that R40 SoC would also need same ratio limits, but other that that, none?
Best regards,
Jernej
Dne sreda, 20. december 2023 ob 08:14:27 CET je Frank Oltmanns napisal(a):
On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
quoted
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
This one is really a tough nut. Unfortunately, the PLL_MIPI clock for
this panel has to run exactly at 6 * panel clock. Let me start by
showing the relevant part of the clock tree (this is on the pinephone
after applying the patches):
pll-video0 393600000
pll-mipi 500945454
tcon0 500945454
tcon-data-clock 125236363
To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit
rate [1]. It's a fixed divisor
The panel I'm proposing to change is defined as this:
static const struct st7703_panel_desc xbd599_desc = {
.mode = &xbd599_mode,
.lanes = 4,
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
.format = MIPI_DSI_FMT_RGB888,
.init_sequence = xbd599_init_sequence,
};
So, we have 24 bpp and 4 lanes. Therefore, the resulting requested
tcon-data-clock rate is
crtc_clock * 1000 * (24 / 4) / 4
tcon-data-clock therefore requests a parent rate of
4 * (crtc_clock * 1000 * (24 / 4) / 4)
The initial 4 is the fixed divisor between tcon0 and tcon-data-clock.
Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi.
Since PLL-MIPI has to run at at least at 500MHz this forces us to have a
crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of
83.502 MHz.
This is much better explanation why this change is needed. Still, I think
adding min and max rate to PLL_MIPI would make sense, so proper rates
are guaranteed.
Anyway, do you know where are all those old values come from? And how did
you come up with new ones? I guess you can't just simply change timings,
there are probably some HW limitations? Do you know if BSP kernel support
this panel and how this situation is solved there?
If we only changed the constraints on the PLL_MIPI without changing the
panel mode, we end up with a mismatch. This, in turn, would result in
dropped frames, right?
From what I read, I think frame rate would be higher than 60 fps. What
exactly would happen depends on the panel.
Best regards,
Jernej
From: Frank Oltmanns <hidden> Date: 2023-12-20 19:37:51
Ok, I've done more detailed testing, and it seems this patch results in
lots of dropped frames. I'm sorry for not being more thorough earlier.
I'll do some more testing without this patch and might have to either
remove it from V2 of this series.
I need to see if the same stability can be achieved when running
PLL-MIPI outside its specied range.
Best regards,
Frank
On 2023-12-20 at 16:18:49 +0100, Jernej Škrabec [off-list ref] wrote:
Dne sreda, 20. december 2023 ob 08:14:27 CET je Frank Oltmanns napisal(a):
quoted
On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
quoted
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
This one is really a tough nut. Unfortunately, the PLL_MIPI clock for
this panel has to run exactly at 6 * panel clock. Let me start by
showing the relevant part of the clock tree (this is on the pinephone
after applying the patches):
pll-video0 393600000
pll-mipi 500945454
tcon0 500945454
tcon-data-clock 125236363
To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit
rate [1]. It's a fixed divisor
The panel I'm proposing to change is defined as this:
static const struct st7703_panel_desc xbd599_desc = {
.mode = &xbd599_mode,
.lanes = 4,
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
.format = MIPI_DSI_FMT_RGB888,
.init_sequence = xbd599_init_sequence,
};
So, we have 24 bpp and 4 lanes. Therefore, the resulting requested
tcon-data-clock rate is
crtc_clock * 1000 * (24 / 4) / 4
tcon-data-clock therefore requests a parent rate of
4 * (crtc_clock * 1000 * (24 / 4) / 4)
The initial 4 is the fixed divisor between tcon0 and tcon-data-clock.
Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi.
Since PLL-MIPI has to run at at least at 500MHz this forces us to have a
crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of
83.502 MHz.
This is much better explanation why this change is needed. Still, I think
adding min and max rate to PLL_MIPI would make sense, so proper rates
are guaranteed.
Anyway, do you know where are all those old values come from? And how did
you come up with new ones? I guess you can't just simply change timings,
there are probably some HW limitations? Do you know if BSP kernel support
this panel and how this situation is solved there?
quoted
If we only changed the constraints on the PLL_MIPI without changing the
panel mode, we end up with a mismatch. This, in turn, would result in
dropped frames, right?
From what I read, I think frame rate would be higher than 60 fps. What
exactly would happen depends on the panel.
Best regards,
Jernej
From: Frank Oltmanns <hidden> Date: 2023-12-22 07:46:22
On 2023-12-20 at 16:12:42 +0100, Jernej Škrabec [off-list ref] wrote:
Dne sreda, 20. december 2023 ob 08:09:28 CET je Frank Oltmanns napisal(a):
quoted
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Above flags are unrelated change, put them in new patch if needed.
You might notice that I am using a new macro for initializing the
pll_video0_clk struct:
New: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO
Old: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST
Setting the two CCU_FEATURE flags is part of the old initialization
macro.
I'll add SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_NM_RATIO_CLOSEST which
hopefully resolves the confusion.
I'm in doubt if we need so many macros. How many users of these macro we'll have?
I see that R40 SoC would also need same ratio limits, but other that that, none?
Ok, IIUC no additional macro and we keep this part of the patch as is.
Best regards,
Frank
From: Frank Oltmanns <hidden> Date: 2023-12-22 09:10:40
On 2023-12-20 at 19:57:06 +0100, Frank Oltmanns [off-list ref] wrote:
Ok, I've done more detailed testing, and it seems this patch results in
lots of dropped frames. I'm sorry for not being more thorough earlier.
I'll do some more testing without this patch and might have to either
remove it from V2 of this series.
I need to see if the same stability can be achieved when running
PLL-MIPI outside its specied range.
I've done some more (load) testing and observing the panel for dropped
frames.
The conclusion I draw from those results is that this patch isn't
necessary for the pinephone. It would be enough to use the correct clock
rate based on the existing values [*]:
- .clock = 69000,
+ .clock = (720 + 40 + 40 + 40) * (1440 + 18 + 10 + 17) * 60 / 1000,
I've asked in the postmarketOS community for a bit more testing. They
already have a merge request that contains these changes [2].
This means that we would continue to drive PLL-MIPI outside it's
specified range. I have, so far, not experienced any downside of doing
so. It seems enough to fix the ratios that are part of the first four
patches in this series without introducing a min and max rate.
In conclusion, I'll soon (after some more feedback from the fine folks
at postmarketOS) submit a V2 that addresses the fixes requested in the
first four patches of this series. I'll drop the existing PATCH 5 and
replace it with the one I sent in February [1] instead.
After that, just for fun, I'll probably look into min_rate and max_rate
for nkm clocks and which consequences it has on the pinephone. I might
or might not send a follow up series for that. However, if the pinephone
runs stable without it, it's not a high priority for me.
Best regards,
Frank
[*] I've already submitted a patch in February '23 [1]. It was of little
use back then because the A64's PLL-MIPI clock was not able to run
close to that rate. But since kernel 6.6 PLL-MIPI is able to set
it's parent rate, so that it can come quite close to the required
rate:
+ Panel requires 74.844 MHz with the current timings.
+-> tcon-data-clock rate should be 112.266 MHz (panel*24/4/4).
+-> PLL-MIPI rate should be 449.064 MHz (TCON0 * 4)
The 6.6 kernel the following rates are possible:
+ PLL-MIPI: ~448.984615 MHz
+-> tcon-data-clock: ~112.246153
+-> panel: ~74.830768 MHz
Which leaves us with a vertical refresh rate of ~59.989 Hz,
deviating less then 0.2% from the ideal 60Hz. That's probably closer
than the accumulated accuracy of all involved components can
reliably achieve. I'd say, let's leave it at that.
[1]: https://lore.kernel.org/lkml/20230219114553.288057-2-frank@oltmanns.dev/
[2]: https://gitlab.com/postmarketOS/pmaports/-/merge_requests/4645
Best regards,
Frank
On 2023-12-20 at 16:18:49 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne sreda, 20. december 2023 ob 08:14:27 CET je Frank Oltmanns napisal(a):
quoted
On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
quoted
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
This one is really a tough nut. Unfortunately, the PLL_MIPI clock for
this panel has to run exactly at 6 * panel clock. Let me start by
showing the relevant part of the clock tree (this is on the pinephone
after applying the patches):
pll-video0 393600000
pll-mipi 500945454
tcon0 500945454
tcon-data-clock 125236363
To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit
rate [1]. It's a fixed divisor
The panel I'm proposing to change is defined as this:
static const struct st7703_panel_desc xbd599_desc = {
.mode = &xbd599_mode,
.lanes = 4,
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
.format = MIPI_DSI_FMT_RGB888,
.init_sequence = xbd599_init_sequence,
};
So, we have 24 bpp and 4 lanes. Therefore, the resulting requested
tcon-data-clock rate is
crtc_clock * 1000 * (24 / 4) / 4
tcon-data-clock therefore requests a parent rate of
4 * (crtc_clock * 1000 * (24 / 4) / 4)
The initial 4 is the fixed divisor between tcon0 and tcon-data-clock.
Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi.
Since PLL-MIPI has to run at at least at 500MHz this forces us to have a
crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of
83.502 MHz.
This is much better explanation why this change is needed. Still, I think
adding min and max rate to PLL_MIPI would make sense, so proper rates
are guaranteed.
Anyway, do you know where are all those old values come from? And how did
you come up with new ones? I guess you can't just simply change timings,
there are probably some HW limitations? Do you know if BSP kernel support
this panel and how this situation is solved there?
quoted
If we only changed the constraints on the PLL_MIPI without changing the
panel mode, we end up with a mismatch. This, in turn, would result in
dropped frames, right?
From what I read, I think frame rate would be higher than 60 fps. What
exactly would happen depends on the panel.
Best regards,
Jernej
Dne petek, 22. december 2023 ob 10:10:25 CET je Frank Oltmanns napisal(a):
On 2023-12-20 at 19:57:06 +0100, Frank Oltmanns [off-list ref] wrote:
quoted
Ok, I've done more detailed testing, and it seems this patch results in
lots of dropped frames. I'm sorry for not being more thorough earlier.
I'll do some more testing without this patch and might have to either
remove it from V2 of this series.
I need to see if the same stability can be achieved when running
PLL-MIPI outside its specied range.
I've done some more (load) testing and observing the panel for dropped
frames.
The conclusion I draw from those results is that this patch isn't
necessary for the pinephone. It would be enough to use the correct clock
rate based on the existing values [*]:
- .clock = 69000,
+ .clock = (720 + 40 + 40 + 40) * (1440 + 18 + 10 + 17) * 60 / 1000,
I've asked in the postmarketOS community for a bit more testing. They
already have a merge request that contains these changes [2].
This patch sounds reasonable and IMO should be merged.
Best regards,
Jernej
This means that we would continue to drive PLL-MIPI outside it's
specified range. I have, so far, not experienced any downside of doing
so. It seems enough to fix the ratios that are part of the first four
patches in this series without introducing a min and max rate.
In conclusion, I'll soon (after some more feedback from the fine folks
at postmarketOS) submit a V2 that addresses the fixes requested in the
first four patches of this series. I'll drop the existing PATCH 5 and
replace it with the one I sent in February [1] instead.
After that, just for fun, I'll probably look into min_rate and max_rate
for nkm clocks and which consequences it has on the pinephone. I might
or might not send a follow up series for that. However, if the pinephone
runs stable without it, it's not a high priority for me.
Best regards,
Frank
[*] I've already submitted a patch in February '23 [1]. It was of little
use back then because the A64's PLL-MIPI clock was not able to run
close to that rate. But since kernel 6.6 PLL-MIPI is able to set
it's parent rate, so that it can come quite close to the required
rate:
+ Panel requires 74.844 MHz with the current timings.
+-> tcon-data-clock rate should be 112.266 MHz (panel*24/4/4).
+-> PLL-MIPI rate should be 449.064 MHz (TCON0 * 4)
The 6.6 kernel the following rates are possible:
+ PLL-MIPI: ~448.984615 MHz
+-> tcon-data-clock: ~112.246153
+-> panel: ~74.830768 MHz
Which leaves us with a vertical refresh rate of ~59.989 Hz,
deviating less then 0.2% from the ideal 60Hz. That's probably closer
than the accumulated accuracy of all involved components can
reliably achieve. I'd say, let's leave it at that.
[1]: https://lore.kernel.org/lkml/20230219114553.288057-2-frank@oltmanns.dev/
[2]: https://gitlab.com/postmarketOS/pmaports/-/merge_requests/4645
quoted
Best regards,
Frank
On 2023-12-20 at 16:18:49 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne sreda, 20. december 2023 ob 08:14:27 CET je Frank Oltmanns napisal(a):
quoted
On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
quoted
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
This one is really a tough nut. Unfortunately, the PLL_MIPI clock for
this panel has to run exactly at 6 * panel clock. Let me start by
showing the relevant part of the clock tree (this is on the pinephone
after applying the patches):
pll-video0 393600000
pll-mipi 500945454
tcon0 500945454
tcon-data-clock 125236363
To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit
rate [1]. It's a fixed divisor
The panel I'm proposing to change is defined as this:
static const struct st7703_panel_desc xbd599_desc = {
.mode = &xbd599_mode,
.lanes = 4,
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
.format = MIPI_DSI_FMT_RGB888,
.init_sequence = xbd599_init_sequence,
};
So, we have 24 bpp and 4 lanes. Therefore, the resulting requested
tcon-data-clock rate is
crtc_clock * 1000 * (24 / 4) / 4
tcon-data-clock therefore requests a parent rate of
4 * (crtc_clock * 1000 * (24 / 4) / 4)
The initial 4 is the fixed divisor between tcon0 and tcon-data-clock.
Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi.
Since PLL-MIPI has to run at at least at 500MHz this forces us to have a
crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of
83.502 MHz.
This is much better explanation why this change is needed. Still, I think
adding min and max rate to PLL_MIPI would make sense, so proper rates
are guaranteed.
Anyway, do you know where are all those old values come from? And how did
you come up with new ones? I guess you can't just simply change timings,
there are probably some HW limitations? Do you know if BSP kernel support
this panel and how this situation is solved there?
quoted
If we only changed the constraints on the PLL_MIPI without changing the
panel mode, we end up with a mismatch. This, in turn, would result in
dropped frames, right?
From what I read, I think frame rate would be higher than 60 fps. What
exactly would happen depends on the panel.
Best regards,
Jernej
From: Frank Oltmanns <hidden> Date: 2023-12-30 21:17:29
On 2023-12-20 at 16:18:49 +0100, Jernej Škrabec [off-list ref] wrote:
Dne sreda, 20. december 2023 ob 08:14:27 CET je Frank Oltmanns napisal(a):
quoted
On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a):
quoted
This panel is used in the pinephone that runs on a Allwinner A64 SOC.
Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more
than 500 MHz.
Therefore, change [hv]sync_(start|end) so that we reach a clock rate
that is high enough to drive PLL-MIPI within its limits.
Signed-off-by: Frank Oltmanns <redacted>
I'm not too sure about this patch. I see that PLL_MIPI doesn't have set
minimum frequency limit in clock driver. If you add it, clock framework
should find rate that is high enough and divisible with target rate.
This one is really a tough nut. Unfortunately, the PLL_MIPI clock for
this panel has to run exactly at 6 * panel clock. Let me start by
showing the relevant part of the clock tree (this is on the pinephone
after applying the patches):
pll-video0 393600000
pll-mipi 500945454
tcon0 500945454
tcon-data-clock 125236363
To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit
rate [1]. It's a fixed divisor
The panel I'm proposing to change is defined as this:
static const struct st7703_panel_desc xbd599_desc = {
.mode = &xbd599_mode,
.lanes = 4,
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
.format = MIPI_DSI_FMT_RGB888,
.init_sequence = xbd599_init_sequence,
};
So, we have 24 bpp and 4 lanes. Therefore, the resulting requested
tcon-data-clock rate is
crtc_clock * 1000 * (24 / 4) / 4
tcon-data-clock therefore requests a parent rate of
4 * (crtc_clock * 1000 * (24 / 4) / 4)
The initial 4 is the fixed divisor between tcon0 and tcon-data-clock.
Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi.
Since PLL-MIPI has to run at at least at 500MHz this forces us to have a
crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of
83.502 MHz.
This is much better explanation why this change is needed. Still, I think
adding min and max rate to PLL_MIPI would make sense, so proper rates
are guaranteed.
Okay, I'll include min and max rate in V2, because you're right that
it's the sane thing to do and actually it wasn't too much work. I (and
others) do experience crashes if pll-mipi is driven below the 500 MHz
mark, so let's fix this once and for all.
Anyway, do you know where are all those old values come from?
I've done some digging on lore and the values were originally submitted
by Icenowy Zheng as part of a series to support the pinephone's LCD [1].
There has been some refactoring after this initial submission and Ondrej
Jirman took over. But the values are still the ones submitted by
Icenowy, so I've added her to CC. I couldn't find any documentation for
this specific panel.
And how did
you come up with new ones?
Trial and no error. :)
No, really, it was just a lucky guess. I know nothing about LCD panels,
so I only looked at the original values:
.htotal = 720 + 40 + 40 + 40,
.vtotal = 1440 + 18 + 10 + 17,
I thought, what if every time I increase a horizontal value by 2, I
increase a vertical value by 1 (very roughly).
So I ended up with:
.htotal = 720 + 65 + 65 + 65,
.vtotal = 1440 + 30 + 22 + 29,
So, in conclusion, I've increased each of the horizontal values by 25
and each of the vertical values by 12. Then I just tried out these new
values, and the world didn't end. :)
If this is stupid, please somebody let me know.
I (and at least one postmarket OS tester) have been daily driving the
panel with these values for about a week now.
I've checked the panel's refresh rate with the following test setup:
- I created a 60 fps video that shows the current frame number in each
frame. The video is 10 seconds (600 frames) long. [2]
- I played that video on my pinephone using vlc. [3]
- I recorded the playback with a Google Pixel 5 phone at 1/8 slow
motion (240 fps).
- I converted the video into individual pictures [4], resized
the pictures to 10% [5], and finally - after deleting some superfluous
pictures at the beginning and end - I created one big collage out of
these [6].
I've uploaded the video[7], resulting collage [8] and the individual
pictures [9].
In the resulting picture you can see that in the beginning frame 2 is
missing and frame 136 is only barely visible because it is stuck too
long on frame 135. Other than that, I think this looks pretty good.
I guess you can't just simply change timings,
there are probably some HW limitations? Do you know if BSP kernel support
this panel and how this situation is solved there?
I'm not aware of any BSP kernel that supports this kernel.
quoted
If we only changed the constraints on the PLL_MIPI without changing the
panel mode, we end up with a mismatch. This, in turn, would result in
dropped frames, right?
From what I read, I think frame rate would be higher than 60 fps. What
exactly would happen depends on the panel.
From: Frank Oltmanns <hidden> Date: 2023-12-31 09:17:30
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec [off-list ref] wrote:
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
I just realized that adding the whole ratio limits for ccu_nm is
superfluous as you could just as well express them in for of a minimum
and maximum range:
Since 8 <= N/M <= 25 and parent_rate = 24 MHz, therefore
192 MHz <= rate <= 600 MHz.
These absolute limits are also listed in Allwinner's A64 manual.
BUT, here the upper limit was raised to 1008 MHz:
5de39acaf34604bd04834f092479cf4dcc946dd "clk: sunxi-ng: a64: Add max.
rate constraint to video PLL"
With this upper limit the ratio limitation is effectively:
8 <= N/M <= 42
Icenowy Zheng (added to CC) had the reasonable explanation that this was
used in the BSP kernel, so we should probably stick to that and ditch
the two PLL-VIDEO0 related patches. What are your thoughts on that?
Dne nedelja, 31. december 2023 ob 10:10:40 CET je Frank Oltmanns napisal(a):
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec [off-list ref] wrote:
quoted
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a):
quoted
The Allwinner A64 manual lists the following constraint for the
PLL-VIDEO0 clock: 8 <= N/M <= 25
Use this constraint.
Signed-off-by: Frank Oltmanns <redacted>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
I just realized that adding the whole ratio limits for ccu_nm is
superfluous as you could just as well express them in for of a minimum
and maximum range:
Since 8 <= N/M <= 25 and parent_rate = 24 MHz, therefore
192 MHz <= rate <= 600 MHz.
Good point!
These absolute limits are also listed in Allwinner's A64 manual.
BUT, here the upper limit was raised to 1008 MHz:
5de39acaf34604bd04834f092479cf4dcc946dd "clk: sunxi-ng: a64: Add max.
rate constraint to video PLL"
With this upper limit the ratio limitation is effectively:
8 <= N/M <= 42
Icenowy Zheng (added to CC) had the reasonable explanation that this was
used in the BSP kernel, so we should probably stick to that and ditch
the two PLL-VIDEO0 related patches. What are your thoughts on that?
Ok, it seems that these patches are really superfluous. Remove them for v2.
Best regards,
Jernej