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.
Roman Beranek (3):
drm: sun4i: rename sun4i_dotclock to sun4i_tcon_dclk
ARM: dts: sunxi: rename tcon's clock output
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/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
9 files changed, 33 insertions(+), 27 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%)
--
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)
While the rate of TCON0's DCLK matches dotclock for parallel and LVDS
outputs, this doesn't hold for DSI. The 'D' in DCLK actually stands for
'Data' according to Allwinner's manuals. The clock is mostly referred to
as dclk throughout this driver already anyway, so stick with that.
Signed-off-by: Roman Beranek <[email protected]>
---
drivers/gpu/drm/sun4i/Makefile | 2 +-
drivers/gpu/drm/sun4i/sun4i_tcon.c | 10 +++++-----
.../drm/sun4i/{sun4i_dotclock.c => sun4i_tcon_dclk.c} | 2 +-
.../drm/sun4i/{sun4i_dotclock.h => sun4i_tcon_dclk.h} | 0
4 files changed, 7 insertions(+), 7 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%)
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0d04f2447b01..bad7497a0d11 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -19,7 +19,7 @@ sun8i-mixer-y += sun8i_mixer.o sun8i_ui_layer.o \
sun8i_vi_scaler.o sun8i_csc.o
sun4i-tcon-y += sun4i_crtc.o
-sun4i-tcon-y += sun4i_dotclock.o
+sun4i-tcon-y += sun4i_tcon_dclk.o
sun4i-tcon-y += sun4i_lvds.o
sun4i-tcon-y += sun4i_tcon.o
sun4i-tcon-y += sun4i_rgb.o
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 523a6d787921..eec26b1faa4b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -31,12 +31,12 @@
#include <uapi/drm/drm_mode.h>
#include "sun4i_crtc.h"
-#include "sun4i_dotclock.h"
#include "sun4i_drv.h"
#include "sun4i_lvds.h"
#include "sun4i_rgb.h"
#include "sun4i_tcon.h"
#include "sun6i_mipi_dsi.h"
+#include "sun4i_tcon_dclk.h"
#include "sun8i_tcon_top.h"
#include "sunxi_engine.h"
@@ -1237,14 +1237,14 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
ret = sun4i_tcon_init_irq(dev, tcon);
if (ret) {
dev_err(dev, "Couldn't init our TCON interrupts\n");
- goto err_free_dotclock;
+ goto err_free_dclk;
}
tcon->crtc = sun4i_crtc_init(drm, engine, tcon);
if (IS_ERR(tcon->crtc)) {
dev_err(dev, "Couldn't create our CRTC\n");
ret = PTR_ERR(tcon->crtc);
- goto err_free_dotclock;
+ goto err_free_dclk;
}
if (tcon->quirks->has_channel_0) {
@@ -1264,7 +1264,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
of_node_put(remote);
if (ret < 0)
- goto err_free_dotclock;
+ goto err_free_dclk;
}
if (tcon->quirks->needs_de_be_mux) {
@@ -1290,7 +1290,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
return 0;
-err_free_dotclock:
+err_free_dclk:
if (tcon->quirks->has_channel_0)
sun4i_dclk_free(tcon);
err_free_clocks:
diff --git a/drivers/gpu/drm/sun4i/sun4i_dotclock.c b/drivers/gpu/drm/sun4i/sun4i_tcon_dclk.c
similarity index 99%
rename from drivers/gpu/drm/sun4i/sun4i_dotclock.c
rename to drivers/gpu/drm/sun4i/sun4i_tcon_dclk.c
index 417ade3d2565..03d7de1911cd 100644
--- a/drivers/gpu/drm/sun4i/sun4i_dotclock.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon_dclk.c
@@ -10,7 +10,7 @@
#include <linux/regmap.h>
#include "sun4i_tcon.h"
-#include "sun4i_dotclock.h"
+#include "sun4i_tcon_dclk.h"
struct sun4i_dclk {
struct clk_hw hw;
diff --git a/drivers/gpu/drm/sun4i/sun4i_dotclock.h b/drivers/gpu/drm/sun4i/sun4i_tcon_dclk.h
similarity index 100%
rename from drivers/gpu/drm/sun4i/sun4i_dotclock.h
rename to drivers/gpu/drm/sun4i/sun4i_tcon_dclk.h
--
2.32.0 (Apple Git-132)
While the rate of TCON0's DCLK matches dotclock for parallel and LVDS
outputs, this doesn't hold for DSI. According manuals from Allwinner,
DCLK is an abbrebiation of Data Clock, not dotclock, so go with that
instead.
Signed-off-by: Roman Beranek <[email protected]>
---
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 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 250d6b87ab4d..2f901a013676 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -286,7 +286,7 @@ tcon0: lcd-controller@1c0c000 {
clock-names = "ahb",
"tcon-ch0",
"tcon-ch1";
- clock-output-names = "tcon-pixel-clock";
+ clock-output-names = "tcon-data-clock";
#clock-cells = <0>;
status = "disabled";
diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index f630ab55bb6a..ddc87cc15e51 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -190,7 +190,7 @@ tcon0: lcd-controller@1c0c000 {
clock-names = "ahb",
"tcon-ch0",
"lvds-alt";
- clock-output-names = "tcon-pixel-clock";
+ clock-output-names = "tcon-data-clock";
#clock-cells = <0>;
resets = <&ccu RST_BUS_LCD>,
<&ccu RST_BUS_LVDS>;
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 82fdb04122ca..94eb3bfc989e 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -456,7 +456,7 @@ tcon0: lcd-controller@1c0c000 {
interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
clock-names = "ahb", "tcon-ch0";
- clock-output-names = "tcon-pixel-clock";
+ clock-output-names = "tcon-data-clock";
#clock-cells = <0>;
resets = <&ccu RST_BUS_TCON0>, <&ccu RST_BUS_LVDS>;
reset-names = "lcd", "lvds";
diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
index db194c606fdc..ab2a0e1235e4 100644
--- a/arch/arm/boot/dts/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
@@ -191,7 +191,7 @@ tcon0: lcd-controller@1c0c000 {
<&ccu CLK_TCON0>;
clock-names = "ahb",
"tcon-ch0";
- clock-output-names = "tcon-pixel-clock";
+ clock-output-names = "tcon-data-clock";
#clock-cells = <0>;
resets = <&ccu RST_BUS_TCON0>;
reset-names = "lcd";
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 62f45f71ec65..e3b17575699c 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -407,7 +407,7 @@ tcon0: lcd-controller@1c0c000 {
interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
clock-names = "ahb", "tcon-ch0";
- clock-output-names = "tcon-pixel-clock";
+ clock-output-names = "tcon-data-clock";
#clock-cells = <0>;
resets = <&ccu RST_BUS_TCON0>, <&ccu RST_BUS_LVDS>;
reset-names = "lcd", "lvds";
--
2.32.0 (Apple Git-132)
Hi Roman,
On 2023-03-31 at 13:02:45 +0200, Roman Beranek <[email protected]> wrote:
> 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);
When apply this to drm-next my panel stays dark. I haven't figured out
yet why, though. The other two patches in this series work fine, i.e.
they have no effect as they are just a refactoring.
I'm testing this on my pinephone. It's the same with the patch I
submitted. For whatever reason, it no longer works on drm-next.
At the time I'm writing this, drm-next is at 82bbec189ab3 "Merge
v6.3-rc4 into drm-next".
Does it work for you? And if so, on which commit are you basing this
series?
Thanks,
Frank
>
> - 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);
--
--
Frank Oltmanns
On Sun Apr 2, 2023 at 12:49 PM CEST, Frank Oltmanns wrote:
>
> When apply this to drm-next my panel stays dark. I haven't figured out
> yet why, though. The other two patches in this series work fine, i.e.
> they have no effect as they are just a refactoring.
>
> I'm testing this on my pinephone. It's the same with the patch I
> submitted. For whatever reason, it no longer works on drm-next.
I've reproduced the issue on my PinePhone and noticed that tcon0 had set
pll-video0-2x as its parent instead of pll-mipi. Having tried a whole
range of pll-video0 rates, I'm now convinced that DSI only works when
tcon0 has pll-mipi as its parent.
As little a change as setting .clock in the default mode of PP's panel
to 73500 can fix it. Better yet, dropping pll-video0-2x from the set
of acceptable parents for tcon0 fixes it universally. And that's what
megi's kernel does, though the measure was introduced with a different
rationale:
<https://github.com/megous/linux/commit/7374d5756aa0cc3f11e494e3cbc54f6c7c01e1a8>
Roman
On 2023-04-03 at 15:52:36 +0200, "Roman Beranek" <[email protected]> wrote:
> On Sun Apr 2, 2023 at 12:49 PM CEST, Frank Oltmanns wrote:
>>
>> When apply this to drm-next my panel stays dark. I haven't figured out
>> yet why, though. The other two patches in this series work fine, i.e.
>> they have no effect as they are just a refactoring.
>>
>> I'm testing this on my pinephone. It's the same with the patch I
>> submitted. For whatever reason, it no longer works on drm-next.
>
> I've reproduced the issue on my PinePhone and noticed that tcon0 had set
> pll-video0-2x as its parent instead of pll-mipi. Having tried a whole
> range of pll-video0 rates, I'm now convinced that DSI only works when
> tcon0 has pll-mipi as its parent.
>
> As little a change as setting .clock in the default mode of PP's panel
> to 73500 can fix it. Better yet, dropping pll-video0-2x from the set
> of acceptable parents for tcon0 fixes it universally. And that's what
> megi's kernel does, though the measure was introduced with a different
> rationale:
> <https://github.com/megous/linux/commit/7374d5756aa0cc3f11e494e3cbc54f6c7c01e1a8>
For sake of completeness, the patch you referenced builds on this patch:
https://github.com/megous/linux/commit/45e0aa8d9e34
Are you saying that your other boards and panels work without these
patches?
Best regards,
Frank
>
> Roman
On Mon Apr 3, 2023 at 5:08 PM CEST, Frank Oltmanns wrote:
>
> On 2023-04-03 at 15:52:36 +0200, "Roman Beranek" <[email protected]> wrote:
> > As little a change as setting .clock in the default mode of PP's panel
> > to 73500 can fix it. Better yet, dropping pll-video0-2x from the set
> > of acceptable parents for tcon0 fixes it universally. And that's what
> > megi's kernel does, though the measure was introduced with a different
> > rationale:
> > <https://github.com/megous/linux/commit/7374d5756aa0cc3f11e494e3cbc54f6c7c01e1a8>
>
> For sake of completeness, the patch you referenced builds on this patch:
> https://github.com/megous/linux/commit/45e0aa8d9e34
>
> Are you saying that your other boards and panels work without these
> patches?
Yes, that was a bit of an oversight on my side as I wrote drivers for
both panels already with the intention of them being used besides
an HDMI output in mind, so I've deliberately picked a timing in each
case such that the dotclock lines up nicely with pll-video0 at 297 MHz.
All the best
Roman Beranek