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):

quoted

On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec [off-list ref] 

wrote:

quoted

quoted

Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):

quoted

On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:

quoted

On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec 

[off-list ref]

quoted

quoted

wrote:

quoted

quoted

quoted

Channel 1 has polarity bits for vsync and hsync signals but driver 

never

quoted

quoted

quoted

quoted

quoted

sets them. It turns out that with pre-HDMI2 controllers seemingly 

there

quoted

quoted

quoted

quoted

quoted

is no issue if polarity is not set. However, with HDMI2 controllers
(H6) there often comes to de-synchronization due to phase shift. 

This

quoted

quoted

quoted

quoted

quoted

causes flickering screen. It's safe to assume that similar issues 

might

quoted

quoted

quoted

quoted

quoted

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")

quoted

quoted

quoted

quoted

quoted

Tested-by: Andre Heider <redacted>
Signed-off-by: Jernej Skrabec <redacted>
---
 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/

quoted

quoted

sun4i_tcon.c

quoted

quoted

quoted

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

quoted

quoted

*tcon,

quoted

quoted

quoted

                     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);

quoted

quoted

quoted

quoted

quoted

+       } 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

quoted

quoted

names like io2_inv.

Just say that we don't know exactly what it is, but it is required for 

things

quoted

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