2023-12-18 13:51:11

by Frank Oltmanns

[permalink] [raw]
Subject: [PATCH 0/5] Pinephone video out fixes (flipping between two frames)

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 <[email protected]>
---
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 <[email protected]>



2023-12-18 13:51:38

by Frank Oltmanns

[permalink] [raw]
Subject: [PATCH 3/5] clk: sunxi-ng: nm: Support constraints on n/m ratio and parent rate

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 <[email protected]>
---
drivers/clk/sunxi-ng/ccu_nm.c | 21 +++++++++++++++++++--
drivers/clk/sunxi-ng/ccu_nm.h | 34 ++++++++++++++++++++++++++++++++--
2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu_nm.c b/drivers/clk/sunxi-ng/ccu_nm.c
index ffac3deb89d6..cfc6981e398b 100644
--- a/drivers/clk/sunxi-ng/ccu_nm.c
+++ b/drivers/clk/sunxi-ng/ccu_nm.c
@@ -27,6 +27,19 @@ static unsigned long ccu_nm_calc_rate(unsigned long parent,
return rate;
}

+static bool ccu_nm_is_valid_rate(struct ccu_common *common, unsigned long n, unsigned long m)
+{
+ struct ccu_nm *nm = container_of(common, struct ccu_nm, common);
+
+ if (nm->max_nm_ratio && (n > nm->max_nm_ratio * m))
+ return false;
+
+ if (nm->min_nm_ratio && (n < nm->min_nm_ratio * m))
+ return false;
+
+ return true;
+}
+
static unsigned long ccu_nm_find_best(struct ccu_common *common, unsigned long parent,
unsigned long rate, struct _ccu_nm *nm)
{
@@ -36,8 +49,12 @@ static unsigned long ccu_nm_find_best(struct ccu_common *common, unsigned long p

for (_n = nm->min_n; _n <= nm->max_n; _n++) {
for (_m = nm->min_m; _m <= nm->max_m; _m++) {
- unsigned long tmp_rate = ccu_nm_calc_rate(parent,
- _n, _m);
+ unsigned long tmp_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;
diff --git a/drivers/clk/sunxi-ng/ccu_nm.h b/drivers/clk/sunxi-ng/ccu_nm.h
index 93c11693574f..0075df6d9697 100644
--- a/drivers/clk/sunxi-ng/ccu_nm.h
+++ b/drivers/clk/sunxi-ng/ccu_nm.h
@@ -31,6 +31,8 @@ struct ccu_nm {
unsigned int fixed_post_div;
unsigned int min_rate;
unsigned int max_rate;
+ unsigned long min_nm_ratio; /* minimum value for m/n */
+ unsigned long max_nm_ratio; /* maximum value for m/n */

struct ccu_common common;
};
@@ -108,7 +110,8 @@ struct ccu_nm {
}, \
}

-#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT(_struct, _name, \
+#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO( \
+ _struct, _name, \
_parent, _reg, \
_min_rate, _max_rate, \
_nshift, _nwidth, \
@@ -117,7 +120,9 @@ struct ccu_nm {
_frac_rate_0, \
_frac_rate_1, \
_gate, _lock, _flags, \
- _features) \
+ _features, \
+ _min_nm_ratio, \
+ _max_nm_ratio) \
struct ccu_nm _struct = { \
.enable = _gate, \
.lock = _lock, \
@@ -128,6 +133,8 @@ struct ccu_nm {
_frac_rate_1), \
.min_rate = _min_rate, \
.max_rate = _max_rate, \
+ .min_nm_ratio = _min_nm_ratio, \
+ .max_nm_ratio = _max_nm_ratio, \
.common = { \
.reg = _reg, \
.features = _features, \
@@ -138,6 +145,29 @@ struct ccu_nm {
}, \
}

+#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT(_struct, _name, \
+ _parent, _reg, \
+ _min_rate, _max_rate, \
+ _nshift, _nwidth, \
+ _mshift, _mwidth, \
+ _frac_en, _frac_sel, \
+ _frac_rate_0, \
+ _frac_rate_1, \
+ _gate, _lock, _flags, \
+ _features) \
+ SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO( \
+ _struct, _name, \
+ _parent, _reg, \
+ _min_rate, _max_rate, \
+ _nshift, _nwidth, \
+ _mshift, _mwidth, \
+ _frac_en, _frac_sel, \
+ _frac_rate_0, \
+ _frac_rate_1, \
+ _gate, _lock, _flags, \
+ _features, \
+ 0, 0)
+
#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX(_struct, _name, \
_parent, _reg, \
_min_rate, _max_rate, \

--
2.43.0


2023-12-19 16:52:56

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH 3/5] clk: sunxi-ng: nm: Support constraints on n/m ratio and parent rate

Dne ponedeljek, 18. december 2023 ob 14:35:21 CET je Frank Oltmanns napisal(a):
> 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 <[email protected]>
> ---
> drivers/clk/sunxi-ng/ccu_nm.c | 21 +++++++++++++++++++--
> drivers/clk/sunxi-ng/ccu_nm.h | 34 ++++++++++++++++++++++++++++++++--
> 2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_nm.c b/drivers/clk/sunxi-ng/ccu_nm.c
> index ffac3deb89d6..cfc6981e398b 100644
> --- a/drivers/clk/sunxi-ng/ccu_nm.c
> +++ b/drivers/clk/sunxi-ng/ccu_nm.c
> @@ -27,6 +27,19 @@ static unsigned long ccu_nm_calc_rate(unsigned long parent,
> return rate;
> }
>
> +static bool ccu_nm_is_valid_rate(struct ccu_common *common, unsigned long n, unsigned long m)
> +{
> + struct ccu_nm *nm = container_of(common, struct ccu_nm, common);
> +
> + if (nm->max_nm_ratio && (n > nm->max_nm_ratio * m))
> + return false;
> +
> + if (nm->min_nm_ratio && (n < nm->min_nm_ratio * m))
> + return false;
> +
> + return true;
> +}
> +
> static unsigned long ccu_nm_find_best(struct ccu_common *common, unsigned long parent,
> unsigned long rate, struct _ccu_nm *nm)
> {
> @@ -36,8 +49,12 @@ static unsigned long ccu_nm_find_best(struct ccu_common *common, unsigned long p
>
> for (_n = nm->min_n; _n <= nm->max_n; _n++) {
> for (_m = nm->min_m; _m <= nm->max_m; _m++) {
> - unsigned long tmp_rate = ccu_nm_calc_rate(parent,
> - _n, _m);
> + unsigned long tmp_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;
> diff --git a/drivers/clk/sunxi-ng/ccu_nm.h b/drivers/clk/sunxi-ng/ccu_nm.h
> index 93c11693574f..0075df6d9697 100644
> --- a/drivers/clk/sunxi-ng/ccu_nm.h
> +++ b/drivers/clk/sunxi-ng/ccu_nm.h
> @@ -31,6 +31,8 @@ struct ccu_nm {
> unsigned int fixed_post_div;
> unsigned int min_rate;
> unsigned int max_rate;
> + unsigned long min_nm_ratio; /* minimum value for m/n */
> + unsigned long max_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

>
> struct ccu_common common;
> };
> @@ -108,7 +110,8 @@ struct ccu_nm {
> }, \
> }
>
> -#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT(_struct, _name, \
> +#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO( \
> + _struct, _name, \
> _parent, _reg, \
> _min_rate, _max_rate, \
> _nshift, _nwidth, \
> @@ -117,7 +120,9 @@ struct ccu_nm {
> _frac_rate_0, \
> _frac_rate_1, \
> _gate, _lock, _flags, \
> - _features) \
> + _features, \
> + _min_nm_ratio, \
> + _max_nm_ratio) \
> struct ccu_nm _struct = { \
> .enable = _gate, \
> .lock = _lock, \
> @@ -128,6 +133,8 @@ struct ccu_nm {
> _frac_rate_1), \
> .min_rate = _min_rate, \
> .max_rate = _max_rate, \
> + .min_nm_ratio = _min_nm_ratio, \
> + .max_nm_ratio = _max_nm_ratio, \
> .common = { \
> .reg = _reg, \
> .features = _features, \
> @@ -138,6 +145,29 @@ struct ccu_nm {
> }, \
> }
>
> +#define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT(_struct, _name, \
> + _parent, _reg, \
> + _min_rate, _max_rate, \
> + _nshift, _nwidth, \
> + _mshift, _mwidth, \
> + _frac_en, _frac_sel, \
> + _frac_rate_0, \
> + _frac_rate_1, \
> + _gate, _lock, _flags, \
> + _features) \
> + SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO( \
> + _struct, _name, \
> + _parent, _reg, \
> + _min_rate, _max_rate, \
> + _nshift, _nwidth, \
> + _mshift, _mwidth, \
> + _frac_en, _frac_sel, \
> + _frac_rate_0, \
> + _frac_rate_1, \
> + _gate, _lock, _flags, \
> + _features, \
> + 0, 0)
> +
> #define SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX(_struct, _name, \
> _parent, _reg, \
> _min_rate, _max_rate, \
>
>