According to Allwinner's BSP code, in DSI mode, TCON0 clock needs to be
running at what's effectively the per-lane datarate of the DSI link.
Given that the TCON DCLK divider is fixed to 4 (SUN6I_DSI_TCON_DIV),
DCLK can't be set equal to the dotclock. Therefore labeling TCON DCLK
as sun4i_dotclock or tcon-pixel-clock shall be avoided.
With bpp bits per pixel transmitted over n DSI lanes, the target DCLK
rate for a given pixel clock is obtained as follows:
DCLK rate = 1/4 * bpp / n * pixel clock
Effect of this change can be observed through the rate of Vblank IRQs
which should now match refresh rate implied by set display mode. It
was verified to do so on a A64 board with a 2-lane and a 4-lane panel.
v2:
1. prevent reparent of tcon0 to pll-video0-2x
2. include pll-video0 in setting TCON0 DCLK rate
3. tested the whole thing also on a PinePhone
v3:
1. accept that pll-video0 can't be included in setting TCON0 DCLK rate
2. reset pll-video0 to its default rate in case u-boot changed it
v4:
1. keep pll-video0 as is
2. assign parent to TCON0 mux in sun50i_a64_ccu_probe (not in DT)
Roman Beranek (4):
clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux
ARM: dts: sunxi: rename tcon's clock output
drm: sun4i: rename sun4i_dotclock to sun4i_tcon_dclk
drm: sun4i: calculate proper DCLK rate for DSI
arch/arm/boot/dts/sun5i.dtsi | 2 +-
arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
arch/arm/boot/dts/sun8i-a83t.dtsi | 2 +-
arch/arm/boot/dts/sun8i-v3s.dtsi | 2 +-
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 2 +-
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++-
drivers/gpu/drm/sun4i/Makefile | 2 +-
drivers/gpu/drm/sun4i/sun4i_tcon.c | 46 +++++++++++--------
.../{sun4i_dotclock.c => sun4i_tcon_dclk.c} | 2 +-
.../{sun4i_dotclock.h => sun4i_tcon_dclk.h} | 0
10 files changed, 46 insertions(+), 28 deletions(-)
rename drivers/gpu/drm/sun4i/{sun4i_dotclock.c => sun4i_tcon_dclk.c} (99%)
rename drivers/gpu/drm/sun4i/{sun4i_dotclock.h => sun4i_tcon_dclk.h} (100%)
base-commit: 8a91b29f1f50ce7742cdbe5cf11d17f128511f3f
--
2.32.0 (Apple Git-132)
TCON0's source clock can be fed from either PLL_MIPI, or PLL_VIDEO0(2X),
however MIPI DSI output only seems to work when PLL_MIPI is selected and
thus the choice must be hardcoded in.
Currently, this driver can't propagate rate change from N-K-M clocks
(such as PLL_MIPI) upwards. This prevents PLL_VIDEO0 from participating
in setting of the TCON0 data clock rate, limiting the precision with
which a target pixel clock can be matched.
For outputs with fixed TCON0 divider, that is DSI and LVDS, the dotclock
can deviate up to 8% off target.
Signed-off-by: Roman Beranek <[email protected]>
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 41519185600a..eb36f8f77d55 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -528,11 +528,18 @@ static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
0x104, 0, 4, 24, 3, BIT(31),
CLK_SET_RATE_PARENT);
+/*
+ * DSI output seems to work only when PLL_MIPI selected. Set it and prevent
+ * the mux from reparenting.
+ */
+#define SUN50I_A64_TCON0_CLK_REG 0x118
+
static const char * const tcon0_parents[] = { "pll-mipi", "pll-video0-2x" };
static const u8 tcon0_table[] = { 0, 2, };
static SUNXI_CCU_MUX_TABLE_WITH_GATE(tcon0_clk, "tcon0", tcon0_parents,
tcon0_table, 0x118, 24, 3, BIT(31),
- CLK_SET_RATE_PARENT);
+ CLK_SET_RATE_PARENT |
+ CLK_SET_RATE_NO_REPARENT);
static const char * const tcon1_parents[] = { "pll-video0", "pll-video1" };
static const u8 tcon1_table[] = { 0, 2, };
@@ -953,6 +960,11 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
+ /* Set PLL MIPI as parent for TCON0 */
+ val = readl(reg + SUN50I_A64_TCON0_CLK_REG);
+ val &= ~GENMASK(26, 24);
+ writel(val | (0 << 24), reg + SUN50I_A64_TCON0_CLK_REG);
+
ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun50i_a64_ccu_desc);
if (ret)
return ret;
--
2.32.0 (Apple Git-132)
In DSI mode, TCON0's data clock is required to run at 1/4 the per-lane
bit rate.
Signed-off-by: Roman Beranek <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 36 +++++++++++++++++-------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eec26b1faa4b..b263de7a8237 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -291,18 +291,6 @@ static int sun4i_tcon_get_clk_delay(const struct drm_display_mode *mode,
return delay;
}
-static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
- const struct drm_display_mode *mode)
-{
- /* Configure the dot clock */
- clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
-
- /* Set the resolution */
- regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
- SUN4I_TCON0_BASIC0_X(mode->crtc_hdisplay) |
- SUN4I_TCON0_BASIC0_Y(mode->crtc_vdisplay));
-}
-
static void sun4i_tcon0_mode_set_dithering(struct sun4i_tcon *tcon,
const struct drm_connector *connector)
{
@@ -367,10 +355,18 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
u32 block_space, start_delay;
u32 tcon_div;
+ /*
+ * dclk is required to run at 1/4 the DSI per-lane bit rate.
+ */
tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
+ clk_set_rate(tcon->dclk, mode->crtc_clock * 1000 * (bpp / lanes)
+ / SUN6I_DSI_TCON_DIV);
- sun4i_tcon0_mode_set_common(tcon, mode);
+ /* Set the resolution */
+ regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
+ SUN4I_TCON0_BASIC0_X(mode->crtc_hdisplay) |
+ SUN4I_TCON0_BASIC0_Y(mode->crtc_vdisplay));
/* Set dithering if needed */
sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
@@ -438,7 +434,12 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
tcon->dclk_min_div = 7;
tcon->dclk_max_div = 7;
- sun4i_tcon0_mode_set_common(tcon, mode);
+ clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
+
+ /* Set the resolution */
+ regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
+ SUN4I_TCON0_BASIC0_X(mode->crtc_hdisplay) |
+ SUN4I_TCON0_BASIC0_Y(mode->crtc_vdisplay));
/* Set dithering if needed */
sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
@@ -515,7 +516,12 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
tcon->dclk_min_div = tcon->quirks->dclk_min_div;
tcon->dclk_max_div = 127;
- sun4i_tcon0_mode_set_common(tcon, mode);
+ clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
+
+ /* Set the resolution */
+ regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
+ SUN4I_TCON0_BASIC0_X(mode->crtc_hdisplay) |
+ SUN4I_TCON0_BASIC0_Y(mode->crtc_vdisplay));
/* Set dithering if needed */
sun4i_tcon0_mode_set_dithering(tcon, connector);
--
2.32.0 (Apple Git-132)
Hi Roman,
On 2023-05-05 at 07:21:06 +0200, Roman Beranek <[email protected]> wrote:
> According to Allwinner's BSP code, in DSI mode, TCON0 clock needs to be
> running at what's effectively the per-lane datarate of the DSI link.
> Given that the TCON DCLK divider is fixed to 4 (SUN6I_DSI_TCON_DIV),
> DCLK can't be set equal to the dotclock. Therefore labeling TCON DCLK
> as sun4i_dotclock or tcon-pixel-clock shall be avoided.
>
> With bpp bits per pixel transmitted over n DSI lanes, the target DCLK
> rate for a given pixel clock is obtained as follows:
>
> DCLK rate = 1/4 * bpp / n * pixel clock
>
> Effect of this change can be observed through the rate of Vblank IRQs
> which should now match refresh rate implied by set display mode. It
> was verified to do so on a A64 board with a 2-lane and a 4-lane panel.
>
> v2:
> 1. prevent reparent of tcon0 to pll-video0-2x
> 2. include pll-video0 in setting TCON0 DCLK rate
> 3. tested the whole thing also on a PinePhone
>
> v3:
> 1. accept that pll-video0 can't be included in setting TCON0 DCLK rate
> 2. reset pll-video0 to its default rate in case u-boot changed it
>
> v4:
> 1. keep pll-video0 as is
> 2. assign parent to TCON0 mux in sun50i_a64_ccu_probe (not in DT)
>
> Roman Beranek (4):
> clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux
> ARM: dts: sunxi: rename tcon's clock output
> drm: sun4i: rename sun4i_dotclock to sun4i_tcon_dclk
> drm: sun4i: calculate proper DCLK rate for DSI
>
> arch/arm/boot/dts/sun5i.dtsi | 2 +-
> arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
> arch/arm/boot/dts/sun8i-a83t.dtsi | 2 +-
> arch/arm/boot/dts/sun8i-v3s.dtsi | 2 +-
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 2 +-
> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++-
> drivers/gpu/drm/sun4i/Makefile | 2 +-
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 46 +++++++++++--------
> .../{sun4i_dotclock.c => sun4i_tcon_dclk.c} | 2 +-
> .../{sun4i_dotclock.h => sun4i_tcon_dclk.h} | 0
> 10 files changed, 46 insertions(+), 28 deletions(-)
> rename drivers/gpu/drm/sun4i/{sun4i_dotclock.c => sun4i_tcon_dclk.c} (99%)
> rename drivers/gpu/drm/sun4i/{sun4i_dotclock.h => sun4i_tcon_dclk.h} (100%)
>
>
> base-commit: 8a91b29f1f50ce7742cdbe5cf11d17f128511f3f
I tested this on my pinephone on drm-next, using additional patches for
the pinephone's panel. [1] [2] [3]
I played back a 60 fps video (10 seconds) and recorded the panel's
output with a 240 fps camera. I noticed only 2 dropped frames, that I
account to the imperfect data rate of 107.8MHz instead of 108 MHz due to
pll-video0's rate being 294MHz instead of the 297 MHz for reasons I
described in the thread on your v2 of this patch [4]).
Tested-by: Frank Oltmanns <[email protected]>
Thanks,
Frank
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/
[3]: https://github.com/megous/linux/commit/e83ffbfe930562257abef1eed4abb8e2251b795a
[4]: https://lore.kernel.org/all/[email protected]/
Hello Frank,
On Mon May 8, 2023 at 10:47 AM CEST, Frank Oltmanns wrote:
> I tested this on my pinephone on drm-next, using additional patches for
> the pinephone's panel. [1] [2] [3]
Thank you for testing this and all the previous version of this
patchset. I appreciate your help.
> I played back a 60 fps video (10 seconds) and recorded the panel's
> output with a 240 fps camera. I noticed only 2 dropped frames, that I
> account to the imperfect data rate of 107.8MHz instead of 108 MHz due to
> pll-video0's rate being 294MHz instead of the 297 MHz for reasons I
> described in the thread on your v2 of this patch [4]).
Yes. That's what should happen, right? Or do you report this as a
defect?
Roman
Hello Roman,
On 2023-05-09 at 13:04:50 +0200, "Roman Beranek" <[email protected]> wrote:
> On Mon May 8, 2023 at 10:47 AM CEST, Frank Oltmanns wrote:
>> I played back a 60 fps video (10 seconds) and recorded the panel's
>> output with a 240 fps camera. I noticed only 2 dropped frames, that I
>> account to the imperfect data rate of 107.8MHz instead of 108 MHz due
>> to pll-video0's rate being 294MHz instead of the 297 MHz for reasons
>> I described in the thread on your v2 of this patch [4]).
>
> Yes. That's what should happen, right? Or do you report this as a
> defect?
Sorry, I didn't communicate more clearly. Without your patch, I'm losing
a number of frames in the realm of three digits. With your patch I only
lost 2 frames which is expetcted. Your patches are a big improvement.
This is the opposite of a defect report. :-)
Thanks,
Frank
>
> Roman
On Fri, May 05, 2023 at 07:21:07AM +0200, Roman Beranek wrote:
> TCON0's source clock can be fed from either PLL_MIPI, or PLL_VIDEO0(2X),
> however MIPI DSI output only seems to work when PLL_MIPI is selected and
> thus the choice must be hardcoded in.
>
> Currently, this driver can't propagate rate change from N-K-M clocks
> (such as PLL_MIPI) upwards. This prevents PLL_VIDEO0 from participating
> in setting of the TCON0 data clock rate, limiting the precision with
> which a target pixel clock can be matched.
>
> For outputs with fixed TCON0 divider, that is DSI and LVDS, the dotclock
> can deviate up to 8% off target.
>
> Signed-off-by: Roman Beranek <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
On Fri, 05 May 2023 07:21:10 +0200, Roman Beranek wrote:
> In DSI mode, TCON0's data clock is required to run at 1/4 the per-lane
> bit rate.
>
>
Applied to drm/drm-misc (drm-misc-next).
Thanks!
Maxime
Dne petek, 05. maj 2023 ob 07:21:07 CEST je Roman Beranek napisal(a):
> TCON0's source clock can be fed from either PLL_MIPI, or PLL_VIDEO0(2X),
> however MIPI DSI output only seems to work when PLL_MIPI is selected and
> thus the choice must be hardcoded in.
>
> Currently, this driver can't propagate rate change from N-K-M clocks
> (such as PLL_MIPI) upwards. This prevents PLL_VIDEO0 from participating
> in setting of the TCON0 data clock rate, limiting the precision with
> which a target pixel clock can be matched.
>
> For outputs with fixed TCON0 divider, that is DSI and LVDS, the dotclock
> can deviate up to 8% off target.
>
> Signed-off-by: Roman Beranek <[email protected]>
Reviewed-by: Jernej Skrabec <[email protected]>
Best regards,
Jernej
Dne petek, 05. maj 2023 ob 07:21:07 CEST je Roman Beranek napisal(a):
> TCON0's source clock can be fed from either PLL_MIPI, or PLL_VIDEO0(2X),
> however MIPI DSI output only seems to work when PLL_MIPI is selected and
> thus the choice must be hardcoded in.
>
> Currently, this driver can't propagate rate change from N-K-M clocks
> (such as PLL_MIPI) upwards. This prevents PLL_VIDEO0 from participating
> in setting of the TCON0 data clock rate, limiting the precision with
> which a target pixel clock can be matched.
>
> For outputs with fixed TCON0 divider, that is DSI and LVDS, the dotclock
> can deviate up to 8% off target.
>
> Signed-off-by: Roman Beranek <[email protected]>
Applied, thanks!
Best regards,
Jernej