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 is outside its limits, and the GPU is not running at a fixed rate.
In this patch series I propose the following changes: 1. sunxi-ng: Adhere to the following constraints given in the Allwinner A64 Manual regarding PLL-MIPI: * M/N <= 3 * (PLL_VIDEO0)/M >= 24MHz * 500MHz <= clockrate <= 1400MHz
2. Remove two operating points from the A64 DTS OPPs, so that the GPU runs at a fixed rate of 432 MHz.
Note, that when pinning the GPU to 432 MHz the issue [1] completely disappears for me. I've searched the BSP and could not find any indication that supports the idea of having the three OPPs. The only frequency I found in the BPSs for A64 is 432 MHz, which has also proven stable for me.
I very much appreciate your feedback!
[1] https://gitlab.com/postmarketOS/pmaports/-/issues/805
Signed-off-by: Frank Oltmanns frank@oltmanns.dev --- Changes in v4: - sunxi-ng: common: Address review comments. - Link to v3: https://lore.kernel.org/r/20240304-pinephone-pll-fixes-v3-0-94ab828f269a@olt...
Changes in v3: - dts: Pin GPU to 432 MHz. - nkm and a64: Move minimum and maximum rate handling to the common part of the sunxi-ng driver. - Removed st7703 patch from series. - Link to v2: https://lore.kernel.org/r/20240205-pinephone-pll-fixes-v2-0-96a46a2d8c9b@olt...
Changes in v2: - dts: Increase minimum GPU frequency to 192 MHz. - nkm and a64: Add minimum and maximum rate for PLL-MIPI. - nkm: Use the same approach for skipping invalid rates in ccu_nkm_find_best() as in ccu_nkm_find_best_with_parent_adj(). - nkm: Improve names for ratio struct members and hence get rid of describing comments. - nkm and a64: Correct description in the commit messages: M/N <= 3 - Remove patches for nm as they were not needed. - st7703: Rework the commit message to cover more background for the change. - Link to v1: https://lore.kernel.org/r/20231218-pinephone-pll-fixes-v1-0-e238b6ed6dc1@olt...
--- Frank Oltmanns (5): clk: sunxi-ng: common: Support minimum and maximum rate clk: sunxi-ng: a64: Set minimum and maximum rate for PLL-MIPI 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 arm64: dts: allwinner: a64: Run GPU at 432 MHz
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 -------- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++++++----- drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ drivers/clk/sunxi-ng/ccu_common.h | 3 +++ drivers/clk/sunxi-ng/ccu_nkm.c | 21 +++++++++++++++++++++ drivers/clk/sunxi-ng/ccu_nkm.h | 2 ++ 6 files changed, 54 insertions(+), 13 deletions(-) --- base-commit: dcb6c8ee6acc6c347caec1e73fb900c0f4ff9806 change-id: 20231218-pinephone-pll-fixes-0ccdfde273e4
Best regards,
The Allwinner SoC's typically have an upper and lower limit for their clocks' rates. Up until now, support for that has been implemented separately for each clock type.
Implement that functionality in the sunxi-ng's common part making use of the CCF rate liming capabilities, so that it is available for all clock types.
Suggested-by: Maxime Ripard mripard@kernel.org Signed-off-by: Frank Oltmanns frank@oltmanns.dev Cc: stable@vger.kernel.org --- drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ drivers/clk/sunxi-ng/ccu_common.h | 3 +++ 2 files changed, 22 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c index 8babce55302f..ac0091b4ce24 100644 --- a/drivers/clk/sunxi-ng/ccu_common.c +++ b/drivers/clk/sunxi-ng/ccu_common.c @@ -44,6 +44,16 @@ bool ccu_is_better_rate(struct ccu_common *common, unsigned long current_rate, unsigned long best_rate) { + unsigned long min_rate, max_rate; + + clk_hw_get_rate_range(&common->hw, &min_rate, &max_rate); + + if (current_rate > max_rate) + return false; + + if (current_rate < min_rate) + return false; + if (common->features & CCU_FEATURE_CLOSEST_RATE) return abs(current_rate - target_rate) < abs(best_rate - target_rate);
@@ -122,6 +132,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
for (i = 0; i < desc->hw_clks->num ; i++) { struct clk_hw *hw = desc->hw_clks->hws[i]; + struct ccu_common *common = hw_to_ccu_common(hw); const char *name;
if (!hw) @@ -136,6 +147,14 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev, pr_err("Couldn't register clock %d - %s\n", i, name); goto err_clk_unreg; } + + if (common->max_rate) + clk_hw_set_rate_range(hw, common->min_rate, + common->max_rate); + else + WARN(common->min_rate, + "No max_rate, ignoring min_rate of clock %d - %s\n", + i, name); }
ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h index 942a72c09437..329734f8cf42 100644 --- a/drivers/clk/sunxi-ng/ccu_common.h +++ b/drivers/clk/sunxi-ng/ccu_common.h @@ -31,6 +31,9 @@ struct ccu_common { u16 lock_reg; u32 prediv;
+ unsigned long min_rate; + unsigned long max_rate; + unsigned long features; spinlock_t *lock; struct clk_hw hw;
Dne nedelja, 10. marec 2024 ob 14:21:11 CET je Frank Oltmanns napisal(a):
The Allwinner SoC's typically have an upper and lower limit for their clocks' rates. Up until now, support for that has been implemented separately for each clock type.
Implement that functionality in the sunxi-ng's common part making use of the CCF rate liming capabilities, so that it is available for all clock types.
Suggested-by: Maxime Ripard mripard@kernel.org Signed-off-by: Frank Oltmanns frank@oltmanns.dev Cc: stable@vger.kernel.org
This looks pretty nice now.
Reviewed-by: Jernej Skrabec jernej.skrabec@gmail.com
Best regards, Jernej
drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ drivers/clk/sunxi-ng/ccu_common.h | 3 +++ 2 files changed, 22 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c index 8babce55302f..ac0091b4ce24 100644 --- a/drivers/clk/sunxi-ng/ccu_common.c +++ b/drivers/clk/sunxi-ng/ccu_common.c @@ -44,6 +44,16 @@ bool ccu_is_better_rate(struct ccu_common *common, unsigned long current_rate, unsigned long best_rate) {
- unsigned long min_rate, max_rate;
- clk_hw_get_rate_range(&common->hw, &min_rate, &max_rate);
- if (current_rate > max_rate)
return false;- if (current_rate < min_rate)
return false;- if (common->features & CCU_FEATURE_CLOSEST_RATE) return abs(current_rate - target_rate) < abs(best_rate - target_rate);
@@ -122,6 +132,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev, for (i = 0; i < desc->hw_clks->num ; i++) { struct clk_hw *hw = desc->hw_clks->hws[i];
struct ccu_common *common = hw_to_ccu_common(hw);if (!hw) @@ -136,6 +147,14 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev, pr_err("Couldn't register clock %d - %s\n", i, name); goto err_clk_unreg; }
if (common->max_rate)clk_hw_set_rate_range(hw, common->min_rate,common->max_rate);elseWARN(common->min_rate,"No max_rate, ignoring min_rate of clock %d - %s\n",i, name);ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h index 942a72c09437..329734f8cf42 100644 --- a/drivers/clk/sunxi-ng/ccu_common.h +++ b/drivers/clk/sunxi-ng/ccu_common.h @@ -31,6 +31,9 @@ struct ccu_common { u16 lock_reg; u32 prediv;
- unsigned long min_rate;
- unsigned long max_rate;
- unsigned long features; spinlock_t *lock; struct clk_hw hw;
On Sun, 10 Mar 2024 14:21:11 +0100, Frank Oltmanns wrote:
The Allwinner SoC's typically have an upper and lower limit for their clocks' rates. Up until now, support for that has been implemented separately for each clock type.
Implement that functionality in the sunxi-ng's common part making use of
[ ... ]
Acked-by: Maxime Ripard mripard@kernel.org
Thanks! Maxime
When the Allwinner A64's TCON0 searches the ideal rate for the connected panel, it may happen that it requests a rate from its parent PLL-MIPI which PLL-MIPI does not support.
This happens for example on the Olimex TERES-I laptop where TCON0 requests PLL-MIPI to change to a rate of several GHz which causes the panel to stay blank. It also happens on the pinephone where a rate of less than 500 MHz is requested which causes instabilities on some phones.
Set the minimum and maximum rate of Allwinner A64's PLL-MIPI according to the Allwinner User Manual.
Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux") Reported-by: Diego Roversi diegor@tiscali.it Closes: https://groups.google.com/g/linux-sunxi/c/Rh-Uqqa66bw Tested-by: Diego Roversi diegor@tiscali.it Cc: stable@vger.kernel.org Reviewed-by: Maxime Ripard mripard@kernel.org Signed-off-by: Frank Oltmanns frank@oltmanns.dev --- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c index 8951ffc14ff5..6a4b2b9ef30a 100644 --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c @@ -182,6 +182,8 @@ static struct ccu_nkm pll_mipi_clk = { &ccu_nkm_ops, CLK_SET_RATE_UNGATE | CLK_SET_RATE_PARENT), .features = CCU_FEATURE_CLOSEST_RATE, + .min_rate = 500000000, + .max_rate = 1400000000, }, };
Dne nedelja, 10. marec 2024 ob 14:21:12 CET je Frank Oltmanns napisal(a):
When the Allwinner A64's TCON0 searches the ideal rate for the connected panel, it may happen that it requests a rate from its parent PLL-MIPI which PLL-MIPI does not support.
This happens for example on the Olimex TERES-I laptop where TCON0 requests PLL-MIPI to change to a rate of several GHz which causes the panel to stay blank. It also happens on the pinephone where a rate of less than 500 MHz is requested which causes instabilities on some phones.
Set the minimum and maximum rate of Allwinner A64's PLL-MIPI according to the Allwinner User Manual.
Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux") Reported-by: Diego Roversi diegor@tiscali.it Closes: https://groups.google.com/g/linux-sunxi/c/Rh-Uqqa66bw Tested-by: Diego Roversi diegor@tiscali.it Cc: stable@vger.kernel.org Reviewed-by: Maxime Ripard mripard@kernel.org Signed-off-by: Frank Oltmanns frank@oltmanns.dev
Reviewed-by: Jernej Skrabec jernej.skrabec@gmail.com
Best regards, Jernej
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c index 8951ffc14ff5..6a4b2b9ef30a 100644 --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c @@ -182,6 +182,8 @@ static struct ccu_nkm pll_mipi_clk = { &ccu_nkm_ops, CLK_SET_RATE_UNGATE | CLK_SET_RATE_PARENT), .features = CCU_FEATURE_CLOSEST_RATE,
.min_rate = 500000000,.max_rate = 1400000000,};
Dear clk and sunxi-ng maintainers,
Patches 1-4 have been reviewed and there are no pending issues. If there is something else you need me to do to get this applied, please let me know.
Thanks, Frank
On 2024-03-10 at 14:21:10 +0100, Frank Oltmanns frank@oltmanns.dev wrote:
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 is outside its limits, and the GPU is not running at a fixed rate.
In this patch series I propose the following changes:
sunxi-ng: Adhere to the following constraints given in the Allwinner A64 Manual regarding PLL-MIPI:
- M/N <= 3
- (PLL_VIDEO0)/M >= 24MHz
- 500MHz <= clockrate <= 1400MHz
Remove two operating points from the A64 DTS OPPs, so that the GPU runs at a fixed rate of 432 MHz.
Note, that when pinning the GPU to 432 MHz the issue [1] completely disappears for me. I've searched the BSP and could not find any indication that supports the idea of having the three OPPs. The only frequency I found in the BPSs for A64 is 432 MHz, which has also proven stable for me.
I very much appreciate your feedback!
[1] https://gitlab.com/postmarketOS/pmaports/-/issues/805
Signed-off-by: Frank Oltmanns frank@oltmanns.dev
Changes in v4:
- sunxi-ng: common: Address review comments.
- Link to v3: https://lore.kernel.org/r/20240304-pinephone-pll-fixes-v3-0-94ab828f269a@olt...
Changes in v3:
- dts: Pin GPU to 432 MHz.
- nkm and a64: Move minimum and maximum rate handling to the common part of the sunxi-ng driver.
- Removed st7703 patch from series.
- Link to v2: https://lore.kernel.org/r/20240205-pinephone-pll-fixes-v2-0-96a46a2d8c9b@olt...
Changes in v2:
- dts: Increase minimum GPU frequency to 192 MHz.
- nkm and a64: Add minimum and maximum rate for PLL-MIPI.
- nkm: Use the same approach for skipping invalid rates in ccu_nkm_find_best() as in ccu_nkm_find_best_with_parent_adj().
- nkm: Improve names for ratio struct members and hence get rid of describing comments.
- nkm and a64: Correct description in the commit messages: M/N <= 3
- Remove patches for nm as they were not needed.
- st7703: Rework the commit message to cover more background for the change.
- Link to v1: https://lore.kernel.org/r/20231218-pinephone-pll-fixes-v1-0-e238b6ed6dc1@olt...
Frank Oltmanns (5): clk: sunxi-ng: common: Support minimum and maximum rate clk: sunxi-ng: a64: Set minimum and maximum rate for PLL-MIPI 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 arm64: dts: allwinner: a64: Run GPU at 432 MHz
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 -------- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++++++----- drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ drivers/clk/sunxi-ng/ccu_common.h | 3 +++ drivers/clk/sunxi-ng/ccu_nkm.c | 21 +++++++++++++++++++++ drivers/clk/sunxi-ng/ccu_nkm.h | 2 ++ 6 files changed, 54 insertions(+), 13 deletions(-)
base-commit: dcb6c8ee6acc6c347caec1e73fb900c0f4ff9806 change-id: 20231218-pinephone-pll-fixes-0ccdfde273e4
Best regards,
Quoting Frank Oltmanns (2024-04-03 08:31:47)
Dear clk and sunxi-ng maintainers,
Patches 1-4 have been reviewed and there are no pending issues. If there is something else you need me to do to get this applied, please let me know.
I'm assuming sunxi maintainers will pick up the clk patches and send them to clk tree in a PR.
Dne sreda, 3. april 2024 ob 17:31:47 GMT +2 je Frank Oltmanns napisal(a):
Dear clk and sunxi-ng maintainers,
Patches 1-4 have been reviewed and there are no pending issues. If there is something else you need me to do to get this applied, please let me know.
Sorry for late patch merge. Patch 1-2 are applied as a fix to 6.9, the rest will go to 6.10.
Best regards, Jernej
Thanks, Frank
On 2024-03-10 at 14:21:10 +0100, Frank Oltmanns frank@oltmanns.dev wrote:
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 is outside its limits, and the GPU is not running at a fixed rate.
In this patch series I propose the following changes:
sunxi-ng: Adhere to the following constraints given in the Allwinner A64 Manual regarding PLL-MIPI:
- M/N <= 3
- (PLL_VIDEO0)/M >= 24MHz
- 500MHz <= clockrate <= 1400MHz
Remove two operating points from the A64 DTS OPPs, so that the GPU runs at a fixed rate of 432 MHz.
Note, that when pinning the GPU to 432 MHz the issue [1] completely disappears for me. I've searched the BSP and could not find any indication that supports the idea of having the three OPPs. The only frequency I found in the BPSs for A64 is 432 MHz, which has also proven stable for me.
I very much appreciate your feedback!
[1] https://gitlab.com/postmarketOS/pmaports/-/issues/805
Signed-off-by: Frank Oltmanns frank@oltmanns.dev
Changes in v4:
- sunxi-ng: common: Address review comments.
- Link to v3: https://lore.kernel.org/r/20240304-pinephone-pll-fixes-v3-0-94ab828f269a@olt...
Changes in v3:
- dts: Pin GPU to 432 MHz.
- nkm and a64: Move minimum and maximum rate handling to the common part of the sunxi-ng driver.
- Removed st7703 patch from series.
- Link to v2: https://lore.kernel.org/r/20240205-pinephone-pll-fixes-v2-0-96a46a2d8c9b@olt...
Changes in v2:
- dts: Increase minimum GPU frequency to 192 MHz.
- nkm and a64: Add minimum and maximum rate for PLL-MIPI.
- nkm: Use the same approach for skipping invalid rates in ccu_nkm_find_best() as in ccu_nkm_find_best_with_parent_adj().
- nkm: Improve names for ratio struct members and hence get rid of describing comments.
- nkm and a64: Correct description in the commit messages: M/N <= 3
- Remove patches for nm as they were not needed.
- st7703: Rework the commit message to cover more background for the change.
- Link to v1: https://lore.kernel.org/r/20231218-pinephone-pll-fixes-v1-0-e238b6ed6dc1@olt...
Frank Oltmanns (5): clk: sunxi-ng: common: Support minimum and maximum rate clk: sunxi-ng: a64: Set minimum and maximum rate for PLL-MIPI 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 arm64: dts: allwinner: a64: Run GPU at 432 MHz
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 -------- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++++++----- drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ drivers/clk/sunxi-ng/ccu_common.h | 3 +++ drivers/clk/sunxi-ng/ccu_nkm.c | 21 +++++++++++++++++++++ drivers/clk/sunxi-ng/ccu_nkm.h | 2 ++ 6 files changed, 54 insertions(+), 13 deletions(-)
base-commit: dcb6c8ee6acc6c347caec1e73fb900c0f4ff9806 change-id: 20231218-pinephone-pll-fixes-0ccdfde273e4
Best regards,
linux-stable-mirror@lists.linaro.org
-
Frank Oltmanns -
Jernej Škrabec -
Maxime Ripard -
Stephen Boyd