Over the year I got plenty of reports of troubles with H6 HDMI signal.
Sometimes monitor flickers, sometimes there was no image at all and
sometimes it didn't play well with AVR.
It turns out there are multiple issues. Patch 1 fixes clock issue,
which didn't adjust parent rate, even if it is allowed to do so. Patch 2
adds polarity config in tcon1. This is seemingly not needed for pre-HDMI2
controllers, although BSP drivers set it accordingly every time. It
turns out that HDMI2 controllers often don't work with monitors if
polarity is not set correctly. Patch 3 always set clock rate for HDMI
controller. Patch 4 fixes cpce PHY setting for 594 MHz. Patch 5 fixes
comment and clock rate limit (wrong reasoning).
Please take a look.
Best regards,
Jernej
Jernej Skrabec (5):
clk: sunxi-ng: mp: fix parent rate change flag check
drm/sun4i: tcon: set sync polarity for tcon1 channel
drm/sun4i: dw-hdmi: always set clock rate
drm/sun4i: Fix H6 HDMI PHY configuration
drm/sun4i: dw-hdmi: Fix max. frequency for H6
drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_tcon.h | 5 +++++
drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 10 +++-------
drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 -
drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 2 +-
6 files changed, 34 insertions(+), 10 deletions(-)
--
2.30.0
CLK_SET_RATE_PARENT flag is checked on parent clock instead of current
one. Fix that.
Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when allowed")
Tested-by: Andre Heider <[email protected]>
Signed-off-by: Jernej Skrabec <[email protected]>
---
drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
index fa4ecb915590..5f40be6d2dfd 100644
--- a/drivers/clk/sunxi-ng/ccu_mp.c
+++ b/drivers/clk/sunxi-ng/ccu_mp.c
@@ -108,7 +108,7 @@ static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux,
max_m = cmp->m.max ?: 1 << cmp->m.width;
max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1);
- if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
+ if (!(clk_hw_get_flags(&cmp->common.hw) & CLK_SET_RATE_PARENT)) {
ccu_mp_find_best(*parent_rate, rate, max_m, max_p, &m, &p);
rate = *parent_rate / p / m;
} else {
--
2.30.0
Channel 1 has polarity bits for vsync and hsync signals but driver never
sets them. It turns out that with pre-HDMI2 controllers seemingly there
is no issue if polarity is not set. However, with HDMI2 controllers
(H6) there often comes to de-synchronization due to phase shift. This
causes flickering screen. It's safe to assume that similar issues might
happen also with pre-HDMI2 controllers.
Solve issue with setting vsync and hsync polarity. Note that display
stacks with tcon top have polarity bits actually in tcon0 polarity
register.
Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Tested-by: Andre Heider <[email protected]>
Signed-off-by: Jernej Skrabec <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_tcon.h | 5 +++++
2 files changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 6b9af4c08cd6..0d132dae58c0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
SUN4I_TCON1_BASIC5_H_SYNC(hsync));
+ /* Setup the polarity of sync signals */
+ if (tcon->quirks->polarity_in_ch0) {
+ val = 0;
+
+ if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+ val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
+
+ if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+ val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
+
+ regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
+ } else {
+ val = SUN4I_TCON1_IO_POL_UNKNOWN;
+
+ if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+ val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
+
+ if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+ val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
+
+ regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
+ }
+
/* Map output pins to channel 1 */
regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
SUN4I_TCON_GCTL_IOMAP_MASK,
@@ -1500,6 +1523,7 @@ static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
.has_channel_1 = true,
+ .polarity_in_ch0 = true,
.set_mux = sun8i_r40_tcon_tv_set_mux,
};
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index c5ac1b02482c..b504fb2d3de5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -154,6 +154,10 @@
#define SUN4I_TCON1_BASIC5_V_SYNC(height) (((height) - 1) & 0x3ff)
#define SUN4I_TCON1_IO_POL_REG 0xf0
+#define SUN4I_TCON1_IO_POL_UNKNOWN BIT(26)
+#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE BIT(25)
+#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE BIT(24)
+
#define SUN4I_TCON1_IO_TRI_REG 0xf4
#define SUN4I_TCON_ECC_FIFO_REG 0xf8
@@ -236,6 +240,7 @@ struct sun4i_tcon_quirks {
bool needs_de_be_mux; /* sun6i needs mux to select backend */
bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */
bool supports_lvds; /* Does the TCON support an LVDS output? */
+ bool polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
u8 dclk_min_div; /* minimum divider for TCON0 DCLK */
/* callback to handle tcon muxing options */
--
2.30.0
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <[email protected]> wrote:
>
> Channel 1 has polarity bits for vsync and hsync signals but driver never
> sets them. It turns out that with pre-HDMI2 controllers seemingly there
> is no issue if polarity is not set. However, with HDMI2 controllers
> (H6) there often comes to de-synchronization due to phase shift. This
> causes flickering screen. It's safe to assume that similar issues might
> happen also with pre-HDMI2 controllers.
>
> Solve issue with setting vsync and hsync polarity. Note that display
> stacks with tcon top have polarity bits actually in tcon0 polarity
> register.
>
> Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> Tested-by: Andre Heider <[email protected]>
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/sun4i/sun4i_tcon.h | 5 +++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 6b9af4c08cd6..0d132dae58c0 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
> SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> SUN4I_TCON1_BASIC5_H_SYNC(hsync));
>
> + /* Setup the polarity of sync signals */
> + if (tcon->quirks->polarity_in_ch0) {
> + val = 0;
> +
> + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> + val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> +
> + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> + val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> +
> + regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> + } else {
> + val = SUN4I_TCON1_IO_POL_UNKNOWN;
I think a comment for the origin of this is warranted.
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
> +
> + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> + val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
> +
> + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> + val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
> +
> + regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
> + }
> +
> /* Map output pins to channel 1 */
> regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
> SUN4I_TCON_GCTL_IOMAP_MASK,
> @@ -1500,6 +1523,7 @@ static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
>
> static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
> .has_channel_1 = true,
> + .polarity_in_ch0 = true,
> .set_mux = sun8i_r40_tcon_tv_set_mux,
> };
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index c5ac1b02482c..b504fb2d3de5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -154,6 +154,10 @@
> #define SUN4I_TCON1_BASIC5_V_SYNC(height) (((height) - 1) & 0x3ff)
>
> #define SUN4I_TCON1_IO_POL_REG 0xf0
> +#define SUN4I_TCON1_IO_POL_UNKNOWN BIT(26)
> +#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE BIT(25)
> +#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE BIT(24)
> +
> #define SUN4I_TCON1_IO_TRI_REG 0xf4
>
> #define SUN4I_TCON_ECC_FIFO_REG 0xf8
> @@ -236,6 +240,7 @@ struct sun4i_tcon_quirks {
> bool needs_de_be_mux; /* sun6i needs mux to select backend */
> bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */
> bool supports_lvds; /* Does the TCON support an LVDS output? */
> + bool polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
> u8 dclk_min_div; /* minimum divider for TCON0 DCLK */
>
> /* callback to handle tcon muxing options */
> --
> 2.30.0
>
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <[email protected]> wrote:
>
> CLK_SET_RATE_PARENT flag is checked on parent clock instead of current
> one. Fix that.
>
> Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when allowed")
> Tested-by: Andre Heider <[email protected]>
> Signed-off-by: Jernej Skrabec <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <[email protected]> wrote:
>
> Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> > On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <[email protected]>
> wrote:
> > > >
> > > > Channel 1 has polarity bits for vsync and hsync signals but driver never
> > > > sets them. It turns out that with pre-HDMI2 controllers seemingly there
> > > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > > (H6) there often comes to de-synchronization due to phase shift. This
> > > > causes flickering screen. It's safe to assume that similar issues might
> > > > happen also with pre-HDMI2 controllers.
> > > >
> > > > Solve issue with setting vsync and hsync polarity. Note that display
> > > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > > register.
> > > >
> > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > > > Tested-by: Andre Heider <[email protected]>
> > > > Signed-off-by: Jernej Skrabec <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 5 +++++
> > > > 2 files changed, 29 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/
> sun4i_tcon.c
> > > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon
> *tcon,
> > > > SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > > SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > > >
> > > > + /* Setup the polarity of sync signals */
> > > > + if (tcon->quirks->polarity_in_ch0) {
> > > > + val = 0;
> > > > +
> > > > + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > + val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > > +
> > > > + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > + val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > +
> > > > + regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> > > > + } else {
> > > > + val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > >
> > > I think a comment for the origin of this is warranted.
> >
> > If it's anything like TCON0, it's the pixel clock polarity
>
> Hard to say, DW HDMI controller has "data enable" polarity along hsync and
> vsync. It could be either or none of those.
>
> What should I write in comment? BSP drivers and documentation use only generic
> names like io2_inv.
Just say that we don't know exactly what it is, but it is required for things
to work properly? Would be interesting to know what happens if you don't set
this bit, but do set VSYNC/HSYNC polarity properly.
ChenYu
Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <[email protected]>
wrote:
> > >
> > > Channel 1 has polarity bits for vsync and hsync signals but driver never
> > > sets them. It turns out that with pre-HDMI2 controllers seemingly there
> > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > (H6) there often comes to de-synchronization due to phase shift. This
> > > causes flickering screen. It's safe to assume that similar issues might
> > > happen also with pre-HDMI2 controllers.
> > >
> > > Solve issue with setting vsync and hsync polarity. Note that display
> > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > register.
> > >
> > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > > Tested-by: Andre Heider <[email protected]>
> > > Signed-off-by: Jernej Skrabec <[email protected]>
> > > ---
> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 5 +++++
> > > 2 files changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/
sun4i_tcon.c
> > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon
*tcon,
> > > SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > >
> > > + /* Setup the polarity of sync signals */
> > > + if (tcon->quirks->polarity_in_ch0) {
> > > + val = 0;
> > > +
> > > + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > + val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > +
> > > + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > + val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > +
> > > + regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> > > + } else {
> > > + val = SUN4I_TCON1_IO_POL_UNKNOWN;
> >
> > I think a comment for the origin of this is warranted.
>
> If it's anything like TCON0, it's the pixel clock polarity
Hard to say, DW HDMI controller has "data enable" polarity along hsync and
vsync. It could be either or none of those.
What should I write in comment? BSP drivers and documentation use only generic
names like io2_inv.
Best regards,
Jernej
On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <[email protected]> wrote:
> >
> > Channel 1 has polarity bits for vsync and hsync signals but driver never
> > sets them. It turns out that with pre-HDMI2 controllers seemingly there
> > is no issue if polarity is not set. However, with HDMI2 controllers
> > (H6) there often comes to de-synchronization due to phase shift. This
> > causes flickering screen. It's safe to assume that similar issues might
> > happen also with pre-HDMI2 controllers.
> >
> > Solve issue with setting vsync and hsync polarity. Note that display
> > stacks with tcon top have polarity bits actually in tcon0 polarity
> > register.
> >
> > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > Tested-by: Andre Heider <[email protected]>
> > Signed-off-by: Jernej Skrabec <[email protected]>
> > ---
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 5 +++++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 6b9af4c08cd6..0d132dae58c0 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
> > SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> >
> > + /* Setup the polarity of sync signals */
> > + if (tcon->quirks->polarity_in_ch0) {
> > + val = 0;
> > +
> > + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > + val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > +
> > + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > + val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > +
> > + regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> > + } else {
> > + val = SUN4I_TCON1_IO_POL_UNKNOWN;
>
> I think a comment for the origin of this is warranted.
If it's anything like TCON0, it's the pixel clock polarity
Maxime