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(-)
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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- 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 {
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net 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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
Reviewed-by: Chen-Yu Tsai wens@csie.org
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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- 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 */
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net 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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
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 wens@csie.org
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 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net 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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
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
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 jernej.skrabec@siol.net
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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
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 Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec jernej.skrabec@siol.net 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 jernej.skrabec@siol.net
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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
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:28:23 CET je Chen-Yu Tsai napisal(a):
On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec jernej.skrabec@siol.net
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
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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
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.
Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At least I didn't notice anything.
Best regards, Jernej
On Fri, Feb 05, 2021 at 07:47:17PM +0100, Jernej Škrabec wrote:
Dne petek, 05. februar 2021 ob 17:28:23 CET je Chen-Yu Tsai napisal(a):
On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec jernej.skrabec@siol.net
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
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 a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
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.
Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At least I didn't notice anything.
That's pretty normal, an inverted pixel clock would at worst give you some weird artifacts and / or pixels being of the wrong color. Data enable on the other hand would very likely stall the HDMI controller since you would have only the blanking periods that would be considered valid.
Maxime
As expected, HDMI controller clock should always match pixel clock. In the past, changing HDMI controller rate would seemingly worsen situation. However, that was the result of other bugs which are now fixed.
Fix that by removing set_rate quirk and always set clock rate.
Fixes: 40bb9d3147b2 ("drm/sun4i: Add support for H6 DW HDMI controller") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 4 +--- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 92add2cef2e7..23773a5e0650 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -21,8 +21,7 @@ static void sun8i_dw_hdmi_encoder_mode_set(struct drm_encoder *encoder, { struct sun8i_dw_hdmi *hdmi = encoder_to_sun8i_dw_hdmi(encoder);
- if (hdmi->quirks->set_rate) - clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000); + clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000); }
static const struct drm_encoder_helper_funcs @@ -295,7 +294,6 @@ static int sun8i_dw_hdmi_remove(struct platform_device *pdev)
static const struct sun8i_dw_hdmi_quirks sun8i_a83t_quirks = { .mode_valid = sun8i_dw_hdmi_mode_valid_a83t, - .set_rate = true, };
static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = { diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index d983746fa194..d4b55af0592f 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -179,7 +179,6 @@ struct sun8i_dw_hdmi_quirks { enum drm_mode_status (*mode_valid)(struct dw_hdmi *hdmi, void *data, const struct drm_display_info *info, const struct drm_display_mode *mode); - unsigned int set_rate : 1; unsigned int use_drm_infoframe : 1; };
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net wrote:
As expected, HDMI controller clock should always match pixel clock. In the past, changing HDMI controller rate would seemingly worsen situation. However, that was the result of other bugs which are now fixed.
Fix that by removing set_rate quirk and always set clock rate.
Fixes: 40bb9d3147b2 ("drm/sun4i: Add support for H6 DW HDMI controller") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
Reviewed-by: Chen-Yu Tsai wens@csie.org
cpce value for 594 MHz is set differently in BSP driver. Fix that.
Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 35c2133724e2..89aff19ddeb4 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -89,7 +89,7 @@ static const struct dw_hdmi_mpll_config sun50i_h6_mpll_cfg[] = { }, }, { 594000000, { - { 0x1a40, 0x0003 }, + { 0x1a7c, 0x0003 }, { 0x3b4c, 0x0003 }, { 0x5a64, 0x0003 }, },
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net wrote:
cpce value for 594 MHz is set differently in BSP driver. Fix that.
Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
Reviewed-by: Chen-Yu Tsai wens@csie.org
Dne petek, 05. februar 2021 ob 04:22:56 CET je Chen-Yu Tsai napisal(a):
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net
wrote:
cpce value for 594 MHz is set differently in BSP driver. Fix that.
Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
Reviewed-by: Chen-Yu Tsai wens@csie.org
Thanks, but I figured that this change is not the proper one. It still gives me issues with my TV. Proper change is to fix current and voltage settings below. I'll replace this patch in v2.
Best regards, Jernej
-- You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to linux-sunxi+unsubscribe@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/
linux-sunxi/ CAGb2v64BpizczmSJdompGosFwWWayNscWvW-7oARLgwNNo%3DteQ%40mail.gmail.com.
It turns out that reasoning for lowering max. supported frequency is wrong. Scrambling works just fine. Several now fixed bugs prevented proper functioning, even with rates lower than 340 MHz. Issues were just more pronounced with higher frequencies.
Fix that by allowing max. supported frequency in HW and fix the comment.
Fixes: cd9063757a22 ("drm/sun4i: DW HDMI: Lower max. supported rate for H6") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 23773a5e0650..bbdfd5e26ec8 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -47,11 +47,9 @@ sun8i_dw_hdmi_mode_valid_h6(struct dw_hdmi *hdmi, void *data, { /* * Controller support maximum of 594 MHz, which correlates to - * 4K@60Hz 4:4:4 or RGB. However, for frequencies greater than - * 340 MHz scrambling has to be enabled. Because scrambling is - * not yet implemented, just limit to 340 MHz for now. + * 4K@60Hz 4:4:4 or RGB. */ - if (mode->clock > 340000) + if (mode->clock > 594000) return MODE_CLOCK_HIGH;
return MODE_OK;
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec jernej.skrabec@siol.net wrote:
It turns out that reasoning for lowering max. supported frequency is wrong. Scrambling works just fine. Several now fixed bugs prevented proper functioning, even with rates lower than 340 MHz. Issues were just more pronounced with higher frequencies.
Fix that by allowing max. supported frequency in HW and fix the comment.
Fixes: cd9063757a22 ("drm/sun4i: DW HDMI: Lower max. supported rate for H6") Tested-by: Andre Heider a.heider@gmail.com Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
Reviewed-by: Chen-Yu Tsai wens@csie.org
dri-devel@lists.freedesktop.org
-
Chen-Yu Tsai -
Jernej Skrabec -
Jernej Škrabec
-
Maxime Ripard